Re: libsepol regressions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dominick Grift <dominick.grift@xxxxxxxxxxx> writes:

> James Carter <jwcart2@xxxxxxxxx> writes:
>
>> On Sun, Aug 1, 2021 at 10:32 AM Dominick Grift
>> <dominick.grift@xxxxxxxxxxx> 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@xxxxxxxxxxx
>>> 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@xxxxxxxxxxx
Key fingerprint = FCD2 3660 5D6B 9D27 7FC6  E0FF DA7E 521F 10F6 4098
https://sks-keyservers.net/pks/lookup?op=get&search=0xDA7E521F10F64098
Dominick Grift



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux