linux-kernel - Re: [PATCH v3 10/14] selinux: validate symbols

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEjxPJ5NVZ4HzSm4doEpTG5pLFD-F7A3MZnrM_noxHh4b-mJAQ@mail.gmail.com>
Date: Wed, 14 May 2025 15:06:07 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: cgzones@...glemail.com
Cc: selinux@...r.kernel.org, Paul Moore <paul@...l-moore.com>, 
	Ondrej Mosnacek <omosnace@...hat.com>, linux-kernel@...r.kernel.org, 
	Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <nick.desaulniers+lkml@...il.com>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, llvm@...ts.linux.dev
Subject: Re: [PATCH v3 10/14] selinux: validate symbols

On Sun, May 11, 2025 at 1:31 PM Christian Göttsche
<cgoettsche@...tendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@...glemail.com>
>
> Some symbol tables need to be validated after indexing, since during
> indexing their referenced entries might not yet have been indexed.
>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@...il.com>

NB role dominance is already gone in checkpolicy and never supported by secilc.
At some point likely should drop corresponding kernel and libsepol support too.

> ---
>  security/selinux/ss/policydb.c | 94 ++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f8d6e993ce89..4559c8918134 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -673,6 +673,84 @@ static int (*const index_f[SYM_NUM])(void *key, void *datum, void *datap) = {
>  };
>  /* clang-format on */
>
> +static int role_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct role_datum *role = datum;
> +       struct ebitmap_node *node;
> +       u32 i;
> +
> +       ebitmap_for_each_positive_bit(&role->dominates, node, i) {
> +               if (!policydb_role_isvalid(p, i))
> +                       goto bad;
> +       }
> +
> +       ebitmap_for_each_positive_bit(&role->types, node, i) {
> +               if (!policydb_type_isvalid(p, i + 1))
> +                       goto bad;
> +       }
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid role %s\n", sym_name(p, SYM_ROLES, role->value - 1));
> +       return -EINVAL;
> +}
> +
> +static int user_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct user_datum *usrdatum = datum;
> +       struct ebitmap_node *node;
> +       u32 i;
> +
> +       ebitmap_for_each_positive_bit(&usrdatum->roles, node, i) {
> +               if (!policydb_role_isvalid(p, i))
> +                       goto bad;
> +       }
> +
> +       if (!mls_range_isvalid(p, &usrdatum->range))
> +               goto bad;
> +
> +       if (!mls_level_isvalid(p, &usrdatum->dfltlevel))
> +               goto bad;
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid user %s\n", sym_name(p, SYM_USERS, usrdatum->value - 1));
> +       return -EINVAL;
> +}
> +
> +static int sens_validate(void *key, void *datum, void *datap)
> +{
> +       const struct policydb *p = datap;
> +       const struct level_datum *levdatum = datum;
> +
> +       if (!mls_level_isvalid(p, &levdatum->level))
> +               goto bad;
> +
> +       return 0;
> +
> +bad:
> +       pr_err("SELinux:  invalid sensitivity\n");
> +       return -EINVAL;
> +}
> +
> +
> +/* clang-format off */
> +static int (*const validate_f[SYM_NUM])(void *key, void *datum, void *datap) = {
> +       NULL, /* Everything validated in common_read() and common_index() */
> +       NULL, /* Everything validated in class_read() and class_index() */
> +       role_validate,
> +       NULL, /* Everything validated in type_read(), type_index() and type_bounds_sanity_check() */
> +       user_validate,
> +       NULL, /* Everything validated in cond_read_bool() and cond_index_bool() */
> +       sens_validate,
> +       NULL, /* Everything validated in cat_read() and cat_index() */
> +};
> +/* clang-format on */
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEBUG
>  static void hash_eval(struct hashtab *h, const char *hash_name,
>                       const char *hash_details)
> @@ -765,6 +843,16 @@ static int policydb_index(struct policydb *p)
>                 if (rc)
>                         goto out;
>         }
> +
> +       for (i = 0; i < SYM_NUM; i++) {
> +               if (!validate_f[i])
> +                       continue;
> +
> +               rc = hashtab_map(&p->symtab[i].table, validate_f[i], p);
> +               if (rc)
> +                       goto out;
> +       }
> +
>         rc = 0;
>  out:
>         return rc;
> @@ -1087,6 +1175,12 @@ static int context_read_and_validate(struct context *c, struct policydb *p,
>                         pr_err("SELinux: error reading MLS range of context\n");
>                         goto out;
>                 }
> +
> +               rc = -EINVAL;
> +               if (!mls_range_isvalid(p, &c->range)) {
> +                       pr_warn("SELinux: invalid range in security context\n");
> +                       goto out;
> +               }
>         }
>
>         rc = -EINVAL;
> --
> 2.49.0
>