Re: [RFC PATCH v2 3/4] checkpolicy: add not-self neverallow support

[James Carter <jwcart2@gmail.com>]

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
>