All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libsepol: Expand role attributes in constraint expressions
@ 2021-03-10 19:30 James Carter
  2021-03-10 20:15 ` Christian Göttsche
  0 siblings, 1 reply; 4+ messages in thread
From: James Carter @ 2021-03-10 19:30 UTC (permalink / raw)
  To: selinux; +Cc: James Carter, Christian Göttsche

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.

Reported-by: Christian Göttsche <cgzones@googlemail.com>
Signed-off-by: James Carter <jwcart2@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] libsepol: Expand role attributes in constraint expressions
  2021-03-10 19:30 [PATCH] libsepol: Expand role attributes in constraint expressions James Carter
@ 2021-03-10 20:15 ` Christian Göttsche
  2021-03-14 20:04   ` Nicolas Iooss
  0 siblings, 1 reply; 4+ messages in thread
From: Christian Göttsche @ 2021-03-10 20:15 UTC (permalink / raw)
  To: James Carter; +Cc: SElinux list

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>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] libsepol: Expand role attributes in constraint expressions
  2021-03-10 20:15 ` Christian Göttsche
@ 2021-03-14 20:04   ` Nicolas Iooss
  2021-03-15 21:06     ` Nicolas Iooss
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Iooss @ 2021-03-14 20:04 UTC (permalink / raw)
  To: Christian Göttsche, James Carter; +Cc: SElinux list

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
> >


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] libsepol: Expand role attributes in constraint expressions
  2021-03-14 20:04   ` Nicolas Iooss
@ 2021-03-15 21:06     ` Nicolas Iooss
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Iooss @ 2021-03-15 21:06 UTC (permalink / raw)
  To: Christian Göttsche, James Carter; +Cc: SElinux list

On Sun, Mar 14, 2021 at 9:04 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> 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>

Merged.

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
> > >


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-15 21:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 19:30 [PATCH] libsepol: Expand role attributes in constraint expressions James Carter
2021-03-10 20:15 ` Christian Göttsche
2021-03-14 20:04   ` Nicolas Iooss
2021-03-15 21:06     ` Nicolas Iooss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.