On Sun, Mar 14, 2021 at 8:59 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> On Mon, Mar 8, 2021 at 9:50 PM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > Roles in an optional block have two datums, one in the global block
> > and one in the avrule_decl where it is declared. The datum in the
> > global block does not have its dominace set. This is a problem because
> > the function set_user_role() sets the user's roles based on the global
> > datum's dominance ebitmap. If a user is declared with an associated role
> > that was declared in an optional block, then it will not have any roles
> > set for it because the dominance ebitmap is empty.
> >
> > Example/
> > # handle_unknown deny
> > class CLASS1
> > sid kernel
> > class CLASS1 { PERM1 }
> > type TYPE1;
> > allow TYPE1 self:CLASS1 PERM1;
> > role ROLE1;
> > role ROLE1 types { TYPE1 };
> > optional {
> > require {
> > class CLASS1 { PERM1 };
> > }
> > role ROLE1A;
> > user USER1A roles ROLE1A;
> > }
> > user USER1 roles ROLE1;
> > sid kernel USER1:ROLE1:TYPE1
> >
> > In this example, USER1A would not have ROLE1A associated with it.
> >
> > Instead of using dominance, which has been deprecated anyway, just
> > set the bit corresponding to the role's value in the user's roles
> > ebitmap in set_user_role().
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> > ---
> > checkpolicy/policy_define.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index c9286f77..6c035716 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -4088,8 +4088,6 @@ cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void *arg2)
> > static int set_user_roles(role_set_t * set, char *id)
> > {
> > role_datum_t *r;
> > - unsigned int i;
> > - ebitmap_node_t *node;
> >
> > if (strcmp(id, "*") == 0) {
> > free(id);
> > @@ -4115,12 +4113,9 @@ static int set_user_roles(role_set_t * set, char *id)
> > return -1;
> > }
> >
> > - /* set the role and every role it dominates */
> > - ebitmap_for_each_positive_bit(&r->dominates, node, i) {
> > - if (ebitmap_set_bit(&set->roles, i, TRUE))
> > - goto oom;
> > - }
> > free(id);
> > + if (ebitmap_set_bit(&set->roles, r->s.value-1, TRUE))
> > + goto oom;
> > return 0;
> > oom:
> > yyerror("out of memory");
>
> In other places there are spaces between the subtraction operator
> ("r->s.value - 1") and it would be nicer if this patch also used
> spaces.
> This is a minor comment anyway, and this patch looks good to me.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
Merged, with added spaces.
Thanks!
Nicolas
