[PATCH RFC v3 58/90] cred: let no id count across cells

[Josef Bacik <josef@toxicpanda.com>]

A cell with a single id runs with the kuid of the user who created it.
That causes problems in both directions:

  - The pid namespace prevents the cell from naming tasks outside of it.
    But the pids of the cell are visible from above. So every other task of
    that user passes the checks that compare uids, and can signal, renice
    or limit the tasks of the cell. The authority over a cell is supposed
    to be the cellfd.
  - The parent of a task that was spawned into a cell is the spawner, which
    is outside of the cell. With KEYCTL_SESSION_TO_PARENT the task could
    replace the session keyring of the spawner with one that it controls.

The ids of two credentials can only be compared if both belong to the same
cell. All the rules that let one task act on another because of its ids
are now in one place, so the restriction can be added once. Add
cred_ids_comparable(), and make every rule call it first. That covers
signals and SIGIO, scheduling attributes and nice values, I/O priorities,
resource limits, ptrace, moving a task in a cgroup v1 hierarchy or between
resctrl groups, and KEYCTL_SESSION_TO_PARENT.

It also applies to euid 0. Some of these rules let a task with euid 0 send
SIGIO to any task, or move any task in a cgroup v1 hierarchy or between
resctrl groups. A cell is a hard boundary for root as well. A cell that
misbehaves can be killed and built again.

The capability checks that sit next to most of these rules are not
changed. A capability that is held in an ancestor user namespace still
applies to a task of a cell for now.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel/cred.c       | 28 ++++++++++++++++++++++++++++
 kernel/cred_kunit.c |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/kernel/cred.c b/kernel/cred.c
index 3aa1b1e3eade..72df625f8c99 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -577,6 +577,10 @@ void __init cred_init(void)
  * Where it says so, a subject with euid 0 may act on anybody. That is a
  * capability by another name, from before there were any.
  *
+ * It is also the one place to say when the ids of two credentials are not to be
+ * compared at all, in which case every rule says no. They are not across cells,
+ * see Documentation/userspace-api/cells.rst.
+ *
  * These functions only speak for the ids. A capability that does as well, and
  * the LSM hook that goes with the operation, remain with the caller.
  *
@@ -588,6 +592,18 @@ void __init cred_init(void)
 #define CRED_EUID	BIT(1)
 #define CRED_SUID	BIT(2)
 
+/*
+ * Are the ids of two credentials to be compared at all? Every rule below asks
+ * this first.
+ *
+ * Across cells an id means nothing, 0 included. A one-id cell runs on the ids
+ * of whoever made it, and authority over a cell is a handle.
+ */
+static bool cred_ids_comparable(const struct cred *subj, const struct cred *obj)
+{
+	return cell_same(subj, obj);
+}
+
 static bool cred_euid_is_root(const struct cred *subj)
 {
 	return uid_eq(subj->euid, GLOBAL_ROOT_UID);
@@ -607,6 +623,9 @@ static bool cred_has_uid(const struct cred *cred, unsigned int ids, kuid_t uid)
 static bool cred_uid_match_any(const struct cred *subj, unsigned int subj_ids,
 			       const struct cred *obj, unsigned int obj_ids)
 {
+	if (!cred_ids_comparable(subj, obj))
+		return false;
+
 	return ((subj_ids & CRED_RUID) && cred_has_uid(obj, obj_ids, subj->uid)) ||
 	       ((subj_ids & CRED_EUID) && cred_has_uid(obj, obj_ids, subj->euid));
 }
@@ -633,6 +652,9 @@ bool cred_ids_allow_signal(const struct cred *subj, const struct cred *obj)
  */
 bool cred_ids_allow_sigio(const struct cred *subj, const struct cred *obj)
 {
+	if (!cred_ids_comparable(subj, obj))
+		return false;
+
 	return cred_euid_is_root(subj) || cred_ids_allow_signal(subj, obj);
 }
 
@@ -671,6 +693,9 @@ bool cred_ids_allow_ioprio(const struct cred *subj, const struct cred *obj)
  */
 bool cred_ids_allow_move(const struct cred *subj, const struct cred *obj)
 {
+	if (!cred_ids_comparable(subj, obj))
+		return false;
+
 	return cred_euid_is_root(subj) ||
 	       cred_uid_match_any(subj, CRED_EUID, obj, CRED_RUID | CRED_SUID);
 }
@@ -693,6 +718,9 @@ bool cred_ids_allow_control(const struct cred *subj, enum cred_ids which,
 	kuid_t uid;
 	kgid_t gid;
 
+	if (!cred_ids_comparable(subj, obj))
+		return false;
+
 	switch (which) {
 	case CRED_IDS_REAL:
 		uid = subj->uid;
diff --git a/kernel/cred_kunit.c b/kernel/cred_kunit.c
index b3a54e8c565f..3921e8f2716f 100644
--- a/kernel/cred_kunit.c
+++ b/kernel/cred_kunit.c
@@ -9,6 +9,7 @@
 #include <kunit/test.h>
 #include <linux/cred.h>
 #include <linux/uidgid.h>
+#include <linux/user_namespace.h>
 
 /* Root, since two of the rules know it, and two others. */
 static const uid_t ids[] = { 0, 1000, 1001 };
@@ -31,6 +32,8 @@ static void uid_combination(unsigned int n, struct cred *cred, struct cred *tcre
 {
 	memset(cred, 0, sizeof(*cred));
 	memset(tcred, 0, sizeof(*tcred));
+	cred->user_ns = &init_user_ns;
+	tcred->user_ns = &init_user_ns;
 	set_uids(cred, &n);
 	set_uids(tcred, &n);
 }
@@ -169,6 +172,8 @@ static void cred_ids_control_rule(struct kunit *test)
 	tcred = kunit_kzalloc(test, sizeof(*tcred), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, cred);
 	KUNIT_ASSERT_NOT_NULL(test, tcred);
+	cred->user_ns = &init_user_ns;
+	tcred->user_ns = &init_user_ns;
 
 	for (n = 0; n < NR_CONTROL_COMBINATIONS; n++) {
 		cred->uid    = KUIDT_INIT(1000 + !!(n & BIT(0)));

-- 
2.55.0