On Fri, Dec 16, 2022 at 1:52 PM Daniel Burgener
<dburgener@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 12/15/2022 4:34 PM, James Carter wrote:
> > I don't expect this to be part of the upcoming userspace release,
> > but I did want to see if this is going to be what Cascade needs.
>
> Thank you! I've been playing with this series locally all morning and
> so far it mostly works as expected. The only minor surprise so far is
> if I do something like:
>
> (type my_type1)
> (type my_type2)
> (type my_type3)
> (typeattributeset my_attr (my_type1 my_type2 my_type3))
> (allow my_attr my_attr (process (signal signull)))
> (deny my_type1 my_attr (process (signal)))
>
> I get rules like this:
>
> $ sesearch -A -s my_attr policy.33
> allow deny_rule_attr_11 my_attr:process { signal signull };
> allow my_attr my_attr:process signull;
>
> Since deny_rule_attr_11 is a subset of my_attr (specifically, my_type2
> and my_type3), both of those types get the "signull" permission from the
> second rule on the attribute, and only strictly need the "signal"
> permission (which my_type1 doesn't get)
>
> It's obviously not a real problem, since it's just redundant policy and
> both versions are functionally equivalent. It's just a little weird
> browsing the rules using sesearch, particularly with large permission
> sets (My first attribute test involved using "all" on the file class and
> denying one permission from a type, and it was mildly challenging to
> manually verify the results looking at the allow rules in a large list.)
>
I knew that I was creating some redundancy in the rules, but I was
trying to eliminate the special cases. Now that I have something that
appears to be working, it will probably be worth it to go back and see
how messy it would be to remove some of the redundancy.
> Anyways, besides that minor issue everything I've tried so far has
> worked well and the overall concept seems sensible and helpful.
> Although, I still need to get my head around the specifics of the
> attribute handling, and I haven't looked thoroughly at the code yet.
>
> I intend to do more thorough testing and code review, but I don't expect
> I'll have time before the holidays, so that will likely come in January.
> Since I don't have time for anything more thorough now, hopefully some
> first impressions are helpful in the interim.
>
I am going to be on vacation for the last two weeks of December, so I
won't be getting back to this until January anyway.
Thanks for the quick look and comments!
Jim
> -Daniel
>
> >
> > This series of patches implements a deny rule in CIL. A deny rule will remove
> > the stated permissions in it from the policy. CIL does this by searching for
> > allow rules that match the deny rule and then writing new allow rules that
> > correspond to the matched allow rule with the permissions from the deny rule
> > removed. The rule uses the same syntax as an allow rule, but with "deny"
> > instead of "allow".
> >
> > (deny SRC TGT (CLASS (PERMS)))
> >
> > Deny rules are processed during post processing (after the AST is resolved,
> > but before the binary policy is written). This means that neverallow checking
> > is done after deny rules are resolved. Deny rules are complimentary to
> > neverallow checking. When an allow rule is found that matches, a deny rule
> > removes permissions while a neverallow rule reports an error.
> >
> > Patch 4 is biggest and most complex since it is the one doing the processing.
> >
> > James Carter (9):
> > libsepol/cil: Parse and add deny rule to AST, but do not process
> > libsepol/cil: Add cil_list_is_empty macro
> > libsepol/cil: Add cil_tree_remove_node function
> > libsepol/cil: Process deny rules
> > libsepol/cil: Add cil_write_post_ast function
> > libsepol: Export the cil_write_post_ast function
> > secilc/secil2tree: Add option to write CIL AST after post processing
> > secilc/test: Add a deny rule test
> > secilc/docs: Add deny rule to CIL documentation
> >
> > libsepol/cil/include/cil/cil.h | 1 +
> > libsepol/cil/src/cil.c | 68 ++
> > libsepol/cil/src/cil_build_ast.c | 56 ++
> > libsepol/cil/src/cil_build_ast.h | 2 +
> > libsepol/cil/src/cil_copy_ast.c | 19 +
> > libsepol/cil/src/cil_copy_ast.h | 1 +
> > libsepol/cil/src/cil_deny.c | 957 +++++++++++++++++++++++++
> > libsepol/cil/src/cil_deny.h | 34 +
> > libsepol/cil/src/cil_flavor.h | 1 +
> > libsepol/cil/src/cil_internal.h | 10 +
> > libsepol/cil/src/cil_list.h | 3 +
> > libsepol/cil/src/cil_post.c | 7 +
> > libsepol/cil/src/cil_reset_ast.c | 8 +
> > libsepol/cil/src/cil_resolve_ast.c | 44 ++
> > libsepol/cil/src/cil_resolve_ast.h | 1 +
> > libsepol/cil/src/cil_tree.c | 27 +
> > libsepol/cil/src/cil_tree.h | 1 +
> > libsepol/cil/src/cil_verify.c | 9 +
> > libsepol/cil/src/cil_write_ast.c | 10 +
> > libsepol/cil/src/cil_write_ast.h | 1 +
> > libsepol/src/libsepol.map.in | 5 +
> > secilc/docs/cil_access_vector_rules.md | 68 ++
> > secilc/secil2tree.c | 8 +-
> > secilc/test/deny_rule_test.cil | 384 ++++++++++
> > 24 files changed, 1724 insertions(+), 1 deletion(-)
> > create mode 100644 libsepol/cil/src/cil_deny.c
> > create mode 100644 libsepol/cil/src/cil_deny.h
> > create mode 100644 secilc/test/deny_rule_test.cil
> >
>
