libsepol regressions

libsepol regressions

[Dominick Grift]

Fedora recently decided to pull in various libsepol patches from
master[1]

My policy has broken down in various way's. Some changes make sense but
some others I have issues with.

An example of something I never expected to be allowed in the first
place is re-declarations of blocks and recent changes exposed some instances
where I declared blocks multiple times and got away with it.

However I also encountered issues that i am not sure how to deal
with.

re-declarations of macros are no longer allowed:

Take this example:
https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil

Here I inherit a set of macros from the
"file.all_macro_template_chr_files" template and then I override some of these
macros by manually re-declaring them with slighty different content (the
xperm rules are appended).

This use to be allowed but I am no longer allowed to redeclare macros.

This would not necessarily be a big problem IF this would instead work:

diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
index 1c0fe66..4f067db 100644
--- a/src/dev/termdev.cil
+++ b/src/dev/termdev.cil
@@ -3,21 +3,9 @@

 (block termdev

-       (macro appendinherited_all_chr_files ((type ARG1))
-             (allow ARG1 typeattr appendinherited_chr_file)
-             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
-
-       (macro readwriteinherited_all_chr_files ((type ARG1))
-             (allow ARG1 typeattr readwriteinherited_chr_file)
-             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
-
        (macro type ((type ARG1))
              (typeattributeset typeattr ARG1))

-       (macro writeinherited_all_chr_files ((type ARG1))
-             (allow ARG1 typeattr writeinherited_chr_file)
-             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
-
        (typeattribute typeattr)

        (blockinherit .file.all_macro_template_chr_files)
@@ -33,3 +21,12 @@

              (allow typeattr termdev.typeatt
                     (chr_file (not (execmod mounton))))))
+
+(in termdev.appendinherited_all_chr_files
+    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
+
+(in termdev.readwriteinherited_all_chr_files
+    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
+
+(in termdev.writeinherited_all_chr_files
+    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))

But the above in-statements cannot be resolved.

This is not the only instance where this approach does not work. I also
have templates that declare blocks. I use to be allowed to re-declare
these blocks so that I could add to them but this is no longer
allowed. However these blocks also cannot be resolved outside of the
templates, so I cannot use "in" to reference them.

It seems as if the "in" blocks are resolved before the "blockinherit"
blocks are expanded.

[1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[Dominick Grift]

Dominick Grift <dominick.grift@defensec.nl> writes:

> Fedora recently decided to pull in various libsepol patches from
> master[1]

Here are two examples (test1.cil and test2.cil)  that I think
demonstrate some of my issues. Comment out the ";;the culprit" blocks to see
it fail.

In the test1.cil scenario I previously was able to get around this by
re-declaring the macro.

In the test2.cil scenario I previously was able to get around this by
re-declaring the block.

I do not mind not being allowed to re-declare macros and blocks
but then I would appreciate if I could use "in" to insert into them
instead. If I cannot either re-declare nor "insert into" then templates
become pretty useless due to the limitations.

 ---

cat > test1.cil <<EOF
;; what is the point of templates when you cannot access them?

(sid kernel)
(sidorder (kernel))
(class CLASS1 (PERM1 PERM2))
(classorder (unordered CLASS1))

(type type)
(typeattribute typeattr) 

(block block_a1
(block block_a2
(blockabstract block_a2)
(macro macro_a1 ((type ARG1))
(allow ARG1 type (CLASS1 (PERM1))))))

(block block_b1
(type type)
(blockinherit block_a1.block_a2))

(block block_c1
(type type)
(call block_b1.macro_a1 (type)))

(block block_d1
(type type)
(blockinherit block_a1.block_a2))

;;the culprit
;;(in block_d1.macro_a1
;;(allow ARG1 type (CLASS (PERM2))))

(block block_e1
(type type)
(call block_d1.macro_a1 (type)))
EOF
secilc test1.cil

 ---

cat > test2.cil <<EOF
;; what is the point of templates when you cannot access them?

(sid kernel)
(sidorder (kernel))
(class CLASS1 (PERM1 PERM2))
(classorder (unordered CLASS1))

(type type)
(typeattribute typeattr) 

(block block_a1
(type type)
(allow type self (CLASS1 (PERM1)))
(block block_a2
(blockabstract block_a2)
(type type)
(allow type self (CLASS1 (PERM1)))
(block block_a3
(type type)
(allow type self (CLASS1 (PERM1))))))

(block block_b1
(blockinherit block_a1.block_a2)
(allow type self (CLASS1 (PERM2))))

;;the culprit
;;(in block_b1.block_a3
;;(block block_b2
;;(type type)))
EOF
secilc test2.cil

>
> My policy has broken down in various way's. Some changes make sense but
> some others I have issues with.
>
> An example of something I never expected to be allowed in the first
> place is re-declarations of blocks and recent changes exposed some instances
> where I declared blocks multiple times and got away with it.
>
> However I also encountered issues that i am not sure how to deal
> with.
>
> re-declarations of macros are no longer allowed:
>
> Take this example:
> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>
> Here I inherit a set of macros from the
> "file.all_macro_template_chr_files" template and then I override some of these
> macros by manually re-declaring them with slighty different content (the
> xperm rules are appended).
>
> This use to be allowed but I am no longer allowed to redeclare macros.
>
> This would not necessarily be a big problem IF this would instead work:
>
> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
> index 1c0fe66..4f067db 100644
> --- a/src/dev/termdev.cil
> +++ b/src/dev/termdev.cil
> @@ -3,21 +3,9 @@
>
>  (block termdev
>
> -       (macro appendinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr appendinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
> -       (macro readwriteinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr readwriteinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
>         (macro type ((type ARG1))
>               (typeattributeset typeattr ARG1))
>
> -       (macro writeinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr writeinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
>         (typeattribute typeattr)
>
>         (blockinherit .file.all_macro_template_chr_files)
> @@ -33,3 +21,12 @@
>
>               (allow typeattr termdev.typeatt
>                      (chr_file (not (execmod mounton))))))
> +
> +(in termdev.appendinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> +
> +(in termdev.readwriteinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> +
> +(in termdev.writeinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>
> But the above in-statements cannot be resolved.
>
> This is not the only instance where this approach does not work. I also
> have templates that declare blocks. I use to be allowed to re-declare
> these blocks so that I could add to them but this is no longer
> allowed. However these blocks also cannot be resolved outside of the
> templates, so I cannot use "in" to reference them.
>
> It seems as if the "in" blocks are resolved before the "blockinherit"
> blocks are expanded.
>
> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[Dominick Grift]

Dominick Grift <dominick.grift@defensec.nl> writes:

> Dominick Grift <dominick.grift@defensec.nl> writes:
>
>> Fedora recently decided to pull in various libsepol patches from
>> master[1]

Here is another simplified take:

Why does this work?

(sid kernel)
(sidorder (kernel))
(class CLASS1 (PERM1 PERM2))
(classorder (unordered CLASS1))
(block test1
(type type1)
(block conf1
(type type1)))
(in test1.conf1
(allow type self (CLASS1 (PERM1))))

> [kcinimod@myguest tests]$ secilc testa.cil
> [kcinimod@myguest tests]$ seinfo policy.33 -t

> Types: 2
>   test1.conf1.type1
>   test1.type1
> [kcinimod@myguest tests]$

Why does this not work?

(sid kernel)
(sidorder (kernel))
(class CLASS2 (PERM1 PERM2))
(classorder (unordered CLASS2))
(block template2
(type type2)
(allow type2 self (CLASS2 (PERM1)))
(block conf2
(type type2)))
(block test2
(blockinherit template2))
(in test2.conf2
(allow type2 self (CLASS2 (PERM2))))

> [kcinimod@myguest tests]$ secilc testb.cil
> Failed to resolve in-statement at testb.cil:12
> Failed to resolve AST
> Failed to compile cildb: -1

I would like to be able to understand from a user perspective. What is
the difference between test1.conf1 and test2.conf2? Why can they not
both be resolved consistently?

What are the implications of this limitation? Does this mean that blocks
that declare blocks and macros cannot be reliably inherited?

If we are not able to reliably declare blocks and macros inside
containers then should we be allowed to do this in the first place? And
what purpose remains there for blockinherit and in given the above?

>
> Here are two examples (test1.cil and test2.cil)  that I think
> demonstrate some of my issues. Comment out the ";;the culprit" blocks to see
> it fail.
>
> In the test1.cil scenario I previously was able to get around this by
> re-declaring the macro.
>
> In the test2.cil scenario I previously was able to get around this by
> re-declaring the block.
>
> I do not mind not being allowed to re-declare macros and blocks
> but then I would appreciate if I could use "in" to insert into them
> instead. If I cannot either re-declare nor "insert into" then templates
> become pretty useless due to the limitations.
>
>  ---
>
> cat > test1.cil <<EOF
> ;; what is the point of templates when you cannot access them?
>
> (sid kernel)
> (sidorder (kernel))
> (class CLASS1 (PERM1 PERM2))
> (classorder (unordered CLASS1))
>
> (type type)
> (typeattribute typeattr) 
>
> (block block_a1
> (block block_a2
> (blockabstract block_a2)
> (macro macro_a1 ((type ARG1))
> (allow ARG1 type (CLASS1 (PERM1))))))
>
> (block block_b1
> (type type)
> (blockinherit block_a1.block_a2))
>
> (block block_c1
> (type type)
> (call block_b1.macro_a1 (type)))
>
> (block block_d1
> (type type)
> (blockinherit block_a1.block_a2))
>
> ;;the culprit
> ;;(in block_d1.macro_a1
> ;;(allow ARG1 type (CLASS (PERM2))))
>
> (block block_e1
> (type type)
> (call block_d1.macro_a1 (type)))
> EOF
> secilc test1.cil
>
>  ---
>
> cat > test2.cil <<EOF
> ;; what is the point of templates when you cannot access them?
>
> (sid kernel)
> (sidorder (kernel))
> (class CLASS1 (PERM1 PERM2))
> (classorder (unordered CLASS1))
>
> (type type)
> (typeattribute typeattr) 
>
> (block block_a1
> (type type)
> (allow type self (CLASS1 (PERM1)))
> (block block_a2
> (blockabstract block_a2)
> (type type)
> (allow type self (CLASS1 (PERM1)))
> (block block_a3
> (type type)
> (allow type self (CLASS1 (PERM1))))))
>
> (block block_b1
> (blockinherit block_a1.block_a2)
> (allow type self (CLASS1 (PERM2))))
>
> ;;the culprit
> ;;(in block_b1.block_a3
> ;;(block block_b2
> ;;(type type)))
> EOF
> secilc test2.cil
>
>>
>> My policy has broken down in various way's. Some changes make sense but
>> some others I have issues with.
>>
>> An example of something I never expected to be allowed in the first
>> place is re-declarations of blocks and recent changes exposed some instances
>> where I declared blocks multiple times and got away with it.
>>
>> However I also encountered issues that i am not sure how to deal
>> with.
>>
>> re-declarations of macros are no longer allowed:
>>
>> Take this example:
>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>>
>> Here I inherit a set of macros from the
>> "file.all_macro_template_chr_files" template and then I override some of these
>> macros by manually re-declaring them with slighty different content (the
>> xperm rules are appended).
>>
>> This use to be allowed but I am no longer allowed to redeclare macros.
>>
>> This would not necessarily be a big problem IF this would instead work:
>>
>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>> index 1c0fe66..4f067db 100644
>> --- a/src/dev/termdev.cil
>> +++ b/src/dev/termdev.cil
>> @@ -3,21 +3,9 @@
>>
>>  (block termdev
>>
>> -       (macro appendinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr appendinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (macro type ((type ARG1))
>>               (typeattributeset typeattr ARG1))
>>
>> -       (macro writeinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr writeinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (typeattribute typeattr)
>>
>>         (blockinherit .file.all_macro_template_chr_files)
>> @@ -33,3 +21,12 @@
>>
>>               (allow typeattr termdev.typeatt
>>                      (chr_file (not (execmod mounton))))))
>> +
>> +(in termdev.appendinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.readwriteinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.writeinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>
>> But the above in-statements cannot be resolved.
>>
>> This is not the only instance where this approach does not work. I also
>> have templates that declare blocks. I use to be allowed to re-declare
>> these blocks so that I could add to them but this is no longer
>> allowed. However these blocks also cannot be resolved outside of the
>> templates, so I cannot use "in" to reference them.
>>
>> It seems as if the "in" blocks are resolved before the "blockinherit"
>> blocks are expanded.
>>
>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[Dominick Grift]

Dominick Grift <dominick.grift@defensec.nl> writes:

> Dominick Grift <dominick.grift@defensec.nl> writes:
>
>> Dominick Grift <dominick.grift@defensec.nl> writes:
>>
>>> Fedora recently decided to pull in various libsepol patches from
>>> master[1]
>
> Here is another simplified take:
>
> Why does this work?
>
> (sid kernel)
> (sidorder (kernel))
> (class CLASS1 (PERM1 PERM2))
> (classorder (unordered CLASS1))
> (block test1
> (type type1)
> (block conf1
> (type type1)))
> (in test1.conf1
> (allow type self (CLASS1 (PERM1))))

Make that type1

>
>> [kcinimod@myguest tests]$ secilc testa.cil
>> [kcinimod@myguest tests]$ seinfo policy.33 -t
>
>> Types: 2
>>   test1.conf1.type1
>>   test1.type1
>> [kcinimod@myguest tests]$
>
> Why does this not work?
>
> (sid kernel)
> (sidorder (kernel))
> (class CLASS2 (PERM1 PERM2))
> (classorder (unordered CLASS2))
> (block template2
> (type type2)
> (allow type2 self (CLASS2 (PERM1)))
> (block conf2
> (type type2)))
> (block test2
> (blockinherit template2))
> (in test2.conf2
> (allow type2 self (CLASS2 (PERM2))))
>
>> [kcinimod@myguest tests]$ secilc testb.cil
>> Failed to resolve in-statement at testb.cil:12
>> Failed to resolve AST
>> Failed to compile cildb: -1
>
> I would like to be able to understand from a user perspective. What is
> the difference between test1.conf1 and test2.conf2? Why can they not
> both be resolved consistently?
>
> What are the implications of this limitation? Does this mean that blocks
> that declare blocks and macros cannot be reliably inherited?
>
> If we are not able to reliably declare blocks and macros inside
> containers then should we be allowed to do this in the first place? And
> what purpose remains there for blockinherit and in given the above?
>
>>
>> Here are two examples (test1.cil and test2.cil)  that I think
>> demonstrate some of my issues. Comment out the ";;the culprit" blocks to see
>> it fail.
>>
>> In the test1.cil scenario I previously was able to get around this by
>> re-declaring the macro.
>>
>> In the test2.cil scenario I previously was able to get around this by
>> re-declaring the block.
>>
>> I do not mind not being allowed to re-declare macros and blocks
>> but then I would appreciate if I could use "in" to insert into them
>> instead. If I cannot either re-declare nor "insert into" then templates
>> become pretty useless due to the limitations.



>>
>>  ---
>>
>> cat > test1.cil <<EOF
>> ;; what is the point of templates when you cannot access them?
>>
>> (sid kernel)
>> (sidorder (kernel))
>> (class CLASS1 (PERM1 PERM2))
>> (classorder (unordered CLASS1))
>>
>> (type type)
>> (typeattribute typeattr) 
>>
>> (block block_a1
>> (block block_a2
>> (blockabstract block_a2)
>> (macro macro_a1 ((type ARG1))
>> (allow ARG1 type (CLASS1 (PERM1))))))
>>
>> (block block_b1
>> (type type)
>> (blockinherit block_a1.block_a2))
>>
>> (block block_c1
>> (type type)
>> (call block_b1.macro_a1 (type)))
>>
>> (block block_d1
>> (type type)
>> (blockinherit block_a1.block_a2))
>>
>> ;;the culprit
>> ;;(in block_d1.macro_a1
>> ;;(allow ARG1 type (CLASS (PERM2))))
>>
>> (block block_e1
>> (type type)
>> (call block_d1.macro_a1 (type)))
>> EOF
>> secilc test1.cil
>>
>>  ---
>>
>> cat > test2.cil <<EOF
>> ;; what is the point of templates when you cannot access them?
>>
>> (sid kernel)
>> (sidorder (kernel))
>> (class CLASS1 (PERM1 PERM2))
>> (classorder (unordered CLASS1))
>>
>> (type type)
>> (typeattribute typeattr) 
>>
>> (block block_a1
>> (type type)
>> (allow type self (CLASS1 (PERM1)))
>> (block block_a2
>> (blockabstract block_a2)
>> (type type)
>> (allow type self (CLASS1 (PERM1)))
>> (block block_a3
>> (type type)
>> (allow type self (CLASS1 (PERM1))))))
>>
>> (block block_b1
>> (blockinherit block_a1.block_a2)
>> (allow type self (CLASS1 (PERM2))))
>>
>> ;;the culprit
>> ;;(in block_b1.block_a3
>> ;;(block block_b2
>> ;;(type type)))
>> EOF
>> secilc test2.cil
>>
>>>
>>> My policy has broken down in various way's. Some changes make sense but
>>> some others I have issues with.
>>>
>>> An example of something I never expected to be allowed in the first
>>> place is re-declarations of blocks and recent changes exposed some instances
>>> where I declared blocks multiple times and got away with it.
>>>
>>> However I also encountered issues that i am not sure how to deal
>>> with.
>>>
>>> re-declarations of macros are no longer allowed:
>>>
>>> Take this example:
>>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>>>
>>> Here I inherit a set of macros from the
>>> "file.all_macro_template_chr_files" template and then I override some of these
>>> macros by manually re-declaring them with slighty different content (the
>>> xperm rules are appended).
>>>
>>> This use to be allowed but I am no longer allowed to redeclare macros.
>>>
>>> This would not necessarily be a big problem IF this would instead work:
>>>
>>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>>> index 1c0fe66..4f067db 100644
>>> --- a/src/dev/termdev.cil
>>> +++ b/src/dev/termdev.cil
>>> @@ -3,21 +3,9 @@
>>>
>>>  (block termdev
>>>
>>> -       (macro appendinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr appendinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>>         (macro type ((type ARG1))
>>>               (typeattributeset typeattr ARG1))
>>>
>>> -       (macro writeinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr writeinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>>         (typeattribute typeattr)
>>>
>>>         (blockinherit .file.all_macro_template_chr_files)
>>> @@ -33,3 +21,12 @@
>>>
>>>               (allow typeattr termdev.typeatt
>>>                      (chr_file (not (execmod mounton))))))
>>> +
>>> +(in termdev.appendinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> +
>>> +(in termdev.readwriteinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> +
>>> +(in termdev.writeinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>>
>>> But the above in-statements cannot be resolved.
>>>
>>> This is not the only instance where this approach does not work. I also
>>> have templates that declare blocks. I use to be allowed to re-declare
>>> these blocks so that I could add to them but this is no longer
>>> allowed. However these blocks also cannot be resolved outside of the
>>> templates, so I cannot use "in" to reference them.
>>>
>>> It seems as if the "in" blocks are resolved before the "blockinherit"
>>> blocks are expanded.
>>>
>>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[James Carter]

On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
>
> Fedora recently decided to pull in various libsepol patches from
> master[1]
>
> My policy has broken down in various way's. Some changes make sense but
> some others I have issues with.
>
> An example of something I never expected to be allowed in the first
> place is re-declarations of blocks and recent changes exposed some instances
> where I declared blocks multiple times and got away with it.
>
> However I also encountered issues that i am not sure how to deal
> with.
>
> re-declarations of macros are no longer allowed:
>

Re-declarations were never supposed to be allowed (other than the
declaration of types and typeattributes when using the -m flag), but
there were not sufficient checks being done when copying the CIL AST
when expanding macro calls, resolving the inheritance of blocks, and
things like that.

The result was behavior that depends on the rule order and one of the
principles of CIL is that there would be no rule order dependencies.

> Take this example:
> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>
> Here I inherit a set of macros from the
> "file.all_macro_template_chr_files" template and then I override some of these
> macros by manually re-declaring them with slighty different content (the
> xperm rules are appended).
>
> This use to be allowed but I am no longer allowed to redeclare macros.
>

I can see that this might be useful behavior.

> This would not necessarily be a big problem IF this would instead work:
>
> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
> index 1c0fe66..4f067db 100644
> --- a/src/dev/termdev.cil
> +++ b/src/dev/termdev.cil
> @@ -3,21 +3,9 @@
>
>  (block termdev
>
> -       (macro appendinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr appendinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
> -       (macro readwriteinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr readwriteinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
>         (macro type ((type ARG1))
>               (typeattributeset typeattr ARG1))
>
> -       (macro writeinherited_all_chr_files ((type ARG1))
> -             (allow ARG1 typeattr writeinherited_chr_file)
> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> -
>         (typeattribute typeattr)
>
>         (blockinherit .file.all_macro_template_chr_files)
> @@ -33,3 +21,12 @@
>
>               (allow typeattr termdev.typeatt
>                      (chr_file (not (execmod mounton))))))
> +
> +(in termdev.appendinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> +
> +(in termdev.readwriteinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> +
> +(in termdev.writeinherited_all_chr_files
> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>
> But the above in-statements cannot be resolved.
>

I think that overriding the macros would make more sense, but I'll
have to think about it.

It would be a pain, but you could do something like:
(block B1
  (macro M1 (...)
    ...
  )
  (macro M2 (...)
    ...
  )
)

(block B2
  (block parent
    (blockinherit B1)
  )
  (macro M1 (...)
    (call parent.M1 (...))
    ...
  )
  (macro M2 (...)
    (call parent.MA (...))
    ...
  )
)

> This is not the only instance where this approach does not work. I also
> have templates that declare blocks. I use to be allowed to re-declare
> these blocks so that I could add to them but this is no longer
> allowed. However these blocks also cannot be resolved outside of the
> templates, so I cannot use "in" to reference them.
>
> It seems as if the "in" blocks are resolved before the "blockinherit"
> blocks are expanded.
>

It is true that in-statements are resolved before inheritance, so what
you have above would not work.

If you are just adding rules and not declarations, then you don't have
to put the rules in the block.
Example:
(block B1
  (block B
    (type t)
   )
)
(block B2
  (blockinherit B1)
  (allow B.t self (CLASS (PERM)))
)

It might be possible to attempt to resolve all in-statements before
inheritance as what currently happens and then try to resolve any
remaining in-statements after inheritance has been resolved.

I'll have to think about the problem a bit.

Thanks for the questions,
Jim



The problem
> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>
> --
> gpg --locate-keys dominick.grift@defensec.nl
> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> Dominick Grift

Re: libsepol regressions

[Dominick Grift]

James Carter <jwcart2@gmail.com> writes:

> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
>>
>>
>> Fedora recently decided to pull in various libsepol patches from
>> master[1]
>>
>> My policy has broken down in various way's. Some changes make sense but
>> some others I have issues with.
>>
>> An example of something I never expected to be allowed in the first
>> place is re-declarations of blocks and recent changes exposed some instances
>> where I declared blocks multiple times and got away with it.
>>
>> However I also encountered issues that i am not sure how to deal
>> with.
>>
>> re-declarations of macros are no longer allowed:
>>
>
> Re-declarations were never supposed to be allowed (other than the
> declaration of types and typeattributes when using the -m flag), but
> there were not sufficient checks being done when copying the CIL AST
> when expanding macro calls, resolving the inheritance of blocks, and
> things like that.
>
> The result was behavior that depends on the rule order and one of the
> principles of CIL is that there would be no rule order dependencies.

True and I like to be clear about this upfront that from what I can tell
now I wouldnt mind refusing re-declaration of block and macro *IF* we
can reliably use "in" consistently and that is currently not the case.

Allowing re-declaration of macros does give more flexibility because it
allows you to re-define whole macros as opposed to in-statements only allowing
you to append to existing macros, but that limitation would be something
I could live with.

The thing is that for me its more important the "in" works reliably in
the longer run. I mean even if I would be allowed to re-declare macros
that then still would not allow be to re-declare blocks and I would like
to have the ability to append to existing blocks.

>
>> Take this example:
>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>>
>> Here I inherit a set of macros from the
>> "file.all_macro_template_chr_files" template and then I override some of these
>> macros by manually re-declaring them with slighty different content (the
>> xperm rules are appended).
>>
>> This use to be allowed but I am no longer allowed to redeclare macros.
>>
>
> I can see that this might be useful behavior.

Yes it is useful but I believe that "in" statements should work
consistently regardless simply because it is not intuitive that you can
only insert into containers that were not declared using templates.

One should be able to insert into containers regardless of how they were
constructed IMHO.

>
>> This would not necessarily be a big problem IF this would instead work:
>>
>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>> index 1c0fe66..4f067db 100644
>> --- a/src/dev/termdev.cil
>> +++ b/src/dev/termdev.cil
>> @@ -3,21 +3,9 @@
>>
>>  (block termdev
>>
>> -       (macro appendinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr appendinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (macro type ((type ARG1))
>>               (typeattributeset typeattr ARG1))
>>
>> -       (macro writeinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr writeinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (typeattribute typeattr)
>>
>>         (blockinherit .file.all_macro_template_chr_files)
>> @@ -33,3 +21,12 @@
>>
>>               (allow typeattr termdev.typeatt
>>                      (chr_file (not (execmod mounton))))))
>> +
>> +(in termdev.appendinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.readwriteinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.writeinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>
>> But the above in-statements cannot be resolved.
>>
>
> I think that overriding the macros would make more sense, but I'll
> have to think about it.

Maybe, but I think that regardless this should just have worked. Would
you not agree? Are there unsurmountable technical difficulties?

>
> It would be a pain, but you could do something like:
> (block B1
>   (macro M1 (...)
>     ...
>   )
>   (macro M2 (...)
>     ...
>   )
> )
>
> (block B2
>   (block parent
>     (blockinherit B1)
>   )
>   (macro M1 (...)
>     (call parent.M1 (...))
>     ...
>   )
>   (macro M2 (...)
>     (call parent.MA (...))
>     ...
>   )
> )
>
>> This is not the only instance where this approach does not work. I also
>> have templates that declare blocks. I use to be allowed to re-declare
>> these blocks so that I could add to them but this is no longer
>> allowed. However these blocks also cannot be resolved outside of the
>> templates, so I cannot use "in" to reference them.
>>
>> It seems as if the "in" blocks are resolved before the "blockinherit"
>> blocks are expanded.
>>
>
> It is true that in-statements are resolved before inheritance, so what
> you have above would not work.

That seems unintuitive to me. I mean you can properly inherit blocks
with all bells and whistless as long as the block does not inherit. If
it inherits then bets are off. That seems inconsistent.

>
> If you are just adding rules and not declarations, then you don't have
> to put the rules in the block.
> Example:
> (block B1
>   (block B
>     (type t)
>    )
> )
> (block B2
>   (blockinherit B1)
>   (allow B.t self (CLASS (PERM)))
> )

Yes sure theres alway's a hack and if all else fails then I guess I will
have to explore my options, but it feels wrong.

>
> It might be possible to attempt to resolve all in-statements before
> inheritance as what currently happens and then try to resolve any
> remaining in-statements after inheritance has been resolved.
>
> I'll have to think about the problem a bit.

Thanks!

>
> Thanks for the questions,
> Jim
>
>
>
> The problem
>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>>
>> --
>> gpg --locate-keys dominick.grift@defensec.nl
>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> Dominick Grift

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[Dominick Grift]

Dominick Grift <dominick.grift@defensec.nl> writes:

> James Carter <jwcart2@gmail.com> writes:
>
>> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
>> <dominick.grift@defensec.nl> wrote:
>>>
>>>
>>> Fedora recently decided to pull in various libsepol patches from
>>> master[1]
>>>
>>> My policy has broken down in various way's. Some changes make sense but
>>> some others I have issues with.
>>>
>>> An example of something I never expected to be allowed in the first
>>> place is re-declarations of blocks and recent changes exposed some instances
>>> where I declared blocks multiple times and got away with it.
>>>
>>> However I also encountered issues that i am not sure how to deal
>>> with.
>>>
>>> re-declarations of macros are no longer allowed:
>>>
>>
>> Re-declarations were never supposed to be allowed (other than the
>> declaration of types and typeattributes when using the -m flag), but
>> there were not sufficient checks being done when copying the CIL AST
>> when expanding macro calls, resolving the inheritance of blocks, and
>> things like that.
>>
>> The result was behavior that depends on the rule order and one of the
>> principles of CIL is that there would be no rule order dependencies.
>
> True and I like to be clear about this upfront that from what I can tell
> now I wouldnt mind refusing re-declaration of block and macro *IF* we
> can reliably use "in" consistently and that is currently not the case.
>
> Allowing re-declaration of macros does give more flexibility because it
> allows you to re-define whole macros as opposed to in-statements only allowing
> you to append to existing macros, but that limitation would be something
> I could live with.

After thinking about the above statement for a bit I started to doubt
the claim that re-declaring a macro allows you to fully replace it. It
is actually a true statement.

The added value of being allowed to replace macros over allowing you to
"insert into" existing macros is low in my opinion though and i stick to
my suggestion that allowing in-statements regardless whether you
insert-into a namespace declared manually or using inheritance should be
possible.

I would probably just don't allow re-declaring macros, blocks and
optionals for simplicity and instead I would focus on makeing
in-statements work more intuitively.

The 3.2 behavior of re-declaring blocks and macros is inconsistent
anyway because:

re-declaring a macro redefines the whole thing
re-declaring a block just behaves as a in-statement and allows you to
insert into an existing block when a in-statement cannot be resolved.

It seems as if we were allowed to re-declare macros and blocks just to
avoid having to address the current limitations of in-statements.

Best to just address the current in-statement limiations head-on
IMHO. Then we don't really need those block/macro re-declarations either
AFAIK.

It will yield a consistent and intuitive behavior, which minimizes confusion.

Its just hard to explain to a user:

You cannot insert into a1.b2 because b2 comes from inheritance but you
can insert into a1.b3 because a1.b3 was declared manually.

That is just unwieldy.

>
> The thing is that for me its more important the "in" works reliably in
> the longer run. I mean even if I would be allowed to re-declare macros
> that then still would not allow be to re-declare blocks and I would like
> to have the ability to append to existing blocks.
>
>>
>>> Take this example:
>>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>>>
>>> Here I inherit a set of macros from the
>>> "file.all_macro_template_chr_files" template and then I override some of these
>>> macros by manually re-declaring them with slighty different content (the
>>> xperm rules are appended).
>>>
>>> This use to be allowed but I am no longer allowed to redeclare macros.
>>>
>>
>> I can see that this might be useful behavior.
>
> Yes it is useful but I believe that "in" statements should work
> consistently regardless simply because it is not intuitive that you can
> only insert into containers that were not declared using templates.
>
> One should be able to insert into containers regardless of how they were
> constructed IMHO.
>
>>
>>> This would not necessarily be a big problem IF this would instead work:
>>>
>>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>>> index 1c0fe66..4f067db 100644
>>> --- a/src/dev/termdev.cil
>>> +++ b/src/dev/termdev.cil
>>> @@ -3,21 +3,9 @@
>>>
>>>  (block termdev
>>>
>>> -       (macro appendinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr appendinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>>         (macro type ((type ARG1))
>>>               (typeattributeset typeattr ARG1))
>>>
>>> -       (macro writeinherited_all_chr_files ((type ARG1))
>>> -             (allow ARG1 typeattr writeinherited_chr_file)
>>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> -
>>>         (typeattribute typeattr)
>>>
>>>         (blockinherit .file.all_macro_template_chr_files)
>>> @@ -33,3 +21,12 @@
>>>
>>>               (allow typeattr termdev.typeatt
>>>                      (chr_file (not (execmod mounton))))))
>>> +
>>> +(in termdev.appendinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> +
>>> +(in termdev.readwriteinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>> +
>>> +(in termdev.writeinherited_all_chr_files
>>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>>
>>> But the above in-statements cannot be resolved.
>>>
>>
>> I think that overriding the macros would make more sense, but I'll
>> have to think about it.
>
> Maybe, but I think that regardless this should just have worked. Would
> you not agree? Are there unsurmountable technical difficulties?
>
>>
>> It would be a pain, but you could do something like:
>> (block B1
>>   (macro M1 (...)
>>     ...
>>   )
>>   (macro M2 (...)
>>     ...
>>   )
>> )
>>
>> (block B2
>>   (block parent
>>     (blockinherit B1)
>>   )
>>   (macro M1 (...)
>>     (call parent.M1 (...))
>>     ...
>>   )
>>   (macro M2 (...)
>>     (call parent.MA (...))
>>     ...
>>   )
>> )
>>
>>> This is not the only instance where this approach does not work. I also
>>> have templates that declare blocks. I use to be allowed to re-declare
>>> these blocks so that I could add to them but this is no longer
>>> allowed. However these blocks also cannot be resolved outside of the
>>> templates, so I cannot use "in" to reference them.
>>>
>>> It seems as if the "in" blocks are resolved before the "blockinherit"
>>> blocks are expanded.
>>>
>>
>> It is true that in-statements are resolved before inheritance, so what
>> you have above would not work.
>
> That seems unintuitive to me. I mean you can properly inherit blocks
> with all bells and whistless as long as the block does not inherit. If
> it inherits then bets are off. That seems inconsistent.
>
>>
>> If you are just adding rules and not declarations, then you don't have
>> to put the rules in the block.
>> Example:
>> (block B1
>>   (block B
>>     (type t)
>>    )
>> )
>> (block B2
>>   (blockinherit B1)
>>   (allow B.t self (CLASS (PERM)))
>> )
>
> Yes sure theres alway's a hack and if all else fails then I guess I will
> have to explore my options, but it feels wrong.
>
>>
>> It might be possible to attempt to resolve all in-statements before
>> inheritance as what currently happens and then try to resolve any
>> remaining in-statements after inheritance has been resolved.
>>
>> I'll have to think about the problem a bit.
>
> Thanks!
>
>>
>> Thanks for the questions,
>> Jim
>>
>>
>>
>> The problem
>>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>>>
>>> --
>>> gpg --locate-keys dominick.grift@defensec.nl
>>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>>> Dominick Grift

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[James Carter]

On Tue, Aug 3, 2021 at 5:49 AM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
> Dominick Grift <dominick.grift@defensec.nl> writes:
>
> > James Carter <jwcart2@gmail.com> writes:
> >
> >> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
> >> <dominick.grift@defensec.nl> wrote:
> >>>
> >>>
> >>> Fedora recently decided to pull in various libsepol patches from
> >>> master[1]
> >>>
> >>> My policy has broken down in various way's. Some changes make sense but
> >>> some others I have issues with.
> >>>
> >>> An example of something I never expected to be allowed in the first
> >>> place is re-declarations of blocks and recent changes exposed some instances
> >>> where I declared blocks multiple times and got away with it.
> >>>
> >>> However I also encountered issues that i am not sure how to deal
> >>> with.
> >>>
> >>> re-declarations of macros are no longer allowed:
> >>>
> >>
> >> Re-declarations were never supposed to be allowed (other than the
> >> declaration of types and typeattributes when using the -m flag), but
> >> there were not sufficient checks being done when copying the CIL AST
> >> when expanding macro calls, resolving the inheritance of blocks, and
> >> things like that.
> >>
> >> The result was behavior that depends on the rule order and one of the
> >> principles of CIL is that there would be no rule order dependencies.
> >
> > True and I like to be clear about this upfront that from what I can tell
> > now I wouldnt mind refusing re-declaration of block and macro *IF* we
> > can reliably use "in" consistently and that is currently not the case.
> >
> > Allowing re-declaration of macros does give more flexibility because it
> > allows you to re-define whole macros as opposed to in-statements only allowing
> > you to append to existing macros, but that limitation would be something
> > I could live with.
>
> After thinking about the above statement for a bit I started to doubt
> the claim that re-declaring a macro allows you to fully replace it. It
> is actually a true statement.
>
> The added value of being allowed to replace macros over allowing you to
> "insert into" existing macros is low in my opinion though and i stick to
> my suggestion that allowing in-statements regardless whether you
> insert-into a namespace declared manually or using inheritance should be
> possible.
>
> I would probably just don't allow re-declaring macros, blocks and
> optionals for simplicity and instead I would focus on makeing
> in-statements work more intuitively.
>
> The 3.2 behavior of re-declaring blocks and macros is inconsistent
> anyway because:
>
> re-declaring a macro redefines the whole thing
> re-declaring a block just behaves as a in-statement and allows you to
> insert into an existing block when a in-statement cannot be resolved.
>
> It seems as if we were allowed to re-declare macros and blocks just to
> avoid having to address the current limitations of in-statements.
>
> Best to just address the current in-statement limiations head-on
> IMHO. Then we don't really need those block/macro re-declarations either
> AFAIK.
>
> It will yield a consistent and intuitive behavior, which minimizes confusion.
>
> Its just hard to explain to a user:
>
> You cannot insert into a1.b2 because b2 comes from inheritance but you
> can insert into a1.b3 because a1.b3 was declared manually.
>
> That is just unwieldy.
>
> >
> > The thing is that for me its more important the "in" works reliably in
> > the longer run. I mean even if I would be allowed to re-declare macros
> > that then still would not allow be to re-declare blocks and I would like
> > to have the ability to append to existing blocks.
> >

The problem with the in-statement is that you might want to work
differently depending on the circumstances.

Imagine that you have the following:

(block b1
  (macro m (...)
    ...
  )
 ...
)

(in b1.m
  (allow ...)
)

(block b2
 (blockinherit b1)
 ...
)

How should the in-statement for b1.m behave?
You might want the in-statement to be resolved before any inheritance,
so that the inherited instance was also changed (imagine block b1
being inherited many times), but it is also possible that you only
want the change to happen for block b1. For better or worse, the
decision was made to have the in-statement be resolved before
inheritance is processed.

We can't change that behavior now, but I do see that there is a need
to support changing only specific instances.

I am thinking about creating a new rule that would act like an
in-statement, but be processed after inheritance. Not sure about the
name though: "append", "add", "addto", "with", "within"?

Jim


> >>
> >>> Take this example:
> >>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
> >>>
> >>> Here I inherit a set of macros from the
> >>> "file.all_macro_template_chr_files" template and then I override some of these
> >>> macros by manually re-declaring them with slighty different content (the
> >>> xperm rules are appended).
> >>>
> >>> This use to be allowed but I am no longer allowed to redeclare macros.
> >>>
> >>
> >> I can see that this might be useful behavior.
> >
> > Yes it is useful but I believe that "in" statements should work
> > consistently regardless simply because it is not intuitive that you can
> > only insert into containers that were not declared using templates.
> >
> > One should be able to insert into containers regardless of how they were
> > constructed IMHO.
> >
> >>
> >>> This would not necessarily be a big problem IF this would instead work:
> >>>
> >>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
> >>> index 1c0fe66..4f067db 100644
> >>> --- a/src/dev/termdev.cil
> >>> +++ b/src/dev/termdev.cil
> >>> @@ -3,21 +3,9 @@
> >>>
> >>>  (block termdev
> >>>
> >>> -       (macro appendinherited_all_chr_files ((type ARG1))
> >>> -             (allow ARG1 typeattr appendinherited_chr_file)
> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>> -
> >>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
> >>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>> -
> >>>         (macro type ((type ARG1))
> >>>               (typeattributeset typeattr ARG1))
> >>>
> >>> -       (macro writeinherited_all_chr_files ((type ARG1))
> >>> -             (allow ARG1 typeattr writeinherited_chr_file)
> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>> -
> >>>         (typeattribute typeattr)
> >>>
> >>>         (blockinherit .file.all_macro_template_chr_files)
> >>> @@ -33,3 +21,12 @@
> >>>
> >>>               (allow typeattr termdev.typeatt
> >>>                      (chr_file (not (execmod mounton))))))
> >>> +
> >>> +(in termdev.appendinherited_all_chr_files
> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>> +
> >>> +(in termdev.readwriteinherited_all_chr_files
> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>> +
> >>> +(in termdev.writeinherited_all_chr_files
> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>>
> >>> But the above in-statements cannot be resolved.
> >>>
> >>
> >> I think that overriding the macros would make more sense, but I'll
> >> have to think about it.
> >
> > Maybe, but I think that regardless this should just have worked. Would
> > you not agree? Are there unsurmountable technical difficulties?
> >
> >>
> >> It would be a pain, but you could do something like:
> >> (block B1
> >>   (macro M1 (...)
> >>     ...
> >>   )
> >>   (macro M2 (...)
> >>     ...
> >>   )
> >> )
> >>
> >> (block B2
> >>   (block parent
> >>     (blockinherit B1)
> >>   )
> >>   (macro M1 (...)
> >>     (call parent.M1 (...))
> >>     ...
> >>   )
> >>   (macro M2 (...)
> >>     (call parent.MA (...))
> >>     ...
> >>   )
> >> )
> >>
> >>> This is not the only instance where this approach does not work. I also
> >>> have templates that declare blocks. I use to be allowed to re-declare
> >>> these blocks so that I could add to them but this is no longer
> >>> allowed. However these blocks also cannot be resolved outside of the
> >>> templates, so I cannot use "in" to reference them.
> >>>
> >>> It seems as if the "in" blocks are resolved before the "blockinherit"
> >>> blocks are expanded.
> >>>
> >>
> >> It is true that in-statements are resolved before inheritance, so what
> >> you have above would not work.
> >
> > That seems unintuitive to me. I mean you can properly inherit blocks
> > with all bells and whistless as long as the block does not inherit. If
> > it inherits then bets are off. That seems inconsistent.
> >
> >>
> >> If you are just adding rules and not declarations, then you don't have
> >> to put the rules in the block.
> >> Example:
> >> (block B1
> >>   (block B
> >>     (type t)
> >>    )
> >> )
> >> (block B2
> >>   (blockinherit B1)
> >>   (allow B.t self (CLASS (PERM)))
> >> )
> >
> > Yes sure theres alway's a hack and if all else fails then I guess I will
> > have to explore my options, but it feels wrong.
> >
> >>
> >> It might be possible to attempt to resolve all in-statements before
> >> inheritance as what currently happens and then try to resolve any
> >> remaining in-statements after inheritance has been resolved.
> >>
> >> I'll have to think about the problem a bit.
> >
> > Thanks!
> >
> >>
> >> Thanks for the questions,
> >> Jim
> >>
> >>
> >>
> >> The problem
> >>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
> >>>
> >>> --
> >>> gpg --locate-keys dominick.grift@defensec.nl
> >>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> >>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> >>> Dominick Grift
>
> --
> gpg --locate-keys dominick.grift@defensec.nl
> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> Dominick Grift

Re: libsepol regressions

[Dominick Grift]

James Carter <jwcart2@gmail.com> writes:

> On Tue, Aug 3, 2021 at 5:49 AM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
>>
>> Dominick Grift <dominick.grift@defensec.nl> writes:
>>
>> > James Carter <jwcart2@gmail.com> writes:
>> >
>> >> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
>> >> <dominick.grift@defensec.nl> wrote:
>> >>>
>> >>>
>> >>> Fedora recently decided to pull in various libsepol patches from
>> >>> master[1]
>> >>>
>> >>> My policy has broken down in various way's. Some changes make sense but
>> >>> some others I have issues with.
>> >>>
>> >>> An example of something I never expected to be allowed in the first
>> >>> place is re-declarations of blocks and recent changes exposed some instances
>> >>> where I declared blocks multiple times and got away with it.
>> >>>
>> >>> However I also encountered issues that i am not sure how to deal
>> >>> with.
>> >>>
>> >>> re-declarations of macros are no longer allowed:
>> >>>
>> >>
>> >> Re-declarations were never supposed to be allowed (other than the
>> >> declaration of types and typeattributes when using the -m flag), but
>> >> there were not sufficient checks being done when copying the CIL AST
>> >> when expanding macro calls, resolving the inheritance of blocks, and
>> >> things like that.
>> >>
>> >> The result was behavior that depends on the rule order and one of the
>> >> principles of CIL is that there would be no rule order dependencies.
>> >
>> > True and I like to be clear about this upfront that from what I can tell
>> > now I wouldnt mind refusing re-declaration of block and macro *IF* we
>> > can reliably use "in" consistently and that is currently not the case.
>> >
>> > Allowing re-declaration of macros does give more flexibility because it
>> > allows you to re-define whole macros as opposed to in-statements only allowing
>> > you to append to existing macros, but that limitation would be something
>> > I could live with.
>>
>> After thinking about the above statement for a bit I started to doubt
>> the claim that re-declaring a macro allows you to fully replace it. It
>> is actually a true statement.
>>
>> The added value of being allowed to replace macros over allowing you to
>> "insert into" existing macros is low in my opinion though and i stick to
>> my suggestion that allowing in-statements regardless whether you
>> insert-into a namespace declared manually or using inheritance should be
>> possible.
>>
>> I would probably just don't allow re-declaring macros, blocks and
>> optionals for simplicity and instead I would focus on makeing
>> in-statements work more intuitively.
>>
>> The 3.2 behavior of re-declaring blocks and macros is inconsistent
>> anyway because:
>>
>> re-declaring a macro redefines the whole thing
>> re-declaring a block just behaves as a in-statement and allows you to
>> insert into an existing block when a in-statement cannot be resolved.
>>
>> It seems as if we were allowed to re-declare macros and blocks just to
>> avoid having to address the current limitations of in-statements.
>>
>> Best to just address the current in-statement limiations head-on
>> IMHO. Then we don't really need those block/macro re-declarations either
>> AFAIK.
>>
>> It will yield a consistent and intuitive behavior, which minimizes confusion.
>>
>> Its just hard to explain to a user:
>>
>> You cannot insert into a1.b2 because b2 comes from inheritance but you
>> can insert into a1.b3 because a1.b3 was declared manually.
>>
>> That is just unwieldy.
>>
>> >
>> > The thing is that for me its more important the "in" works reliably in
>> > the longer run. I mean even if I would be allowed to re-declare macros
>> > that then still would not allow be to re-declare blocks and I would like
>> > to have the ability to append to existing blocks.
>> >
>
> The problem with the in-statement is that you might want to work
> differently depending on the circumstances.
>
> Imagine that you have the following:
>
> (block b1
>   (macro m (...)
>     ...
>   )
>  ...
> )
>
> (in b1.m
>   (allow ...)
> )
>
> (block b2
>  (blockinherit b1)
>  ...
> )
>
> How should the in-statement for b1.m behave?
> You might want the in-statement to be resolved before any inheritance,
> so that the inherited instance was also changed (imagine block b1
> being inherited many times), but it is also possible that you only
> want the change to happen for block b1. For better or worse, the
> decision was made to have the in-statement be resolved before
> inheritance is processed.

To start at the end. I agree that since the decision was already made that it is
probably best to not break compatibility now.

I have difficulty imagining a practical use-case where one might want to resolve
before inheritance, but given the compatibility aspect I think it does
not matter whether I can or cannot imagine a practical use-case. Last
thing I want to do is promote abstracting away any of the flexibility
that defines SELinux.

>
> We can't change that behavior now, but I do see that there is a need
> to support changing only specific instances.
>
> I am thinking about creating a new rule that would act like an
> in-statement, but be processed after inheritance. Not sure about the
> name though: "append", "add", "addto", "with", "within"?

None of them perfectly describe "same-as-in-but-then-with-inheritance". I
guess closest to that is "within". It is the functionality that matters
most to me.

Thanks!

>
> Jim
>
>
>> >>
>> >>> Take this example:
>> >>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>> >>>
>> >>> Here I inherit a set of macros from the
>> >>> "file.all_macro_template_chr_files" template and then I override some of these
>> >>> macros by manually re-declaring them with slighty different content (the
>> >>> xperm rules are appended).
>> >>>
>> >>> This use to be allowed but I am no longer allowed to redeclare macros.
>> >>>
>> >>
>> >> I can see that this might be useful behavior.
>> >
>> > Yes it is useful but I believe that "in" statements should work
>> > consistently regardless simply because it is not intuitive that you can
>> > only insert into containers that were not declared using templates.
>> >
>> > One should be able to insert into containers regardless of how they were
>> > constructed IMHO.
>> >
>> >>
>> >>> This would not necessarily be a big problem IF this would instead work:
>> >>>
>> >>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>> >>> index 1c0fe66..4f067db 100644
>> >>> --- a/src/dev/termdev.cil
>> >>> +++ b/src/dev/termdev.cil
>> >>> @@ -3,21 +3,9 @@
>> >>>
>> >>>  (block termdev
>> >>>
>> >>> -       (macro appendinherited_all_chr_files ((type ARG1))
>> >>> -             (allow ARG1 typeattr appendinherited_chr_file)
>> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>> -
>> >>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>> >>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>> -
>> >>>         (macro type ((type ARG1))
>> >>>               (typeattributeset typeattr ARG1))
>> >>>
>> >>> -       (macro writeinherited_all_chr_files ((type ARG1))
>> >>> -             (allow ARG1 typeattr writeinherited_chr_file)
>> >>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>> -
>> >>>         (typeattribute typeattr)
>> >>>
>> >>>         (blockinherit .file.all_macro_template_chr_files)
>> >>> @@ -33,3 +21,12 @@
>> >>>
>> >>>               (allow typeattr termdev.typeatt
>> >>>                      (chr_file (not (execmod mounton))))))
>> >>> +
>> >>> +(in termdev.appendinherited_all_chr_files
>> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>> +
>> >>> +(in termdev.readwriteinherited_all_chr_files
>> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>> +
>> >>> +(in termdev.writeinherited_all_chr_files
>> >>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>>
>> >>> But the above in-statements cannot be resolved.
>> >>>
>> >>
>> >> I think that overriding the macros would make more sense, but I'll
>> >> have to think about it.
>> >
>> > Maybe, but I think that regardless this should just have worked. Would
>> > you not agree? Are there unsurmountable technical difficulties?
>> >
>> >>
>> >> It would be a pain, but you could do something like:
>> >> (block B1
>> >>   (macro M1 (...)
>> >>     ...
>> >>   )
>> >>   (macro M2 (...)
>> >>     ...
>> >>   )
>> >> )
>> >>
>> >> (block B2
>> >>   (block parent
>> >>     (blockinherit B1)
>> >>   )
>> >>   (macro M1 (...)
>> >>     (call parent.M1 (...))
>> >>     ...
>> >>   )
>> >>   (macro M2 (...)
>> >>     (call parent.MA (...))
>> >>     ...
>> >>   )
>> >> )
>> >>
>> >>> This is not the only instance where this approach does not work. I also
>> >>> have templates that declare blocks. I use to be allowed to re-declare
>> >>> these blocks so that I could add to them but this is no longer
>> >>> allowed. However these blocks also cannot be resolved outside of the
>> >>> templates, so I cannot use "in" to reference them.
>> >>>
>> >>> It seems as if the "in" blocks are resolved before the "blockinherit"
>> >>> blocks are expanded.
>> >>>
>> >>
>> >> It is true that in-statements are resolved before inheritance, so what
>> >> you have above would not work.
>> >
>> > That seems unintuitive to me. I mean you can properly inherit blocks
>> > with all bells and whistless as long as the block does not inherit. If
>> > it inherits then bets are off. That seems inconsistent.
>> >
>> >>
>> >> If you are just adding rules and not declarations, then you don't have
>> >> to put the rules in the block.
>> >> Example:
>> >> (block B1
>> >>   (block B
>> >>     (type t)
>> >>    )
>> >> )
>> >> (block B2
>> >>   (blockinherit B1)
>> >>   (allow B.t self (CLASS (PERM)))
>> >> )
>> >
>> > Yes sure theres alway's a hack and if all else fails then I guess I will
>> > have to explore my options, but it feels wrong.
>> >
>> >>
>> >> It might be possible to attempt to resolve all in-statements before
>> >> inheritance as what currently happens and then try to resolve any
>> >> remaining in-statements after inheritance has been resolved.
>> >>
>> >> I'll have to think about the problem a bit.
>> >
>> > Thanks!
>> >
>> >>
>> >> Thanks for the questions,
>> >> Jim
>> >>
>> >>
>> >>
>> >> The problem
>> >>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>> >>>
>> >>> --
>> >>> gpg --locate-keys dominick.grift@defensec.nl
>> >>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> >>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> >>> Dominick Grift
>>
>> --
>> gpg --locate-keys dominick.grift@defensec.nl
>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> Dominick Grift

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[Dominick Grift]

James Carter <jwcart2@gmail.com> writes:

> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
>>
>>
>> Fedora recently decided to pull in various libsepol patches from
>> master[1]
>>
>> My policy has broken down in various way's. Some changes make sense but
>> some others I have issues with.
>>
>> An example of something I never expected to be allowed in the first
>> place is re-declarations of blocks and recent changes exposed some instances
>> where I declared blocks multiple times and got away with it.
>>
>> However I also encountered issues that i am not sure how to deal
>> with.
>>
>> re-declarations of macros are no longer allowed:
>>
>
> Re-declarations were never supposed to be allowed (other than the
> declaration of types and typeattributes when using the -m flag), but
> there were not sufficient checks being done when copying the CIL AST
> when expanding macro calls, resolving the inheritance of blocks, and
> things like that.
>
> The result was behavior that depends on the rule order and one of the
> principles of CIL is that there would be no rule order dependencies.
>
>> Take this example:
>> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>>
>> Here I inherit a set of macros from the
>> "file.all_macro_template_chr_files" template and then I override some of these
>> macros by manually re-declaring them with slighty different content (the
>> xperm rules are appended).
>>
>> This use to be allowed but I am no longer allowed to redeclare macros.
>>
>
> I can see that this might be useful behavior.
>
>> This would not necessarily be a big problem IF this would instead work:
>>
>> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>> index 1c0fe66..4f067db 100644
>> --- a/src/dev/termdev.cil
>> +++ b/src/dev/termdev.cil
>> @@ -3,21 +3,9 @@
>>
>>  (block termdev
>>
>> -       (macro appendinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr appendinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (macro type ((type ARG1))
>>               (typeattributeset typeattr ARG1))
>>
>> -       (macro writeinherited_all_chr_files ((type ARG1))
>> -             (allow ARG1 typeattr writeinherited_chr_file)
>> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> -
>>         (typeattribute typeattr)
>>
>>         (blockinherit .file.all_macro_template_chr_files)
>> @@ -33,3 +21,12 @@
>>
>>               (allow typeattr termdev.typeatt
>>                      (chr_file (not (execmod mounton))))))
>> +
>> +(in termdev.appendinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.readwriteinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> +
>> +(in termdev.writeinherited_all_chr_files
>> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>>
>> But the above in-statements cannot be resolved.
>>
>
> I think that overriding the macros would make more sense, but I'll
> have to think about it.
>
> It would be a pain, but you could do something like:
> (block B1
>   (macro M1 (...)
>     ...
>   )
>   (macro M2 (...)
>     ...
>   )
> )
>
> (block B2
>   (block parent
>     (blockinherit B1)
>   )
>   (macro M1 (...)
>     (call parent.M1 (...))
>     ...
>   )
>   (macro M2 (...)
>     (call parent.MA (...))
>     ...
>   )
> )
>
>> This is not the only instance where this approach does not work. I also
>> have templates that declare blocks. I use to be allowed to re-declare
>> these blocks so that I could add to them but this is no longer
>> allowed. However these blocks also cannot be resolved outside of the
>> templates, so I cannot use "in" to reference them.
>>
>> It seems as if the "in" blocks are resolved before the "blockinherit"
>> blocks are expanded.
>>
>
> It is true that in-statements are resolved before inheritance, so what
> you have above would not work.
>
> If you are just adding rules and not declarations, then you don't have
> to put the rules in the block.
> Example:
> (block B1
>   (block B
>     (type t)
>    )
> )
> (block B2
>   (blockinherit B1)
>   (allow B.t self (CLASS (PERM)))
> )

Ive been trying to translate this to a actual use-case in my policy to
see if that would suffice.

I have these high level "content" templates that basically allow you to
inherit a whole skeleton without filecon's because a skeleton can be
generic but filecon's are specific so they have to be added later.

(block skel
  (type type)
  (allow ...)
  (block conf
  (type type)
  (allow ...))
  (block exec
  (type type)
  (allow ,,,))
  etc...)

These skeletons can then be inherited and the filecons (and custom rules)
can be added later:

(block myapp
  (blockinherit skel)
  (allow ...))

With 3.2 I was able to mimic in-statements by re-declaring the
blocks so that I would be able to insert the filecons (and custom rules)
later:

(block myapp
  (blockinherit shel)
  (allow ...)
  (block conf
    (filecon "/etc/myapp" file file_context))
  (block exec
    (filecon "/bin/myapp" file file_context)))

If in-statements would be resolved after inheritance then I could do:

(block myapp
  (blockiherit skel)
  (allow ...))

(in myapp.conf
  (filecon "/etc/myapp" file file_context))

(in myapp.exec
  (filecon "/bin/myapp" file file_context))

Your suggestion would be suffice if I would use raw filecons instead of
context:

(block myapp
  (blockinherit skel)
  (allow ...)
  (filecon "/etc/myapp" file (sys.id sys.role myapp.conf.type
  ((s0)(s0))))
  (filecon "/bin/myapp" file (sys.id sys.role myapp.exec.type
  ((s0)(s0)))))

But then that would be yet another inconsistency that would be hard to explain:
Can only use file_context keyword in filecon if you don't use
inheritance.

Not to mention macro's. If there is one thing I dislike then its having to
duplicate macros everywhere. I like to template macros from the ground
up to reduce points of failure and to encourage consistency. This
implies inheritance as well.

I try not to use high level abstractions like these myself, but the idea
is that users of my policy are able to create/script higher level abstractions
on top of my policy. So that they can for example create policy
generation scripts using higher level templates.

This is why consistency matters to me. I can get used to buts-and-ifs but can
the end-user of my policy do that as well?

Eventually I envision something like:

;; generic high level base template used in a script created by end-users
(block myapp
  (blockinherit skel))

;; custom stuff added by the script based on script input
(in myapp
  (allow type self (class (perm))))

(in myapp.conf
  (filecon "/etc/myapp" file file_context)
  (call .tmp.associate (type))

(in myapp.exec
  (filecon "/bin/myapp" file file_context))

(in user
  (call .myapp.run (type)))

>
> It might be possible to attempt to resolve all in-statements before
> inheritance as what currently happens and then try to resolve any
> remaining in-statements after inheritance has been resolved.
>
> I'll have to think about the problem a bit.
>
> Thanks for the questions,
> Jim
>
>
>
> The problem
>> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>>
>> --
>> gpg --locate-keys dominick.grift@defensec.nl
>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> Dominick Grift

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift

Re: libsepol regressions

[James Carter]

On Wed, Aug 4, 2021 at 3:36 AM Dominick Grift
<dominick.grift@defensec.nl> wrote:
>
> James Carter <jwcart2@gmail.com> writes:
>
> > On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
> > <dominick.grift@defensec.nl> wrote:
> >>
> >>
> >> Fedora recently decided to pull in various libsepol patches from
> >> master[1]
> >>
> >> My policy has broken down in various way's. Some changes make sense but
> >> some others I have issues with.
> >>
> >> An example of something I never expected to be allowed in the first
> >> place is re-declarations of blocks and recent changes exposed some instances
> >> where I declared blocks multiple times and got away with it.
> >>
> >> However I also encountered issues that i am not sure how to deal
> >> with.
> >>
> >> re-declarations of macros are no longer allowed:
> >>
> >
> > Re-declarations were never supposed to be allowed (other than the
> > declaration of types and typeattributes when using the -m flag), but
> > there were not sufficient checks being done when copying the CIL AST
> > when expanding macro calls, resolving the inheritance of blocks, and
> > things like that.
> >
> > The result was behavior that depends on the rule order and one of the
> > principles of CIL is that there would be no rule order dependencies.
> >
> >> Take this example:
> >> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
> >>
> >> Here I inherit a set of macros from the
> >> "file.all_macro_template_chr_files" template and then I override some of these
> >> macros by manually re-declaring them with slighty different content (the
> >> xperm rules are appended).
> >>
> >> This use to be allowed but I am no longer allowed to redeclare macros.
> >>
> >
> > I can see that this might be useful behavior.
> >
> >> This would not necessarily be a big problem IF this would instead work:
> >>
> >> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
> >> index 1c0fe66..4f067db 100644
> >> --- a/src/dev/termdev.cil
> >> +++ b/src/dev/termdev.cil
> >> @@ -3,21 +3,9 @@
> >>
> >>  (block termdev
> >>
> >> -       (macro appendinherited_all_chr_files ((type ARG1))
> >> -             (allow ARG1 typeattr appendinherited_chr_file)
> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >> -
> >> -       (macro readwriteinherited_all_chr_files ((type ARG1))
> >> -             (allow ARG1 typeattr readwriteinherited_chr_file)
> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >> -
> >>         (macro type ((type ARG1))
> >>               (typeattributeset typeattr ARG1))
> >>
> >> -       (macro writeinherited_all_chr_files ((type ARG1))
> >> -             (allow ARG1 typeattr writeinherited_chr_file)
> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >> -
> >>         (typeattribute typeattr)
> >>
> >>         (blockinherit .file.all_macro_template_chr_files)
> >> @@ -33,3 +21,12 @@
> >>
> >>               (allow typeattr termdev.typeatt
> >>                      (chr_file (not (execmod mounton))))))
> >> +
> >> +(in termdev.appendinherited_all_chr_files
> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >> +
> >> +(in termdev.readwriteinherited_all_chr_files
> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >> +
> >> +(in termdev.writeinherited_all_chr_files
> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
> >>
> >> But the above in-statements cannot be resolved.
> >>
> >
> > I think that overriding the macros would make more sense, but I'll
> > have to think about it.
> >
> > It would be a pain, but you could do something like:
> > (block B1
> >   (macro M1 (...)
> >     ...
> >   )
> >   (macro M2 (...)
> >     ...
> >   )
> > )
> >
> > (block B2
> >   (block parent
> >     (blockinherit B1)
> >   )
> >   (macro M1 (...)
> >     (call parent.M1 (...))
> >     ...
> >   )
> >   (macro M2 (...)
> >     (call parent.MA (...))
> >     ...
> >   )
> > )
> >
> >> This is not the only instance where this approach does not work. I also
> >> have templates that declare blocks. I use to be allowed to re-declare
> >> these blocks so that I could add to them but this is no longer
> >> allowed. However these blocks also cannot be resolved outside of the
> >> templates, so I cannot use "in" to reference them.
> >>
> >> It seems as if the "in" blocks are resolved before the "blockinherit"
> >> blocks are expanded.
> >>
> >
> > It is true that in-statements are resolved before inheritance, so what
> > you have above would not work.
> >
> > If you are just adding rules and not declarations, then you don't have
> > to put the rules in the block.
> > Example:
> > (block B1
> >   (block B
> >     (type t)
> >    )
> > )
> > (block B2
> >   (blockinherit B1)
> >   (allow B.t self (CLASS (PERM)))
> > )
>
> Ive been trying to translate this to a actual use-case in my policy to
> see if that would suffice.
>
> I have these high level "content" templates that basically allow you to
> inherit a whole skeleton without filecon's because a skeleton can be
> generic but filecon's are specific so they have to be added later.
>
> (block skel
>   (type type)
>   (allow ...)
>   (block conf
>   (type type)
>   (allow ...))
>   (block exec
>   (type type)
>   (allow ,,,))
>   etc...)
>
> These skeletons can then be inherited and the filecons (and custom rules)
> can be added later:
>
> (block myapp
>   (blockinherit skel)
>   (allow ...))
>
> With 3.2 I was able to mimic in-statements by re-declaring the
> blocks so that I would be able to insert the filecons (and custom rules)
> later:
>
> (block myapp
>   (blockinherit shel)
>   (allow ...)
>   (block conf
>     (filecon "/etc/myapp" file file_context))
>   (block exec
>     (filecon "/bin/myapp" file file_context)))
>
> If in-statements would be resolved after inheritance then I could do:
>
> (block myapp
>   (blockiherit skel)
>   (allow ...))
>
> (in myapp.conf
>   (filecon "/etc/myapp" file file_context))
>
> (in myapp.exec
>   (filecon "/bin/myapp" file file_context))
>
> Your suggestion would be suffice if I would use raw filecons instead of
> context:
>
> (block myapp
>   (blockinherit skel)
>   (allow ...)
>   (filecon "/etc/myapp" file (sys.id sys.role myapp.conf.type
>   ((s0)(s0))))
>   (filecon "/bin/myapp" file (sys.id sys.role myapp.exec.type
>   ((s0)(s0)))))
>
> But then that would be yet another inconsistency that would be hard to explain:
> Can only use file_context keyword in filecon if you don't use
> inheritance.
>
> Not to mention macro's. If there is one thing I dislike then its having to
> duplicate macros everywhere. I like to template macros from the ground
> up to reduce points of failure and to encourage consistency. This
> implies inheritance as well.
>
> I try not to use high level abstractions like these myself, but the idea
> is that users of my policy are able to create/script higher level abstractions
> on top of my policy. So that they can for example create policy
> generation scripts using higher level templates.
>
> This is why consistency matters to me. I can get used to buts-and-ifs but can
> the end-user of my policy do that as well?
>
> Eventually I envision something like:
>
> ;; generic high level base template used in a script created by end-users
> (block myapp
>   (blockinherit skel))
>
> ;; custom stuff added by the script based on script input
> (in myapp
>   (allow type self (class (perm))))
>
> (in myapp.conf
>   (filecon "/etc/myapp" file file_context)
>   (call .tmp.associate (type))
>
> (in myapp.exec
>   (filecon "/bin/myapp" file file_context))
>
> (in user
>   (call .myapp.run (type)))
>

I am definitely sympathetic to this use case and feel that you have
identified a gap in CIL's capabilities that should be filled. But like
I said, it will have to have a different name. I don't think that it
will be too hard to add, but we'll see.

Thanks for details,
Jim

> >
> > It might be possible to attempt to resolve all in-statements before
> > inheritance as what currently happens and then try to resolve any
> > remaining in-statements after inheritance has been resolved.
> >
> > I'll have to think about the problem a bit.
> >
> > Thanks for the questions,
> > Jim
> >
> >
> >
> > The problem
> >> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
> >>
> >> --
> >> gpg --locate-keys dominick.grift@defensec.nl
> >> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> >> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> >> Dominick Grift
>
> --
> gpg --locate-keys dominick.grift@defensec.nl
> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
> Dominick Grift

Re: libsepol regressions

[Dominick Grift]

James Carter <jwcart2@gmail.com> writes:

> On Wed, Aug 4, 2021 at 3:36 AM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
>>
>> James Carter <jwcart2@gmail.com> writes:
>>
>> > On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
>> > <dominick.grift@defensec.nl> wrote:
>> >>
>> >>
>> >> Fedora recently decided to pull in various libsepol patches from
>> >> master[1]
>> >>
>> >> My policy has broken down in various way's. Some changes make sense but
>> >> some others I have issues with.
>> >>
>> >> An example of something I never expected to be allowed in the first
>> >> place is re-declarations of blocks and recent changes exposed some instances
>> >> where I declared blocks multiple times and got away with it.
>> >>
>> >> However I also encountered issues that i am not sure how to deal
>> >> with.
>> >>
>> >> re-declarations of macros are no longer allowed:
>> >>
>> >
>> > Re-declarations were never supposed to be allowed (other than the
>> > declaration of types and typeattributes when using the -m flag), but
>> > there were not sufficient checks being done when copying the CIL AST
>> > when expanding macro calls, resolving the inheritance of blocks, and
>> > things like that.
>> >
>> > The result was behavior that depends on the rule order and one of the
>> > principles of CIL is that there would be no rule order dependencies.
>> >
>> >> Take this example:
>> >> https://github.com/DefenSec/dssp5/blob/dev/src/dev/termdev.cil
>> >>
>> >> Here I inherit a set of macros from the
>> >> "file.all_macro_template_chr_files" template and then I override some of these
>> >> macros by manually re-declaring them with slighty different content (the
>> >> xperm rules are appended).
>> >>
>> >> This use to be allowed but I am no longer allowed to redeclare macros.
>> >>
>> >
>> > I can see that this might be useful behavior.
>> >
>> >> This would not necessarily be a big problem IF this would instead work:
>> >>
>> >> diff --git a/src/dev/termdev.cil b/src/dev/termdev.cil
>> >> index 1c0fe66..4f067db 100644
>> >> --- a/src/dev/termdev.cil
>> >> +++ b/src/dev/termdev.cil
>> >> @@ -3,21 +3,9 @@
>> >>
>> >>  (block termdev
>> >>
>> >> -       (macro appendinherited_all_chr_files ((type ARG1))
>> >> -             (allow ARG1 typeattr appendinherited_chr_file)
>> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >> -
>> >> -       (macro readwriteinherited_all_chr_files ((type ARG1))
>> >> -             (allow ARG1 typeattr readwriteinherited_chr_file)
>> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >> -
>> >>         (macro type ((type ARG1))
>> >>               (typeattributeset typeattr ARG1))
>> >>
>> >> -       (macro writeinherited_all_chr_files ((type ARG1))
>> >> -             (allow ARG1 typeattr writeinherited_chr_file)
>> >> -             (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >> -
>> >>         (typeattribute typeattr)
>> >>
>> >>         (blockinherit .file.all_macro_template_chr_files)
>> >> @@ -33,3 +21,12 @@
>> >>
>> >>               (allow typeattr termdev.typeatt
>> >>                      (chr_file (not (execmod mounton))))))
>> >> +
>> >> +(in termdev.appendinherited_all_chr_files
>> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >> +
>> >> +(in termdev.readwriteinherited_all_chr_files
>> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >> +
>> >> +(in termdev.writeinherited_all_chr_files
>> >> +    (allowx ARG1 typeattr (ioctl chr_file (not (0x5412)))))
>> >>
>> >> But the above in-statements cannot be resolved.
>> >>
>> >
>> > I think that overriding the macros would make more sense, but I'll
>> > have to think about it.
>> >
>> > It would be a pain, but you could do something like:
>> > (block B1
>> >   (macro M1 (...)
>> >     ...
>> >   )
>> >   (macro M2 (...)
>> >     ...
>> >   )
>> > )
>> >
>> > (block B2
>> >   (block parent
>> >     (blockinherit B1)
>> >   )
>> >   (macro M1 (...)
>> >     (call parent.M1 (...))
>> >     ...
>> >   )
>> >   (macro M2 (...)
>> >     (call parent.MA (...))
>> >     ...
>> >   )
>> > )
>> >
>> >> This is not the only instance where this approach does not work. I also
>> >> have templates that declare blocks. I use to be allowed to re-declare
>> >> these blocks so that I could add to them but this is no longer
>> >> allowed. However these blocks also cannot be resolved outside of the
>> >> templates, so I cannot use "in" to reference them.
>> >>
>> >> It seems as if the "in" blocks are resolved before the "blockinherit"
>> >> blocks are expanded.
>> >>
>> >
>> > It is true that in-statements are resolved before inheritance, so what
>> > you have above would not work.
>> >
>> > If you are just adding rules and not declarations, then you don't have
>> > to put the rules in the block.
>> > Example:
>> > (block B1
>> >   (block B
>> >     (type t)
>> >    )
>> > )
>> > (block B2
>> >   (blockinherit B1)
>> >   (allow B.t self (CLASS (PERM)))
>> > )
>>
>> Ive been trying to translate this to a actual use-case in my policy to
>> see if that would suffice.
>>
>> I have these high level "content" templates that basically allow you to
>> inherit a whole skeleton without filecon's because a skeleton can be
>> generic but filecon's are specific so they have to be added later.
>>
>> (block skel
>>   (type type)
>>   (allow ...)
>>   (block conf
>>   (type type)
>>   (allow ...))
>>   (block exec
>>   (type type)
>>   (allow ,,,))
>>   etc...)
>>
>> These skeletons can then be inherited and the filecons (and custom rules)
>> can be added later:
>>
>> (block myapp
>>   (blockinherit skel)
>>   (allow ...))
>>
>> With 3.2 I was able to mimic in-statements by re-declaring the
>> blocks so that I would be able to insert the filecons (and custom rules)
>> later:
>>
>> (block myapp
>>   (blockinherit shel)
>>   (allow ...)
>>   (block conf
>>     (filecon "/etc/myapp" file file_context))
>>   (block exec
>>     (filecon "/bin/myapp" file file_context)))
>>
>> If in-statements would be resolved after inheritance then I could do:
>>
>> (block myapp
>>   (blockiherit skel)
>>   (allow ...))
>>
>> (in myapp.conf
>>   (filecon "/etc/myapp" file file_context))
>>
>> (in myapp.exec
>>   (filecon "/bin/myapp" file file_context))
>>
>> Your suggestion would be suffice if I would use raw filecons instead of
>> context:
>>
>> (block myapp
>>   (blockinherit skel)
>>   (allow ...)
>>   (filecon "/etc/myapp" file (sys.id sys.role myapp.conf.type
>>   ((s0)(s0))))
>>   (filecon "/bin/myapp" file (sys.id sys.role myapp.exec.type
>>   ((s0)(s0)))))
>>
>> But then that would be yet another inconsistency that would be hard to explain:
>> Can only use file_context keyword in filecon if you don't use
>> inheritance.
>>
>> Not to mention macro's. If there is one thing I dislike then its having to
>> duplicate macros everywhere. I like to template macros from the ground
>> up to reduce points of failure and to encourage consistency. This
>> implies inheritance as well.
>>
>> I try not to use high level abstractions like these myself, but the idea
>> is that users of my policy are able to create/script higher level abstractions
>> on top of my policy. So that they can for example create policy
>> generation scripts using higher level templates.
>>
>> This is why consistency matters to me. I can get used to buts-and-ifs but can
>> the end-user of my policy do that as well?
>>
>> Eventually I envision something like:
>>
>> ;; generic high level base template used in a script created by end-users
>> (block myapp
>>   (blockinherit skel))
>>
>> ;; custom stuff added by the script based on script input
>> (in myapp
>>   (allow type self (class (perm))))
>>
>> (in myapp.conf
>>   (filecon "/etc/myapp" file file_context)
>>   (call .tmp.associate (type))
>>
>> (in myapp.exec
>>   (filecon "/bin/myapp" file file_context))
>>
>> (in user
>>   (call .myapp.run (type)))
>>
>
> I am definitely sympathetic to this use case and feel that you have
> identified a gap in CIL's capabilities that should be filled. But like
> I said, it will have to have a different name. I don't think that it
> will be too hard to add, but we'll see.

Yes sure I don't mind the name. Just wanted to ensure we arent
overlooking something.

>
> Thanks for details,
> Jim
>
>> >
>> > It might be possible to attempt to resolve all in-statements before
>> > inheritance as what currently happens and then try to resolve any
>> > remaining in-statements after inheritance has been resolved.
>> >
>> > I'll have to think about the problem a bit.
>> >
>> > Thanks for the questions,
>> > Jim
>> >
>> >
>> >
>> > The problem
>> >> [1] https://src.fedoraproject.org/rpms/libsepol/c/c59879b8aa30ceb601ac4e449ee5e958c6659fbc?branch=rawhide
>> >>
>> >> --
>> >> gpg --locate-keys dominick.grift@defensec.nl
>> >> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> >> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> >> Dominick Grift
>>
>> --
>> gpg --locate-keys dominick.grift@defensec.nl
>> Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
>> https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
>> Dominick Grift

-- 
gpg --locate-keys dominick.grift@defensec.nl
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift