| Message ID | 20211124190815.12757-3-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,v2,1/4] libsepol: introduce ebitmap_subtract() | expand |
On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > Add support for using negated or complemented self in the target type of > neverallow rules. > > Some refpolicy examples: > > neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *; > # no violations > > neverallow domain domain:file ~{ append read_file_perms write }; > > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create }; > libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; > > neverallow domain { domain -self }:file ~{ append read_file_perms write }; > > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; > libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; > > Using negated self in a complement `~{ domain -self }` is not supported. > I am thinking about what to do with this patch set. If this is valuable in checkpolicy, then I would like it in CIL as well. But CIL obviously cannot support the above syntax. What would be lost if we used "notself"? We could define the behavior of "neverallow SRC notself . . ." so that if SRC was a type, the rule would be expanded so that the target types would be all types except SRC, and if SRC was an attribute the rules would be expanded into multiple rules with all combinations of the types in SRC used for the source and target except for the cases where the source and target type would be the same. That would make your examples work. "neverallow * notself . . ." would expand like "neverallow * ~self . . ." "neverallow domain notself . . ." would expand like "neverallow domain { domain -self } . . ." "neverallow domain notself . . ." and "neverallow domain ~domain . . ." together would expand like "neverallow domain ~self . . ." (I think) What would be missing would be the ability to express "neverallow domain { subdomain -self } . . ." A few minor comments below. > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > v2: > - fix neverallowxperm usage > --- > checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++----- > checkpolicy/test/dismod.c | 6 ++++- > 2 files changed, 45 insertions(+), 7 deletions(-) > > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c > index d3eb6111..f27a6f33 100644 > --- a/checkpolicy/policy_define.c > +++ b/checkpolicy/policy_define.c > @@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) > while ((id = queue_remove(id_queue))) { > if (strcmp(id, "self") == 0) { > free(id); > - if (add == 0) { > - yyerror("-self is not supported"); > + if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) { > + yyerror("-self is only supported in neverallowxperm rules"); "-self is only supported in neverallow and neverallowxperm rules" > + ret = -1; > + goto out; > + } > + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); > + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { > + yyerror("self and -self is not supported"); > ret = -1; > goto out; > } > - avrule->flags |= RULE_SELF; > continue; > } > if (set_types > @@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) > } > } > > + if ((avrule->ttypes.flags & TYPE_COMP)) { > + if (avrule->flags & RULE_NOTSELF) { > + yyerror("-self is not supported in complements"); > + ret = -1; > + goto out; > + } > + if (avrule->flags & RULE_SELF) { > + avrule->flags &= ~RULE_SELF; > + avrule->flags |= RULE_NOTSELF; > + } > + } > + > ebitmap_init(&tclasses); > ret = read_classes(&tclasses); > if (ret) > @@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) > while ((id = queue_remove(id_queue))) { > if (strcmp(id, "self") == 0) { > free(id); > - if (add == 0) { > - yyerror("-self is not supported"); > + if (add == 0 && which != AVRULE_NEVERALLOW) { > + yyerror("-self is only supported in neverallow rules"); "-self is only supported in neverallow and neverallowxperm rules" Thanks, Jim > + ret = -1; > + goto out; > + } > + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); > + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { > + yyerror("self and -self is not supported"); > ret = -1; > goto out; > } > - avrule->flags |= RULE_SELF; > continue; > } > if (set_types > @@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) > } > } > > + if ((avrule->ttypes.flags & TYPE_COMP)) { > + if (avrule->flags & RULE_NOTSELF) { > + yyerror("-self is not supported in complements"); > + ret = -1; > + goto out; > + } > + if (avrule->flags & RULE_SELF) { > + avrule->flags &= ~RULE_SELF; > + avrule->flags |= RULE_NOTSELF; > + } > + } > + > ebitmap_init(&tclasses); > ret = read_classes(&tclasses); > if (ret) > diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c > index ec2a3e9a..a2d74d42 100644 > --- a/checkpolicy/test/dismod.c > +++ b/checkpolicy/test/dismod.c > @@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic > } > > num_types = 0; > - if (flags & RULE_SELF) { > + if (flags & (RULE_SELF | RULE_NOTSELF)) { > num_types++; > } > > @@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic > fprintf(fp, " self"); > } > > + if (flags & RULE_NOTSELF) { > + fprintf(fp, " -self"); > + } > + > if (num_types > 1) > fprintf(fp, " }"); > > -- > 2.34.0 >
On Fri, 3 Dec 2021 at 22:56, James Carter <jwcart2@gmail.com> wrote: > > On Thu, Nov 25, 2021 at 3:03 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Add support for using negated or complemented self in the target type of > > neverallow rules. > > > > Some refpolicy examples: > > > > neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *; > > # no violations > > > > neverallow domain domain:file ~{ append read_file_perms write }; > > > > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; > > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create }; > > libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; > > > > neverallow domain { domain -self }:file ~{ append read_file_perms write }; > > > > libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; > > libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; > > > > Using negated self in a complement `~{ domain -self }` is not supported. > > > > I am thinking about what to do with this patch set. If this is > valuable in checkpolicy, then I would like it in CIL as well. But CIL > obviously cannot support the above syntax. > > What would be lost if we used "notself"? > > We could define the behavior of "neverallow SRC notself . . ." so that > if SRC was a type, the rule would be expanded so that the target types > would be all types except SRC, and if SRC was an attribute the rules > would be expanded into multiple rules with all combinations of the > types in SRC used for the source and target except for the cases where > the source and target type would be the same. > That would make your examples work. > "neverallow * notself . . ." would expand like "neverallow * ~self . . ." > "neverallow domain notself . . ." would expand like "neverallow domain > { domain -self } . . ." > "neverallow domain notself . . ." and "neverallow domain ~domain . . > ." together would expand like "neverallow domain ~self . . ." (I > think) > > What would be missing would be the ability to express "neverallow > domain { subdomain -self } . . ." > I would suggest to introduce `all` as a new keyword for neverallow rules. Yes, this would break backwards compatibility, but in the long run it makes a saner language and with appropriate communication and good CIL error messages the transition should be manageable. Then one could write something like: neverallow * ~self:process fork; (neverallow all (not self) (process (fork))) neverallow domain { domain -self -foo_t }:dir create; (neverallow domain (and domain (not (and self foo_t))) (dir (create))) > A few minor comments below. > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > v2: > > - fix neverallowxperm usage > > --- > > checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++----- > > checkpolicy/test/dismod.c | 6 ++++- > > 2 files changed, 45 insertions(+), 7 deletions(-) > > > > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c > > index d3eb6111..f27a6f33 100644 > > --- a/checkpolicy/policy_define.c > > +++ b/checkpolicy/policy_define.c > > @@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) > > while ((id = queue_remove(id_queue))) { > > if (strcmp(id, "self") == 0) { > > free(id); > > - if (add == 0) { > > - yyerror("-self is not supported"); > > + if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) { > > + yyerror("-self is only supported in neverallowxperm rules"); > > "-self is only supported in neverallow and neverallowxperm rules" > > > + ret = -1; > > + goto out; > > + } > > + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); > > + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { > > + yyerror("self and -self is not supported"); > > ret = -1; > > goto out; > > } > > - avrule->flags |= RULE_SELF; > > continue; > > } > > if (set_types > > @@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) > > } > > } > > > > + if ((avrule->ttypes.flags & TYPE_COMP)) { > > + if (avrule->flags & RULE_NOTSELF) { > > + yyerror("-self is not supported in complements"); > > + ret = -1; > > + goto out; > > + } > > + if (avrule->flags & RULE_SELF) { > > + avrule->flags &= ~RULE_SELF; > > + avrule->flags |= RULE_NOTSELF; > > + } > > + } > > + > > ebitmap_init(&tclasses); > > ret = read_classes(&tclasses); > > if (ret) > > @@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) > > while ((id = queue_remove(id_queue))) { > > if (strcmp(id, "self") == 0) { > > free(id); > > - if (add == 0) { > > - yyerror("-self is not supported"); > > + if (add == 0 && which != AVRULE_NEVERALLOW) { > > + yyerror("-self is only supported in neverallow rules"); > > "-self is only supported in neverallow and neverallowxperm rules" > > Thanks, > Jim > > > > + ret = -1; > > + goto out; > > + } > > + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); > > + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { > > + yyerror("self and -self is not supported"); > > ret = -1; > > goto out; > > } > > - avrule->flags |= RULE_SELF; > > continue; > > } > > if (set_types > > @@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) > > } > > } > > > > + if ((avrule->ttypes.flags & TYPE_COMP)) { > > + if (avrule->flags & RULE_NOTSELF) { > > + yyerror("-self is not supported in complements"); > > + ret = -1; > > + goto out; > > + } > > + if (avrule->flags & RULE_SELF) { > > + avrule->flags &= ~RULE_SELF; > > + avrule->flags |= RULE_NOTSELF; > > + } > > + } > > + > > ebitmap_init(&tclasses); > > ret = read_classes(&tclasses); > > if (ret) > > diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c > > index ec2a3e9a..a2d74d42 100644 > > --- a/checkpolicy/test/dismod.c > > +++ b/checkpolicy/test/dismod.c > > @@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic > > } > > > > num_types = 0; > > - if (flags & RULE_SELF) { > > + if (flags & (RULE_SELF | RULE_NOTSELF)) { > > num_types++; > > } > > > > @@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic > > fprintf(fp, " self"); > > } > > > > + if (flags & RULE_NOTSELF) { > > + fprintf(fp, " -self"); > > + } > > + > > if (num_types > 1) > > fprintf(fp, " }"); > > > > -- > > 2.34.0 > >
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c index d3eb6111..f27a6f33 100644 --- a/checkpolicy/policy_define.c +++ b/checkpolicy/policy_define.c @@ -2067,12 +2067,17 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) while ((id = queue_remove(id_queue))) { if (strcmp(id, "self") == 0) { free(id); - if (add == 0) { - yyerror("-self is not supported"); + if (add == 0 && which != AVRULE_XPERMS_NEVERALLOW) { + yyerror("-self is only supported in neverallowxperm rules"); + ret = -1; + goto out; + } + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { + yyerror("self and -self is not supported"); ret = -1; goto out; } - avrule->flags |= RULE_SELF; continue; } if (set_types @@ -2083,6 +2088,18 @@ static int define_te_avtab_xperms_helper(int which, avrule_t ** rule) } } + if ((avrule->ttypes.flags & TYPE_COMP)) { + if (avrule->flags & RULE_NOTSELF) { + yyerror("-self is not supported in complements"); + ret = -1; + goto out; + } + if (avrule->flags & RULE_SELF) { + avrule->flags &= ~RULE_SELF; + avrule->flags |= RULE_NOTSELF; + } + } + ebitmap_init(&tclasses); ret = read_classes(&tclasses); if (ret) @@ -2528,12 +2545,17 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) while ((id = queue_remove(id_queue))) { if (strcmp(id, "self") == 0) { free(id); - if (add == 0) { - yyerror("-self is not supported"); + if (add == 0 && which != AVRULE_NEVERALLOW) { + yyerror("-self is only supported in neverallow rules"); + ret = -1; + goto out; + } + avrule->flags |= (add ? RULE_SELF : RULE_NOTSELF); + if ((avrule->flags & RULE_SELF) && (avrule->flags & RULE_NOTSELF)) { + yyerror("self and -self is not supported"); ret = -1; goto out; } - avrule->flags |= RULE_SELF; continue; } if (set_types @@ -2544,6 +2566,18 @@ static int define_te_avtab_helper(int which, avrule_t ** rule) } } + if ((avrule->ttypes.flags & TYPE_COMP)) { + if (avrule->flags & RULE_NOTSELF) { + yyerror("-self is not supported in complements"); + ret = -1; + goto out; + } + if (avrule->flags & RULE_SELF) { + avrule->flags &= ~RULE_SELF; + avrule->flags |= RULE_NOTSELF; + } + } + ebitmap_init(&tclasses); ret = read_classes(&tclasses); if (ret) diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c index ec2a3e9a..a2d74d42 100644 --- a/checkpolicy/test/dismod.c +++ b/checkpolicy/test/dismod.c @@ -124,7 +124,7 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic } num_types = 0; - if (flags & RULE_SELF) { + if (flags & (RULE_SELF | RULE_NOTSELF)) { num_types++; } @@ -169,6 +169,10 @@ static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * polic fprintf(fp, " self"); } + if (flags & RULE_NOTSELF) { + fprintf(fp, " -self"); + } + if (num_types > 1) fprintf(fp, " }");
Add support for using negated or complemented self in the target type of neverallow rules. Some refpolicy examples: neverallow * ~self:{ capability cap_userns capability2 cap2_userns } *; # no violations neverallow domain domain:file ~{ append read_file_perms write }; libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow chromium_t chromium_t:file { create }; libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; neverallow domain { domain -self }:file ~{ append read_file_perms write }; libsepol.report_failure: neverallow on line 565 of policy/modules/kernel/kernel.te (or line 30300 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:file { create setattr relabelfrom relabelto unlink link rename }; libsepol.report_failure: neverallow on line 564 of policy/modules/kernel/kernel.te (or line 30299 of policy.conf) violated by allow sysadm_t httpd_bugzilla_script_t:dir { create }; Using negated self in a complement `~{ domain -self }` is not supported. Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- v2: - fix neverallowxperm usage --- checkpolicy/policy_define.c | 46 ++++++++++++++++++++++++++++++++----- checkpolicy/test/dismod.c | 6 ++++- 2 files changed, 45 insertions(+), 7 deletions(-)