Re: [PATCH v3 4/8] checkpolicy: add front-end support for segregate attributes

From: "Christian Göttsche" <cgzones@googlemail.com>
To: James Carter <jwcart2@gmail.com>
Cc: selinux@vger.kernel.org
Subject: Re: [PATCH v3 4/8] checkpolicy: add front-end support for segregate attributes
Date: Fri, 11 Aug 2023 18:38:29 +0200	[thread overview]
Message-ID: <CAJ2a_DcOeDWotazAexEKGh8rLs69F8BkRRNUu8e39mDCBmvJ1w@mail.gmail.com> (raw)
In-Reply-To: <CAP+JOzTZeqY10FX8znd0bReEkszhE33YtLB0-_JDvzfHdi6fNA@mail.gmail.com>

On Mon, 8 Aug 2022 at 19:09, James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Jul 21, 2022 at 11:11 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Support specifying segregate attributes.
> >
> > The following two blocks are equivalent:
> >
> >     segregate_attributes attr1, attr2, attr3;
> >
> >     segregate_attributes attr1, attr2;
> >     segregate_attributes attr1, attr3;
> >     segregate_attributes attr2, attr3;
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  checkpolicy/policy_define.c | 66 +++++++++++++++++++++++++++++++++++++
> >  checkpolicy/policy_define.h |  1 +
> >  checkpolicy/policy_parse.y  |  5 +++
> >  checkpolicy/policy_scan.l   |  2 ++
> >  4 files changed, 74 insertions(+)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index 8bf36859..cf6fbf08 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -1220,6 +1220,72 @@ exit:
> >         return rc;
> >  }
> >
> > +int define_segregate_attributes(void)
> > +{
> > +       char *id = NULL;
> > +       segregate_attributes_rule_t *sattr = NULL;
> > +       int rc = -1;
> > +
> > +       if (pass == 1) {
> > +               while ((id = queue_remove(id_queue)))
> > +                       free(id);
> > +               return 0;
> > +       }
> > +
> > +       sattr = malloc(sizeof(segregate_attributes_rule_t));
> > +       if (!sattr) {
> > +               yyerror("Out of memory!");
> > +               goto exit;
> > +       }
> > +
> > +       ebitmap_init(&sattr->attrs);
> > +
> > +       while ((id = queue_remove(id_queue))) {
> > +               const type_datum_t *attr;
> > +
> > +               if (!is_id_in_scope(SYM_TYPES, id)) {
> > +                       yyerror2("attribute %s is not within scope", id);
> > +                       goto exit;
> > +               }
> > +
> > +               attr = hashtab_search(policydbp->p_types.table, id);
> > +               if (!attr) {
> > +                       yyerror2("attribute %s is not declared", id);
> > +                       goto exit;
> > +               }
> > +
> > +               if (attr->flavor != TYPE_ATTRIB) {
> > +                       yyerror2("%s is a type, not an attribute", id);
> > +                       goto exit;
> > +               }
> > +
>
> It seems like it would be useful to check a type, so an error would be
> given if the type is associated with the attribute.
>

I am not exactly sure what you mean.
Do you like to have a policy statement like

    nevertypeattribute TYPE ATTRIBUTE;

that checks at compile time a type is not associated with an attribute?

> > +               if (ebitmap_get_bit(&sattr->attrs, attr->s.value - 1)) {
> > +                       yyerror2("attribute %s used multiple times", id);
> > +                       goto exit;
> > +               }
> > +
> > +               if (ebitmap_set_bit(&sattr->attrs, attr->s.value - 1, TRUE)) {
> > +                       yyerror("Out of memory!");
> > +                       goto exit;
> > +               }
> > +
> > +               free(id);
> > +       }
> > +
> > +       sattr->next = policydbp->segregate_attributes;
> > +       policydbp->segregate_attributes = sattr;
> > +
> > +       sattr = NULL;
> > +       rc = 0;
> > +exit:
> > +       if (sattr) {
> > +               ebitmap_destroy(&sattr->attrs);
> > +               free(sattr);
> > +       }
> > +       free(id);
> > +       return rc;
> > +}
> > +
> >  static int add_aliases_to_type(type_datum_t * type)
> >  {
> >         char *id;
> > diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
> > index 50a7ba78..f55d0b17 100644
> > --- a/checkpolicy/policy_define.h
> > +++ b/checkpolicy/policy_define.h
> > @@ -68,6 +68,7 @@ int define_type(int alias);
> >  int define_user(void);
> >  int define_validatetrans(constraint_expr_t *expr);
> >  int expand_attrib(void);
> > +int define_segregate_attributes(void);
> >  int insert_id(const char *id,int push);
> >  int insert_separator(int push);
> >  role_datum_t *define_role_dom(role_datum_t *r);
> > diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
> > index 45f973ff..acd6096d 100644
> > --- a/checkpolicy/policy_parse.y
> > +++ b/checkpolicy/policy_parse.y
> > @@ -104,6 +104,7 @@ typedef int (* require_func_t)(int pass);
> >  %token ALIAS
> >  %token ATTRIBUTE
> >  %token EXPANDATTRIBUTE
> > +%token SEGREGATEATTRIBUTES
> >  %token BOOL
> >  %token TUNABLE
> >  %token IF
> > @@ -320,6 +321,7 @@ rbac_decl           : attribute_role_def
> >                         ;
> >  te_decl                        : attribute_def
> >                          | expandattribute_def
> > +                        | segregateattributes_def
> >                          | type_def
> >                          | typealias_def
> >                          | typeattribute_def
> > @@ -337,6 +339,9 @@ attribute_def           : ATTRIBUTE identifier ';'
> >  expandattribute_def     : EXPANDATTRIBUTE names bool_val ';'
> >                          { if (expand_attrib()) return -1;}
> >                          ;
> > +segregateattributes_def : SEGREGATEATTRIBUTES identifier ',' id_comma_list ';'
> > +                        { if (define_segregate_attributes()) return -1;}
> > +
>
> I don't see the need for comparing more than two at a time.
>
> Something like:
> disjoint_types attr1 attr2;

That would lead to quadratic growth of statements, for example in the
Reference Policy example of

    ibendport_type, packet_type, sysctl_type, device_node,
ibpkey_type, sysfs_types, domain, boolean_type, netif_type, file_type,
node_type, proc_type, port_type

Also one could see supporting more than two attributes as syntactic
sugar, which the traditional language already supports, e.g.

    allow { TYPE1 TYPE2 } { TYPE3 TYPE4 } : { CLASS1 CLASS2 } perm_list;

>
> Thanks,
> Jim
>
>                        ;
> >  type_def               : TYPE identifier alias_def opt_attr_list ';'
> >                          {if (define_type(1)) return -1;}
> >                         | TYPE identifier opt_attr_list ';'
> > diff --git a/checkpolicy/policy_scan.l b/checkpolicy/policy_scan.l
> > index 9fefea7b..d865dcb6 100644
> > --- a/checkpolicy/policy_scan.l
> > +++ b/checkpolicy/policy_scan.l
> > @@ -123,6 +123,8 @@ ATTRIBUTE |
> >  attribute                      { return(ATTRIBUTE); }
> >  EXPANDATTRIBUTE |
> >  expandattribute                 { return(EXPANDATTRIBUTE); }
> > +SEGREGATE_ATTRIBUTES |
> > +segregate_attributes           { return(SEGREGATEATTRIBUTES); }
> >  TYPE_TRANSITION |
> >  type_transition                        { return(TYPE_TRANSITION); }
> >  TYPE_MEMBER |
> > --
> > 2.36.1
> >