It's that time of the year again where we debate security settings for user
namespaces ;)
I’ve been experimenting with different approaches to address the gripe
around user namespaces being used as attack vectors.
After invaluable feedback from Serge and Christian offline, this is what I
came up with.
There are obviously a lot of things we could do differently but I feel this
is the right balance between functionality, simplicity and security. This
also serves as a good foundation and could always be extended if the need
arises in the future.
Notes:
- Adding a new capability set is far from ideal, but trying to reuse the
existing capability framework was deemed both impractical and
questionable security-wise, so here we are.
- We might want to add new capabilities for some of the checks instead of
reusing CAP_SETPCAP every time. Serge mentioned something like
CAP_SYS_LIMIT?
- In the last patch, we could decide to have stronger requirements and
perform checks inside cap_capable() in case we want to retroactively
prevent capabilities in old namespaces, this might be an overreach though
so I left it out.
I'm also not fond of the ulong logic for setting the sysctl parameter, on
the other hand, the usermodhelper code always uses two u32s which makes it
very confusing to set in userspace.
Jonathan Calmels (3):
capabilities: user namespace capabilities
capabilities: add securebit for strict userns caps
capabilities: add cap userns sysctl mask
fs/proc/array.c | 9 ++++
include/linux/cred.h | 3 ++
include/linux/securebits.h | 1 +
include/linux/user_namespace.h | 7 +++
include/uapi/linux/prctl.h | 7 +++
include/uapi/linux/securebits.h | 11 ++++-
kernel/cred.c | 3 ++
kernel/sysctl.c | 10 ++++
kernel/umh.c | 16 +++++++
kernel/user_namespace.c | 83 ++++++++++++++++++++++++++++++---
security/commoncap.c | 59 +++++++++++++++++++++++
security/keys/process_keys.c | 3 ++
12 files changed, 204 insertions(+), 8 deletions(-)
--
2.45.0
Attackers often rely on user namespaces to get elevated (yet confined)
privileges in order to target specific subsystems (e.g. [1]). Distributions
have been pretty adamant that they need a way to configure these, most of
them carry out-of-tree patches to do so, or plainly refuse to enable them.
As a result, there have been multiple efforts over the years to introduce
various knobs to control and/or disable user namespaces (e.g. [2][3][4]).
While we acknowledge that there are already ways to control the creation of
such namespaces (the most recent being a LSM hook), there are inherent
issues with these approaches. Preventing the user namespace creation is not
fine-grained enough, and in some cases, incompatible with various userspace
expectations (e.g. container runtimes, browser sandboxing, service
isolation)
This patch addresses these limitations by introducing an additional
capability set used to restrict the permissions granted when creating user
namespaces. This way, processes can apply the principle of least privilege
by configuring only the capabilities they need for their namespaces.
For compatibility reasons, processes always start with a full userns
capability set.
On namespace creation, the userns capability set (pU) is assigned to the
new effective (pE), permitted (pP) and bounding set (X) of the task:
pU = pE = pP = X
The userns capability set obeys the invariant that no bit can ever be set
if it is not already part of the task’s bounding set. This ensures that no
namespace can ever gain more privileges than its predecessors.
Additionally, if a task is not privileged over CAP_SETPCAP, setting any bit
in the userns set requires its corresponding bit to be set in the permitted
set. This effectively mimics the inheritable set rules and means that, by
default, only root in the initial user namespace can gain userns
capabilities:
p’U = (pE & CAP_SETPCAP) ? X : (X & pP)
Note that since userns capabilities are strictly hierarchical, policies can
be enforced at various levels (e.g. init, pam_cap) and inherited by every
child namespace.
Here is a sample program that can be used to verify the functionality:
/*
* Test program that drops CAP_SYS_RAWIO from subsequent user namespaces.
*
* ./cap_userns_test unshare -r grep Cap /proc/self/status
* CapInh: 0000000000000000
* CapPrm: 000001fffffdffff
* CapEff: 000001fffffdffff
* CapBnd: 000001fffffdffff
* CapAmb: 0000000000000000
* CapUNs: 000001fffffdffff
*/
int main(int argc, char *argv[])
{
if (prctl(PR_CAP_USERNS, PR_CAP_USERNS_LOWER, CAP_SYS_RAWIO, 0, 0) < 0)
err(1, "cannot drop userns cap");
execvp(argv[1], argv + 1);
err(1, "cannot exec");
}
Link: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html
Link: https://lore.kernel.org/lkml/[email protected]
Link: https://lore.kernel.org/lkml/[email protected]
Link: https://lore.kernel.org/containers/[email protected]
Signed-off-by: Jonathan Calmels <[email protected]>
---
fs/proc/array.c | 9 ++++++
include/linux/cred.h | 3 ++
include/uapi/linux/prctl.h | 7 +++++
kernel/cred.c | 3 ++
kernel/umh.c | 16 ++++++++++
kernel/user_namespace.c | 12 +++-----
security/commoncap.c | 59 ++++++++++++++++++++++++++++++++++++
security/keys/process_keys.c | 3 ++
8 files changed, 105 insertions(+), 7 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 34a47fb0c57f..364e8bb19f9d 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -313,6 +313,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
const struct cred *cred;
kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
cap_bset, cap_ambient;
+#ifdef CONFIG_USER_NS
+ kernel_cap_t cap_userns;
+#endif
rcu_read_lock();
cred = __task_cred(p);
@@ -321,6 +324,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
cap_effective = cred->cap_effective;
cap_bset = cred->cap_bset;
cap_ambient = cred->cap_ambient;
+#ifdef CONFIG_USER_NS
+ cap_userns = cred->cap_userns;
+#endif
rcu_read_unlock();
render_cap_t(m, "CapInh:\t", &cap_inheritable);
@@ -328,6 +334,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
render_cap_t(m, "CapEff:\t", &cap_effective);
render_cap_t(m, "CapBnd:\t", &cap_bset);
render_cap_t(m, "CapAmb:\t", &cap_ambient);
+#ifdef CONFIG_USER_NS
+ render_cap_t(m, "CapUNs:\t", &cap_userns);
+#endif
}
static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 2976f534a7a3..adab0031443e 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -124,6 +124,9 @@ struct cred {
kernel_cap_t cap_effective; /* caps we can actually use */
kernel_cap_t cap_bset; /* capability bounding set */
kernel_cap_t cap_ambient; /* Ambient capability set */
+#ifdef CONFIG_USER_NS
+ kernel_cap_t cap_userns; /* User namespace capability set */
+#endif
#ifdef CONFIG_KEYS
unsigned char jit_keyring; /* default keyring to attach requested
* keys to */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 370ed14b1ae0..e09475171f62 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -198,6 +198,13 @@ struct prctl_mm_map {
# define PR_CAP_AMBIENT_LOWER 3
# define PR_CAP_AMBIENT_CLEAR_ALL 4
+/* Control the userns capability set */
+#define PR_CAP_USERNS 48
+# define PR_CAP_USERNS_IS_SET 1
+# define PR_CAP_USERNS_RAISE 2
+# define PR_CAP_USERNS_LOWER 3
+# define PR_CAP_USERNS_CLEAR_ALL 4
+
/* arm64 Scalable Vector Extension controls */
/* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
#define PR_SVE_SET_VL 50 /* set task vector length */
diff --git a/kernel/cred.c b/kernel/cred.c
index 075cfa7c896f..9912c6f3bc6b 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -56,6 +56,9 @@ struct cred init_cred = {
.cap_permitted = CAP_FULL_SET,
.cap_effective = CAP_FULL_SET,
.cap_bset = CAP_FULL_SET,
+#ifdef CONFIG_USER_NS
+ .cap_userns = CAP_FULL_SET,
+#endif
.user = INIT_USER,
.user_ns = &init_user_ns,
.group_info = &init_groups,
diff --git a/kernel/umh.c b/kernel/umh.c
index 1b13c5d34624..51f1e1d25d49 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -32,6 +32,9 @@
#include <trace/events/module.h>
+#ifdef CONFIG_USER_NS
+static kernel_cap_t usermodehelper_userns = CAP_FULL_SET;
+#endif
static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
static DEFINE_SPINLOCK(umh_sysctl_lock);
@@ -94,6 +97,10 @@ static int call_usermodehelper_exec_async(void *data)
new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
new->cap_inheritable);
+#ifdef CONFIG_USER_NS
+ new->cap_userns = cap_intersect(usermodehelper_userns,
+ new->cap_userns);
+#endif
spin_unlock(&umh_sysctl_lock);
if (sub_info->init) {
@@ -560,6 +567,15 @@ static struct ctl_table usermodehelper_table[] = {
.mode = 0600,
.proc_handler = proc_cap_handler,
},
+#ifdef CONFIG_USER_NS
+ {
+ .procname = "userns",
+ .data = &usermodehelper_userns,
+ .maxlen = 2 * sizeof(unsigned long),
+ .mode = 0600,
+ .proc_handler = proc_cap_handler,
+ },
+#endif
{ }
};
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0b0b95418b16..7e624607330b 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -42,15 +42,13 @@ static void dec_user_namespaces(struct ucounts *ucounts)
static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
{
- /* Start with the same capabilities as init but useless for doing
- * anything as the capabilities are bound to the new user namespace.
- */
- cred->securebits = SECUREBITS_DEFAULT;
+ /* Start with the capabilities defined in the userns set. */
+ cred->cap_bset = cred->cap_userns;
+ cred->cap_permitted = cred->cap_userns;
+ cred->cap_effective = cred->cap_userns;
cred->cap_inheritable = CAP_EMPTY_SET;
- cred->cap_permitted = CAP_FULL_SET;
- cred->cap_effective = CAP_FULL_SET;
cred->cap_ambient = CAP_EMPTY_SET;
- cred->cap_bset = CAP_FULL_SET;
+ cred->securebits = SECUREBITS_DEFAULT;
#ifdef CONFIG_KEYS
key_put(cred->request_key_auth);
cred->request_key_auth = NULL;
diff --git a/security/commoncap.c b/security/commoncap.c
index 162d96b3a676..b3d3372bf910 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -228,6 +228,28 @@ static inline int cap_inh_is_capped(void)
return 1;
}
+/*
+ * Determine whether a userns capability can be raised.
+ * Returns 1 if it can, 0 otherwise.
+ */
+#ifdef CONFIG_USER_NS
+static inline int cap_uns_is_raiseable(unsigned long cap)
+{
+ if (!!cap_raised(current_cred()->cap_userns, cap))
+ return 1;
+ /* a capability cannot be raised unless the current task has it in
+ * its bounding set and, without CAP_SETPCAP, its permitted set.
+ */
+ if (!cap_raised(current_cred()->cap_bset, cap))
+ return 0;
+ if (cap_capable(current_cred(), current_cred()->user_ns,
+ CAP_SETPCAP, CAP_OPT_NONE) != 0 &&
+ !cap_raised(current_cred()->cap_permitted, cap))
+ return 0;
+ return 1;
+}
+#endif
+
/**
* cap_capset - Validate and apply proposed changes to current's capabilities
* @new: The proposed new credentials; alterations should be made here
@@ -1382,6 +1404,43 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
return commit_creds(new);
}
+#ifdef CONFIG_USER_NS
+ case PR_CAP_USERNS:
+ if (arg2 == PR_CAP_USERNS_CLEAR_ALL) {
+ if (arg3 | arg4 | arg5)
+ return -EINVAL;
+
+ new = prepare_creds();
+ if (!new)
+ return -ENOMEM;
+ cap_clear(new->cap_userns);
+ return commit_creds(new);
+ }
+
+ if (((!cap_valid(arg3)) | arg4 | arg5))
+ return -EINVAL;
+
+ if (arg2 == PR_CAP_USERNS_IS_SET) {
+ return !!cap_raised(current_cred()->cap_userns, arg3);
+ } else if (arg2 != PR_CAP_USERNS_RAISE &&
+ arg2 != PR_CAP_USERNS_LOWER) {
+ return -EINVAL;
+ } else {
+ if (arg2 == PR_CAP_USERNS_RAISE &&
+ !cap_uns_is_raiseable(arg3))
+ return -EPERM;
+
+ new = prepare_creds();
+ if (!new)
+ return -ENOMEM;
+ if (arg2 == PR_CAP_USERNS_RAISE)
+ cap_raise(new->cap_userns, arg3);
+ else
+ cap_lower(new->cap_userns, arg3);
+ return commit_creds(new);
+ }
+#endif
+
default:
/* No functionality available - continue with default */
return -ENOSYS;
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index b5d5333ab330..e3670d815435 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -944,6 +944,9 @@ void key_change_session_keyring(struct callback_head *twork)
new->cap_effective = old->cap_effective;
new->cap_ambient = old->cap_ambient;
new->cap_bset = old->cap_bset;
+#ifdef CONFIG_USER_NS
+ new->cap_userns = old->cap_userns;
+#endif
new->jit_keyring = old->jit_keyring;
new->thread_keyring = key_get(old->thread_keyring);
--
2.45.0
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
Signed-off-by: Jonathan Calmels <[email protected]>
---
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. */
+ 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.0
This patch adds a new system-wide userns capability mask designed to mask
off capabilities in user namespaces.
This mask is controlled through a sysctl and can be set early in the boot
process or on the kernel command line to exclude known capabilities from
ever being gained in namespaces. Once set, it can be further restricted to
exert dynamic policies on the system (e.g. ward off a potential exploit).
Changing this mask requires privileges over CAP_SYS_ADMIN and CAP_SETPCAP
in the initial user namespace.
Example:
# sysctl -qw kernel.cap_userns_mask=0x1fffffdffff && \
unshare -r grep Cap /proc/self/status
CapInh: 0000000000000000
CapPrm: 000001fffffdffff
CapEff: 000001fffffdffff
CapBnd: 000001fffffdffff
CapAmb: 0000000000000000
CapUNs: 000001fffffdffff
Signed-off-by: Jonathan Calmels <[email protected]>
---
include/linux/user_namespace.h | 7 ++++
kernel/sysctl.c | 10 ++++++
kernel/user_namespace.c | 66 ++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 6030a8235617..e3478bd54ee5 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_USER_NAMESPACE_H
#define _LINUX_USER_NAMESPACE_H
+#include <linux/capability.h>
#include <linux/kref.h>
#include <linux/nsproxy.h>
#include <linux/ns_common.h>
@@ -14,6 +15,12 @@
#define UID_GID_MAP_MAX_BASE_EXTENTS 5
#define UID_GID_MAP_MAX_EXTENTS 340
+#ifdef CONFIG_SYSCTL
+extern kernel_cap_t cap_userns_mask;
+int proc_cap_userns_handler(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos);
+#endif
+
struct uid_gid_extent {
u32 first;
u32 lower_first;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 81cc974913bb..1546eebd6aea 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -62,6 +62,7 @@
#include <linux/sched/sysctl.h>
#include <linux/mount.h>
#include <linux/userfaultfd_k.h>
+#include <linux/user_namespace.h>
#include <linux/pid.h>
#include "../lib/kstrtox.h"
@@ -1846,6 +1847,15 @@ static struct ctl_table kern_table[] = {
.mode = 0444,
.proc_handler = proc_dointvec,
},
+#ifdef CONFIG_USER_NS
+ {
+ .procname = "cap_userns_mask",
+ .data = &cap_userns_mask,
+ .maxlen = sizeof(kernel_cap_t),
+ .mode = 0644,
+ .proc_handler = proc_cap_userns_handler,
+ },
+#endif
#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86)
{
.procname = "unknown_nmi_panic",
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 53848e2b68cd..e0cf606e9140 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -26,6 +26,66 @@
static struct kmem_cache *user_ns_cachep __ro_after_init;
static DEFINE_MUTEX(userns_state_mutex);
+#ifdef CONFIG_SYSCTL
+static DEFINE_SPINLOCK(cap_userns_lock);
+kernel_cap_t cap_userns_mask = CAP_FULL_SET;
+
+int proc_cap_userns_handler(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ struct ctl_table t;
+ unsigned long mask_array[2];
+ kernel_cap_t new_mask, *mask;
+ int err;
+
+ if (write && (!capable(CAP_SETPCAP) ||
+ !capable(CAP_SYS_ADMIN)))
+ return -EPERM;
+
+ /*
+ * convert from the global kernel_cap_t to the ulong array to print to
+ * userspace if this is a read.
+ *
+ * capabilities are exposed as one 64-bit value or two 32-bit values
+ * depending on the architecture
+ */
+ mask = table->data;
+ spin_lock(&cap_userns_lock);
+ mask_array[0] = (unsigned long) mask->val;
+#if BITS_PER_LONG != 64
+ mask_array[1] = mask->val >> BITS_PER_LONG;
+#endif
+ spin_unlock(&cap_userns_lock);
+
+ t = *table;
+ t.data = &mask_array;
+
+ /*
+ * actually read or write and array of ulongs from userspace. Remember
+ * these are least significant bits first
+ */
+ err = proc_doulongvec_minmax(&t, write, buffer, lenp, ppos);
+ if (err < 0)
+ return err;
+
+ new_mask.val = mask_array[0];
+#if BITS_PER_LONG != 64
+ new_mask.val += (u64)mask_array[1] << BITS_PER_LONG;
+#endif
+
+ /*
+ * Drop everything not in the new_mask (but don't add things)
+ */
+ if (write) {
+ spin_lock(&cap_userns_lock);
+ *mask = cap_intersect(*mask, new_mask);
+ spin_unlock(&cap_userns_lock);
+ }
+
+ return 0;
+}
+#endif
+
static bool new_idmap_permitted(const struct file *file,
struct user_namespace *ns, int cap_setid,
struct uid_gid_map *map);
@@ -46,6 +106,12 @@ static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
/* Limit userns capabilities to our parent's bounding set. */
if (iscredsecure(cred, SECURE_USERNS_STRICT_CAPS))
cred->cap_userns = cap_intersect(cred->cap_userns, cred->cap_bset);
+#ifdef CONFIG_SYSCTL
+ /* Mask off userns capabilities that are not permitted by the system-wide mask. */
+ spin_lock(&cap_userns_lock);
+ cred->cap_userns = cap_intersect(cred->cap_userns, cap_userns_mask);
+ spin_unlock(&cap_userns_lock);
+#endif
/* Start with the capabilities defined in the userns set. */
cred->cap_bset = cred->cap_userns;
--
2.45.0
Some quick remarks, no bandwidth to understand this.
First of all short summary does not define any action so it should be
rather e.g.
"capabilities: Add userns capabilities"
Much more understandable.
On Thu May 16, 2024 at 12:22 PM EEST, Jonathan Calmels wrote:
> Attackers often rely on user namespaces to get elevated (yet confined)
> privileges in order to target specific subsystems (e.g. [1]). Distributions
> have been pretty adamant that they need a way to configure these, most of
> them carry out-of-tree patches to do so, or plainly refuse to enable them.
> As a result, there have been multiple efforts over the years to introduce
> various knobs to control and/or disable user namespaces (e.g. [2][3][4]).
>
> While we acknowledge that there are already ways to control the creation of
> such namespaces (the most recent being a LSM hook), there are inherent
> issues with these approaches. Preventing the user namespace creation is not
> fine-grained enough, and in some cases, incompatible with various userspace
> expectations (e.g. container runtimes, browser sandboxing, service
> isolation)
>
> This patch addresses these limitations by introducing an additional
> capability set used to restrict the permissions granted when creating user
> namespaces. This way, processes can apply the principle of least privilege
> by configuring only the capabilities they need for their namespaces.
>
> For compatibility reasons, processes always start with a full userns
> capability set.
>
> On namespace creation, the userns capability set (pU) is assigned to the
> new effective (pE), permitted (pP) and bounding set (X) of the task:
>
> pU = pE = pP = X
>
> The userns capability set obeys the invariant that no bit can ever be set
> if it is not already part of the task’s bounding set. This ensures that no
> namespace can ever gain more privileges than its predecessors.
> Additionally, if a task is not privileged over CAP_SETPCAP, setting any bit
> in the userns set requires its corresponding bit to be set in the permitted
> set. This effectively mimics the inheritable set rules and means that, by
> default, only root in the initial user namespace can gain userns
> capabilities:
>
> p’U = (pE & CAP_SETPCAP) ? X : (X & pP)
>
> Note that since userns capabilities are strictly hierarchical, policies can
> be enforced at various levels (e.g. init, pam_cap) and inherited by every
> child namespace.
>
> Here is a sample program that can be used to verify the functionality:
>
> /*
> * Test program that drops CAP_SYS_RAWIO from subsequent user namespaces.
> *
> * ./cap_userns_test unshare -r grep Cap /proc/self/status
> * CapInh: 0000000000000000
> * CapPrm: 000001fffffdffff
> * CapEff: 000001fffffdffff
> * CapBnd: 000001fffffdffff
> * CapAmb: 0000000000000000
> * CapUNs: 000001fffffdffff
> */
>
> int main(int argc, char *argv[])
> {
> if (prctl(PR_CAP_USERNS, PR_CAP_USERNS_LOWER, CAP_SYS_RAWIO, 0, 0) < 0)
> err(1, "cannot drop userns cap");
>
> execvp(argv[1], argv + 1);
> err(1, "cannot exec");
> }
>
> Link: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/containers/[email protected]
>
> Signed-off-by: Jonathan Calmels <[email protected]>
> ---
> fs/proc/array.c | 9 ++++++
> include/linux/cred.h | 3 ++
> include/uapi/linux/prctl.h | 7 +++++
> kernel/cred.c | 3 ++
> kernel/umh.c | 16 ++++++++++
> kernel/user_namespace.c | 12 +++-----
> security/commoncap.c | 59 ++++++++++++++++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 8 files changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 34a47fb0c57f..364e8bb19f9d 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -313,6 +313,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> const struct cred *cred;
> kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> cap_bset, cap_ambient;
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns;
> +#endif
>
> rcu_read_lock();
> cred = __task_cred(p);
> @@ -321,6 +324,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> cap_effective = cred->cap_effective;
> cap_bset = cred->cap_bset;
> cap_ambient = cred->cap_ambient;
> +#ifdef CONFIG_USER_NS
> + cap_userns = cred->cap_userns;
> +#endif
> rcu_read_unlock();
>
> render_cap_t(m, "CapInh:\t", &cap_inheritable);
> @@ -328,6 +334,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapEff:\t", &cap_effective);
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> render_cap_t(m, "CapAmb:\t", &cap_ambient);
> +#ifdef CONFIG_USER_NS
> + render_cap_t(m, "CapUNs:\t", &cap_userns);
> +#endif
> }
>
> static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 2976f534a7a3..adab0031443e 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -124,6 +124,9 @@ struct cred {
> kernel_cap_t cap_effective; /* caps we can actually use */
> kernel_cap_t cap_bset; /* capability bounding set */
> kernel_cap_t cap_ambient; /* Ambient capability set */
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns; /* User namespace capability set */
> +#endif
> #ifdef CONFIG_KEYS
> unsigned char jit_keyring; /* default keyring to attach requested
> * keys to */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 370ed14b1ae0..e09475171f62 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -198,6 +198,13 @@ struct prctl_mm_map {
> # define PR_CAP_AMBIENT_LOWER 3
> # define PR_CAP_AMBIENT_CLEAR_ALL 4
>
> +/* Control the userns capability set */
> +#define PR_CAP_USERNS 48
> +# define PR_CAP_USERNS_IS_SET 1
> +# define PR_CAP_USERNS_RAISE 2
> +# define PR_CAP_USERNS_LOWER 3
> +# define PR_CAP_USERNS_CLEAR_ALL 4
Kernel coding style does not support this way but instead recommends
enum but apparently the whole header is not following that so I
guess it is fine ;-)
> +
> /* arm64 Scalable Vector Extension controls */
> /* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
> #define PR_SVE_SET_VL 50 /* set task vector length */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 075cfa7c896f..9912c6f3bc6b 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -56,6 +56,9 @@ struct cred init_cred = {
> .cap_permitted = CAP_FULL_SET,
> .cap_effective = CAP_FULL_SET,
> .cap_bset = CAP_FULL_SET,
> +#ifdef CONFIG_USER_NS
> + .cap_userns = CAP_FULL_SET,
> +#endif
> .user = INIT_USER,
> .user_ns = &init_user_ns,
> .group_info = &init_groups,
> diff --git a/kernel/umh.c b/kernel/umh.c
> index 1b13c5d34624..51f1e1d25d49 100644
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -32,6 +32,9 @@
>
> #include <trace/events/module.h>
>
> +#ifdef CONFIG_USER_NS
> +static kernel_cap_t usermodehelper_userns = CAP_FULL_SET;
> +#endif
> static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
> static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
> static DEFINE_SPINLOCK(umh_sysctl_lock);
> @@ -94,6 +97,10 @@ static int call_usermodehelper_exec_async(void *data)
> new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
> new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
> new->cap_inheritable);
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = cap_intersect(usermodehelper_userns,
> + new->cap_userns);
Could be also a single line (checkpatch.pl does not complain).
> +#endif
> spin_unlock(&umh_sysctl_lock);
>
> if (sub_info->init) {
> @@ -560,6 +567,15 @@ static struct ctl_table usermodehelper_table[] = {
> .mode = 0600,
> .proc_handler = proc_cap_handler,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "userns",
> + .data = &usermodehelper_userns,
> + .maxlen = 2 * sizeof(unsigned long),
> + .mode = 0600,
> + .proc_handler = proc_cap_handler,
> + },
> +#endif
> { }
> };
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0b0b95418b16..7e624607330b 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -42,15 +42,13 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>
> static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> {
> - /* Start with the same capabilities as init but useless for doing
> - * anything as the capabilities are bound to the new user namespace.
> - */
> - cred->securebits = SECUREBITS_DEFAULT;
> + /* Start with the capabilities defined in the userns set. */
> + cred->cap_bset = cred->cap_userns;
> + cred->cap_permitted = cred->cap_userns;
> + cred->cap_effective = cred->cap_userns;
> cred->cap_inheritable = CAP_EMPTY_SET;
> - cred->cap_permitted = CAP_FULL_SET;
> - cred->cap_effective = CAP_FULL_SET;
> cred->cap_ambient = CAP_EMPTY_SET;
> - cred->cap_bset = CAP_FULL_SET;
> + cred->securebits = SECUREBITS_DEFAULT;
> #ifdef CONFIG_KEYS
> key_put(cred->request_key_auth);
> cred->request_key_auth = NULL;
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 162d96b3a676..b3d3372bf910 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -228,6 +228,28 @@ static inline int cap_inh_is_capped(void)
> return 1;
> }
>
> +/*
> + * Determine whether a userns capability can be raised.
> + * Returns 1 if it can, 0 otherwise.
> + */
> +#ifdef CONFIG_USER_NS
> +static inline int cap_uns_is_raiseable(unsigned long cap)
> +{
> + if (!!cap_raised(current_cred()->cap_userns, cap))
> + return 1;
Empty line.
> + /* a capability cannot be raised unless the current task has it in
Incorrectly formatted comment:
/*
* Foo
That is what's the correct format.
> + * its bounding set and, without CAP_SETPCAP, its permitted set.
> + */
> + if (!cap_raised(current_cred()->cap_bset, cap))
> + return 0;
Empty line might be appropriate here.
> + if (cap_capable(current_cred(), current_cred()->user_ns,
> + CAP_SETPCAP, CAP_OPT_NONE) != 0 &&
> + !cap_raised(current_cred()->cap_permitted, cap))
I'd consider being dummy here to make this more easy to verify
also in the future: create two bools and use them for final
comparison.
My head hurts reading that.
> + return 0;
> + return 1;
> +}
> +#endif
> +
> /**
> * cap_capset - Validate and apply proposed changes to current's capabilities
> * @new: The proposed new credentials; alterations should be made here
> @@ -1382,6 +1404,43 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> return commit_creds(new);
> }
>
> +#ifdef CONFIG_USER_NS
> + case PR_CAP_USERNS:
> + if (arg2 == PR_CAP_USERNS_CLEAR_ALL) {
> + if (arg3 | arg4 | arg5)
> + return -EINVAL;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + cap_clear(new->cap_userns);
> + return commit_creds(new);
> + }
> +
> + if (((!cap_valid(arg3)) | arg4 | arg5))
> + return -EINVAL;
> +
> + if (arg2 == PR_CAP_USERNS_IS_SET) {
> + return !!cap_raised(current_cred()->cap_userns, arg3);
> + } else if (arg2 != PR_CAP_USERNS_RAISE &&
> + arg2 != PR_CAP_USERNS_LOWER) {
> + return -EINVAL;
> + } else {
> + if (arg2 == PR_CAP_USERNS_RAISE &&
> + !cap_uns_is_raiseable(arg3))
> + return -EPERM;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + if (arg2 == PR_CAP_USERNS_RAISE)
> + cap_raise(new->cap_userns, arg3);
> + else
> + cap_lower(new->cap_userns, arg3);
> + return commit_creds(new);
> + }
> +#endif
> +
> default:
> /* No functionality available - continue with default */
> return -ENOSYS;
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index b5d5333ab330..e3670d815435 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -944,6 +944,9 @@ void key_change_session_keyring(struct callback_head *twork)
> new->cap_effective = old->cap_effective;
> new->cap_ambient = old->cap_ambient;
> new->cap_bset = old->cap_bset;
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = old->cap_userns;
> +#endif
>
> new->jit_keyring = old->jit_keyring;
> new->thread_keyring = key_get(old->thread_keyring);
BR, Jarkko
Maintainer dependent but at least on x86 patches people tend to prefer
capital letter in the short summary i.e. s/add/Add/
On Thu May 16, 2024 at 12:22 PM EEST, 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
>
> Signed-off-by: Jonathan Calmels <[email protected]>
> ---
> 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. */
> + 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;
BR, Jarkko
On Thu May 16, 2024 at 12:22 PM EEST, Jonathan Calmels wrote:
> This patch adds a new system-wide userns capability mask designed to mask
> off capabilities in user namespaces.
>
> This mask is controlled through a sysctl and can be set early in the boot
> process or on the kernel command line to exclude known capabilities from
> ever being gained in namespaces. Once set, it can be further restricted to
> exert dynamic policies on the system (e.g. ward off a potential exploit).
>
> Changing this mask requires privileges over CAP_SYS_ADMIN and CAP_SETPCAP
> in the initial user namespace.
>
> Example:
>
> # sysctl -qw kernel.cap_userns_mask=0x1fffffdffff && \
> unshare -r grep Cap /proc/self/status
> CapInh: 0000000000000000
> CapPrm: 000001fffffdffff
> CapEff: 000001fffffdffff
> CapBnd: 000001fffffdffff
> CapAmb: 0000000000000000
> CapUNs: 000001fffffdffff
>
> Signed-off-by: Jonathan Calmels <[email protected]>
> ---
> include/linux/user_namespace.h | 7 ++++
> kernel/sysctl.c | 10 ++++++
> kernel/user_namespace.c | 66 ++++++++++++++++++++++++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 6030a8235617..e3478bd54ee5 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -2,6 +2,7 @@
> #ifndef _LINUX_USER_NAMESPACE_H
> #define _LINUX_USER_NAMESPACE_H
>
> +#include <linux/capability.h>
> #include <linux/kref.h>
> #include <linux/nsproxy.h>
> #include <linux/ns_common.h>
> @@ -14,6 +15,12 @@
> #define UID_GID_MAP_MAX_BASE_EXTENTS 5
> #define UID_GID_MAP_MAX_EXTENTS 340
>
> +#ifdef CONFIG_SYSCTL
> +extern kernel_cap_t cap_userns_mask;
> +int proc_cap_userns_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos);
> +#endif
> +
> struct uid_gid_extent {
> u32 first;
> u32 lower_first;
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 81cc974913bb..1546eebd6aea 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -62,6 +62,7 @@
> #include <linux/sched/sysctl.h>
> #include <linux/mount.h>
> #include <linux/userfaultfd_k.h>
> +#include <linux/user_namespace.h>
> #include <linux/pid.h>
>
> #include "../lib/kstrtox.h"
> @@ -1846,6 +1847,15 @@ static struct ctl_table kern_table[] = {
> .mode = 0444,
> .proc_handler = proc_dointvec,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "cap_userns_mask",
> + .data = &cap_userns_mask,
> + .maxlen = sizeof(kernel_cap_t),
> + .mode = 0644,
> + .proc_handler = proc_cap_userns_handler,
> + },
> +#endif
> #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86)
> {
> .procname = "unknown_nmi_panic",
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 53848e2b68cd..e0cf606e9140 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -26,6 +26,66 @@
> static struct kmem_cache *user_ns_cachep __ro_after_init;
> static DEFINE_MUTEX(userns_state_mutex);
>
> +#ifdef CONFIG_SYSCTL
> +static DEFINE_SPINLOCK(cap_userns_lock);
Generally new global or file-local locks are better to have a comment
that describes their use.
> +kernel_cap_t cap_userns_mask = CAP_FULL_SET;
> +
Non-static symbol should have appropriate kdoc with alll arguments
and return values documented.
> +int proc_cap_userns_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos)
> +{
> + struct ctl_table t;
> + unsigned long mask_array[2];
> + kernel_cap_t new_mask, *mask;
> + int err;
> +
> + if (write && (!capable(CAP_SETPCAP) ||
> + !capable(CAP_SYS_ADMIN)))
> + return -EPERM;
> +
> + /*
> + * convert from the global kernel_cap_t to the ulong array to print to
> + * userspace if this is a read.
> + *
> + * capabilities are exposed as one 64-bit value or two 32-bit values
> + * depending on the architecture
> + */
> + mask = table->data;
> + spin_lock(&cap_userns_lock);
> + mask_array[0] = (unsigned long) mask->val;
> +#if BITS_PER_LONG != 64
> + mask_array[1] = mask->val >> BITS_PER_LONG;
> +#endif
Why not just "if (BITS_PER_LONG != 64)"?
Compiler will do its job here.
> + spin_unlock(&cap_userns_lock);
> +
> + t = *table;
> + t.data = &mask_array;
> +
> + /*
> + * actually read or write and array of ulongs from userspace. Remember
> + * these are least significant bits first
> + */
> + err = proc_doulongvec_minmax(&t, write, buffer, lenp, ppos);
> + if (err < 0)
> + return err;
> +
> + new_mask.val = mask_array[0];
> +#if BITS_PER_LONG != 64
> + new_mask.val += (u64)mask_array[1] << BITS_PER_LONG;
> +#endif
Ditto.
> +
> + /*
> + * Drop everything not in the new_mask (but don't add things)
> + */
> + if (write) {
> + spin_lock(&cap_userns_lock);
> + *mask = cap_intersect(*mask, new_mask);
> + spin_unlock(&cap_userns_lock);
> + }
> +
> + return 0;
> +}
> +#endif
> +
> static bool new_idmap_permitted(const struct file *file,
> struct user_namespace *ns, int cap_setid,
> struct uid_gid_map *map);
> @@ -46,6 +106,12 @@ static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> /* Limit userns capabilities to our parent's bounding set. */
> if (iscredsecure(cred, SECURE_USERNS_STRICT_CAPS))
> cred->cap_userns = cap_intersect(cred->cap_userns, cred->cap_bset);
> +#ifdef CONFIG_SYSCTL
> + /* Mask off userns capabilities that are not permitted by the system-wide mask. */
> + spin_lock(&cap_userns_lock);
> + cred->cap_userns = cap_intersect(cred->cap_userns, cap_userns_mask);
> + spin_unlock(&cap_userns_lock);
> +#endif
>
> /* Start with the capabilities defined in the userns set. */
> cred->cap_bset = cred->cap_userns;
BR, Jarkko
On Thu, May 16, 2024 at 02:22:02 -0700, Jonathan Calmels wrote:
> Jonathan Calmels (3):
> capabilities: user namespace capabilities
> capabilities: add securebit for strict userns caps
> capabilities: add cap userns sysctl mask
>
> fs/proc/array.c | 9 ++++
> include/linux/cred.h | 3 ++
> include/linux/securebits.h | 1 +
> include/linux/user_namespace.h | 7 +++
> include/uapi/linux/prctl.h | 7 +++
> include/uapi/linux/securebits.h | 11 ++++-
> kernel/cred.c | 3 ++
> kernel/sysctl.c | 10 ++++
> kernel/umh.c | 16 +++++++
> kernel/user_namespace.c | 83 ++++++++++++++++++++++++++++++---
> security/commoncap.c | 59 +++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 12 files changed, 204 insertions(+), 8 deletions(-)
I note a lack of any changes to `Documentation/` which seems quite
glaring for something with such a userspace visibility aspect to it.
--Ben
On Thu May 16, 2024 at 4:30 PM EEST, Ben Boeckel wrote:
> On Thu, May 16, 2024 at 02:22:02 -0700, Jonathan Calmels wrote:
> > Jonathan Calmels (3):
> > capabilities: user namespace capabilities
> > capabilities: add securebit for strict userns caps
> > capabilities: add cap userns sysctl mask
> >
> > fs/proc/array.c | 9 ++++
> > include/linux/cred.h | 3 ++
> > include/linux/securebits.h | 1 +
> > include/linux/user_namespace.h | 7 +++
> > include/uapi/linux/prctl.h | 7 +++
> > include/uapi/linux/securebits.h | 11 ++++-
> > kernel/cred.c | 3 ++
> > kernel/sysctl.c | 10 ++++
> > kernel/umh.c | 16 +++++++
> > kernel/user_namespace.c | 83 ++++++++++++++++++++++++++++++---
> > security/commoncap.c | 59 +++++++++++++++++++++++
> > security/keys/process_keys.c | 3 ++
> > 12 files changed, 204 insertions(+), 8 deletions(-)
>
> I note a lack of any changes to `Documentation/` which seems quite
> glaring for something with such a userspace visibility aspect to it.
>
> --Ben
Yeah, also in cover letter it would be nice to refresh what is
a bounding set. I had to xref that (recalled what it is), and
then got bored reading the rest :-)
Not exactly in the nutshell cover letter tbh, but maybe the
content in that would be better put to Documentation/
BR, Jarkko
On Thu, May 16, 2024 at 5:21 AM Jonathan Calmels <[email protected]> wrote:
>
> It's that time of the year again where we debate security settings for user
> namespaces ;)
>
> I’ve been experimenting with different approaches to address the gripe
> around user namespaces being used as attack vectors.
> After invaluable feedback from Serge and Christian offline, this is what I
> came up with.
As Serge is the capabilities maintainer it would be good to hear his
thoughts on-list about this proposal.
> There are obviously a lot of things we could do differently but I feel this
> is the right balance between functionality, simplicity and security. This
> also serves as a good foundation and could always be extended if the need
> arises in the future.
>
> Notes:
>
> - Adding a new capability set is far from ideal, but trying to reuse the
> existing capability framework was deemed both impractical and
> questionable security-wise, so here we are.
>
> - We might want to add new capabilities for some of the checks instead of
> reusing CAP_SETPCAP every time. Serge mentioned something like
> CAP_SYS_LIMIT?
>
> - In the last patch, we could decide to have stronger requirements and
> perform checks inside cap_capable() in case we want to retroactively
> prevent capabilities in old namespaces, this might be an overreach though
> so I left it out.
>
> I'm also not fond of the ulong logic for setting the sysctl parameter, on
> the other hand, the usermodhelper code always uses two u32s which makes it
> very confusing to set in userspace.
>
>
> Jonathan Calmels (3):
> capabilities: user namespace capabilities
> capabilities: add securebit for strict userns caps
> capabilities: add cap userns sysctl mask
>
> fs/proc/array.c | 9 ++++
> include/linux/cred.h | 3 ++
> include/linux/securebits.h | 1 +
> include/linux/user_namespace.h | 7 +++
> include/uapi/linux/prctl.h | 7 +++
> include/uapi/linux/securebits.h | 11 ++++-
> kernel/cred.c | 3 ++
> kernel/sysctl.c | 10 ++++
> kernel/umh.c | 16 +++++++
> kernel/user_namespace.c | 83 ++++++++++++++++++++++++++++++---
> security/commoncap.c | 59 +++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 12 files changed, 204 insertions(+), 8 deletions(-)
--
paul-moore.com
On Thu May 16, 2024 at 7:23 PM EEST, Paul Moore wrote:
> On Thu, May 16, 2024 at 5:21 AM Jonathan Calmels <[email protected]> wrote:
> >
> > It's that time of the year again where we debate security settings for user
> > namespaces ;)
> >
> > I’ve been experimenting with different approaches to address the gripe
> > around user namespaces being used as attack vectors.
> > After invaluable feedback from Serge and Christian offline, this is what I
> > came up with.
>
> As Serge is the capabilities maintainer it would be good to hear his
> thoughts on-list about this proposal.
Also it would make sense to make this just a bit more digestible to a
wider group of maintainers, i.e. a better introduction to the topic
instead of huge list of references (no bandwidth to read them all).
This is exactly kind of patch set that makes you ignore it unless
you are pro-active exactly in this domain.
I think this could bring more actually useful feedback.
BR, Jarkko
On Thu May 16, 2024 at 10:29 PM EEST, Jarkko Sakkinen wrote:
> On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
> > I suggest that adding a capability set for user namespaces is a bad idea:
> > - It is in no way obvious what problem it solves
> > - It is not obvious how it solves any problem
> > - The capability mechanism has not been popular, and relying on a
> > community (e.g. container developers) to embrace it based on this
> > enhancement is a recipe for failure
> > - Capabilities are already more complicated than modern developers
> > want to deal with. Adding another, special purpose set, is going
> > to make them even more difficult to use.
>
> What Inh, Prm, Eff, Bnd and Amb is not dead obvious to you? ;-)
> One UNs cannot hurt...
>
> I'm not following containers that much but didn't seccomp profiles
> supposed to be the silver bullet?
Also, I think Kata Containers style way of doing containers is pretty
solid. I've heard that some video streaming service at least in recent
past did launch VM per stream so it's not like VM's cannot be made to
scale I guess.
BR, Jarkko
On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
> I suggest that adding a capability set for user namespaces is a bad idea:
> - It is in no way obvious what problem it solves
> - It is not obvious how it solves any problem
> - The capability mechanism has not been popular, and relying on a
> community (e.g. container developers) to embrace it based on this
> enhancement is a recipe for failure
> - Capabilities are already more complicated than modern developers
> want to deal with. Adding another, special purpose set, is going
> to make them even more difficult to use.
What Inh, Prm, Eff, Bnd and Amb is not dead obvious to you? ;-)
One UNs cannot hurt...
I'm not following containers that much but didn't seccomp profiles
supposed to be the silver bullet?
BR, Jarkko
On Thu May 16, 2024 at 10:31 PM EEST, Jarkko Sakkinen wrote:
> On Thu May 16, 2024 at 10:29 PM EEST, Jarkko Sakkinen wrote:
> > On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
> > > I suggest that adding a capability set for user namespaces is a bad idea:
> > > - It is in no way obvious what problem it solves
> > > - It is not obvious how it solves any problem
> > > - The capability mechanism has not been popular, and relying on a
> > > community (e.g. container developers) to embrace it based on this
> > > enhancement is a recipe for failure
> > > - Capabilities are already more complicated than modern developers
> > > want to deal with. Adding another, special purpose set, is going
> > > to make them even more difficult to use.
> >
> > What Inh, Prm, Eff, Bnd and Amb is not dead obvious to you? ;-)
> > One UNs cannot hurt...
> >
> > I'm not following containers that much but didn't seccomp profiles
> > supposed to be the silver bullet?
>
> Also, I think Kata Containers style way of doing containers is pretty
> solid. I've heard that some video streaming service at least in recent
> past did launch VM per stream so it's not like VM's cannot be made to
> scale I guess.
Sorry for multiple responses but this actually nails the key question:
who will use this? Even if this would work out somehow, is there someone
who will actually use this, and not few other more robust solutions
available? I mean it is worth of time to maintain it, if there is no
potential users for a feature.
In addition to "show me the code", there is always also "show me the payload".
BR, Jarkko
On 5/16/24 02:22, Jonathan Calmels wrote:
> Attackers often rely on user namespaces to get elevated (yet confined)
> privileges in order to target specific subsystems (e.g. [1]). Distributions
> have been pretty adamant that they need a way to configure these, most of
> them carry out-of-tree patches to do so, or plainly refuse to enable them.
> As a result, there have been multiple efforts over the years to introduce
> various knobs to control and/or disable user namespaces (e.g. [2][3][4]).
>
> While we acknowledge that there are already ways to control the creation of
> such namespaces (the most recent being a LSM hook), there are inherent
> issues with these approaches. Preventing the user namespace creation is not
> fine-grained enough, and in some cases, incompatible with various userspace
agreed, though it really is application dependent. Some applications handle
the denial at userns creation better, than the capability after. Others
like anything based on QTWebEngine will crash on denial of userns creation
but handle denial of the capability within the userns just fine, and some
applications just crash regardless.
The userns cred from the LSM hook can be modified, yes it is currently
specified as const but is still under construction so it can be safely
modified the LSM hook just needs a small update.
The advantage of doing it under the LSM is an LSM can have a richer policy
around what can use them and tracking of what is allowed. That is to say the
LSM has the capability of being finer grained than doing it via capabilities.
I am not opposed to adding another mechanism to control user namespaces,
I am just not currently convinced that capabilities are the right
mechanism.
> expectations (e.g. container runtimes, browser sandboxing, service
> isolation)
>
> This patch addresses these limitations by introducing an additional
> capability set used to restrict the permissions granted when creating user
> namespaces. This way, processes can apply the principle of least privilege
> by configuring only the capabilities they need for their namespaces.
>
> For compatibility reasons, processes always start with a full userns
> capability set.
>
> On namespace creation, the userns capability set (pU) is assigned to the
> new effective (pE), permitted (pP) and bounding set (X) of the task:
>
> pU = pE = pP = X
>
this should be bounded by the creating task's bounding set, other wise
the capability model's bounding invariant will be broken, but having the
capabilities that the userns want to access in the task's bounding set is
a problem for all the unprivileged processes wanting access to user
namespaces.
Simply setting the userns fcap on the programs that want access to user
namespaces, does certainly reduce the attack surface, but really is
insufficient for utilities like unshare, bwrap, lxd etc. They can be
used to trivially by-pass the restriction.
> The userns capability set obeys the invariant that no bit can ever be set
> if it is not already part of the task’s bounding set. This ensures that no
> namespace can ever gain more privileges than its predecessors.
> Additionally, if a task is not privileged over CAP_SETPCAP, setting any bit
> in the userns set requires its corresponding bit to be set in the permitted
> set. This effectively mimics the inheritable set rules and means that, by
> default, only root in the initial user namespace can gain userns
> capabilities:
>
> p’U = (pE & CAP_SETPCAP) ? X : (X & pP)
>
If I am reading this right for unprivileged processes the capabilities in
the userns are bounded by the processes permitted set before the userns is
created?
This is only being respected in PR_CTL, the user mode helper is straight
setting the caps.
> Note that since userns capabilities are strictly hierarchical, policies can
> be enforced at various levels (e.g. init, pam_cap) and inherited by every
> child namespace.
>
> Here is a sample program that can be used to verify the functionality:
>
> /*
> * Test program that drops CAP_SYS_RAWIO from subsequent user namespaces.
> *
> * ./cap_userns_test unshare -r grep Cap /proc/self/status
> * CapInh: 0000000000000000
> * CapPrm: 000001fffffdffff
> * CapEff: 000001fffffdffff
> * CapBnd: 000001fffffdffff
> * CapAmb: 0000000000000000
> * CapUNs: 000001fffffdffff
> */
>
> int main(int argc, char *argv[])
> {
> if (prctl(PR_CAP_USERNS, PR_CAP_USERNS_LOWER, CAP_SYS_RAWIO, 0, 0) < 0)
> err(1, "cannot drop userns cap");
>
> execvp(argv[1], argv + 1);
> err(1, "cannot exec");
> }
>
> Link: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/containers/[email protected]
>
> Signed-off-by: Jonathan Calmels <[email protected]>
> ---
> fs/proc/array.c | 9 ++++++
> include/linux/cred.h | 3 ++
> include/uapi/linux/prctl.h | 7 +++++
> kernel/cred.c | 3 ++
> kernel/umh.c | 16 ++++++++++
> kernel/user_namespace.c | 12 +++-----
> security/commoncap.c | 59 ++++++++++++++++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 8 files changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 34a47fb0c57f..364e8bb19f9d 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -313,6 +313,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> const struct cred *cred;
> kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> cap_bset, cap_ambient;
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns;
> +#endif
>
> rcu_read_lock();
> cred = __task_cred(p);
> @@ -321,6 +324,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> cap_effective = cred->cap_effective;
> cap_bset = cred->cap_bset;
> cap_ambient = cred->cap_ambient;
> +#ifdef CONFIG_USER_NS
> + cap_userns = cred->cap_userns;
> +#endif
> rcu_read_unlock();
>
> render_cap_t(m, "CapInh:\t", &cap_inheritable);
> @@ -328,6 +334,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapEff:\t", &cap_effective);
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> render_cap_t(m, "CapAmb:\t", &cap_ambient);
> +#ifdef CONFIG_USER_NS
> + render_cap_t(m, "CapUNs:\t", &cap_userns);
> +#endif
> }
>
> static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 2976f534a7a3..adab0031443e 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -124,6 +124,9 @@ struct cred {
> kernel_cap_t cap_effective; /* caps we can actually use */
> kernel_cap_t cap_bset; /* capability bounding set */
> kernel_cap_t cap_ambient; /* Ambient capability set */
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns; /* User namespace capability set */
> +#endif
> #ifdef CONFIG_KEYS
> unsigned char jit_keyring; /* default keyring to attach requested
> * keys to */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 370ed14b1ae0..e09475171f62 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -198,6 +198,13 @@ struct prctl_mm_map {
> # define PR_CAP_AMBIENT_LOWER 3
> # define PR_CAP_AMBIENT_CLEAR_ALL 4
>
> +/* Control the userns capability set */
> +#define PR_CAP_USERNS 48
> +# define PR_CAP_USERNS_IS_SET 1
> +# define PR_CAP_USERNS_RAISE 2
> +# define PR_CAP_USERNS_LOWER 3
> +# define PR_CAP_USERNS_CLEAR_ALL 4
> +
> /* arm64 Scalable Vector Extension controls */
> /* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
> #define PR_SVE_SET_VL 50 /* set task vector length */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 075cfa7c896f..9912c6f3bc6b 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -56,6 +56,9 @@ struct cred init_cred = {
> .cap_permitted = CAP_FULL_SET,
> .cap_effective = CAP_FULL_SET,
> .cap_bset = CAP_FULL_SET,
> +#ifdef CONFIG_USER_NS
> + .cap_userns = CAP_FULL_SET,
> +#endif
> .user = INIT_USER,
> .user_ns = &init_user_ns,
> .group_info = &init_groups,
> diff --git a/kernel/umh.c b/kernel/umh.c
> index 1b13c5d34624..51f1e1d25d49 100644
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -32,6 +32,9 @@
>
> #include <trace/events/module.h>
>
> +#ifdef CONFIG_USER_NS
> +static kernel_cap_t usermodehelper_userns = CAP_FULL_SET;
> +#endif
> static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
> static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
> static DEFINE_SPINLOCK(umh_sysctl_lock);
> @@ -94,6 +97,10 @@ static int call_usermodehelper_exec_async(void *data)
> new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
> new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
> new->cap_inheritable);
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = cap_intersect(usermodehelper_userns,
> + new->cap_userns);
> +#endif
> spin_unlock(&umh_sysctl_lock);
>
> if (sub_info->init) {
> @@ -560,6 +567,15 @@ static struct ctl_table usermodehelper_table[] = {
> .mode = 0600,
> .proc_handler = proc_cap_handler,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "userns",
> + .data = &usermodehelper_userns,
> + .maxlen = 2 * sizeof(unsigned long),
> + .mode = 0600,
> + .proc_handler = proc_cap_handler,
> + },
> +#endif
> { }
> };
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0b0b95418b16..7e624607330b 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -42,15 +42,13 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>
> static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> {
> - /* Start with the same capabilities as init but useless for doing
> - * anything as the capabilities are bound to the new user namespace.
> - */
> - cred->securebits = SECUREBITS_DEFAULT;
> + /* Start with the capabilities defined in the userns set. */
> + cred->cap_bset = cred->cap_userns;
> + cred->cap_permitted = cred->cap_userns;
> + cred->cap_effective = cred->cap_userns;
> cred->cap_inheritable = CAP_EMPTY_SET;
> - cred->cap_permitted = CAP_FULL_SET;
> - cred->cap_effective = CAP_FULL_SET;
> cred->cap_ambient = CAP_EMPTY_SET;
> - cred->cap_bset = CAP_FULL_SET;
> + cred->securebits = SECUREBITS_DEFAULT;
> #ifdef CONFIG_KEYS
> key_put(cred->request_key_auth);
> cred->request_key_auth = NULL;
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 162d96b3a676..b3d3372bf910 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -228,6 +228,28 @@ static inline int cap_inh_is_capped(void)
> return 1;
> }
>
> +/*
> + * Determine whether a userns capability can be raised.
> + * Returns 1 if it can, 0 otherwise.
> + */
> +#ifdef CONFIG_USER_NS
> +static inline int cap_uns_is_raiseable(unsigned long cap)
> +{
> + if (!!cap_raised(current_cred()->cap_userns, cap))
> + return 1;
> + /* a capability cannot be raised unless the current task has it in
> + * its bounding set and, without CAP_SETPCAP, its permitted set.
> + */
> + if (!cap_raised(current_cred()->cap_bset, cap))
> + return 0;
> + if (cap_capable(current_cred(), current_cred()->user_ns,
> + CAP_SETPCAP, CAP_OPT_NONE) != 0 &&
> + !cap_raised(current_cred()->cap_permitted, cap))
> + return 0;
> + return 1;
> +}
> +#endif
> +
> /**
> * cap_capset - Validate and apply proposed changes to current's capabilities
> * @new: The proposed new credentials; alterations should be made here
> @@ -1382,6 +1404,43 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> return commit_creds(new);
> }
>
> +#ifdef CONFIG_USER_NS
> + case PR_CAP_USERNS:
> + if (arg2 == PR_CAP_USERNS_CLEAR_ALL) {
> + if (arg3 | arg4 | arg5)
> + return -EINVAL;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + cap_clear(new->cap_userns);
> + return commit_creds(new);
> + }
> +
> + if (((!cap_valid(arg3)) | arg4 | arg5))
> + return -EINVAL;
> +
> + if (arg2 == PR_CAP_USERNS_IS_SET) {
> + return !!cap_raised(current_cred()->cap_userns, arg3);
> + } else if (arg2 != PR_CAP_USERNS_RAISE &&
> + arg2 != PR_CAP_USERNS_LOWER) {
> + return -EINVAL;
> + } else {
> + if (arg2 == PR_CAP_USERNS_RAISE &&
> + !cap_uns_is_raiseable(arg3))
> + return -EPERM;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + if (arg2 == PR_CAP_USERNS_RAISE)
> + cap_raise(new->cap_userns, arg3);
> + else
> + cap_lower(new->cap_userns, arg3);
> + return commit_creds(new);
> + }
> +#endif
> +
> default:
> /* No functionality available - continue with default */
> return -ENOSYS;
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index b5d5333ab330..e3670d815435 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -944,6 +944,9 @@ void key_change_session_keyring(struct callback_head *twork)
> new->cap_effective = old->cap_effective;
> new->cap_ambient = old->cap_ambient;
> new->cap_bset = old->cap_bset;
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = old->cap_userns;
> +#endif
>
> new->jit_keyring = old->jit_keyring;
> new->thread_keyring = key_get(old->thread_keyring);
On 5/16/2024 2:22 AM, Jonathan Calmels wrote:
> It's that time of the year again where we debate security settings for user
> namespaces ;)
>
> I’ve been experimenting with different approaches to address the gripe
> around user namespaces being used as attack vectors.
> After invaluable feedback from Serge and Christian offline, this is what I
> came up with.
>
> There are obviously a lot of things we could do differently but I feel this
> is the right balance between functionality, simplicity and security. This
> also serves as a good foundation and could always be extended if the need
> arises in the future.
>
> Notes:
>
> - Adding a new capability set is far from ideal, but trying to reuse the
> existing capability framework was deemed both impractical and
> questionable security-wise, so here we are.
I suggest that adding a capability set for user namespaces is a bad idea:
- It is in no way obvious what problem it solves
- It is not obvious how it solves any problem
- The capability mechanism has not been popular, and relying on a
community (e.g. container developers) to embrace it based on this
enhancement is a recipe for failure
- Capabilities are already more complicated than modern developers
want to deal with. Adding another, special purpose set, is going
to make them even more difficult to use.
> - We might want to add new capabilities for some of the checks instead of
> reusing CAP_SETPCAP every time. Serge mentioned something like
> CAP_SYS_LIMIT?
>
> - In the last patch, we could decide to have stronger requirements and
> perform checks inside cap_capable() in case we want to retroactively
> prevent capabilities in old namespaces, this might be an overreach though
> so I left it out.
>
> I'm also not fond of the ulong logic for setting the sysctl parameter, on
> the other hand, the usermodhelper code always uses two u32s which makes it
> very confusing to set in userspace.
>
>
> Jonathan Calmels (3):
> capabilities: user namespace capabilities
> capabilities: add securebit for strict userns caps
> capabilities: add cap userns sysctl mask
>
> fs/proc/array.c | 9 ++++
> include/linux/cred.h | 3 ++
> include/linux/securebits.h | 1 +
> include/linux/user_namespace.h | 7 +++
> include/uapi/linux/prctl.h | 7 +++
> include/uapi/linux/securebits.h | 11 ++++-
> kernel/cred.c | 3 ++
> kernel/sysctl.c | 10 ++++
> kernel/umh.c | 16 +++++++
> kernel/user_namespace.c | 83 ++++++++++++++++++++++++++++++---
> security/commoncap.c | 59 +++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 12 files changed, 204 insertions(+), 8 deletions(-)
>
On Thu, May 16, 2024 at 04:36:07PM GMT, Jarkko Sakkinen wrote:
> On Thu May 16, 2024 at 4:30 PM EEST, Ben Boeckel wrote:
> > I note a lack of any changes to `Documentation/` which seems quite
> > glaring for something with such a userspace visibility aspect to it.
> >
> > --Ben
>
> Yeah, also in cover letter it would be nice to refresh what is
> a bounding set. I had to xref that (recalled what it is), and
> then got bored reading the rest :-)
Thanks for reminding me, I actually meant to do it, just forgot.
Having said that, `Documentation/security/credentials.rst` is not the
best documention when it comes to capabilities. I will definitely add
few more lines in there, but it's probably not what you're looking for.
capabilities(7) is where everything is explained, I should have
mentioned it. I could try to summarize the existing sets, but honestly I
will probably do a worse job than the man page.
I do plan to update the man page though if it comes to that.
On 5/17/24 03:51, Jonathan Calmels wrote:
> On Thu, May 16, 2024 at 03:07:28PM GMT, John Johansen wrote:
>> agreed, though it really is application dependent. Some applications handle
>> the denial at userns creation better, than the capability after. Others
>> like anything based on QTWebEngine will crash on denial of userns creation
>> but handle denial of the capability within the userns just fine, and some
>> applications just crash regardless.
>
> Yes this is application specific, but I would argue that the latter is
> much more preferable. For example, having one application crash in a
> container is probably ok, but not being able to start the container in
> the first place is probably not. Similarly, preventing the network
> namespace creation breaks services which rely on systemd’s
> PrivateNetwork, even though they most likely use it to prevent any
> networking from being done.
>
Agred the solution has to be application/usage model specific. Some of
them are easy, and others not so much.
>> The userns cred from the LSM hook can be modified, yes it is currently
>> specified as const but is still under construction so it can be safely
>> modified the LSM hook just needs a small update.
>>
>> The advantage of doing it under the LSM is an LSM can have a richer policy
>> around what can use them and tracking of what is allowed. That is to say the
>> LSM has the capability of being finer grained than doing it via capabilities.
>
> Sure, we could modify the LSM hook to do all sorts of things, but
> leveraging it would be quite cumbersome, will take time to show up in
> userspace, or simply never be adopted.
> We’re already seeing it in Ubuntu which started requiring Apparmor profiles.
>
yes, I would argue that is a metric of adoption.
> This new capability set would be a universal thing that could be
> leveraged today without modification to userspace. Moreover, it’s a
> simple framework that can be extended.
I would argue that is a problem. Userspace has to change for this to be
secure. Is it an improvement over the current state yes.
> As you mentioned, LSMs are even finer grained, and that’s the idea,
> those could be used hand in hand eventually. You could envision LSM
> hooks controlling the userns capability set, and thus enforce policies
> on the creation of nested namespaces without limiting the other tasks’
> capabilities.
>
>> I am not opposed to adding another mechanism to control user namespaces,
>> I am just not currently convinced that capabilities are the right
>> mechanism.
>
> Well that’s the thing, from past conversations, there is a lot of
> disagreement about restricting namespaces. By restricting the
> capabilities granted by namespaces instead, we’re actually treating the
> root cause of most concerns.
>
no disagreement there. This is actually Ubuntu's posture with user namespaces
atm. Where the user namespace is allowed but the capabilities within it
are denied.
It does however when not handled correctly result in some very odd failures
and would be easier to debug if the use of user namespaces were just
cleanly denied.
> Today user namespaces are "special" and always grant full caps. Adding a
> new capability set to limit this behavior is logical; same way it's done
> for usual process transitions.
> Essentially this set is to namespaces what the inheritable set is to
> root.
>
its not so much the capabilities set as the inheritable part that is
problematic. Yes I am well aware of where that is required but I question
that capabilities provides the needed controls here.
>> this should be bounded by the creating task's bounding set, other wise
>> the capability model's bounding invariant will be broken, but having the
>> capabilities that the userns want to access in the task's bounding set is
>> a problem for all the unprivileged processes wanting access to user
>> namespaces.
>
> This is possible with the security bit introduced in the second patch.
> The idea of having those separate is that a service which has dropped
> its capabilities can still create a fully privileged user namespace.
yes, which is the problem. Not that we don't do that with say setuid
applications, but the difference is that they were known to be doing
something dangerous and took measures around that.
We are starting from a different posture here. Where applications have
assumed that user namespaces where safe and no measures were needed.
Tools like unshare and bwrap if set to allow user namespaces in their
fcaps will allow exploits a trivial by-pass.
> For example, systemd’s machined drops capabilities from its bounding set,
> yet it should be able to create unprivileged containers.
> The invariant is sound because a child userns can never regain what it
> doesn’t have in its bounding set. If it helps you can view the userns
> set as a “namespace bounding set” since it defines the future bounding
> sets of namespaced tasks.
>
sure I get it, some of the use cases work, some not so well
>> If I am reading this right for unprivileged processes the capabilities in
>> the userns are bounded by the processes permitted set before the userns is
>> created?
>
> Yes, unprivileged processes that want to raise a capability in their
> userns set need it in their permitted set (as well as their bounding
> set). This is similar to inheritable capabilities.
Right.
> Recall that processes start with a full set of userns capabilities, so
> if you drop a userns capability (or something else did, e.g.
> init/pam/sysctl/parent) you will never be able to regain it, and
> namespaces you create won't have it included.
sure, that part of the behavior is fine
> Now, if you’re root (or cap privileged) you can always regain it.
>
yes
What I was trying to get at is two points.
1. The written description wasn't clear enough, leaving room for
ambiguity.
2. That I quest that the behavior should be allowed given the
current set of tools that use user namespaces. It reduces exploit
codes ability to directly use unprivileged user namespaces but
makes it all to easy to by-pass the restriction because of the
behavior of the current tool set. ie. user space has to change.
>> This is only being respected in PR_CTL, the user mode helper is straight
>> setting the caps.
>
> Usermod helper requires CAP_SYS_MODULE and CAP_SETPCAP in the initns so
> the permitted set is irrelevant there. It starts with a full set but from
> there you can only lower caps, so the invariant holds.
>
sure, I get what is happening. Again the description needs work. It was
ambiguous as to whether it was applying to the fcaps or only the pcaps.
But again, I believe the fcaps behavior is wrong, because of the state of
current software. If this had been a proposal where there was no existing
software infrastructure I would be starting from a different stance.
Jonathan Calmels <[email protected]> writes:
> Attackers often rely on user namespaces to get elevated (yet confined)
> privileges in order to target specific subsystems (e.g. [1]). Distributions
> have been pretty adamant that they need a way to configure these, most of
> them carry out-of-tree patches to do so, or plainly refuse to enable
> them.
Pointers please?
That sentence sounds about 5 years out of date.
Eric
On Thu, May 16, 2024 at 03:07:28PM GMT, John Johansen wrote:
> agreed, though it really is application dependent. Some applications handle
> the denial at userns creation better, than the capability after. Others
> like anything based on QTWebEngine will crash on denial of userns creation
> but handle denial of the capability within the userns just fine, and some
> applications just crash regardless.
Yes this is application specific, but I would argue that the latter is
much more preferable. For example, having one application crash in a
container is probably ok, but not being able to start the container in
the first place is probably not. Similarly, preventing the network
namespace creation breaks services which rely on systemd’s
PrivateNetwork, even though they most likely use it to prevent any
networking from being done.
> The userns cred from the LSM hook can be modified, yes it is currently
> specified as const but is still under construction so it can be safely
> modified the LSM hook just needs a small update.
>
> The advantage of doing it under the LSM is an LSM can have a richer policy
> around what can use them and tracking of what is allowed. That is to say the
> LSM has the capability of being finer grained than doing it via capabilities.
Sure, we could modify the LSM hook to do all sorts of things, but
leveraging it would be quite cumbersome, will take time to show up in
userspace, or simply never be adopted.
We’re already seeing it in Ubuntu which started requiring Apparmor profiles.
This new capability set would be a universal thing that could be
leveraged today without modification to userspace. Moreover, it’s a
simple framework that can be extended.
As you mentioned, LSMs are even finer grained, and that’s the idea,
those could be used hand in hand eventually. You could envision LSM
hooks controlling the userns capability set, and thus enforce policies
on the creation of nested namespaces without limiting the other tasks’
capabilities.
> I am not opposed to adding another mechanism to control user namespaces,
> I am just not currently convinced that capabilities are the right
> mechanism.
Well that’s the thing, from past conversations, there is a lot of
disagreement about restricting namespaces. By restricting the
capabilities granted by namespaces instead, we’re actually treating the
root cause of most concerns.
Today user namespaces are "special" and always grant full caps. Adding a
new capability set to limit this behavior is logical; same way it's done
for usual process transitions.
Essentially this set is to namespaces what the inheritable set is to
root.
> this should be bounded by the creating task's bounding set, other wise
> the capability model's bounding invariant will be broken, but having the
> capabilities that the userns want to access in the task's bounding set is
> a problem for all the unprivileged processes wanting access to user
> namespaces.
This is possible with the security bit introduced in the second patch.
The idea of having those separate is that a service which has dropped
its capabilities can still create a fully privileged user namespace.
For example, systemd’s machined drops capabilities from its bounding set,
yet it should be able to create unprivileged containers.
The invariant is sound because a child userns can never regain what it
doesn’t have in its bounding set. If it helps you can view the userns
set as a “namespace bounding set” since it defines the future bounding
sets of namespaced tasks.
> If I am reading this right for unprivileged processes the capabilities in
> the userns are bounded by the processes permitted set before the userns is
> created?
Yes, unprivileged processes that want to raise a capability in their
userns set need it in their permitted set (as well as their bounding
set). This is similar to inheritable capabilities.
Recall that processes start with a full set of userns capabilities, so
if you drop a userns capability (or something else did, e.g.
init/pam/sysctl/parent) you will never be able to regain it, and
namespaces you create won't have it included.
Now, if you’re root (or cap privileged) you can always regain it.
> This is only being respected in PR_CTL, the user mode helper is straight
> setting the caps.
Usermod helper requires CAP_SYS_MODULE and CAP_SETPCAP in the initns so
the permitted set is irrelevant there. It starts with a full set but from
there you can only lower caps, so the invariant holds.
On 5/17/24 04:55, Jonathan Calmels wrote:
> On Fri, May 17, 2024 at 06:32:46AM GMT, Eric W. Biederman wrote:
>>
>> Pointers please?
>>
>> That sentence sounds about 5 years out of date.
>
> The link referenced is from last year.
> Here are some others often cited by distributions:
>
> https://nvd.nist.gov/vuln/detail/CVE-2022-0185
> https://nvd.nist.gov/vuln/detail/CVE-2022-1015
> https://nvd.nist.gov/vuln/detail/CVE-2022-2078
> https://nvd.nist.gov/vuln/detail/CVE-2022-24122
> https://nvd.nist.gov/vuln/detail/CVE-2022-25636
>
> Recent thread discussing this too:
> https://seclists.org/oss-sec/2024/q2/128
>
they were used in 2020, 2021, and 2022 pwn2own exploits. Sorry I don't remember the exact numbers and will have to dig.
pwn2own 2023 4/5 hacks used them
https://www.zerodayinitiative.com/blog/2023/3/23/pwn2own-vancouver-2023-day-two-results
I will need to dig to find the CVEs associated with them.
pwn2own 2024 I can not discuss atm
but its not just pwn2own, the actual list of kernel CVEs that unprivileged user namespaces make exploitable is much larger.
On Fri, May 17, 2024 at 06:32:46AM GMT, Eric W. Biederman wrote:
>
> Pointers please?
>
> That sentence sounds about 5 years out of date.
The link referenced is from last year.
Here are some others often cited by distributions:
https://nvd.nist.gov/vuln/detail/CVE-2022-0185
https://nvd.nist.gov/vuln/detail/CVE-2022-1015
https://nvd.nist.gov/vuln/detail/CVE-2022-2078
https://nvd.nist.gov/vuln/detail/CVE-2022-24122
https://nvd.nist.gov/vuln/detail/CVE-2022-25636
Recent thread discussing this too:
https://seclists.org/oss-sec/2024/q2/128
> > > On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
> > > > I suggest that adding a capability set for user namespaces is a bad idea:
> > > > - It is in no way obvious what problem it solves
> > > > - It is not obvious how it solves any problem
> > > > - The capability mechanism has not been popular, and relying on a
> > > > community (e.g. container developers) to embrace it based on this
> > > > enhancement is a recipe for failure
> > > > - Capabilities are already more complicated than modern developers
> > > > want to deal with. Adding another, special purpose set, is going
> > > > to make them even more difficult to use.
Sorry if the commit wasn't clear enough. Basically:
- Today user namespaces grant full capabilities.
This behavior is often abused to attack various kernel subsystems.
Only option is to disable them altogether which breaks a lot of
userspace stuff.
This goes against the least privilege principle.
- It adds a new capability set.
This set dictates what capabilities are granted in namespaces (instead
of always getting full caps).
This brings namespaces in line with the rest of the system, user
namespaces are no more "special".
They now work the same way as say a transition to root does with
inheritable caps.
- This isn't intended to be used by end users per se (although they could).
This would be used at the same places where existing capabalities are
used today (e.g. init system, pam, container runtime, browser
sandbox), or by system administrators.
To give you some ideas of things you could do:
# E.g. prevent alice from getting CAP_NET_ADMIN in user namespaces under SSH
echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
echo "!cap_net_admin alice" >> /etc/security/capability.conf.
# E.g. prevent any Docker container from ever getting CAP_DAC_OVERRIDE
systemd-run -p CapabilityBoundingSet=~CAP_DAC_OVERRIDE \
-p SecureBits=userns-strict-caps \
/usr/bin/dockerd
# E.g. kernel could be vulnerable to CAP_SYS_RAWIO exploits
# Prevent users from ever gaining it
sysctl -w cap_bound_userns_mask=0x1fffffdffff
Jonathan Calmels <[email protected]> writes:
> On Fri, May 17, 2024 at 06:32:46AM GMT, Eric W. Biederman wrote:
>>
>> Pointers please?
>>
>> That sentence sounds about 5 years out of date.
>
> The link referenced is from last year.
> Here are some others often cited by distributions:
>
> https://nvd.nist.gov/vuln/detail/CVE-2022-0185
> https://nvd.nist.gov/vuln/detail/CVE-2022-1015
> https://nvd.nist.gov/vuln/detail/CVE-2022-2078
> https://nvd.nist.gov/vuln/detail/CVE-2022-24122
> https://nvd.nist.gov/vuln/detail/CVE-2022-25636
>
> Recent thread discussing this too:
> https://seclists.org/oss-sec/2024/q2/128
My apologies perhaps I trimmed too much.
I know that user namespaces enlarge the attack surface.
How much and how serious could be debated but for unprivileged
users the attack surface is undoubtedly enlarged.
As I read your introduction you were justifying the introduction
of a new security mechanism with the observation that distributions
were carrying distribution specific patches.
To the best of my knowledge distribution specific patches and
distributions disabling user namespaces have been gone for quite a
while. So if that has changed recently I would like to know.
Thank you,
Eric
On 5/17/2024 4:42 AM, Jonathan Calmels wrote:
>>>> On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
>>>>> I suggest that adding a capability set for user namespaces is a bad idea:
>>>>> - It is in no way obvious what problem it solves
>>>>> - It is not obvious how it solves any problem
>>>>> - The capability mechanism has not been popular, and relying on a
>>>>> community (e.g. container developers) to embrace it based on this
>>>>> enhancement is a recipe for failure
>>>>> - Capabilities are already more complicated than modern developers
>>>>> want to deal with. Adding another, special purpose set, is going
>>>>> to make them even more difficult to use.
> Sorry if the commit wasn't clear enough.
While, as others have pointed out, the commit description left
much to be desired, that isn't the biggest problem with the change
you're proposing.
> Basically:
>
> - Today user namespaces grant full capabilities.
Of course they do. I have been following the use of capabilities
in Linux since before they were implemented. The uptake has been
disappointing in all use cases.
> This behavior is often abused to attack various kernel subsystems.
Yes. The problems of a single, all powerful root privilege scheme are
well documented.
> Only option
Hardly.
> is to disable them altogether which breaks a lot of
> userspace stuff.
Updating userspace components to behave properly in a capabilities
environment has never been a popular activity, but is the right way
to address this issue. And before you start on the "no one can do that,
it's too hard", I'll point out that multiple UNIX systems supported
rootless, all capabilities based systems back in the day.
> This goes against the least privilege principle.
If you're going to run userspace that *requires* privilege, you have
to have a way to *allow* privilege. If the userspace insists on a root
based privilege model, you're stuck supporting it. Regardless of your
principles.
>
> - It adds a new capability set.
Which is a really, really bad idea. The equation for calculating effective
privilege is already more complicated than userspace developers are generally
willing to put up with.
> This set dictates what capabilities are granted in namespaces (instead
> of always getting full caps).
I would not expect container developers to be eager to learn how to use
this facility.
> This brings namespaces in line with the rest of the system, user
> namespaces are no more "special".
I'm sorry, but this makes no sense to me whatsoever. You want to introduce
a capability set explicitly for namespaces in order to make them less
special? Maybe I'm just old and cranky.
> They now work the same way as say a transition to root does with
> inheritable caps.
That needs some explanation.
>
> - This isn't intended to be used by end users per se (although they could).
> This would be used at the same places where existing capabalities are
> used today (e.g. init system, pam, container runtime, browser
> sandbox), or by system administrators.
I understand that. It is for containers. Containers are not kernel entities.
>
> To give you some ideas of things you could do:
>
> # E.g. prevent alice from getting CAP_NET_ADMIN in user namespaces under SSH
> echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
> echo "!cap_net_admin alice" >> /etc/security/capability.conf.
>
> # E.g. prevent any Docker container from ever getting CAP_DAC_OVERRIDE
> systemd-run -p CapabilityBoundingSet=~CAP_DAC_OVERRIDE \
> -p SecureBits=userns-strict-caps \
> /usr/bin/dockerd
>
> # E.g. kernel could be vulnerable to CAP_SYS_RAWIO exploits
> # Prevent users from ever gaining it
> sysctl -w cap_bound_userns_mask=0x1fffffdffff
> > On Fri, May 17, 2024 at 06:32:46AM GMT, Eric W. Biederman wrote:
> As I read your introduction you were justifying the introduction
> of a new security mechanism with the observation that distributions
> were carrying distribution specific patches.
>
> To the best of my knowledge distribution specific patches and
> distributions disabling user namespaces have been gone for quite a
> while. So if that has changed recently I would like to know.
On the top of my head:
- RHEL based:
namespace.unpriv_enable
user_namespace.enable
- Arch/Debian based:
kernel.unprivileged_userns_clone
- Ubuntu based:
kernel.apparmor_restrict_unprivileged_userns
I'm not sure which exact version those apply to, but it's definitely
still out there.
The observation is that while you can disable namespaces today, in
practice it breaks userspace in various ways. Hence, being able to
control capabilities is a better way to approach it.
For example, today's big hammer to prevent CAP_NET_ADMIN in userns:
# sysctl -qw user.max_net_namespaces=0
$ unshare -U -r -n ip tuntap add mode tap tap0 && echo OK
unshare: unshare failed: No space left on device
With patch, this becomes manageable:
# capsh --drop=cap_net_admin --secbits=$((1 << 8)) --user=$USER -- \
-c 'unshare -U -r -n ip tuntap add mode tap tap0 && echo OK'
ioctl(TUNSETIFF): Operation not permitted
On Fri, May 17, 2024 at 10:53:24AM GMT, Casey Schaufler wrote:
> Of course they do. I have been following the use of capabilities
> in Linux since before they were implemented. The uptake has been
> disappointing in all use cases.
Why "Of course"?
What if they should not get *all* privileges?
> Yes. The problems of a single, all powerful root privilege scheme are
> well documented.
That's my point, it doesn't have to be this way.
> Hardly.
Maybe I'm missing something, then.
How do I restrict my users from gaining say CAP_NET_ADMIN in their
userns today?
> If you're going to run userspace that *requires* privilege, you have
> to have a way to *allow* privilege. If the userspace insists on a root
> based privilege model, you're stuck supporting it. Regardless of your
> principles.
I want *some* privileges, not *all* of them.
> Which is a really, really bad idea. The equation for calculating effective
> privilege is already more complicated than userspace developers are generally
> willing to put up with.
This is generally true, but this set is way more straightforward than
the other sets, it's always:
pU = pP = pE = X
If you look at the patch, there is no transition logic or anything
complicated, it's just a set of caps behind inherited.
> I would not expect container developers to be eager to learn how to use
> this facility.
And they probably wouldn't.
For most use cases it's going to be enforced through system policies
(init, pam, etc). Other than that, usage won't change, you will run your
usual `docker run --cap-add ...` to get caps, except now it works in
userns.
> I'm sorry, but this makes no sense to me whatsoever. You want to introduce
> a capability set explicitly for namespaces in order to make them less
> special? Maybe I'm just old and cranky.
>
> > They now work the same way as say a transition to root does with
> > inheritable caps.
>
> That needs some explanation.
From man capabilities(7):
In order to mirror traditional UNIX semantics, the kernel performs
special treatment of file capabilities when a process with UID 0 (root)
executes a program [...]
Thus, when [...] a process whose real and effective UIDs are
zero execve(2)s a program, the calculation of the process's new
permitted capabilities simplifies to:
P'(permitted) = P(inheritable) | P(bounding)
P'(effective) = P'(permitted)
So, the same way a root process is bounded by its inheritable set when
it execs, a "rootless" process is bounded by its userns set when it
unshares.
On Fri, May 17, 2024 at 04:59:41AM GMT, John Johansen wrote:
> On 5/17/24 03:51, Jonathan Calmels wrote:
> > This new capability set would be a universal thing that could be
> > leveraged today without modification to userspace. Moreover, it’s a
> > simple framework that can be extended.
>
> I would argue that is a problem. Userspace has to change for this to be
> secure. Is it an improvement over the current state yes.
Well, yes and no. With those patches, I can lock down things today on my
system and I don't need to change anything.
For example I can decide that none of my rootless containers started
under SSH will get CAP_NET_ADMIN:
# echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
# echo "!cap_net_admin $USER" >> /etc/security/capability.conf
# capsh --secbits=$((1 << 8)) -- -c /usr/sbin/sshd
$ ssh localhost 'unshare -r capsh --current'
Current: =ep cap_net_admin-ep
Current IAB: !cap_net_admin
Or I can decide than I don't ever want CAP_SYS_RAWIO in my namespaces:
# sysctl -w cap_bound_userns_mask=0x1fffffdffff
This doesn't require changes to userspace.
Now, granted if you want to have finer-grained controls, it will require
*some* changes in *some* places (e.g. adding new systemd property like
UserNSSet=).
> > Well that’s the thing, from past conversations, there is a lot of
> > disagreement about restricting namespaces. By restricting the
> > capabilities granted by namespaces instead, we’re actually treating the
> > root cause of most concerns.
> >
> no disagreement there. This is actually Ubuntu's posture with user namespaces
> atm. Where the user namespace is allowed but the capabilities within it
> are denied.
>
> It does however when not handled correctly result in some very odd failures
> and would be easier to debug if the use of user namespaces were just
> cleanly denied.
Yes but as we established it depends on the use case, both are not
mutually exclusive.
> its not so much the capabilities set as the inheritable part that is
> problematic. Yes I am well aware of where that is required but I question
> that capabilities provides the needed controls here.
Again, I'm not opposed to doing this with LSMs. I just think both could
work well together. We already do that with standard capabilities vs
LSMs, both have their strength and weaknesses.
It's always a tradeoff, do you want a setting that's universal and
coarse, or do you want one that's tailored to specific things but less
ubiquitous.
It's also a tradeoff on usability. If this doesn't get used in practice,
then there is no point.
I would argue that even though capabilities are complicated, they are
more widely understood than LSMs. Are capabilities insufficient in
certain scenarios, absolutely, and that's usually where LSMs come in.
> > This is possible with the security bit introduced in the second patch.
> > The idea of having those separate is that a service which has dropped
> > its capabilities can still create a fully privileged user namespace.
>
> yes, which is the problem. Not that we don't do that with say setuid
> applications, but the difference is that they were known to be doing
> something dangerous and took measures around that.
>
> We are starting from a different posture here. Where applications have
> assumed that user namespaces where safe and no measures were needed.
> Tools like unshare and bwrap if set to allow user namespaces in their
> fcaps will allow exploits a trivial by-pass.
Agreed, but we can't really walk back this decision unfortunately.
At least with this patch series system administrators have the ability
to limit such tools.
> What I was trying to get at is two points.
> 1. The written description wasn't clear enough, leaving room for
> ambiguity.
> 2. That I quest that the behavior should be allowed given the
> current set of tools that use user namespaces. It reduces exploit
> codes ability to directly use unprivileged user namespaces but
> makes it all to easy to by-pass the restriction because of the
> behavior of the current tool set. ie. user space has to change.
> But again, I believe the fcaps behavior is wrong, because of the state of
> current software. If this had been a proposal where there was no existing
> software infrastructure I would be starting from a different stance.
As mentioned above, userspace doesn't necessarily have to change. I'm
also not sure what you mean by easy to by-pass? If I mask off some
capabilities system wide or in a given process tree, I know for a fact
that no namespace will ever get those capabilities.
On Fri May 17, 2024 at 10:11 PM EEST, Jonathan Calmels wrote:
> On Fri, May 17, 2024 at 10:53:24AM GMT, Casey Schaufler wrote:
> > Of course they do. I have been following the use of capabilities
> > in Linux since before they were implemented. The uptake has been
> > disappointing in all use cases.
>
> Why "Of course"?
> What if they should not get *all* privileges?
They do the job given a real-world workload and stress test.
Here the problem is based on a theory and an experiment.
Even a formal model does not necessarily map all "unknown unknowns".
BR, Jarkko
On Sat May 18, 2024 at 2:08 PM EEST, Jarkko Sakkinen wrote:
> On Fri May 17, 2024 at 10:11 PM EEST, Jonathan Calmels wrote:
> > On Fri, May 17, 2024 at 10:53:24AM GMT, Casey Schaufler wrote:
> > > Of course they do. I have been following the use of capabilities
> > > in Linux since before they were implemented. The uptake has been
> > > disappointing in all use cases.
> >
> > Why "Of course"?
> > What if they should not get *all* privileges?
>
> They do the job given a real-world workload and stress test.
>
> Here the problem is based on a theory and an experiment.
>
> Even a formal model does not necessarily map all "unknown unknowns".
So this was like the worst "sales pitch" ever:
1. The cover letter starts with the idea of having to argue about name
spaces, and have fun while doing that ;-) We all have our own ways to
entertain ourselves but "name space duels" are not my thing. Why not
just start with why we all want this instead? Maybe we don't want it
then. Maybe this is just useless spam given the angle presented?
2. There's shitloads of computer science and set theory but nothing
that would make common sense. You need to build more understandable
model. There's zero "gist" in this work.
Maybe this does make sense but the story around it sucks so far.
BR, Jarkko
On Sat May 18, 2024 at 2:17 PM EEST, Jarkko Sakkinen wrote:
> On Sat May 18, 2024 at 2:08 PM EEST, Jarkko Sakkinen wrote:
> > On Fri May 17, 2024 at 10:11 PM EEST, Jonathan Calmels wrote:
> > > On Fri, May 17, 2024 at 10:53:24AM GMT, Casey Schaufler wrote:
> > > > Of course they do. I have been following the use of capabilities
> > > > in Linux since before they were implemented. The uptake has been
> > > > disappointing in all use cases.
> > >
> > > Why "Of course"?
> > > What if they should not get *all* privileges?
> >
> > They do the job given a real-world workload and stress test.
> >
> > Here the problem is based on a theory and an experiment.
> >
> > Even a formal model does not necessarily map all "unknown unknowns".
>
> So this was like the worst "sales pitch" ever:
>
> 1. The cover letter starts with the idea of having to argue about name
> spaces, and have fun while doing that ;-) We all have our own ways to
> entertain ourselves but "name space duels" are not my thing. Why not
> just start with why we all want this instead? Maybe we don't want it
> then. Maybe this is just useless spam given the angle presented?
> 2. There's shitloads of computer science and set theory but nothing
> that would make common sense. You need to build more understandable
> model. There's zero "gist" in this work.
>
> Maybe this does make sense but the story around it sucks so far.
One tip: I think this is wrong forum to present namespace ideas in the
first place. It would be probably better to talk about this with e.g.
systemd or podman developers, and similar groups. There's zero evidence
of the usefulness. Then when you go that route and come back with actual
users, things click much more easily. Now this is all in the void.
BR, Jarkko
On Fri, May 17, 2024 at 10:53:24AM -0700, Casey Schaufler wrote:
> On 5/17/2024 4:42 AM, Jonathan Calmels wrote:
> >>>> On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
> >>>>> I suggest that adding a capability set for user namespaces is a bad idea:
> >>>>> - It is in no way obvious what problem it solves
> >>>>> - It is not obvious how it solves any problem
> >>>>> - The capability mechanism has not been popular, and relying on a
> >>>>> community (e.g. container developers) to embrace it based on this
> >>>>> enhancement is a recipe for failure
> >>>>> - Capabilities are already more complicated than modern developers
> >>>>> want to deal with. Adding another, special purpose set, is going
> >>>>> to make them even more difficult to use.
> > Sorry if the commit wasn't clear enough.
>
> While, as others have pointed out, the commit description left
> much to be desired, that isn't the biggest problem with the change
> you're proposing.
>
> > Basically:
> >
> > - Today user namespaces grant full capabilities.
>
> Of course they do. I have been following the use of capabilities
> in Linux since before they were implemented. The uptake has been
> disappointing in all use cases.
>
> > This behavior is often abused to attack various kernel subsystems.
>
> Yes. The problems of a single, all powerful root privilege scheme are
> well documented.
>
> > Only option
>
> Hardly.
>
> > is to disable them altogether which breaks a lot of
> > userspace stuff.
>
> Updating userspace components to behave properly in a capabilities
> environment has never been a popular activity, but is the right way
> to address this issue. And before you start on the "no one can do that,
> it's too hard", I'll point out that multiple UNIX systems supported
> rootless, all capabilities based systems back in the day.
>
> > This goes against the least privilege principle.
>
> If you're going to run userspace that *requires* privilege, you have
> to have a way to *allow* privilege. If the userspace insists on a root
> based privilege model, you're stuck supporting it. Regardless of your
> principles.
Casey,
I might be wrong, but I think you're misreading this patchset. It is not
about limiting capabilities in the init user ns at all. It's about limiting
the capabilities which a process in a child userns can get.
Any unprivileged task can create a new userns, and get a process with
all capabilities in that namespace. Always. User namespaces were a
great success in that we can do this without any resulting privilege
against host owned resources. The unaddressed issue is the expanded
kernel code surface area.
You say, above, (quoting out of place here)
> Updating userspace components to behave properly in a capabilities
> environment has never been a popular activity, but is the right way
> to address this issue. And before you start on the "no one can do that,
> it's too hard", I'll point out that multiple UNIX systems supported
He's not saying no one can do that. He's saying, correctly, that the
kernel currently offers no way for userspace to do this limiting. His
patchset offers two ways: one system wide capability mask (which applies
only to non-initial user namespaces) and on per-process inherited one
which - yay - userspace can use to limit what its children will be
able to get if they unshare a user namespace.
> > - It adds a new capability set.
>
> Which is a really, really bad idea. The equation for calculating effective
> privilege is already more complicated than userspace developers are generally
> willing to put up with.
This is somewhat true, but I think the semantics of what is proposed here are
about as straightforward as you could hope for, and you can basically reason
about them completely independently of the other sets. Only when reasoning
about the correctness of this code do you need to consider the other sets. Not
when administering a system.
If you want root in a child user namespace to not have CAP_MAC_ADMIN, you drop
it from your pU. Simple as that.
> > This set dictates what capabilities are granted in namespaces (instead
> > of always getting full caps).
>
> I would not expect container developers to be eager to learn how to use
> this facility.
I'm a container developer, and I'm excited about it :)
> > This brings namespaces in line with the rest of the system, user
> > namespaces are no more "special".
>
> I'm sorry, but this makes no sense to me whatsoever. You want to introduce
> a capability set explicitly for namespaces in order to make them less
> special?
Yes, exactly.
> Maybe I'm just old and cranky.
That's fine.
> > They now work the same way as say a transition to root does with
> > inheritable caps.
>
> That needs some explanation.
>
> >
> > - This isn't intended to be used by end users per se (although they could).
> > This would be used at the same places where existing capabalities are
> > used today (e.g. init system, pam, container runtime, browser
> > sandbox), or by system administrators.
>
> I understand that. It is for containers. Containers are not kernel entities.
User namespaces are.
This patch set provides userspace a way of limiting the kernel code exposed
to untrusted children, which currently does not exist.
> > To give you some ideas of things you could do:
> >
> > # E.g. prevent alice from getting CAP_NET_ADMIN in user namespaces under SSH
> > echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
> > echo "!cap_net_admin alice" >> /etc/security/capability.conf.
> >
> > # E.g. prevent any Docker container from ever getting CAP_DAC_OVERRIDE
> > systemd-run -p CapabilityBoundingSet=~CAP_DAC_OVERRIDE \
> > -p SecureBits=userns-strict-caps \
> > /usr/bin/dockerd
> >
> > # E.g. kernel could be vulnerable to CAP_SYS_RAWIO exploits
> > # Prevent users from ever gaining it
> > sysctl -w cap_bound_userns_mask=0x1fffffdffff
On 5/17/24 20:50, Jonathan Calmels wrote:
> On Fri, May 17, 2024 at 04:59:41AM GMT, John Johansen wrote:
>> On 5/17/24 03:51, Jonathan Calmels wrote:
>>> This new capability set would be a universal thing that could be
>>> leveraged today without modification to userspace. Moreover, it’s a
>>> simple framework that can be extended.
>>
>> I would argue that is a problem. Userspace has to change for this to be
>> secure. Is it an improvement over the current state yes.
>
> Well, yes and no. With those patches, I can lock down things today on my
> system and I don't need to change anything.
>
sure, same as with the big no user ns toggle. This is finer and allows
selectively enabling on a per application basis.
> For example I can decide that none of my rootless containers started
> under SSH will get CAP_NET_ADMIN:
>
> # echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
> # echo "!cap_net_admin $USER" >> /etc/security/capability.conf
> # capsh --secbits=$((1 << 8)) -- -c /usr/sbin/sshd
>
> $ ssh localhost 'unshare -r capsh --current'
> Current: =ep cap_net_admin-ep
> Current IAB: !cap_net_admin
>
> Or I can decide than I don't ever want CAP_SYS_RAWIO in my namespaces:
>
> # sysctl -w cap_bound_userns_mask=0x1fffffdffff
>
> This doesn't require changes to userspace.
> Now, granted if you want to have finer-grained controls, it will require
> *some* changes in *some* places (e.g. adding new systemd property like
> UserNSSet=).
>
yep
>>> Well that’s the thing, from past conversations, there is a lot of
>>> disagreement about restricting namespaces. By restricting the
>>> capabilities granted by namespaces instead, we’re actually treating the
>>> root cause of most concerns.
>>>
>> no disagreement there. This is actually Ubuntu's posture with user namespaces
>> atm. Where the user namespace is allowed but the capabilities within it
>> are denied.
>>
>> It does however when not handled correctly result in some very odd failures
>> and would be easier to debug if the use of user namespaces were just
>> cleanly denied.
>
> Yes but as we established it depends on the use case, both are not
> mutually exclusive.
>
yep
>> its not so much the capabilities set as the inheritable part that is
>> problematic. Yes I am well aware of where that is required but I question
>> that capabilities provides the needed controls here.
>
> Again, I'm not opposed to doing this with LSMs. I just think both could
> work well together. We already do that with standard capabilities vs
> LSMs, both have their strength and weaknesses.
>
yes, don't get me wrong I am not necessarily advocating an LSM solution
as being necessary. I just want to make sure the trade-offs of the
capabilities solution get discussed to help evaluate whether extending
the current capability model is worth it.
> It's always a tradeoff, do you want a setting that's universal and
> coarse, or do you want one that's tailored to specific things but less
> ubiquitous.
>
yep
> It's also a tradeoff on usability. If this doesn't get used in practice,
> then there is no point.
agreed
> I would argue that even though capabilities are complicated, they are
> more widely understood than LSMs. Are capabilities insufficient in
> certain scenarios, absolutely, and that's usually where LSMs come in.
>
hrmmm, I am not sure I would agree with capabilities are better understood
than LSMs, At the base level of capability(X) to get permission yes, but
the whole permitting, bounding, ... Really I think most people are just
confused by both
>>> This is possible with the security bit introduced in the second patch.
>>> The idea of having those separate is that a service which has dropped
>>> its capabilities can still create a fully privileged user namespace.
>>
>> yes, which is the problem. Not that we don't do that with say setuid
>> applications, but the difference is that they were known to be doing
>> something dangerous and took measures around that.
>>
>> We are starting from a different posture here. Where applications have
>> assumed that user namespaces where safe and no measures were needed.
>> Tools like unshare and bwrap if set to allow user namespaces in their
>> fcaps will allow exploits a trivial by-pass.
>
> Agreed, but we can't really walk back this decision unfortunately.
And that is partly the crux of the issue, if we can't walk back the
decision then the solution becomes more complex
> At least with this patch series system administrators have the ability
> to limit such tools.
>
agreed
>> What I was trying to get at is two points.
>> 1. The written description wasn't clear enough, leaving room for
>> ambiguity.
>> 2. That I quest that the behavior should be allowed given the
>> current set of tools that use user namespaces. It reduces exploit
>> codes ability to directly use unprivileged user namespaces but
>> makes it all to easy to by-pass the restriction because of the
>> behavior of the current tool set. ie. user space has to change.
>
>> But again, I believe the fcaps behavior is wrong, because of the state of
>> current software. If this had been a proposal where there was no existing
>> software infrastructure I would be starting from a different stance.
>
> As mentioned above, userspace doesn't necessarily have to change. I'm
> also not sure what you mean by easy to by-pass? If I mask off some
> capabilities system wide or in a given process tree, I know for a fact
> that no namespace will ever get those capabilities.
so by-pass will very much depend on the system but from a distro pov
we pretty much have to have bwrap enabled if users want to use flatpaks
(and they do), same story for several other tools. Since this basically
means said tools need to be available by default, most systems the
distro is installed on are vulnerable by default. The trivial by-pass
then becomes the exploit running its payload through one of these tools,
and yes I have tested it.
Could a distro disable these tools by default, and require the user/admin
to enable them, yes though there would be a lot of friction, push back,
and in the end most systems would still end up with them enabled.
With the capibilities approach can a user/admin make their system
more secure than the current situation, absolutely.
Note, that regardless of what happens with patch 1, and 2. I think we
either need the big sysctl toggle, or a version of your patch 3
On Sat, May 18, 2024 at 05:27:27AM GMT, John Johansen wrote:
> On 5/17/24 20:50, Jonathan Calmels wrote:
> > As mentioned above, userspace doesn't necessarily have to change. I'm
> > also not sure what you mean by easy to by-pass? If I mask off some
> > capabilities system wide or in a given process tree, I know for a fact
> > that no namespace will ever get those capabilities.
>
> so by-pass will very much depend on the system but from a distro pov
> we pretty much have to have bwrap enabled if users want to use flatpaks
> (and they do), same story for several other tools. Since this basically
> means said tools need to be available by default, most systems the
> distro is installed on are vulnerable by default. The trivial by-pass
> then becomes the exploit running its payload through one of these tools,
> and yes I have tested it.
>
> Could a distro disable these tools by default, and require the user/admin
> to enable them, yes though there would be a lot of friction, push back,
> and in the end most systems would still end up with them enabled.
>
> With the capibilities approach can a user/admin make their system
> more secure than the current situation, absolutely.
>
> Note, that regardless of what happens with patch 1, and 2. I think we
> either need the big sysctl toggle, or a version of your patch 3
Ah ok, I get you concerns. Unfortunately, I can't really speak for
distros or tooling about how this gets leveraged.
I've never claimed this was going to be bulletproof day 1.
All I'm saying is that they now have the option to do so.
As you pointed out, we're coming from a model where today it's open-bar.
Only now they can put a bouncer in front of it, so to speak :)
Regarding distros:
Maybe they ship with an empty userns mask by default and admins have
to tweak it, understanding full well the consequences of doing so.
Maybe they ship with a conservative mask and use pam rules to
adjust it.
Maybe they introduce something like a wheel/sudo group that you need
to be part of to gain extra privileges in your userns.
Maybe only some system services (e.g. dockerd, lxd/incusd, machined)
get confined.
Maybe they need highly specific policies, and this is where you'll
would want LSM support. Say an Apparmor profile targetting
unshare(1) specifically.
Regarding tools:
Maybe bwrap has its own group you need to be part of to get full
caps.
Maybe docker uses this set behind `--cap-add` `--cap-drop`.
Maybe lxd/incusd imlement ACL restricting who can do what.
Maybe steam always drops everything it doesn't need,
I'm sure this won't cover every single corner cases, but as stated in
the headline, this is a start, a simple framework we can always
extend if needed in the future.
On 5/18/2024 5:20 AM, Serge Hallyn wrote:
> On Fri, May 17, 2024 at 10:53:24AM -0700, Casey Schaufler wrote:
>> On 5/17/2024 4:42 AM, Jonathan Calmels wrote:
>>>>>> On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
>>>>>>> I suggest that adding a capability set for user namespaces is a bad idea:
>>>>>>> - It is in no way obvious what problem it solves
>>>>>>> - It is not obvious how it solves any problem
>>>>>>> - The capability mechanism has not been popular, and relying on a
>>>>>>> community (e.g. container developers) to embrace it based on this
>>>>>>> enhancement is a recipe for failure
>>>>>>> - Capabilities are already more complicated than modern developers
>>>>>>> want to deal with. Adding another, special purpose set, is going
>>>>>>> to make them even more difficult to use.
>>> Sorry if the commit wasn't clear enough.
>> While, as others have pointed out, the commit description left
>> much to be desired, that isn't the biggest problem with the change
>> you're proposing.
>>
>>> Basically:
>>>
>>> - Today user namespaces grant full capabilities.
>> Of course they do. I have been following the use of capabilities
>> in Linux since before they were implemented. The uptake has been
>> disappointing in all use cases.
>>
>>> This behavior is often abused to attack various kernel subsystems.
>> Yes. The problems of a single, all powerful root privilege scheme are
>> well documented.
>>
>>> Only option
>> Hardly.
>>
>>> is to disable them altogether which breaks a lot of
>>> userspace stuff.
>> Updating userspace components to behave properly in a capabilities
>> environment has never been a popular activity, but is the right way
>> to address this issue. And before you start on the "no one can do that,
>> it's too hard", I'll point out that multiple UNIX systems supported
>> rootless, all capabilities based systems back in the day.
>>
>>> This goes against the least privilege principle.
>> If you're going to run userspace that *requires* privilege, you have
>> to have a way to *allow* privilege. If the userspace insists on a root
>> based privilege model, you're stuck supporting it. Regardless of your
>> principles.
> Casey,
>
> I might be wrong, but I think you're misreading this patchset. It is not
> about limiting capabilities in the init user ns at all. It's about limiting
> the capabilities which a process in a child userns can get.
I do understand that. My objection is not to the intent, but to the approach.
Adding a capability set to the general mechanism in support of a limited, specific
use case seems wrong to me. I would rather see a mechanism in userns to limit
the capabilities in a user namespace than a mechanism in capabilities that is
specific to user namespaces.
> Any unprivileged task can create a new userns, and get a process with
> all capabilities in that namespace. Always. User namespaces were a
> great success in that we can do this without any resulting privilege
> against host owned resources. The unaddressed issue is the expanded
> kernel code surface area.
An option to clone() then, to limit the capabilities available?
I honestly can't recall if that has been suggested elsewhere, and
apologize if it's already been dismissed as a stoopid idea.
>
> You say, above, (quoting out of place here)
>
>> Updating userspace components to behave properly in a capabilities
>> environment has never been a popular activity, but is the right way
>> to address this issue. And before you start on the "no one can do that,
>> it's too hard", I'll point out that multiple UNIX systems supported
> He's not saying no one can do that. He's saying, correctly, that the
> kernel currently offers no way for userspace to do this limiting. His
> patchset offers two ways: one system wide capability mask (which applies
> only to non-initial user namespaces) and on per-process inherited one
> which - yay - userspace can use to limit what its children will be
> able to get if they unshare a user namespace.
>
>>> - It adds a new capability set.
>> Which is a really, really bad idea. The equation for calculating effective
>> privilege is already more complicated than userspace developers are generally
>> willing to put up with.
> This is somewhat true, but I think the semantics of what is proposed here are
> about as straightforward as you could hope for, and you can basically reason
> about them completely independently of the other sets. Only when reasoning
> about the correctness of this code do you need to consider the other sets. Not
> when administering a system.
>
> If you want root in a child user namespace to not have CAP_MAC_ADMIN, you drop
> it from your pU. Simple as that.
>
>>> This set dictates what capabilities are granted in namespaces (instead
>>> of always getting full caps).
>> I would not expect container developers to be eager to learn how to use
>> this facility.
> I'm a container developer, and I'm excited about it :)
OK, well, I'm wrong. It's happened before and will happen again.
>
>>> This brings namespaces in line with the rest of the system, user
>>> namespaces are no more "special".
>> I'm sorry, but this makes no sense to me whatsoever. You want to introduce
>> a capability set explicitly for namespaces in order to make them less
>> special?
> Yes, exactly.
Hmm. I can't say I buy that. It makes a whole lot more sense to me to
change userns than to change capabilities.
>
>> Maybe I'm just old and cranky.
> That's fine.
>
>>> They now work the same way as say a transition to root does with
>>> inheritable caps.
>> That needs some explanation.
>>
>>> - This isn't intended to be used by end users per se (although they could).
>>> This would be used at the same places where existing capabalities are
>>> used today (e.g. init system, pam, container runtime, browser
>>> sandbox), or by system administrators.
>> I understand that. It is for containers. Containers are not kernel entities.
> User namespaces are.
>
> This patch set provides userspace a way of limiting the kernel code exposed
> to untrusted children, which currently does not exist.
Yes, I understand. I would rather see a change to userns in support of a userns
specific need than a change to capabilities for a userns specific need.
>>> To give you some ideas of things you could do:
>>>
>>> # E.g. prevent alice from getting CAP_NET_ADMIN in user namespaces under SSH
>>> echo "auth optional pam_cap.so" >> /etc/pam.d/sshd
>>> echo "!cap_net_admin alice" >> /etc/security/capability.conf.
>>>
>>> # E.g. prevent any Docker container from ever getting CAP_DAC_OVERRIDE
>>> systemd-run -p CapabilityBoundingSet=~CAP_DAC_OVERRIDE \
>>> -p SecureBits=userns-strict-caps \
>>> /usr/bin/dockerd
>>>
>>> # E.g. kernel could be vulnerable to CAP_SYS_RAWIO exploits
>>> # Prevent users from ever gaining it
>>> sysctl -w cap_bound_userns_mask=0x1fffffdffff
On Sun, May 19, 2024 at 10:03:29AM GMT, Casey Schaufler wrote:
> I do understand that. My objection is not to the intent, but to the approach.
> Adding a capability set to the general mechanism in support of a limited, specific
> use case seems wrong to me. I would rather see a mechanism in userns to limit
> the capabilities in a user namespace than a mechanism in capabilities that is
> specific to user namespaces.
> An option to clone() then, to limit the capabilities available?
> I honestly can't recall if that has been suggested elsewhere, and
> apologize if it's already been dismissed as a stoopid idea.
No and you're right, this would also make sense. This was considered as
well as things like ioctl_ns() (basically introducing the concept of
capabilities in the user_namespace struct). I also considered reusing
the existing sets with various schemes to no avail.
The main issue with this approach is that you've to consider how this is
going to be used. This ties into the other thread we've had with John
and Eric.
Basically, we're coming from a model where things are wide open and
we're trying to tighten things down.
Quoting John here:
> We are starting from a different posture here. Where applications have
> assumed that user namespaces where safe and no measures were needed.
> Tools like unshare and bwrap if set to allow user namespaces in their
> fcaps will allow exploits a trivial by-pass.
We can't really expect userspace to patch every single userns callsite
and opt-in this new security mechanism.
You said it well yourself:
> Capabilities are already more complicated than modern developers
> want to deal with.
Moreover, policies are not necessarily enforced at said callsites. Take
for example a service like systemd-machined, or a PAM session. Those
need to be able to place restrictions on any processes spawned under
them.
If we do this in clone() (or similar), we'll also need to come up with
inheritance rules, being able to query capabilities, etc.
At this point we're just reinventing capability sets.
Finally the nice thing about having it as a capability set, is that we
can easily define rules between them. Patch 2 is a good example of this.
It constrains the userns set to the bounding set of a task. Thus,
requiring minimal/no change to userspace, and helping with adoption.
> Yes, I understand. I would rather see a change to userns in support of a userns
> specific need than a change to capabilities for a userns specific need.
Valid point, but at the end of the day, those are really just tasks'
capabilities. The unshare() just happens to trigger specific rules when it
comes to the tasks' creds. This isn't so different than the other sets
and their specific rules for execve() or UID 0.
This could also be reframed as:
Why would setting capabilities on taks in a userns be so different than
tasks outside of it?
On Thu, May 16, 2024 at 02:22:03AM -0700, Jonathan Calmels wrote:
> Attackers often rely on user namespaces to get elevated (yet confined)
> privileges in order to target specific subsystems (e.g. [1]). Distributions
> have been pretty adamant that they need a way to configure these, most of
> them carry out-of-tree patches to do so, or plainly refuse to enable them.
> As a result, there have been multiple efforts over the years to introduce
> various knobs to control and/or disable user namespaces (e.g. [2][3][4]).
>
> While we acknowledge that there are already ways to control the creation of
> such namespaces (the most recent being a LSM hook), there are inherent
> issues with these approaches. Preventing the user namespace creation is not
> fine-grained enough, and in some cases, incompatible with various userspace
> expectations (e.g. container runtimes, browser sandboxing, service
> isolation)
>
> This patch addresses these limitations by introducing an additional
> capability set used to restrict the permissions granted when creating user
> namespaces. This way, processes can apply the principle of least privilege
> by configuring only the capabilities they need for their namespaces.
>
> For compatibility reasons, processes always start with a full userns
> capability set.
>
> On namespace creation, the userns capability set (pU) is assigned to the
> new effective (pE), permitted (pP) and bounding set (X) of the task:
>
> pU = pE = pP = X
>
> The userns capability set obeys the invariant that no bit can ever be set
> if it is not already part of the task’s bounding set. This ensures that no
> namespace can ever gain more privileges than its predecessors.
> Additionally, if a task is not privileged over CAP_SETPCAP, setting any bit
> in the userns set requires its corresponding bit to be set in the permitted
> set. This effectively mimics the inheritable set rules and means that, by
> default, only root in the initial user namespace can gain userns
> capabilities:
>
> p’U = (pE & CAP_SETPCAP) ? X : (X & pP)
>
> Note that since userns capabilities are strictly hierarchical, policies can
> be enforced at various levels (e.g. init, pam_cap) and inherited by every
> child namespace.
>
> Here is a sample program that can be used to verify the functionality:
>
> /*
> * Test program that drops CAP_SYS_RAWIO from subsequent user namespaces.
> *
> * ./cap_userns_test unshare -r grep Cap /proc/self/status
> * CapInh: 0000000000000000
> * CapPrm: 000001fffffdffff
> * CapEff: 000001fffffdffff
> * CapBnd: 000001fffffdffff
> * CapAmb: 0000000000000000
> * CapUNs: 000001fffffdffff
> */
>
> int main(int argc, char *argv[])
> {
> if (prctl(PR_CAP_USERNS, PR_CAP_USERNS_LOWER, CAP_SYS_RAWIO, 0, 0) < 0)
> err(1, "cannot drop userns cap");
>
> execvp(argv[1], argv + 1);
> err(1, "cannot exec");
> }
>
> Link: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/containers/[email protected]
>
> Signed-off-by: Jonathan Calmels <[email protected]>
Thanks! Of course we'llnsee how the conversations fall out, but
Reviewed-by: Serge Hallyn <[email protected]>
> ---
> fs/proc/array.c | 9 ++++++
> include/linux/cred.h | 3 ++
> include/uapi/linux/prctl.h | 7 +++++
> kernel/cred.c | 3 ++
> kernel/umh.c | 16 ++++++++++
> kernel/user_namespace.c | 12 +++-----
> security/commoncap.c | 59 ++++++++++++++++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 8 files changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 34a47fb0c57f..364e8bb19f9d 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -313,6 +313,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> const struct cred *cred;
> kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> cap_bset, cap_ambient;
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns;
> +#endif
>
> rcu_read_lock();
> cred = __task_cred(p);
> @@ -321,6 +324,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> cap_effective = cred->cap_effective;
> cap_bset = cred->cap_bset;
> cap_ambient = cred->cap_ambient;
> +#ifdef CONFIG_USER_NS
> + cap_userns = cred->cap_userns;
> +#endif
> rcu_read_unlock();
>
> render_cap_t(m, "CapInh:\t", &cap_inheritable);
> @@ -328,6 +334,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapEff:\t", &cap_effective);
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> render_cap_t(m, "CapAmb:\t", &cap_ambient);
> +#ifdef CONFIG_USER_NS
> + render_cap_t(m, "CapUNs:\t", &cap_userns);
> +#endif
> }
>
> static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 2976f534a7a3..adab0031443e 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -124,6 +124,9 @@ struct cred {
> kernel_cap_t cap_effective; /* caps we can actually use */
> kernel_cap_t cap_bset; /* capability bounding set */
> kernel_cap_t cap_ambient; /* Ambient capability set */
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns; /* User namespace capability set */
> +#endif
> #ifdef CONFIG_KEYS
> unsigned char jit_keyring; /* default keyring to attach requested
> * keys to */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 370ed14b1ae0..e09475171f62 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -198,6 +198,13 @@ struct prctl_mm_map {
> # define PR_CAP_AMBIENT_LOWER 3
> # define PR_CAP_AMBIENT_CLEAR_ALL 4
>
> +/* Control the userns capability set */
> +#define PR_CAP_USERNS 48
> +# define PR_CAP_USERNS_IS_SET 1
> +# define PR_CAP_USERNS_RAISE 2
> +# define PR_CAP_USERNS_LOWER 3
> +# define PR_CAP_USERNS_CLEAR_ALL 4
> +
> /* arm64 Scalable Vector Extension controls */
> /* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
> #define PR_SVE_SET_VL 50 /* set task vector length */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 075cfa7c896f..9912c6f3bc6b 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -56,6 +56,9 @@ struct cred init_cred = {
> .cap_permitted = CAP_FULL_SET,
> .cap_effective = CAP_FULL_SET,
> .cap_bset = CAP_FULL_SET,
> +#ifdef CONFIG_USER_NS
> + .cap_userns = CAP_FULL_SET,
> +#endif
> .user = INIT_USER,
> .user_ns = &init_user_ns,
> .group_info = &init_groups,
> diff --git a/kernel/umh.c b/kernel/umh.c
> index 1b13c5d34624..51f1e1d25d49 100644
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -32,6 +32,9 @@
>
> #include <trace/events/module.h>
>
> +#ifdef CONFIG_USER_NS
> +static kernel_cap_t usermodehelper_userns = CAP_FULL_SET;
> +#endif
> static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
> static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
> static DEFINE_SPINLOCK(umh_sysctl_lock);
> @@ -94,6 +97,10 @@ static int call_usermodehelper_exec_async(void *data)
> new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
> new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
> new->cap_inheritable);
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = cap_intersect(usermodehelper_userns,
> + new->cap_userns);
> +#endif
> spin_unlock(&umh_sysctl_lock);
>
> if (sub_info->init) {
> @@ -560,6 +567,15 @@ static struct ctl_table usermodehelper_table[] = {
> .mode = 0600,
> .proc_handler = proc_cap_handler,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "userns",
> + .data = &usermodehelper_userns,
> + .maxlen = 2 * sizeof(unsigned long),
> + .mode = 0600,
> + .proc_handler = proc_cap_handler,
> + },
> +#endif
> { }
> };
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0b0b95418b16..7e624607330b 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -42,15 +42,13 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>
> static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> {
> - /* Start with the same capabilities as init but useless for doing
> - * anything as the capabilities are bound to the new user namespace.
> - */
> - cred->securebits = SECUREBITS_DEFAULT;
> + /* Start with the capabilities defined in the userns set. */
> + cred->cap_bset = cred->cap_userns;
> + cred->cap_permitted = cred->cap_userns;
> + cred->cap_effective = cred->cap_userns;
> cred->cap_inheritable = CAP_EMPTY_SET;
> - cred->cap_permitted = CAP_FULL_SET;
> - cred->cap_effective = CAP_FULL_SET;
> cred->cap_ambient = CAP_EMPTY_SET;
> - cred->cap_bset = CAP_FULL_SET;
> + cred->securebits = SECUREBITS_DEFAULT;
> #ifdef CONFIG_KEYS
> key_put(cred->request_key_auth);
> cred->request_key_auth = NULL;
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 162d96b3a676..b3d3372bf910 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -228,6 +228,28 @@ static inline int cap_inh_is_capped(void)
> return 1;
> }
>
> +/*
> + * Determine whether a userns capability can be raised.
> + * Returns 1 if it can, 0 otherwise.
> + */
> +#ifdef CONFIG_USER_NS
> +static inline int cap_uns_is_raiseable(unsigned long cap)
> +{
> + if (!!cap_raised(current_cred()->cap_userns, cap))
> + return 1;
> + /* a capability cannot be raised unless the current task has it in
> + * its bounding set and, without CAP_SETPCAP, its permitted set.
> + */
> + if (!cap_raised(current_cred()->cap_bset, cap))
> + return 0;
> + if (cap_capable(current_cred(), current_cred()->user_ns,
> + CAP_SETPCAP, CAP_OPT_NONE) != 0 &&
> + !cap_raised(current_cred()->cap_permitted, cap))
> + return 0;
> + return 1;
> +}
> +#endif
> +
> /**
> * cap_capset - Validate and apply proposed changes to current's capabilities
> * @new: The proposed new credentials; alterations should be made here
> @@ -1382,6 +1404,43 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> return commit_creds(new);
> }
>
> +#ifdef CONFIG_USER_NS
> + case PR_CAP_USERNS:
> + if (arg2 == PR_CAP_USERNS_CLEAR_ALL) {
> + if (arg3 | arg4 | arg5)
> + return -EINVAL;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + cap_clear(new->cap_userns);
> + return commit_creds(new);
> + }
> +
> + if (((!cap_valid(arg3)) | arg4 | arg5))
> + return -EINVAL;
> +
> + if (arg2 == PR_CAP_USERNS_IS_SET) {
> + return !!cap_raised(current_cred()->cap_userns, arg3);
> + } else if (arg2 != PR_CAP_USERNS_RAISE &&
> + arg2 != PR_CAP_USERNS_LOWER) {
> + return -EINVAL;
> + } else {
> + if (arg2 == PR_CAP_USERNS_RAISE &&
> + !cap_uns_is_raiseable(arg3))
> + return -EPERM;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + if (arg2 == PR_CAP_USERNS_RAISE)
> + cap_raise(new->cap_userns, arg3);
> + else
> + cap_lower(new->cap_userns, arg3);
> + return commit_creds(new);
> + }
> +#endif
> +
> default:
> /* No functionality available - continue with default */
> return -ENOSYS;
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index b5d5333ab330..e3670d815435 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -944,6 +944,9 @@ void key_change_session_keyring(struct callback_head *twork)
> new->cap_effective = old->cap_effective;
> new->cap_ambient = old->cap_ambient;
> new->cap_bset = old->cap_bset;
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = old->cap_userns;
> +#endif
>
> new->jit_keyring = old->jit_keyring;
> new->thread_keyring = key_get(old->thread_keyring);
> --
> 2.45.0
>
On Thu, May 16, 2024 at 02:22:03AM -0700, Jonathan Calmels wrote:
> Attackers often rely on user namespaces to get elevated (yet confined)
> privileges in order to target specific subsystems (e.g. [1]). Distributions
> have been pretty adamant that they need a way to configure these, most of
> them carry out-of-tree patches to do so, or plainly refuse to enable them.
> As a result, there have been multiple efforts over the years to introduce
> various knobs to control and/or disable user namespaces (e.g. [2][3][4]).
>
> While we acknowledge that there are already ways to control the creation of
> such namespaces (the most recent being a LSM hook), there are inherent
> issues with these approaches. Preventing the user namespace creation is not
> fine-grained enough, and in some cases, incompatible with various userspace
> expectations (e.g. container runtimes, browser sandboxing, service
> isolation)
>
> This patch addresses these limitations by introducing an additional
> capability set used to restrict the permissions granted when creating user
> namespaces. This way, processes can apply the principle of least privilege
> by configuring only the capabilities they need for their namespaces.
>
> For compatibility reasons, processes always start with a full userns
> capability set.
>
> On namespace creation, the userns capability set (pU) is assigned to the
> new effective (pE), permitted (pP) and bounding set (X) of the task:
>
> pU = pE = pP = X
>
> The userns capability set obeys the invariant that no bit can ever be set
> if it is not already part of the task’s bounding set. This ensures that no
> namespace can ever gain more privileges than its predecessors.
> Additionally, if a task is not privileged over CAP_SETPCAP, setting any bit
> in the userns set requires its corresponding bit to be set in the permitted
> set. This effectively mimics the inheritable set rules and means that, by
> default, only root in the initial user namespace can gain userns
> capabilities:
>
> p’U = (pE & CAP_SETPCAP) ? X : (X & pP)
>
> Note that since userns capabilities are strictly hierarchical, policies can
> be enforced at various levels (e.g. init, pam_cap) and inherited by every
> child namespace.
>
> Here is a sample program that can be used to verify the functionality:
>
> /*
> * Test program that drops CAP_SYS_RAWIO from subsequent user namespaces.
> *
> * ./cap_userns_test unshare -r grep Cap /proc/self/status
> * CapInh: 0000000000000000
> * CapPrm: 000001fffffdffff
> * CapEff: 000001fffffdffff
> * CapBnd: 000001fffffdffff
> * CapAmb: 0000000000000000
> * CapUNs: 000001fffffdffff
> */
>
> int main(int argc, char *argv[])
> {
> if (prctl(PR_CAP_USERNS, PR_CAP_USERNS_LOWER, CAP_SYS_RAWIO, 0, 0) < 0)
> err(1, "cannot drop userns cap");
>
> execvp(argv[1], argv + 1);
> err(1, "cannot exec");
> }
>
> Link: https://security.googleblog.com/2023/06/learnings-from-kctf-vrps-42-linux.html
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/lkml/[email protected]
> Link: https://lore.kernel.org/containers/[email protected]
>
> Signed-off-by: Jonathan Calmels <[email protected]>
> ---
> fs/proc/array.c | 9 ++++++
> include/linux/cred.h | 3 ++
> include/uapi/linux/prctl.h | 7 +++++
> kernel/cred.c | 3 ++
> kernel/umh.c | 16 ++++++++++
> kernel/user_namespace.c | 12 +++-----
> security/commoncap.c | 59 ++++++++++++++++++++++++++++++++++++
> security/keys/process_keys.c | 3 ++
> 8 files changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 34a47fb0c57f..364e8bb19f9d 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -313,6 +313,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> const struct cred *cred;
> kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
> cap_bset, cap_ambient;
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns;
> +#endif
>
> rcu_read_lock();
> cred = __task_cred(p);
> @@ -321,6 +324,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> cap_effective = cred->cap_effective;
> cap_bset = cred->cap_bset;
> cap_ambient = cred->cap_ambient;
> +#ifdef CONFIG_USER_NS
> + cap_userns = cred->cap_userns;
> +#endif
> rcu_read_unlock();
>
> render_cap_t(m, "CapInh:\t", &cap_inheritable);
> @@ -328,6 +334,9 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
> render_cap_t(m, "CapEff:\t", &cap_effective);
> render_cap_t(m, "CapBnd:\t", &cap_bset);
> render_cap_t(m, "CapAmb:\t", &cap_ambient);
> +#ifdef CONFIG_USER_NS
> + render_cap_t(m, "CapUNs:\t", &cap_userns);
> +#endif
> }
>
> static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 2976f534a7a3..adab0031443e 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -124,6 +124,9 @@ struct cred {
> kernel_cap_t cap_effective; /* caps we can actually use */
> kernel_cap_t cap_bset; /* capability bounding set */
> kernel_cap_t cap_ambient; /* Ambient capability set */
> +#ifdef CONFIG_USER_NS
> + kernel_cap_t cap_userns; /* User namespace capability set */
> +#endif
> #ifdef CONFIG_KEYS
> unsigned char jit_keyring; /* default keyring to attach requested
> * keys to */
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 370ed14b1ae0..e09475171f62 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -198,6 +198,13 @@ struct prctl_mm_map {
> # define PR_CAP_AMBIENT_LOWER 3
> # define PR_CAP_AMBIENT_CLEAR_ALL 4
>
> +/* Control the userns capability set */
> +#define PR_CAP_USERNS 48
> +# define PR_CAP_USERNS_IS_SET 1
> +# define PR_CAP_USERNS_RAISE 2
> +# define PR_CAP_USERNS_LOWER 3
> +# define PR_CAP_USERNS_CLEAR_ALL 4
> +
> /* arm64 Scalable Vector Extension controls */
> /* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
> #define PR_SVE_SET_VL 50 /* set task vector length */
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 075cfa7c896f..9912c6f3bc6b 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -56,6 +56,9 @@ struct cred init_cred = {
> .cap_permitted = CAP_FULL_SET,
> .cap_effective = CAP_FULL_SET,
> .cap_bset = CAP_FULL_SET,
> +#ifdef CONFIG_USER_NS
> + .cap_userns = CAP_FULL_SET,
> +#endif
> .user = INIT_USER,
> .user_ns = &init_user_ns,
> .group_info = &init_groups,
> diff --git a/kernel/umh.c b/kernel/umh.c
> index 1b13c5d34624..51f1e1d25d49 100644
> --- a/kernel/umh.c
> +++ b/kernel/umh.c
> @@ -32,6 +32,9 @@
>
> #include <trace/events/module.h>
>
> +#ifdef CONFIG_USER_NS
> +static kernel_cap_t usermodehelper_userns = CAP_FULL_SET;
> +#endif
> static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
> static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
> static DEFINE_SPINLOCK(umh_sysctl_lock);
> @@ -94,6 +97,10 @@ static int call_usermodehelper_exec_async(void *data)
> new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
> new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
> new->cap_inheritable);
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = cap_intersect(usermodehelper_userns,
> + new->cap_userns);
> +#endif
> spin_unlock(&umh_sysctl_lock);
>
> if (sub_info->init) {
> @@ -560,6 +567,15 @@ static struct ctl_table usermodehelper_table[] = {
> .mode = 0600,
> .proc_handler = proc_cap_handler,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "userns",
> + .data = &usermodehelper_userns,
> + .maxlen = 2 * sizeof(unsigned long),
> + .mode = 0600,
> + .proc_handler = proc_cap_handler,
> + },
> +#endif
> { }
> };
>
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 0b0b95418b16..7e624607330b 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -42,15 +42,13 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>
> static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> {
> - /* Start with the same capabilities as init but useless for doing
> - * anything as the capabilities are bound to the new user namespace.
> - */
> - cred->securebits = SECUREBITS_DEFAULT;
> + /* Start with the capabilities defined in the userns set. */
> + cred->cap_bset = cred->cap_userns;
> + cred->cap_permitted = cred->cap_userns;
> + cred->cap_effective = cred->cap_userns;
> cred->cap_inheritable = CAP_EMPTY_SET;
> - cred->cap_permitted = CAP_FULL_SET;
> - cred->cap_effective = CAP_FULL_SET;
> cred->cap_ambient = CAP_EMPTY_SET;
> - cred->cap_bset = CAP_FULL_SET;
> + cred->securebits = SECUREBITS_DEFAULT;
> #ifdef CONFIG_KEYS
> key_put(cred->request_key_auth);
> cred->request_key_auth = NULL;
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 162d96b3a676..b3d3372bf910 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -228,6 +228,28 @@ static inline int cap_inh_is_capped(void)
> return 1;
> }
>
> +/*
> + * Determine whether a userns capability can be raised.
> + * Returns 1 if it can, 0 otherwise.
> + */
> +#ifdef CONFIG_USER_NS
> +static inline int cap_uns_is_raiseable(unsigned long cap)
> +{
> + if (!!cap_raised(current_cred()->cap_userns, cap))
> + return 1;
> + /* a capability cannot be raised unless the current task has it in
> + * its bounding set and, without CAP_SETPCAP, its permitted set.
> + */
> + if (!cap_raised(current_cred()->cap_bset, cap))
> + return 0;
> + if (cap_capable(current_cred(), current_cred()->user_ns,
> + CAP_SETPCAP, CAP_OPT_NONE) != 0 &&
> + !cap_raised(current_cred()->cap_permitted, cap))
> + return 0;
> + return 1;
> +}
> +#endif
> +
> /**
> * cap_capset - Validate and apply proposed changes to current's capabilities
> * @new: The proposed new credentials; alterations should be made here
> @@ -1382,6 +1404,43 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> return commit_creds(new);
> }
>
> +#ifdef CONFIG_USER_NS
> + case PR_CAP_USERNS:
> + if (arg2 == PR_CAP_USERNS_CLEAR_ALL) {
> + if (arg3 | arg4 | arg5)
> + return -EINVAL;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + cap_clear(new->cap_userns);
> + return commit_creds(new);
> + }
> +
> + if (((!cap_valid(arg3)) | arg4 | arg5))
> + return -EINVAL;
> +
> + if (arg2 == PR_CAP_USERNS_IS_SET) {
> + return !!cap_raised(current_cred()->cap_userns, arg3);
> + } else if (arg2 != PR_CAP_USERNS_RAISE &&
> + arg2 != PR_CAP_USERNS_LOWER) {
> + return -EINVAL;
> + } else {
Sorry, I meabt to say, one nit would be that this next block
does not need to be in an else, since every other condition returns.
> + if (arg2 == PR_CAP_USERNS_RAISE &&
> + !cap_uns_is_raiseable(arg3))
> + return -EPERM;
> +
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + if (arg2 == PR_CAP_USERNS_RAISE)
> + cap_raise(new->cap_userns, arg3);
> + else
> + cap_lower(new->cap_userns, arg3);
> + return commit_creds(new);
> + }
> +#endif
> +
> default:
> /* No functionality available - continue with default */
> return -ENOSYS;
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index b5d5333ab330..e3670d815435 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -944,6 +944,9 @@ void key_change_session_keyring(struct callback_head *twork)
> new->cap_effective = old->cap_effective;
> new->cap_ambient = old->cap_ambient;
> new->cap_bset = old->cap_bset;
> +#ifdef CONFIG_USER_NS
> + new->cap_userns = old->cap_userns;
> +#endif
>
> new->jit_keyring = old->jit_keyring;
> new->thread_keyring = key_get(old->thread_keyring);
> --
> 2.45.0
>
On Thu, May 16, 2024 at 02:22:04AM -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
>
> Signed-off-by: Jonathan Calmels <[email protected]>
Reviewed-by: Serge Hallyn <[email protected]>
> ---
> 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. */
> + 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.0
>
On Thu, May 16, 2024 at 02:22:05AM -0700, Jonathan Calmels wrote:
> This patch adds a new system-wide userns capability mask designed to mask
> off capabilities in user namespaces.
>
> This mask is controlled through a sysctl and can be set early in the boot
> process or on the kernel command line to exclude known capabilities from
> ever being gained in namespaces. Once set, it can be further restricted to
> exert dynamic policies on the system (e.g. ward off a potential exploit).
>
> Changing this mask requires privileges over CAP_SYS_ADMIN and CAP_SETPCAP
> in the initial user namespace.
>
> Example:
>
> # sysctl -qw kernel.cap_userns_mask=0x1fffffdffff && \
> unshare -r grep Cap /proc/self/status
> CapInh: 0000000000000000
> CapPrm: 000001fffffdffff
> CapEff: 000001fffffdffff
> CapBnd: 000001fffffdffff
> CapAmb: 0000000000000000
> CapUNs: 000001fffffdffff
>
> Signed-off-by: Jonathan Calmels <[email protected]>
Reviewed-by: Serge Hallyn <[email protected]>
> ---
> include/linux/user_namespace.h | 7 ++++
> kernel/sysctl.c | 10 ++++++
> kernel/user_namespace.c | 66 ++++++++++++++++++++++++++++++++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 6030a8235617..e3478bd54ee5 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -2,6 +2,7 @@
> #ifndef _LINUX_USER_NAMESPACE_H
> #define _LINUX_USER_NAMESPACE_H
>
> +#include <linux/capability.h>
> #include <linux/kref.h>
> #include <linux/nsproxy.h>
> #include <linux/ns_common.h>
> @@ -14,6 +15,12 @@
> #define UID_GID_MAP_MAX_BASE_EXTENTS 5
> #define UID_GID_MAP_MAX_EXTENTS 340
>
> +#ifdef CONFIG_SYSCTL
> +extern kernel_cap_t cap_userns_mask;
> +int proc_cap_userns_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos);
> +#endif
> +
> struct uid_gid_extent {
> u32 first;
> u32 lower_first;
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 81cc974913bb..1546eebd6aea 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -62,6 +62,7 @@
> #include <linux/sched/sysctl.h>
> #include <linux/mount.h>
> #include <linux/userfaultfd_k.h>
> +#include <linux/user_namespace.h>
> #include <linux/pid.h>
>
> #include "../lib/kstrtox.h"
> @@ -1846,6 +1847,15 @@ static struct ctl_table kern_table[] = {
> .mode = 0444,
> .proc_handler = proc_dointvec,
> },
> +#ifdef CONFIG_USER_NS
> + {
> + .procname = "cap_userns_mask",
> + .data = &cap_userns_mask,
> + .maxlen = sizeof(kernel_cap_t),
> + .mode = 0644,
> + .proc_handler = proc_cap_userns_handler,
> + },
> +#endif
> #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86)
> {
> .procname = "unknown_nmi_panic",
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 53848e2b68cd..e0cf606e9140 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -26,6 +26,66 @@
> static struct kmem_cache *user_ns_cachep __ro_after_init;
> static DEFINE_MUTEX(userns_state_mutex);
>
> +#ifdef CONFIG_SYSCTL
> +static DEFINE_SPINLOCK(cap_userns_lock);
> +kernel_cap_t cap_userns_mask = CAP_FULL_SET;
> +
> +int proc_cap_userns_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos)
> +{
> + struct ctl_table t;
> + unsigned long mask_array[2];
> + kernel_cap_t new_mask, *mask;
> + int err;
> +
> + if (write && (!capable(CAP_SETPCAP) ||
> + !capable(CAP_SYS_ADMIN)))
> + return -EPERM;
> +
> + /*
> + * convert from the global kernel_cap_t to the ulong array to print to
> + * userspace if this is a read.
> + *
> + * capabilities are exposed as one 64-bit value or two 32-bit values
> + * depending on the architecture
> + */
> + mask = table->data;
> + spin_lock(&cap_userns_lock);
> + mask_array[0] = (unsigned long) mask->val;
> +#if BITS_PER_LONG != 64
> + mask_array[1] = mask->val >> BITS_PER_LONG;
> +#endif
> + spin_unlock(&cap_userns_lock);
> +
> + t = *table;
> + t.data = &mask_array;
> +
> + /*
> + * actually read or write and array of ulongs from userspace. Remember
> + * these are least significant bits first
> + */
> + err = proc_doulongvec_minmax(&t, write, buffer, lenp, ppos);
> + if (err < 0)
> + return err;
> +
> + new_mask.val = mask_array[0];
> +#if BITS_PER_LONG != 64
> + new_mask.val += (u64)mask_array[1] << BITS_PER_LONG;
> +#endif
> +
> + /*
> + * Drop everything not in the new_mask (but don't add things)
> + */
> + if (write) {
> + spin_lock(&cap_userns_lock);
> + *mask = cap_intersect(*mask, new_mask);
> + spin_unlock(&cap_userns_lock);
> + }
> +
> + return 0;
> +}
> +#endif
> +
> static bool new_idmap_permitted(const struct file *file,
> struct user_namespace *ns, int cap_setid,
> struct uid_gid_map *map);
> @@ -46,6 +106,12 @@ static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
> /* Limit userns capabilities to our parent's bounding set. */
> if (iscredsecure(cred, SECURE_USERNS_STRICT_CAPS))
> cred->cap_userns = cap_intersect(cred->cap_userns, cred->cap_bset);
> +#ifdef CONFIG_SYSCTL
> + /* Mask off userns capabilities that are not permitted by the system-wide mask. */
> + spin_lock(&cap_userns_lock);
> + cred->cap_userns = cap_intersect(cred->cap_userns, cap_userns_mask);
> + spin_unlock(&cap_userns_lock);
> +#endif
>
> /* Start with the capabilities defined in the userns set. */
> cred->cap_bset = cred->cap_userns;
> --
> 2.45.0
>
Hi Jonathan,
On Thu, May 16, 2024 at 02:22:05AM -0700, Jonathan Calmels wrote:
> +int proc_cap_userns_handler(struct ctl_table *table, int write,
> + void *buffer, size_t *lenp, loff_t *ppos)
> +{
there is an ongoing effort (started at [0]) to constify the first arg
here, since you're not supposed to write to it. Your usage looks
correct to me, so I think all it needs is a literal "const" here.
[0]: https://lore.kernel.org/lkml/[email protected]/
> + struct ctl_table t;
> + unsigned long mask_array[2];
> + kernel_cap_t new_mask, *mask;
> + int err;
> +
> + if (write && (!capable(CAP_SETPCAP) ||
> + !capable(CAP_SYS_ADMIN)))
> + return -EPERM;
..why CAP_SYS_ADMIN? You mention it in the changelog, but don't
explain why.
Tycho
On Mon, May 20, 2024 at 07:30:14AM GMT, Tycho Andersen wrote:
> there is an ongoing effort (started at [0]) to constify the first arg
> here, since you're not supposed to write to it. Your usage looks
> correct to me, so I think all it needs is a literal "const" here.
Will do, along with the suggestions from Jarkko
> > + struct ctl_table t;
> > + unsigned long mask_array[2];
> > + kernel_cap_t new_mask, *mask;
> > + int err;
> > +
> > + if (write && (!capable(CAP_SETPCAP) ||
> > + !capable(CAP_SYS_ADMIN)))
> > + return -EPERM;
>
> ...why CAP_SYS_ADMIN? You mention it in the changelog, but don't
> explain why.
No reason really, I was hoping we could decide what we want here.
UMH uses CAP_SYS_MODULE, Serge mentioned adding a new cap maybe.
On Mon, May 20, 2024 at 12:25:27PM -0700, Jonathan Calmels wrote:
> On Mon, May 20, 2024 at 07:30:14AM GMT, Tycho Andersen wrote:
> > there is an ongoing effort (started at [0]) to constify the first arg
> > here, since you're not supposed to write to it. Your usage looks
> > correct to me, so I think all it needs is a literal "const" here.
>
> Will do, along with the suggestions from Jarkko
>
> > > + struct ctl_table t;
> > > + unsigned long mask_array[2];
> > > + kernel_cap_t new_mask, *mask;
> > > + int err;
> > > +
> > > + if (write && (!capable(CAP_SETPCAP) ||
> > > + !capable(CAP_SYS_ADMIN)))
> > > + return -EPERM;
> >
> > ...why CAP_SYS_ADMIN? You mention it in the changelog, but don't
> > explain why.
>
> No reason really, I was hoping we could decide what we want here.
> UMH uses CAP_SYS_MODULE, Serge mentioned adding a new cap maybe.
I don't have a strong preference between SETPCAP and a new capability,
but I do think it should be just one. SYS_ADMIN is already god mode
enough, IMO.
Tycho
On Tue May 21, 2024 at 12:13 AM EEST, Tycho Andersen wrote:
> On Mon, May 20, 2024 at 12:25:27PM -0700, Jonathan Calmels wrote:
> > On Mon, May 20, 2024 at 07:30:14AM GMT, Tycho Andersen wrote:
> > > there is an ongoing effort (started at [0]) to constify the first arg
> > > here, since you're not supposed to write to it. Your usage looks
> > > correct to me, so I think all it needs is a literal "const" here.
> >
> > Will do, along with the suggestions from Jarkko
> >
> > > > + struct ctl_table t;
> > > > + unsigned long mask_array[2];
> > > > + kernel_cap_t new_mask, *mask;
> > > > + int err;
> > > > +
> > > > + if (write && (!capable(CAP_SETPCAP) ||
> > > > + !capable(CAP_SYS_ADMIN)))
> > > > + return -EPERM;
> > >
> > > ...why CAP_SYS_ADMIN? You mention it in the changelog, but don't
> > > explain why.
> >
> > No reason really, I was hoping we could decide what we want here.
> > UMH uses CAP_SYS_MODULE, Serge mentioned adding a new cap maybe.
>
> I don't have a strong preference between SETPCAP and a new capability,
> but I do think it should be just one. SYS_ADMIN is already god mode
> enough, IMO.
Sometimes I think would it make more sense to invent something
completely new like capabilities but more modern and robust, instead of
increasing complexity of a broken mechanism (especially thanks to
CAP_MAC_ADMIN).
I kind of liked the idea of privilege tokens both in Symbian and Maemo
(have been involved professionally in both). Emphasis on the idea not
necessarily on implementation.
Not an LSM but like something that you could use in the place of POSIX
caps. Probably quite tedious effort tho because you would need to pull
the whole industry with the new thing...
BR, Jarkko
On 5/18/24 04:21, Jarkko Sakkinen wrote:
> On Sat May 18, 2024 at 2:17 PM EEST, Jarkko Sakkinen wrote:
>> On Sat May 18, 2024 at 2:08 PM EEST, Jarkko Sakkinen wrote:
>>> On Fri May 17, 2024 at 10:11 PM EEST, Jonathan Calmels wrote:
>>>> On Fri, May 17, 2024 at 10:53:24AM GMT, Casey Schaufler wrote:
>>>>> Of course they do. I have been following the use of capabilities
>>>>> in Linux since before they were implemented. The uptake has been
>>>>> disappointing in all use cases.
>>>>
>>>> Why "Of course"?
>>>> What if they should not get *all* privileges?
>>>
>>> They do the job given a real-world workload and stress test.
>>>
>>> Here the problem is based on a theory and an experiment.
>>>
>>> Even a formal model does not necessarily map all "unknown unknowns".
>>
>> So this was like the worst "sales pitch" ever:
>>
>> 1. The cover letter starts with the idea of having to argue about name
>> spaces, and have fun while doing that ;-) We all have our own ways to
>> entertain ourselves but "name space duels" are not my thing. Why not
>> just start with why we all want this instead? Maybe we don't want it
>> then. Maybe this is just useless spam given the angle presented?
>> 2. There's shitloads of computer science and set theory but nothing
>> that would make common sense. You need to build more understandable
>> model. There's zero "gist" in this work.
>>
>> Maybe this does make sense but the story around it sucks so far.
>
> One tip: I think this is wrong forum to present namespace ideas in the
> first place. It would be probably better to talk about this with e.g.
> systemd or podman developers, and similar groups. There's zero evidence
> of the usefulness. Then when you go that route and come back with actual
> users, things click much more easily. Now this is all in the void.
>
> BR, Jarkko
Jarkko,
this is very much the right forum. User namespaces exist today. This
is a discussion around trying to reduce the exposed kernel surface
that is being used to attack the kernel.
On Tue May 21, 2024 at 4:57 PM EEST, John Johansen wrote:
> > One tip: I think this is wrong forum to present namespace ideas in the
> > first place. It would be probably better to talk about this with e.g.
> > systemd or podman developers, and similar groups. There's zero evidence
> > of the usefulness. Then when you go that route and come back with actual
> > users, things click much more easily. Now this is all in the void.
> >
> > BR, Jarkko
>
> Jarkko,
>
> this is very much the right forum. User namespaces exist today. This
> is a discussion around trying to reduce the exposed kernel surface
> that is being used to attack the kernel.
Agreed, that was harsh way to put it. What I mean is that if this
feature was included, would it be enabled by distributions?
This user base part or potential user space part is not very well
described in the cover letter. I.e. "motivation" to put it short.
I mean the technical details are really in detail in this patch set but
it would help to digest them if there was some even rough description
how this would be deployed.
If the motivation should be obvious, then it is beyond me, and thus
would be nice if that obvious thing was stated that everyone else gets.
E.g. I like to sometimes just test quite alien patch sets for the sake
of learning and fun (or not so fun, depends) but this patch set does not
deliver enough information to do anything at all.
Hope this clears a bit where I stand. IMHO a good patch set should bring
the details to the specialists on the topic but also have some wider
audience motivational stuff in order to make clear where it fits in this
world :-)
BR, Jarkko
On 5/18/24 05:20, Serge Hallyn wrote:
> On Fri, May 17, 2024 at 10:53:24AM -0700, Casey Schaufler wrote:
>> On 5/17/2024 4:42 AM, Jonathan Calmels wrote:
>>>>>> On Thu May 16, 2024 at 10:07 PM EEST, Casey Schaufler wrote:
>>>>>>> I suggest that adding a capability set for user namespaces is a bad idea:
>>>>>>> - It is in no way obvious what problem it solves
>>>>>>> - It is not obvious how it solves any problem
>>>>>>> - The capability mechanism has not been popular, and relying on a
>>>>>>> community (e.g. container developers) to embrace it based on this
>>>>>>> enhancement is a recipe for failure
>>>>>>> - Capabilities are already more complicated than modern developers
>>>>>>> want to deal with. Adding another, special purpose set, is going
>>>>>>> to make them even more difficult to use.
>>> Sorry if the commit wasn't clear enough.
>>
>> While, as others have pointed out, the commit description left
>> much to be desired, that isn't the biggest problem with the change
>> you're proposing.
>>
>>> Basically:
>>>
>>> - Today user namespaces grant full capabilities.
>>
>> Of course they do. I have been following the use of capabilities
>> in Linux since before they were implemented. The uptake has been
>> disappointing in all use cases.
>>
>>> This behavior is often abused to attack various kernel subsystems.
>>
>> Yes. The problems of a single, all powerful root privilege scheme are
>> well documented.
>>
>>> Only option
>>
>> Hardly.
>>
>>> is to disable them altogether which breaks a lot of
>>> userspace stuff.
>>
>> Updating userspace components to behave properly in a capabilities
>> environment has never been a popular activity, but is the right way
>> to address this issue. And before you start on the "no one can do that,
>> it's too hard", I'll point out that multiple UNIX systems supported
>> rootless, all capabilities based systems back in the day.
>>
>>> This goes against the least privilege principle.
>>
>> If you're going to run userspace that *requires* privilege, you have
>> to have a way to *allow* privilege. If the userspace insists on a root
>> based privilege model, you're stuck supporting it. Regardless of your
>> principles.
>
> Casey,
>
> I might be wrong, but I think you're misreading this patchset. It is not
> about limiting capabilities in the init user ns at all. It's about limiting
> the capabilities which a process in a child userns can get.
>
> Any unprivileged task can create a new userns, and get a process with
> all capabilities in that namespace. Always. User namespaces were a
> great success in that we can do this without any resulting privilege
> against host owned resources. The unaddressed issue is the expanded
> kernel code surface area.
>
> You say, above, (quoting out of place here)
>
>> Updating userspace components to behave properly in a capabilities
>> environment has never been a popular activity, but is the right way
>> to address this issue. And before you start on the "no one can do that,
>> it's too hard", I'll point out that multiple UNIX systems supported
>
> He's not saying no one can do that. He's saying, correctly, that the
> kernel currently offers no way for userspace to do this limiting. His
> patchset offers two ways: one system wide capability mask (which applies
> only to non-initial user namespaces) and on per-process inherited one
> which - yay - userspace can use to limit what its children will be
> able to get if they unshare a user namespace.
>
>>> - It adds a new capability set.
>>
>> Which is a really, really bad idea. The equation for calculating effective
>> privilege is already more complicated than userspace developers are generally
>> willing to put up with.
>
> This is somewhat true, but I think the semantics of what is proposed here are
> about as straightforward as you could hope for, and you can basically reason
> about them completely independently of the other sets. Only when reasoning
> about the correctness of this code do you need to consider the other sets. Not
> when administering a system.
>
> If you want root in a child user namespace to not have CAP_MAC_ADMIN, you drop
> it from your pU. Simple as that.
>
>>> This set dictates what capabilities are granted in namespaces (instead
>>> of always getting full caps).
>>
>> I would not expect container developers to be eager to learn how to use
>> this facility.
>
> I'm a container developer, and I'm excited about it :)
>
>>> This brings namespaces in line with the rest of the system, user
>>> namespaces are no more "special".
>>
>> I'm sorry, but this makes no sense to me whatsoever. You want to introduce
>> a capability set explicitly for namespaces in order to make them less
>> special?
>
> Yes, exactly.
>
>> Maybe I'm just old and cranky.
>
> That's fine.
>
>>> They now work the same way as say a transition to root does with
>>> inheritable caps.
>>
>> That needs some explanation.
>>
>>>
>>> - This isn't intended to be used by end users per se (although they could).
>>> This would be used at the same places where existing capabalities are
>>> used today (e.g. init system, pam, container runtime, browser
>>> sandbox), or by system administrators.
>>
>> I understand that. It is for containers. Containers are not kernel entities.
>
> User namespaces are.
>
> This patch set provides userspace a way of limiting the kernel code exposed
> to untrusted children, which currently does not exist.
>
theoretically, I am worried that in practice the existing utils allow
untrusted code to still access user namespaces.
In practice we have found that we need to allow a different set of capabilities
when bwrap is called from flatpak than when called on its own etc. We see the
same pattern with unshare and other utilities around launching applications
in user namespaces.
In practice at the distro level I don't see this approach actually helping.
Because we have so many uses that require exposing close to the full capabilities
set in multiple utilities that are required by many different applications.
To be clear this doesn't stop distros from doing something more, but is it
worth the added complexity if in practice it can't be used effectively.
I really don't have the answer.
On Tue May 21, 2024 at 5:29 PM EEST, Tycho Andersen wrote:
> On Tue, May 21, 2024 at 01:12:57AM +0300, Jarkko Sakkinen wrote:
> > On Tue May 21, 2024 at 12:13 AM EEST, Tycho Andersen wrote:
> > > On Mon, May 20, 2024 at 12:25:27PM -0700, Jonathan Calmels wrote:
> > > > On Mon, May 20, 2024 at 07:30:14AM GMT, Tycho Andersen wrote:
> > > > > there is an ongoing effort (started at [0]) to constify the first arg
> > > > > here, since you're not supposed to write to it. Your usage looks
> > > > > correct to me, so I think all it needs is a literal "const" here.
> > > >
> > > > Will do, along with the suggestions from Jarkko
> > > >
> > > > > > + struct ctl_table t;
> > > > > > + unsigned long mask_array[2];
> > > > > > + kernel_cap_t new_mask, *mask;
> > > > > > + int err;
> > > > > > +
> > > > > > + if (write && (!capable(CAP_SETPCAP) ||
> > > > > > + !capable(CAP_SYS_ADMIN)))
> > > > > > + return -EPERM;
> > > > >
> > > > > ...why CAP_SYS_ADMIN? You mention it in the changelog, but don't
> > > > > explain why.
> > > >
> > > > No reason really, I was hoping we could decide what we want here.
> > > > UMH uses CAP_SYS_MODULE, Serge mentioned adding a new cap maybe.
> > >
> > > I don't have a strong preference between SETPCAP and a new capability,
> > > but I do think it should be just one. SYS_ADMIN is already god mode
> > > enough, IMO.
> >
> > Sometimes I think would it make more sense to invent something
> > completely new like capabilities but more modern and robust, instead of
> > increasing complexity of a broken mechanism (especially thanks to
> > CAP_MAC_ADMIN).
> >
> > I kind of liked the idea of privilege tokens both in Symbian and Maemo
> > (have been involved professionally in both). Emphasis on the idea not
> > necessarily on implementation.
> >
> > Not an LSM but like something that you could use in the place of POSIX
> > caps. Probably quite tedious effort tho because you would need to pull
> > the whole industry with the new thing...
>
> And then we have LSM hooks, (ns_)capable(), __secure_computing() plus
> a new set of hooks for this new thing sprinkled around. I guess
> kernel developers wouldn't be excited about it, let alone the rest of
> the industry :)
>
> Thinking out loud: I wonder if fixing the seccomp TOCTOU against
> pointers would help here. I guess you'd still have issues where your
> policy engine resolves a path arg to open() and that inode changes
> between the decision and the actual vfs access, you have just changed
> the TOCTOU.
>
> Or even scarier: what if you could change the return value at any
> kprobe? :)
I had one crazy idea related to seccomp filters once.
What if there was way to compose tokens that would be just a seccomp
filter like the one that you pass to PR_SET_SECCOMP but presented with a
file descriptor?
Then you could send these with SCM_RIGHTS to other processes and they
could upgrade their existing filter with them. So it would be a kind of
extension mechanism for a seccomp filter.
Not something I'm seriously suggesting but though to flush this out now
that we are on these topics anyhow ;-)
> Tycho
PS. Sorry if my language was a bit harsh earlier but I think I had also
a point related to at least to the patch set presentation. I.e. you
are very precise describing the mechanism but motivation and bringing
topic somehow to a context is equally important :-)
BR, Jarkko
On Tue, May 21, 2024 at 01:12:57AM +0300, Jarkko Sakkinen wrote:
> On Tue May 21, 2024 at 12:13 AM EEST, Tycho Andersen wrote:
> > On Mon, May 20, 2024 at 12:25:27PM -0700, Jonathan Calmels wrote:
> > > On Mon, May 20, 2024 at 07:30:14AM GMT, Tycho Andersen wrote:
> > > > there is an ongoing effort (started at [0]) to constify the first arg
> > > > here, since you're not supposed to write to it. Your usage looks
> > > > correct to me, so I think all it needs is a literal "const" here.
> > >
> > > Will do, along with the suggestions from Jarkko
> > >
> > > > > + struct ctl_table t;
> > > > > + unsigned long mask_array[2];
> > > > > + kernel_cap_t new_mask, *mask;
> > > > > + int err;
> > > > > +
> > > > > + if (write && (!capable(CAP_SETPCAP) ||
> > > > > + !capable(CAP_SYS_ADMIN)))
> > > > > + return -EPERM;
> > > >
> > > > ...why CAP_SYS_ADMIN? You mention it in the changelog, but don't
> > > > explain why.
> > >
> > > No reason really, I was hoping we could decide what we want here.
> > > UMH uses CAP_SYS_MODULE, Serge mentioned adding a new cap maybe.
> >
> > I don't have a strong preference between SETPCAP and a new capability,
> > but I do think it should be just one. SYS_ADMIN is already god mode
> > enough, IMO.
>
> Sometimes I think would it make more sense to invent something
> completely new like capabilities but more modern and robust, instead of
> increasing complexity of a broken mechanism (especially thanks to
> CAP_MAC_ADMIN).
>
> I kind of liked the idea of privilege tokens both in Symbian and Maemo
> (have been involved professionally in both). Emphasis on the idea not
> necessarily on implementation.
>
> Not an LSM but like something that you could use in the place of POSIX
> caps. Probably quite tedious effort tho because you would need to pull
> the whole industry with the new thing...
And then we have LSM hooks, (ns_)capable(), __secure_computing() plus
a new set of hooks for this new thing sprinkled around. I guess
kernel developers wouldn't be excited about it, let alone the rest of
the industry :)
Thinking out loud: I wonder if fixing the seccomp TOCTOU against
pointers would help here. I guess you'd still have issues where your
policy engine resolves a path arg to open() and that inode changes
between the decision and the actual vfs access, you have just changed
the TOCTOU.
Or even scarier: what if you could change the return value at any
kprobe? :)
Tycho
On 5/21/24 07:12, Jarkko Sakkinen wrote:
> On Tue May 21, 2024 at 4:57 PM EEST, John Johansen wrote:
>>> One tip: I think this is wrong forum to present namespace ideas in the
>>> first place. It would be probably better to talk about this with e.g.
>>> systemd or podman developers, and similar groups. There's zero evidence
>>> of the usefulness. Then when you go that route and come back with actual
>>> users, things click much more easily. Now this is all in the void.
>>>
>>> BR, Jarkko
>>
>> Jarkko,
>>
>> this is very much the right forum. User namespaces exist today. This
>> is a discussion around trying to reduce the exposed kernel surface
>> that is being used to attack the kernel.
>
> Agreed, that was harsh way to put it. What I mean is that if this
> feature was included, would it be enabled by distributions?
>
Enabled, maybe? It requires the debian distros to make sure their
packaging supports xattrs correctly. It should be good but it isn't
well exercised. It also requires the work to set these on multiple
applications. From experience we are talking 100s.
It will break out of repo applications, and require an extra step for
users to enable. Ubuntu is already breaking these but for many, of the
more popular ones they are shipping profiles so the users don't have
to take an extra step. Things like appimages remain broken and wil
require an approach similar to the Mac with unverified software
downloaded from the internet.
Nor does this fix the bwrap, unshare, ... use case. Which means the
distro is going to have to continue shipping an alternate solution
that covers those. For Ubuntu atm this is just an extra point of
friction but I expect we would still end up enabling it to tick the
checkbox at some point if it goes into the upstream kernel.
> This user base part or potential user space part is not very well
> described in the cover letter. I.e. "motivation" to put it short.
>
yes the cover letter needs work
> I mean the technical details are really in detail in this patch set but
> it would help to digest them if there was some even rough description
> how this would be deployed.
>
yes
> If the motivation should be obvious, then it is beyond me, and thus
> would be nice if that obvious thing was stated that everyone else gets.
>
sure. The cover letter will get updated with this. Seeing as I have been
dealing with this a lot lately. It comes down to user namespaces allow
unprivileged code to access kernel surface area that is usually protected
behind capabilities. This has been leveraged as part of the exploit chain
in the majority of kernel exploits we are seeing.
> E.g. I like to sometimes just test quite alien patch sets for the sake
> of learning and fun (or not so fun, depends) but this patch set does not
> deliver enough information to do anything at all.
>
under stood, I am playing devils advocate here. Its not that I don't see
value in the proposal, but that I am not sure I see enough value with
the current situation, where so much code has been written around the
assumption that unprivileged user namespaces are safe. Trying to fix
the situation without breaking everything is complicated.
> Hope this clears a bit where I stand. IMHO a good patch set should bring
> the details to the specialists on the topic but also have some wider
> audience motivational stuff in order to make clear where it fits in this
> world :-)
>
> BR, Jarkko
>
On 5/17/24 07:22, Eric W. Biederman wrote:
> Jonathan Calmels <[email protected]> writes:
>
>> On Fri, May 17, 2024 at 06:32:46AM GMT, Eric W. Biederman wrote:
>>>
>>> Pointers please?
>>>
>>> That sentence sounds about 5 years out of date.
>>
>> The link referenced is from last year.
>> Here are some others often cited by distributions:
>>
>> https://nvd.nist.gov/vuln/detail/CVE-2022-0185
>> https://nvd.nist.gov/vuln/detail/CVE-2022-1015
>> https://nvd.nist.gov/vuln/detail/CVE-2022-2078
>> https://nvd.nist.gov/vuln/detail/CVE-2022-24122
>> https://nvd.nist.gov/vuln/detail/CVE-2022-25636
>>
>> Recent thread discussing this too:
>> https://seclists.org/oss-sec/2024/q2/128
>
> My apologies perhaps I trimmed too much.
>
> I know that user namespaces enlarge the attack surface.
> How much and how serious could be debated but for unprivileged
> users the attack surface is undoubtedly enlarged.
>
> As I read your introduction you were justifying the introduction
> of a new security mechanism with the observation that distributions
> were carrying distribution specific patches.
>
> To the best of my knowledge distribution specific patches and
> distributions disabling user namespaces have been gone for quite a
> while. So if that has changed recently I would like to know.
>
almost all the distros are carrying the out of try sysctl to disable
user namepsaces. Its disabled by default but is available. Ubuntu in
its 24.04 release is now limiting unprivileged use of user namespaces
to known code. At a generic code level they are allowed but with no
capabilities within the user namespace.
On Tue, May 21, 2024 at 07:45:20AM GMT, John Johansen wrote:
> On 5/21/24 07:12, Jarkko Sakkinen wrote:
> > On Tue May 21, 2024 at 4:57 PM EEST, John Johansen wrote:
> > > > One tip: I think this is wrong forum to present namespace ideas in the
> > > > first place. It would be probably better to talk about this with e.g.
> > > > systemd or podman developers, and similar groups. There's zero evidence
> > > > of the usefulness. Then when you go that route and come back with actual
> > > > users, things click much more easily. Now this is all in the void.
> > > >
> > > > BR, Jarkko
> > >
> > > Jarkko,
> > >
> > > this is very much the right forum. User namespaces exist today. This
> > > is a discussion around trying to reduce the exposed kernel surface
> > > that is being used to attack the kernel.
> >
> > Agreed, that was harsh way to put it. What I mean is that if this
> > feature was included, would it be enabled by distributions?
> >
> Enabled, maybe? It requires the debian distros to make sure their
> packaging supports xattrs correctly. It should be good but it isn't
> well exercised. It also requires the work to set these on multiple
> applications. From experience we are talking 100s.
>
> It will break out of repo applications, and require an extra step for
> users to enable. Ubuntu is already breaking these but for many, of the
> more popular ones they are shipping profiles so the users don't have
> to take an extra step. Things like appimages remain broken and wil
> require an approach similar to the Mac with unverified software
> downloaded from the internet.
>
> Nor does this fix the bwrap, unshare, ... use case. Which means the
> distro is going to have to continue shipping an alternate solution
> that covers those. For Ubuntu atm this is just an extra point of
> friction but I expect we would still end up enabling it to tick the
> checkbox at some point if it goes into the upstream kernel.
I'm not sure I understand your point here and how this relates to xattrs.
This patchset has nothing to do with file capabilities. The userns
capability set is purely a process based capability set and in no way
influenced by file attributes.
> > This user base part or potential user space part is not very well
> > described in the cover letter. I.e. "motivation" to put it short.
> >
> yes the cover letter needs work
Yes, it's been mentioned several times already.
While not in the cover letter, the motivation is stated in the first
patch and provides several references to past discussions on the topic.
This is nothing new, this subject has been contentious for years now and
discussed over and over on these lists (Eric would know :)). As
mentioned in the patch also, this recently warranted the inclusion of
new LSM hooks.
But again, I wrongfully assumed that this problem was well understood
and still relatively fresh, that's my bad.
> > I mean the technical details are really in detail in this patch set but
> > it would help to digest them if there was some even rough description
> > how this would be deployed.
> >
> yes
Yes, this was purposefully left out so as not to influence any specific
implementation. There is a mention of where this could be done (i.e.
init, pam), but at the end of the day, this is going to depend on each
use case.
Having said that, since it appears to be confusing, maybe we could add
some of the examples I sent out in this thread or the other ones.
I want to reiterate that this is a generic capability set, this is not
magic switch you turn on to secure the whole system.
Its implementation is going to vary across environments and it is going
to be dictated by your threat model.
For example, John's threat model of securing a multi-user Ubuntu Desktop
is going to be very different than say securing a server where all the
userspace is fixed and known.
The former might require additional integration with the LSM subsystem.
Thankfully, this patch should synergize well with it.
Fundamentally, and at its core, it's very simple. Serge put it nicely:
> If you want root in a child user namespace to not have CAP_MAC_ADMIN,
> you drop it from your pU. Simple as that.
From there, you can imagine any integration you want in userspace and
ways to enforce your own policies.
TLDR, this is a first step towards empowering userspace with control
over capabilities granted by a userns. At present, the kernel does not
offer ways to do this. By itself, it is not a comprehensive solution
designed to thwart threat actors. However, it gives userspace the option
to do so.
On 5/21/24 17:45, Jonathan Calmels wrote:
> On Tue, May 21, 2024 at 07:45:20AM GMT, John Johansen wrote:
>> On 5/21/24 07:12, Jarkko Sakkinen wrote:
>>> On Tue May 21, 2024 at 4:57 PM EEST, John Johansen wrote:
>>>>> One tip: I think this is wrong forum to present namespace ideas in the
>>>>> first place. It would be probably better to talk about this with e.g.
>>>>> systemd or podman developers, and similar groups. There's zero evidence
>>>>> of the usefulness. Then when you go that route and come back with actual
>>>>> users, things click much more easily. Now this is all in the void.
>>>>>
>>>>> BR, Jarkko
>>>>
>>>> Jarkko,
>>>>
>>>> this is very much the right forum. User namespaces exist today. This
>>>> is a discussion around trying to reduce the exposed kernel surface
>>>> that is being used to attack the kernel.
>>>
>>> Agreed, that was harsh way to put it. What I mean is that if this
>>> feature was included, would it be enabled by distributions?
>>>
>> Enabled, maybe? It requires the debian distros to make sure their
>> packaging supports xattrs correctly. It should be good but it isn't
>> well exercised. It also requires the work to set these on multiple
>> applications. From experience we are talking 100s.
>>
>> It will break out of repo applications, and require an extra step for
>> users to enable. Ubuntu is already breaking these but for many, of the
>> more popular ones they are shipping profiles so the users don't have
>> to take an extra step. Things like appimages remain broken and wil
>> require an approach similar to the Mac with unverified software
>> downloaded from the internet.
>>
>> Nor does this fix the bwrap, unshare, ... use case. Which means the
>> distro is going to have to continue shipping an alternate solution
>> that covers those. For Ubuntu atm this is just an extra point of
>> friction but I expect we would still end up enabling it to tick the
>> checkbox at some point if it goes into the upstream kernel.
>
> I'm not sure I understand your point here and how this relates to xattrs.
> This patchset has nothing to do with file capabilities. The userns
> capability set is purely a process based capability set and in no way
> influenced by file attributes.
>
Oopps sorry the fcaps bit is crossing over a side discussion.
>>> This user base part or potential user space part is not very well
>>> described in the cover letter. I.e. "motivation" to put it short.
>>>
>> yes the cover letter needs work
>
> Yes, it's been mentioned several times already.
> While not in the cover letter, the motivation is stated in the first
> patch and provides several references to past discussions on the topic.
>
> This is nothing new, this subject has been contentious for years now and
> discussed over and over on these lists (Eric would know :)). As
> mentioned in the patch also, this recently warranted the inclusion of
> new LSM hooks.
>
> But again, I wrongfully assumed that this problem was well understood
> and still relatively fresh, that's my bad.
>
>>> I mean the technical details are really in detail in this patch set but
>>> it would help to digest them if there was some even rough description
>>> how this would be deployed.
>>>
>> yes
>
> Yes, this was purposefully left out so as not to influence any specific
> implementation. There is a mention of where this could be done (i.e.
> init, pam), but at the end of the day, this is going to depend on each
> use case.
> Having said that, since it appears to be confusing, maybe we could add
> some of the examples I sent out in this thread or the other ones.
>
examples would help, especially for people not too familiar with this.
> I want to reiterate that this is a generic capability set, this is not
> magic switch you turn on to secure the whole system.
> Its implementation is going to vary across environments and it is going
> to be dictated by your threat model.
>
yeah
> For example, John's threat model of securing a multi-user Ubuntu Desktop
> is going to be very different than say securing a server where all the
> userspace is fixed and known.
> The former might require additional integration with the LSM subsystem.
> Thankfully, this patch should synergize well with it.
>
hrmmm, maybe, I will be happy if they just don't end up complicating
each other
> Fundamentally, and at its core, it's very simple. Serge put it nicely:
>
yes it is, and yet it still worries me a great deal. I have some of
the same worries as Casey, and also worry that people will take this
as a solution for all use cases, without understanding the issues.
On the other hand walking back the current state of unprivileged use of
user namespaces is a huge issue. Having another approach also pushing
will actually be helpful in some ways.
>> If you want root in a child user namespace to not have CAP_MAC_ADMIN,
>> you drop it from your pU. Simple as that.
>
> From there, you can imagine any integration you want in userspace and
> ways to enforce your own policies.
>
> TLDR, this is a first step towards empowering userspace with control
> over capabilities granted by a userns. At present, the kernel does not
> offer ways to do this. By itself, it is not a comprehensive solution
yep
> designed to thwart threat actors. However, it gives userspace the option
> to do so.
again, I don't believe the capabilities system is actually capable of
doing this, it covers some of the use cases. To be fair the LSM doesn't
cover everything either, there are current use cases that just aren't safe,
you either break them or allow them and accept the risks. It relies on
people understanding threat models, and sadly I have become grown quite
grumpy about that topic.
Anyways I will try to finish up my review of the code this weekend.