[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6df822d8-8413-413e-8ecd-cac1cfae4a3b@schaufler-ca.com>
Date: Wed, 13 Dec 2023 10:35:31 -0800
From: Casey Schaufler <casey@...aufler-ca.com>
To: Michael Weiß <michael.weiss@...ec.fraunhofer.de>,
Christian Brauner <brauner@...nel.org>,
Alexander Mikhalitsyn <alexander@...alicyn.com>,
Alexei Starovoitov <ast@...nel.org>,
Paul Moore <paul@...l-moore.com>
Cc: Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Quentin Monnet <quentin@...valent.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Miklos Szeredi <miklos@...redi.hu>,
Amir Goldstein <amir73il@...il.com>,
"Serge E. Hallyn" <serge@...lyn.com>, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-security-module@...r.kernel.org,
gyroidos@...ec.fraunhofer.de,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [RFC PATCH v3 3/3] devguard: added device guard for mknod in
non-initial userns
On 12/13/2023 6:38 AM, Michael Weiß wrote:
> devguard is a simple LSM to allow CAP_MKNOD in non-initial user
> namespace in cooperation of an attached cgroup device program. We
> just need to implement the security_inode_mknod() hook for this.
> In the hook, we check if the current task is guarded by a device
> cgroup using the lately introduced cgroup_bpf_current_enabled()
> helper. If so, we strip out SB_I_NODEV from the super block.
>
> Access decisions to those device nodes are then guarded by existing
> device cgroups mechanism.
>
> Signed-off-by: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
> ---
> security/Kconfig | 11 +++++----
> security/Makefile | 1 +
> security/devguard/Kconfig | 12 ++++++++++
> security/devguard/Makefile | 2 ++
> security/devguard/devguard.c | 44 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 65 insertions(+), 5 deletions(-)
> create mode 100644 security/devguard/Kconfig
> create mode 100644 security/devguard/Makefile
> create mode 100644 security/devguard/devguard.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 52c9af08ad35..7ec4017745d4 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -194,6 +194,7 @@ source "security/yama/Kconfig"
> source "security/safesetid/Kconfig"
> source "security/lockdown/Kconfig"
> source "security/landlock/Kconfig"
> +source "security/devguard/Kconfig"
>
> source "security/integrity/Kconfig"
>
> @@ -233,11 +234,11 @@ endchoice
>
> config LSM
> string "Ordered list of enabled LSMs"
> - default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
> - default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
> - default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
> - default "landlock,lockdown,yama,loadpin,safesetid,bpf" if DEFAULT_SECURITY_DAC
> - default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf"
> + default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf,devguard" if DEFAULT_SECURITY_SMACK
> + default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf,devguard" if DEFAULT_SECURITY_APPARMOR
> + default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf,devguard" if DEFAULT_SECURITY_TOMOYO
> + default "landlock,lockdown,yama,loadpin,safesetid,bpf,devguard" if DEFAULT_SECURITY_DAC
> + default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf,devguard"
> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list, except for those with order
> diff --git a/security/Makefile b/security/Makefile
> index 18121f8f85cd..82a0d8cab3c3 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/
> obj-$(CONFIG_CGROUPS) += device_cgroup.o
> obj-$(CONFIG_BPF_LSM) += bpf/
> obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/
> +obj-$(CONFIG_SECURITY_DEVGUARD) += devguard/
>
> # Object integrity file lists
> obj-$(CONFIG_INTEGRITY) += integrity/
> diff --git a/security/devguard/Kconfig b/security/devguard/Kconfig
> new file mode 100644
> index 000000000000..592684615a8f
> --- /dev/null
> +++ b/security/devguard/Kconfig
> @@ -0,0 +1,12 @@
> +config SECURITY_DEVGUARD
> + bool "Devguard for device node creation"
> + depends on SECURITY
> + depends on CGROUP_BPF
> + default n
> + help
> + This enables devguard, an LSM that allows to guard device node
> + creation in non-initial user namespace. It may allow mknod
> + in cooperation of an attached cgroup device program.
> + This security module stacks with other LSMs.
> +
> + If you are unsure how to answer this question, answer N.
> diff --git a/security/devguard/Makefile b/security/devguard/Makefile
> new file mode 100644
> index 000000000000..fdaff8dc2fea
> --- /dev/null
> +++ b/security/devguard/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_SECURITY_DEVGUARD) += devguard.o
> diff --git a/security/devguard/devguard.c b/security/devguard/devguard.c
> new file mode 100644
> index 000000000000..3a0c9c27a691
> --- /dev/null
> +++ b/security/devguard/devguard.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device guard security module
> + *
> + * Simple in-kernel LSM to allow cap_mknod in non-initial
> + * user namespace if current task is guarded by device cgroup.
> + *
> + * Copyright (C) 2023 Fraunhofer AISEC. All rights reserved.
> + *
> + * Authors: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
> + */
> +
> +#include <linux/bpf-cgroup.h>
> +#include <linux/lsm_hooks.h>
> +
> +static int devguard_inode_mknod(struct inode *dir, struct dentry *dentry,
> + umode_t mode, dev_t dev)
> +{
> + if (dentry->d_sb->s_iflags & ~SB_I_NODEV)
> + return 0;
> +
> + // strip SB_I_NODEV on super block if device cgroup is active
Please use block style comments. We don't use // comments here.
/*
* Strip SB_I_NODEV on super block if device cgroup is active
*/
> + if (cgroup_bpf_current_enabled(CGROUP_DEVICE))
> + dentry->d_sb->s_iflags &= ~SB_I_NODEV;
> +
> + return 0;
> +}
> +
> +static struct security_hook_list devguard_hooks[] __ro_after_init = {
> + LSM_HOOK_INIT(inode_mknod, devguard_inode_mknod),
> +};
> +
> +static int __init devguard_init(void)
> +{
> + security_add_hooks(devguard_hooks, ARRAY_SIZE(devguard_hooks),
> + "devguard");
> + pr_info("devguard: initialized\n");
> + return 0;
> +}
> +
> +DEFINE_LSM(devguard) = {
> + .name = "devguard",
> + .init = devguard_init,
> +};
Powered by blists - more mailing lists