sec,mknod: enforce that caller has CAP_MKNOD

While it wouldn't matter much if a user in the targer user namespace
does not have CAP_MKNOD when creating safe devices, it ultimately is a
breach in documented behavior that can subtly change existing programs
relying on that behavior. One example is checking for EPERM and falling
back to different logic like bind-mounting, which some programs that
manipulate namespaces (including bst) do.

Author: Snaipe

capable.c

@@ -35,6 +35,28 @@ static int bst_capget(cap_user_header_t header, cap_user_data_t data)
 	return (int) syscall(SYS_capget, header, data);
 }
 
+int tid_capget(pid_t tid, struct capabilities *caps)
+{
+	struct __user_cap_header_struct hdr = {
+		.version = _LINUX_CAPABILITY_VERSION_3,
+		.pid     = tid,
+	};
+
+	struct __user_cap_data_struct current[2];
+
+	if (bst_capget(&hdr, current) == -1) {
+		return -1;
+	}
+
+	*caps = (struct capabilities) {
+		.inheritable = (uint64_t) current[1].inheritable << 32 | current[0].inheritable,
+		.permitted   = (uint64_t) current[1].permitted << 32   | current[0].permitted,
+		.effective   = (uint64_t) current[1].effective << 32   | current[0].effective,
+	};
+
+	return 0;
+}
+
 void init_capabilities(void)
 {
 	if (bst_capget(&hdr, current) == -1) {

capable.h

@@ -10,6 +10,7 @@
 # include <stdbool.h>
 # include <stdint.h>
 # include <linux/capability.h>
+# include <unistd.h>
 
 /* Define more useful capability constants */
 # define BST_CAP_SYS_ADMIN      ((uint64_t) 1 << CAP_SYS_ADMIN)
@@ -30,6 +31,14 @@ void make_capable(uint64_t cap);
 void reset_capabilities(void);
 void drop_capabilities(void);
 
+struct capabilities {
+	uint64_t inheritable;
+	uint64_t permitted;
+	uint64_t effective;
+};
+
+int tid_capget(pid_t tid, struct capabilities *capabilities);
+
 # define with_capable(capmask) \
 	for (int __ok = ((void) make_capable(capmask), 1); __ok; (void) reset_capabilities(), __ok = 0)
 

sec.c

@@ -113,13 +113,13 @@ static int run_in_process_context(int seccomp_fd, int procfd,
 	}
 
 	if (memfd == -1) {
-		warn("open /proc/<pid>/mem");
+		warn("open /proc/%d/mem", req->pid);
 		rc = -EINVAL;
 		goto error_close;
 	}
 
 	if (mntns == -1) {
-		warn("open /proc/<pid>/ns/mnt");
+		warn("open /proc/%d/ns/mnt", req->pid);
 		rc = -EINVAL;
 		goto error_close;
 	}
@@ -199,30 +199,46 @@ static int run_in_process_context(int seccomp_fd, int procfd,
 }
 
 struct mknodat_args {
+	pid_t pid;
 	int dirfd;
 	mode_t mode;
 	dev_t dev;
 	char pathname[PATH_MAX];
 
 	struct proc_status status;
+	struct capabilities caps;
 };
 
 static int sec__mknodat_init(int procfd, void *cookie)
 {
 	struct mknodat_args *args = cookie;
 
 	if (proc_read_status(procfd, &args->status) == -1) {
-		warn("proc_read_status /proc/<pid>/status");
+		warn("proc_read_status /proc/%d/status", args->pid);
 		return -EINVAL;
 	}
 
+	if (tid_capget(args->pid, &args->caps) == -1) {
+		int err = errno;
+		warn("capget %d", args->pid);
+		return -err;
+	}
+
 	return 0;
 }
 
 static int sec__mknodat_callback(int procfd, void *cookie)
 {
 	struct mknodat_args *args = cookie;
 
+	/* It wouldn't technically matter for a user without CAP_MKNOD to be
+	   able to create safe devices, but some programs handle EPERM with
+	   fallback code to account for root and non-root execution. Deny
+	   the syscall in order to be faithful to that behavior */
+	if (!(args->caps.effective & BST_CAP_MKNOD)) {
+		return -EPERM;
+	}
+
 	mode_t old_umask = umask(args->status.umask);
 
 	uid_t uid = geteuid();
@@ -297,6 +313,7 @@ safe: {}
 	}
 
 	struct mknodat_args args = {
+		.pid = req->pid,
 		.dirfd = realdirfd,
 		.dev = dev,
 		.mode = mode,