linux-kernel - Re: [RFC PATCH 11/22] selinux: more strict policy parsing

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] [day] [month] [year] [list]

Message-ID: <CA+zpnLdkeMnakS_pP_F1xLd45BOmWfQqUGqqJT34OB4E+kkE9w@mail.gmail.com>
Date: Tue, 3 Dec 2024 11:34:00 +1100
From: Thiébaud Weksteen <tweek@...gle.com>
To: cgzones@...glemail.com
Cc: selinux@...r.kernel.org, Paul Moore <paul@...l-moore.com>, 
	Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej Mosnacek <omosnace@...hat.com>, 
	Jacob Satterfield <jsatterfield.linux@...il.com>, Eric Suen <ericsu@...ux.microsoft.com>, 
	Bram Bonné <brambonne@...gle.com>, 
	Canfeng Guo <guocanfeng@...ontech.com>, Casey Schaufler <casey@...aufler-ca.com>, 
	GUO Zihua <guozihua@...wei.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 11/22] selinux: more strict policy parsing

On Sat, Nov 16, 2024 at 12:37 AM Christian Göttsche
<cgoettsche@...tendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@...glemail.com>
>
> Be more strict during parsing of policies and reject invalid values.
>
> Add some error messages in the case of policy parse failures, to
> enhance debugging, either on a malformed policy or a too strict check.
>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
> ---

Thanks for the patch.

> +               switch (xperms.specified) {
> +               case AVTAB_XPERMS_IOCTLFUNCTION:
> +               case AVTAB_XPERMS_IOCTLDRIVER:
> +               case AVTAB_XPERMS_NLMSG:
> +                       break;
> +               default:
> +                       pr_err("SELinux: avtab: invalid xperm specifier %#x\n", xperms.specified);
> +                       return -EINVAL;
> +               }
>                 rc = next_entry(&xperms.driver, fp, sizeof(u8));

I think this is too restrictive. We should be able to add extended
permissions in a future policy and this should be gracefully handled
by the kernel. You could use a pr_info instead, similarly to what is
done in selinux_set_mapping for unknown permissions.