Re: [PATCH] libsepol: Expand role attributes in constraint expressions

[Nicolas Iooss <nicolas.iooss@m4x.org>]

On Wed, Mar 10, 2021 at 9:16 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Am Mi., 10. März 2021 um 20:30 Uhr schrieb James Carter <jwcart2@gmail.com>:
> >
> > When creating the kernel binary policy, role attributes in constraint
> > expressions are not expanded. This causes the constraint expression
> > to refer to a non-existent role in the kernel policy. This can lead
> > to a segfault when converting the binary policy back to conf or CIL
> > source or when using policy tools such as seinfo.
> >
> > Expand role attributes in constraint expressions when creating the
> > kernel binary policy.
>
>
> Thanks for the quick fix.
> Tested role attribute constraints with bare 3.2, leading to setfiles
> failing with `libsepol.validate_constraint_nodes: Invalid constraint
> expr`.
> Works fine with this patch.
> Also seinfo does not crash on the newly generated policy anymore.
>
> Tested-by: Christian Göttsche <cgzones@googlemail.com>
>
>
> >
> > Reported-by: Christian Göttsche <cgzones@googlemail.com>
> > Signed-off-by: James Carter <jwcart2@gmail.com>

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks,
Nicolas

> > ---
> >  libsepol/src/expand.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> > index eac7e450..2d9cb566 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -71,6 +71,38 @@ static int map_ebitmap(ebitmap_t * src, ebitmap_t * dst, uint32_t * map)
> >         return 0;
> >  }
> >
> > +static int ebitmap_expand_roles(policydb_t *p, ebitmap_t *roles)
> > +{
> > +       ebitmap_node_t *node;
> > +       unsigned int bit;
> > +       role_datum_t *role;
> > +       ebitmap_t tmp;
> > +
> > +       ebitmap_init(&tmp);
> > +       ebitmap_for_each_positive_bit(roles, node, bit) {
> > +               role = p->role_val_to_struct[bit];
> > +               assert(role);
> > +               if (role->flavor != ROLE_ATTRIB) {
> > +                       if (ebitmap_set_bit(&tmp, bit, 1)) {
> > +                               ebitmap_destroy(&tmp);
> > +                               return -1;
> > +                       }
> > +               } else {
> > +                       if (ebitmap_union(&tmp, &role->roles)) {
> > +                               ebitmap_destroy(&tmp);
> > +                               return -1;
> > +                       }
> > +               }
> > +       }
> > +       ebitmap_destroy(roles);
> > +       if (ebitmap_cpy(roles, &tmp)) {
> > +               ebitmap_destroy(&tmp);
> > +               return -1;
> > +       }
> > +       ebitmap_destroy(&tmp);
> > +       return 0;
> > +}
> > +
> >  static int type_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
> >                               void *data)
> >  {
> > @@ -333,6 +365,9 @@ static int constraint_node_clone(constraint_node_t ** dst,
> >                                         if (map_ebitmap(&expr->names, &new_expr->names, state->rolemap)) {
> >                                                 goto out_of_mem;
> >                                         }
> > +                                       if (ebitmap_expand_roles(state->out, &new_expr->names)) {
> > +                                               goto out_of_mem;
> > +                                       }
> >                                 } else if (new_expr->attr & CEXPR_USER) {
> >                                         if (map_ebitmap(&expr->names, &new_expr->names, state->usermap)) {
> >                                                 goto out_of_mem;
> > --
> > 2.26.2
> >