Re: [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions

From: Ondrej Mosnacek <omosnace@redhat.com>
To: "Christian Göttsche" <cgzones@googlemail.com>
Cc: SElinux list <selinux@vger.kernel.org>,
	Dominick Grift <dominick.grift@defensec.nl>
Subject: Re: [PATCH userspace 2/2] libsepol,checkpolicy: add support for self keyword in type transitions
Date: Mon, 25 Apr 2022 13:32:20 +0200	[thread overview]
Message-ID: <CAFqZXNsRR+e1gn2ktGRNXNy1inSfVZtMmp3Lhq13TmjNPVOfFg@mail.gmail.com> (raw)
In-Reply-To: <CAJ2a_DcOi6QmRPYq1CtyP15STrnK6KgPKqJ-qpyOZ_QDWELvsQ@mail.gmail.com>

On Sat, Apr 23, 2022 at 9:43 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
> On Fri, 22 Apr 2022 at 17:44, Ondrej Mosnacek <omosnace@redhat.com> 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.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  checkpolicy/policy_define.c                | 42 ++++++++++++-
> >  libsepol/include/sepol/policydb/policydb.h |  4 +-
> >  libsepol/src/expand.c                      | 69 ++++++++++++++--------
> >  libsepol/src/link.c                        |  1 +
> >  libsepol/src/module_to_cil.c               | 30 ++++++----
> >  libsepol/src/policydb.c                    | 33 +++++++++--
> >  libsepol/src/write.c                       | 19 +++---
> >  7 files changed, 148 insertions(+), 50 deletions(-)
> >
[...]
> > diff --git a/libsepol/src/write.c b/libsepol/src/write.c
> > index d7ac2b25..0b4f5d9a 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)
> > @@ -1962,10 +1963,14 @@ static int filename_trans_rule_write(filename_trans_rule_t * t, struct policy_fi
> >
> >                 buf[0] = cpu_to_le32(ftr->tclass);
> >                 buf[1] = cpu_to_le32(ftr->otype);
> > +               buf[2] = cpu_to_le32(ftr->flags);
> >
> > -               items = put_entry(buf, sizeof(uint32_t), 2, fp);
> > -               if (items != 2)
> > -                       return POLICYDB_ERROR;
> > +               if (p->policyvers >= MOD_POLICYDB_VERSION_SELF_TYPETRANS)
> > +                       entries = 3;
> > +               else
> > +                       entries = 2;
> > +
> > +               items = put_entry(buf, sizeof(uint32_t), entries, fp);
>
> + if (items != entries)
> +     return POLICYDB_ERROR;
>
> >         }
> >         return POLICYDB_SUCCESS;
> >  }
> > @@ -2039,7 +2044,7 @@ static int avrule_decl_write(avrule_decl_t * decl, int num_scope_syms,
> >         }
> >
> >         if (p->policyvers >= MOD_POLICYDB_VERSION_FILENAME_TRANS &&
> > -           filename_trans_rule_write(decl->filename_trans_rules, fp))
> > +           filename_trans_rule_write(p, decl->filename_trans_rules, fp))
> >                 return POLICYDB_ERROR;
> >
> >         if (p->policyvers >= MOD_POLICYDB_VERSION_RANGETRANS &&
> > --
> > 2.35.1
> >
>
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -1184,6 +1184,14 @@ static int
> validate_filename_trans_rules(sepol_handle_t *handle, filename_trans_
>                        goto bad;
>                if (validate_value(filename_trans->otype, &flavors[SYM_TYPES]))
>                        goto bad;
> +
> +               switch (filename_trans->flags) {
> +               case 0:
> +               case RULE_SELF:
> +                       break;
> +               default:
> +                       goto bad;
> +               }
>        }
>
>        return 0;

Good catches, thanks! I will include these changes in the next respin.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.