On Fri, Nov 15, 2024 at 8:58 AM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Wed, Aug 28, 2024 at 11:16 AM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > When a user is created in an optional block, a user datum is added
> > to both the avrule_decl's symtab and the policydb's symtab, but
> > the semantic MLS information is only added to the avrule_decl's
> > user datum. This causes an error to occur during policy expansion
> > when user_copy_callback() is called. If this error did not occur
> > then the policydb's user datum would be written without any MLS
> > info and the policy would fail validation when read later.
> >
> > When creating a user datum, search for a user datum with the same
> > key in the policydb's symtab. If that datum has no MLS information,
> > then copy the MLS information from the avrule_decl's datum. If it
> > does, then compare the default level, low level, and high level
> > sensitivities and give an error if they do not match. There is not
> > enough information to expand the categories for the high and low
> > levels, so merge the semantic categories. If the two category sets
> > are not equal an error will occur during the expansion phase.
> >
> > A minimum policy to demonstrate the bug:
> > class CLASS1
> > sid kernel
> > class CLASS1 { PERM1 }
> > sensitivity SENS1;
> > dominance { SENS1 }
> > level SENS1;
> > mlsconstrain CLASS1 { PERM1 } ((h1 dom h2) and (l1 domby h1));
> > type TYPE1;
> > allow TYPE1 self : CLASS1 PERM1;
> > role ROLE1;
> > role ROLE1 types TYPE1;
> > optional {
> > require {
> > role ROLE1;
> > }
> > user USER2 roles ROLE1 level SENS1 range SENS1;
> > }
> > user USER1 roles ROLE1 level SENS1 range SENS1;
> > sid kernel USER1:ROLE1:TYPE1:SENS1
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
>
> I plan on merging this soon.
> Jim
>
This patch has been merged.
Jim
> > ---
> > v2:
> > - Fixed mls_semantic_cats_merge() so that it keeps existing cats in dst
> > - Made src const in mls_add_or_check_level()
> > v3:
> > - Free id in the error path
> >
> > checkpolicy/policy_define.c | 72 ++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 71 insertions(+), 1 deletion(-)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index bfeda86b..af8d007c 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -4175,6 +4175,50 @@ static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats
> > return 0;
> > }
> >
> > +static int mls_semantic_cats_merge(mls_semantic_cat_t ** dst,
> > + const mls_semantic_cat_t * src)
> > +{
> > + mls_semantic_cat_t *new;
> > +
> > + while (src) {
> > + new = (mls_semantic_cat_t *) malloc(sizeof(mls_semantic_cat_t));
> > + if (!new)
> > + return -1;
> > +
> > + mls_semantic_cat_init(new);
> > + new->low = src->low;
> > + new->high = src->high;
> > + new->next = *dst;
> > + *dst = new;
> > +
> > + src = src->next;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mls_add_or_check_level(mls_semantic_level_t *dst, const mls_semantic_level_t *src)
> > +{
> > + if (!dst->sens) {
> > + if (mls_semantic_level_cpy(dst, src) < 0) {
> > + yyerror("out of memory");
> > + return -1;
> > + }
> > + } else {
> > + if (dst->sens != src->sens) {
> > + return -1;
> > + }
> > + /* Duplicate cats won't cause problems, but different cats will
> > + * result in an error during expansion */
> > + if (mls_semantic_cats_merge(&dst->cat, src->cat) < 0) {
> > + yyerror("out of memory");
> > + return -1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int parse_semantic_categories(char *id, level_datum_t * levdatum __attribute__ ((unused)),
> > mls_semantic_cat_t ** cats)
> > {
> > @@ -4233,7 +4277,7 @@ static int parse_semantic_categories(char *id, level_datum_t * levdatum __attrib
> > int define_user(void)
> > {
> > char *id;
> > - user_datum_t *usrdatum;
> > + user_datum_t *usrdatum, *usr_global;
> > level_datum_t *levdatum;
> > int l;
> >
> > @@ -4258,10 +4302,16 @@ int define_user(void)
> > return 0;
> > }
> >
> > + id = strdup(queue_head(id_queue));
> > +
> > if ((usrdatum = declare_user()) == NULL) {
> > + free(id);
> > return -1;
> > }
> >
> > + usr_global = hashtab_search(policydbp->p_users.table, (hashtab_key_t) id);
> > + free(id);
> > +
> > while ((id = queue_remove(id_queue))) {
> > if (set_user_roles(&usrdatum->roles, id))
> > return -1;
> > @@ -4288,6 +4338,7 @@ int define_user(void)
> > usrdatum->dfltlevel.sens = levdatum->level->sens;
> >
> > while ((id = queue_remove(id_queue))) {
> > + /* This will add to any already existing categories */
> > if (parse_semantic_categories(id, levdatum,
> > &usrdatum->dfltlevel.cat)) {
> > free(id);
> > @@ -4313,6 +4364,7 @@ int define_user(void)
> > usrdatum->range.level[l].sens = levdatum->level->sens;
> >
> > while ((id = queue_remove(id_queue))) {
> > + /* This will add to any already existing categories */
> > if (parse_semantic_categories(id, levdatum,
> > &usrdatum->range.level[l].cat)) {
> > free(id);
> > @@ -4333,6 +4385,24 @@ int define_user(void)
> > return -1;
> > }
> > }
> > +
> > + if (usr_global && usr_global != usrdatum) {
> > + if (mls_add_or_check_level(&usr_global->dfltlevel,
> > + &usrdatum->dfltlevel)) {
> > + yyerror("Problem with user default level");
> > + return -1;
> > + }
> > + if (mls_add_or_check_level(&usr_global->range.level[0],
> > + &usrdatum->range.level[0])) {
> > + yyerror("Problem with user low level");
> > + return -1;
> > + }
> > + if (mls_add_or_check_level(&usr_global->range.level[1],
> > + &usrdatum->range.level[1])) {
> > + yyerror("Problem with user high level");
> > + return -1;
> > + }
> > + }
> > }
> > return 0;
> > }
> > --
> > 2.46.0
> >
