Received-SPF: pass (google.com: domain of linux-kernel+bounces-189146-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 24 May 2024 16:26:38 -0500
From: "Serge E. Hallyn" <serge@hallyn.com>
To: lkml <linux-kernel@vger.kernel.org>, Andy Lutomirski <luto@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Tycho Andersen <tycho@tycho.pizza>
Subject: [PATCH 1/1] user_namespace map_write: allow CAP_SETUID/CAP_SETGID
Message-ID: <20240524212638.GA1898944@mail.hallyn.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Before 41c21e351e a task with CAP_SETUID could write to /proc/self/uid_map, or
a task with CAP_SETGID to gid_map.  41c21e351e was an important fix in checking
the capabilities against the opener of the file rather than the writer's
namespace, but it erred in replacing CAP_SETXID with CAP_SYS_ADMIN.  This means
that a task with CAP_SETXID is no longer able to configure its user.  The
argument in the commit message that:

  Changing uid/gid/projid mappings doesn't change your id within the
  namespace; it reconfigures the namespace.

is disputed: First, privilege was needed, the patch only switched the needed
capabilitiy.  Secondly, creating and configuring a new namespace while
getting to choose uids from the parent namespace to bind into the child
namespace is in fact akin to being able to setuid to the newly mapped
uids.

This patch fixes that regression.  Since in the meantime a system
may have started using CAP_SYS_ADMIN, support either now, to avoid
regressing other programs.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Fixes: 41c21e351e ("userns: Changing any namespace id mappings should require privileges")
---
 kernel/user_namespace.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 0b0b95418b16..8fbf1ef337bb 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -920,6 +920,17 @@ static bool verify_root_map(const struct file *file,
 	return true;
 }
 
+static inline bool map_write_allowed(struct file *file, struct user_namespace *map_ns, int cap_setid)
+{
+	// if the cap is -1, then anyone is allowed to write
+	if (!cap_valid(cap_setid))
+		return true;
+
+	// Otherwise, require either cap_setid or CAP_SYS_ADMIN
+	return (file_ns_capable(file, map_ns, cap_setid) ||
+		file_ns_capable(file, map_ns, CAP_SYS_ADMIN));
+}
+
 static ssize_t map_write(struct file *file, const char __user *buf,
 			 size_t count, loff_t *ppos,
 			 int cap_setid,
@@ -974,7 +985,7 @@ static ssize_t map_write(struct file *file, const char __user *buf,
 	/*
 	 * Adjusting namespace settings requires capabilities on the target.
 	 */
-	if (cap_valid(cap_setid) && !file_ns_capable(file, map_ns, CAP_SYS_ADMIN))
+	if (!map_write_allowed(file, map_ns, cap_setid))
 		goto out;
 
 	/* Parse the user data */
-- 
2.34.1