On Thu, Jun 1, 2023 at 10:59 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Wed, May 31, 2023 at 7:51 AM Juraj Marcin <juraj@xxxxxxxxxxxxxxx> wrote:
> >
> > This patch extends the structures for module and base policy (avrule_t)
> > to support prefix/suffix transitions. In addition to this, it implements
> > the necessary changes to functions for reading and writing the binary
> > policy as well as parsing the policy conf.
> >
>
> I would like to see an example of the new syntax.
> Something like:
> type_transition ta tb : CLASS03 tc "file03" match_exact;
> type_transition ta tb : CLASS04 tc "file04" match_prefix;
> type_transition ta tb : CLASS05 tc "file05" match_suffix;
>
> > Reviewed-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > Signed-off-by: Juraj Marcin <juraj@xxxxxxxxxxxxxxx>
> > ---
> > checkpolicy/policy_define.c | 13 ++++---
> > checkpolicy/policy_define.h | 2 +-
> > checkpolicy/policy_parse.y | 15 +++++---
> > checkpolicy/policy_scan.l | 6 ++++
> > checkpolicy/test/dismod.c | 14 ++++++++
> > checkpolicy/test/dispol.c | 2 +-
> > libsepol/cil/src/cil_binary.c | 4 ++-
> > libsepol/include/sepol/policydb/avtab.h | 1 +
> > libsepol/include/sepol/policydb/policydb.h | 13 ++++---
> > libsepol/src/avtab.c | 30 ++++++++++++----
> > libsepol/src/expand.c | 6 +++-
> > libsepol/src/kernel_to_common.h | 2 +-
> > libsepol/src/link.c | 1 +
> > libsepol/src/module_to_cil.c | 25 +++++++++++---
> > libsepol/src/policydb.c | 23 ++++++++++++-
> > libsepol/src/write.c | 40 ++++++++++++++++------
> > 16 files changed, 154 insertions(+), 43 deletions(-)
<snip>
> > @@ -452,13 +453,19 @@ cond_dontaudit_def : DONTAUDIT names names ':' names names ';'
> > ;
> > ;
> > transition_def : TYPE_TRANSITION names names ':' names identifier filename ';'
> > - {if (define_compute_type(AVRULE_TRANSITION, 1)) return -1; }
> > + {if (define_compute_type(AVRULE_TRANSITION, 1, NAME_TRANS_MATCH_EXACT)) return -1;}
> > + | TYPE_TRANSITION names names ':' names identifier filename MATCH_EXACT ';'
> > + {if (define_compute_type(AVRULE_TRANSITION, 1, NAME_TRANS_MATCH_EXACT)) return -1;}
> > + | TYPE_TRANSITION names names ':' names identifier filename MATCH_PREFIX ';'
> > + {if (define_compute_type(AVRULE_TRANSITION, 1, NAME_TRANS_MATCH_PREFIX)) return -1;}
> > + | TYPE_TRANSITION names names ':' names identifier filename MATCH_SUFFIX ';'
> > + {if (define_compute_type(AVRULE_TRANSITION, 1, NAME_TRANS_MATCH_SUFFIX)) return -1;}
> > | TYPE_TRANSITION names names ':' names identifier ';'
> > - {if (define_compute_type(AVRULE_TRANSITION, 0)) return -1;}
> > + {if (define_compute_type(AVRULE_TRANSITION, 0, NAME_TRANS_MATCH_EXACT)) return -1;}
> > | TYPE_MEMBER names names ':' names identifier ';'
> > - {if (define_compute_type(AVRULE_MEMBER, 0)) return -1;}
> > + {if (define_compute_type(AVRULE_MEMBER, 0, NAME_TRANS_MATCH_EXACT)) return -1;}
> > | TYPE_CHANGE names names ':' names identifier ';'
> > - {if (define_compute_type(AVRULE_CHANGE, 0)) return -1;}
> > + {if (define_compute_type(AVRULE_CHANGE, 0, NAME_TRANS_MATCH_EXACT)) return -1;}
> > ;
> > range_trans_def : RANGE_TRANSITION names names mls_range_def ';'
> > { if (define_range_trans(0)) return -1; }
> > diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
> > index 9fefea7b..3f568701 100644
> > --- a/checkpolicy/policy_scan.l
> > +++ b/checkpolicy/policy_scan.l
> > @@ -125,6 +125,12 @@ EXPANDATTRIBUTE |
> > expandattribute { return(EXPANDATTRIBUTE); }
> > TYPE_TRANSITION |
> > type_transition { return(TYPE_TRANSITION); }
> > +MATCH_EXACT |
> > +match_exact { return(MATCH_EXACT); }
> > +MATCH_PREFIX |
> > +match_prefix { return(MATCH_PREFIX); }
> > +MATCH_SUFFIX |
> > +match_suffix { return(MATCH_SUFFIX); }
>
> I would prefer just "exact", "prefix", and "suffix" without the
> "match_" prefix, but I can live with it if others think that the
> shorter keywords will cause problems.
I slightly prefer the "match_" in the keyword, since it makes the
semantic more clear when reading the policy (especially for people
that are only learning SELinux). But I guess in the case of "prefix"
and "suffix" it doesn't make as much difference as in the case of
"exact". Which leads me to the question whether we want to even add
the redundant "exact"/"match_exact" keyword as opposed to just leaving
the current syntax (which we need to keep either way for
compatibility). IIRC, when we discussed this with Juraj, we were not
sure which way to go, so we are open to dropping it if that's
preferred by others.
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
