[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240610023301.GA2183903@mail.hallyn.com>
Date: Sun, 9 Jun 2024 21:33:01 -0500
From: "Serge E. Hallyn" <serge@...lyn.com>
To: Jonathan Calmels <jcalmels@...0.net>
Cc: brauner@...nel.org, ebiederm@...ssion.com,
Jonathan Corbet <corbet@....net>, Paul Moore <paul@...l-moore.com>,
James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>, KP Singh <kpsingh@...nel.org>,
Matt Bobrowski <mattbobrowski@...gle.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Luis Chamberlain <mcgrof@...nel.org>,
Kees Cook <kees@...nel.org>, Joel Granados <j.granados@...sung.com>,
John Johansen <john.johansen@...onical.com>,
David Howells <dhowells@...hat.com>,
Jarkko Sakkinen <jarkko@...nel.org>,
Stephen Smalley <stephen.smalley.work@...il.com>,
Ondrej Mosnacek <omosnace@...hat.com>,
Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
containers@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-security-module@...r.kernel.org, bpf@...r.kernel.org,
apparmor@...ts.ubuntu.com, keyrings@...r.kernel.org,
selinux@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 2/4] capabilities: Add securebit to restrict userns
caps
On Sun, Jun 09, 2024 at 03:43:35AM -0700, Jonathan Calmels wrote:
> This patch adds a new capability security bit designed to constrain a
> task’s userns capability set to its bounding set. The reason for this is
> twofold:
>
> - This serves as a quick and easy way to lock down a set of capabilities
> for a task, thus ensuring that any namespace it creates will never be
> more privileged than itself is.
> - This helps userspace transition to more secure defaults by not requiring
> specific logic for the userns capability set, or libcap support.
>
> Example:
>
> # capsh --secbits=$((1 << 8)) --drop=cap_sys_rawio -- \
> -c 'unshare -r grep Cap /proc/self/status'
> CapInh: 0000000000000000
> CapPrm: 000001fffffdffff
> CapEff: 000001fffffdffff
> CapBnd: 000001fffffdffff
> CapAmb: 0000000000000000
> CapUNs: 000001fffffdffff
But you are not (that I can see, in this or the previous patch)
keeping SECURE_USERNS_STRICT_CAPS in securebits on the next
level unshare. Though I think it's ok, because by then both
cap_userns and cap_bset are reduced and cap_userns can't be
expanded. (Sorry, just thinking aloud here)
> Signed-off-by: Jonathan Calmels <jcalmels@...0.net>
> ---
> include/linux/securebits.h | 1 +
> include/uapi/linux/securebits.h | 11 ++++++++++-
> kernel/user_namespace.c | 5 +++++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/securebits.h b/include/linux/securebits.h
> index 656528673983..5f9d85cd69c3 100644
> --- a/include/linux/securebits.h
> +++ b/include/linux/securebits.h
> @@ -5,4 +5,5 @@
> #include <uapi/linux/securebits.h>
>
> #define issecure(X) (issecure_mask(X) & current_cred_xxx(securebits))
> +#define iscredsecure(cred, X) (issecure_mask(X) & cred->securebits)
> #endif /* !_LINUX_SECUREBITS_H */
> diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
> index d6d98877ff1a..2da3f4be4531 100644
> --- a/include/uapi/linux/securebits.h
> +++ b/include/uapi/linux/securebits.h
> @@ -52,10 +52,19 @@
> #define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
> (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
>
> +/* When set, user namespace capabilities are restricted to their parent's bounding set. */
> +#define SECURE_USERNS_STRICT_CAPS 8
> +#define SECURE_USERNS_STRICT_CAPS_LOCKED 9 /* make bit-8 immutable */
> +
> +#define SECBIT_USERNS_STRICT_CAPS (issecure_mask(SECURE_USERNS_STRICT_CAPS))
> +#define SECBIT_USERNS_STRICT_CAPS_LOCKED \
> + (issecure_mask(SECURE_USERNS_STRICT_CAPS_LOCKED))
> +
> #define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
> issecure_mask(SECURE_NO_SETUID_FIXUP) | \
> issecure_mask(SECURE_KEEP_CAPS) | \
> - issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
> + issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
> + issecure_mask(SECURE_USERNS_STRICT_CAPS))
> #define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
>
> #endif /* _UAPI_LINUX_SECUREBITS_H */
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 7e624607330b..53848e2b68cd 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -10,6 +10,7 @@
> #include <linux/cred.h>
> #include <linux/securebits.h>
> #include <linux/security.h>
> +#include <linux/capability.h>
> #include <linux/keyctl.h>
> #include <linux/key-type.h>
> #include <keys/user-type.h>
> @@ -42,6 +43,10 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>
> static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> {
> + /* Limit userns capabilities to our parent's bounding set. */
In the case of userns_install(), it will be the target user namespace
creator's bounding set, right? Not "our parent's"?
> + if (iscredsecure(cred, SECURE_USERNS_STRICT_CAPS))
> + cred->cap_userns = cap_intersect(cred->cap_userns, cred->cap_bset);
> +
> /* Start with the capabilities defined in the userns set. */
> cred->cap_bset = cred->cap_userns;
> cred->cap_permitted = cred->cap_userns;
> --
> 2.45.2
Powered by blists - more mailing lists