On Fri, Aug 11, 2023 at 12:38 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> On Mon, 8 Aug 2022 at 19:09, James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > On Thu, Jul 21, 2022 at 11:11 AM Christian Göttsche
> > <cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > > ---
> > > 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;
>
The case above would be a pain to do and making it a list would be
better. I guess using a list is not that big of a deal.
The problem with checking that attributes are disjoint is that it does
not tell me *why* they should be disjoint.
It would be better to use more neverallow rules because they express
the goals of the security policy.
If a neverallow rule cannot be written to say why two attributes
should be disjoint, then either the policy is not fine-grained enough
for it to matter or there is a problem with the policy.
Jim
> >
> > 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
> > >
