On Tue, Aug 3, 2021 at 5:49 AM Dominick Grift
<dominick.grift@xxxxxxxxxxx> wrote:
>
> 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.
> >
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@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
