Re: [PATCH] checkpolicy: Fix MLS users in optional blocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 12 Aug 2024 at 21:41, 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;
> }

Maybe add this optional block to checkpolicy/tests/policy_allonce_mls.conf?

> user USER1 roles ROLE1 level SENS1 range SENS1;
> sid kernel USER1:ROLE1:TYPE1:SENS1
>
> Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> ---
>  checkpolicy/policy_define.c | 76 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index bfeda86b..93a1397e 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -4175,6 +4175,55 @@ 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, *dcat;
> +
> +       dcat = *dst;

Does this always work?
*dst is the pointer to the first category (which might be NULL).
What if the list is not empty, so dcat->next is not NULL, then we
would override it later and lose the existing tail of the list.

> +       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;
> +
> +               if (dcat)
> +                       dcat->next = new;
> +               else
> +                       *dst = new;
> +               dcat = new;
> +
> +               src = src->next;
> +       }
> +
> +       return 0;
> +}
> +
> +static int mls_add_or_check_level(mls_semantic_level_t *dst, mls_semantic_level_t *src)

src can be declared const

> +{
> +       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 +4282,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 +4307,15 @@ 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 +4342,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 +4368,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 +4389,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
>
>




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux