On Thu, Apr 28, 2022 at 9:23 PM James Carter <jwcart2@xxxxxxxxx> wrote:
> On Wed, Apr 27, 2022 at 6:14 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> >
> > With the addition of the anon_inode class in the kernel, 'self'
> > transition rules became useful, but haven't been implemented.
> >
> > This patch implements the self keyword in all 'typetransition'
> > statements at the TE language level and adds the support to the module
> > policydb format. Note that changing the kernel policydb format is not
> > necessary at all, as type transitions are always expanded in the kernel
> > policydb.
> >
>
> These patches also cause "self" to be allowed in type change and type
> member rules as well. That is fine, but it should be stated.
Good point, I'll update the commit messages.
[...]
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index d7ac2b25..5f593c1d 100644
> > --- a/libsepol/src/write.c
> > +++ b/libsepol/src/write.c
> > @@ -1929,11 +1929,12 @@ static int role_allow_rule_write(role_allow_rule_t * r, struct policy_file *fp)
> > return POLICYDB_SUCCESS;
> > }
> >
> > -static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_file *fp)
> > +static int filename_trans_rule_write(policydb_t *p, filename_trans_rule_t *t,
> > + struct policy_file *fp)
> > {
> > int nel = 0;
> > - size_t items;
> > - uint32_t buf[2], len;
> > + size_t items, entries;
> > + uint32_t buf[3], len;
> > filename_trans_rule_t *ftr;
> >
> > for (ftr = t; ftr; ftr = ftr->next)
>
>
> The flags field needs to be cleared if the policy version being
> written is < MOD_POLICYDB_VERSION_SELF_TYPETRANS.
>
> Something like:
> + if ((p->policyvers < MOD_POLICYDB_VERSION_SELF_TYPETRANS) &&
> + (ftr->flags & RULE_SELF)) {
> + WARN(fp->handle, "Module policy version %u does not support using "
> + "\"self\" in a filename type transition rule", p->policyvers);
> + ftr->flags &= ~RULE_SELF;
> + }
>
> A similar check is needed in avrule_write(), but there needs to be a
> check that the rule is an AVRULE_TYPE as well.
I agree we need to handle this case, but should we really just discard
the flag? It seems that currently avrule->ttypes is empty if
(avrule->flags & RULE_SELF) is set, so we would create an empty (or
maybe even invalid?) rule in the policy module. Setting ttypes =
stypes would not be equivalent and expanding the rule also doesn't
seem like a great idea. IMHO we should just return an error on any
attempt to downgrade a module containing self AVRULE_TYPE rules below
MOD_POLICYDB_VERSION_SELF_TYPETRANS.
What do you think?
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
