[lxc-devel] [lxd/master] seccomp: more security enhancements

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190711/73c3c873/attachment.bin>
-------------- next part --------------
From 45d9266290459f78c837407d19f0f2f57bc6aa9c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 11 Jul 2019 16:36:30 +0200
Subject: [PATCH 1/2] forkmknod: check for MS_NODEV

Note that we use MS_NODEV even though statfs says ST_NODEV.
MS_NODEV and ST_NODEV are equivalent. Note, that this is __not__ true
for all all MS_* and ST_* flags. So don't blindly copy what I'm doing
here.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/main_forkmknod.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/main_forkmknod.go b/lxd/main_forkmknod.go
index 6402580989..4d23cd3453 100644
--- a/lxd/main_forkmknod.go
+++ b/lxd/main_forkmknod.go
@@ -197,7 +197,7 @@ void forkmknod()
 		_exit(EXIT_FAILURE);
 	}
 
-	if (!same_fsinfo(&s1, &s2, &sfs1, &sfs2)) {
+	if (!same_fsinfo(&s1, &s2, &sfs1, &sfs2) || !(sfs2.f_flags & MS_NODEV)) {
 		fprintf(stderr, "%d", ENOMEDIUM);
 		_exit(EXIT_FAILURE);
 	}

From f06eede3f7d5ba55636a7ee892fa3a39a15b61a5 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 11 Jul 2019 16:42:46 +0200
Subject: [PATCH 2/2] seccomp: check permissions before handling mknod via
 device injection

Note that this is inherently racy until we switch to pidfd_open()...

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/main_forkmknod.go |  5 +++++
 lxd/seccomp.go        | 12 +++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lxd/main_forkmknod.go b/lxd/main_forkmknod.go
index 4d23cd3453..226309ba5e 100644
--- a/lxd/main_forkmknod.go
+++ b/lxd/main_forkmknod.go
@@ -114,6 +114,7 @@ void forkmknod()
 	struct stat s1, s2;
 	struct statfs sfs1, sfs2;
 	cap_t caps;
+	int chk_perm_only;
 
 	// Get the subcommand
 	cur = advance_arg(false);
@@ -135,6 +136,7 @@ void forkmknod()
 	target_host = advance_arg(true);
 	uid = atoi(advance_arg(true));
 	gid = atoi(advance_arg(true));
+	chk_perm_only = atoi(advance_arg(true));
 
 	snprintf(cwd, sizeof(cwd), "/proc/%d/cwd", pid);
 	target_fd = open(cwd, O_PATH | O_RDONLY | O_CLOEXEC);
@@ -202,6 +204,9 @@ void forkmknod()
 		_exit(EXIT_FAILURE);
 	}
 
+	if (chk_perm_only)
+		_exit(EXIT_SUCCESS);
+
 	// basename() can modify its argument so accessing target_host is
 	// invalid from now on.
 	ret = mknodat(target_fd, target, mode, dev);
diff --git a/lxd/seccomp.go b/lxd/seccomp.go
index 6e5a497e17..2c9e8e5132 100644
--- a/lxd/seccomp.go
+++ b/lxd/seccomp.go
@@ -771,7 +771,7 @@ func taskUidGid(pid int) (error, int32, int32) {
 	return nil, uid, gid
 }
 
-func (s *SeccompServer) doMknod(c container, dev types.Device, requestPID int) (error, int) {
+func (s *SeccompServer) doMknod(c container, dev types.Device, requestPID int, permissionsOnly bool) (error, int) {
 	goErrno := int(-C.EPERM)
 
 	cwdLink := fmt.Sprintf("/proc/%d/cwd", requestPID)
@@ -796,7 +796,8 @@ func (s *SeccompServer) doMknod(c container, dev types.Device, requestPID int) (
 	errnoMsg, err := shared.RunCommand(util.GetExecPath(),
 		"forkmknod", dev["pid"], dev["path"],
 		dev["mode_t"], dev["dev_t"], dev["hostpath"],
-		fmt.Sprintf("%d", uid), fmt.Sprintf("%d", gid))
+		fmt.Sprintf("%d", uid), fmt.Sprintf("%d", gid),
+		fmt.Sprintf("%d", permissionsOnly))
 	if err != nil {
 		tmp, err2 := strconv.Atoi(errnoMsg)
 		if err2 == nil {
@@ -865,9 +866,14 @@ func (s *SeccompServer) Handler(c net.Conn, clientFd int, ucred *ucred,
 			}
 
 			if s.d.os.Shiftfs && !c.IsPrivileged() && diskIdmap == nil {
+				err, _ = s.doMknod(c, dev, int(cPid), true)
+				if err != nil {
+					goErrno = int(-C.EPERM)
+					goto send_response
+				}
 				err = c.InsertSeccompUnixDevice(fmt.Sprintf("forkmknod.unix.%d", int(cPid)), dev, int(cPid))
 			} else {
-				err, goErrno = s.doMknod(c, dev, int(cPid))
+				err, goErrno = s.doMknod(c, dev, int(cPid), false)
 				if err != nil && (goErrno == int(-C.ENOMEDIUM)) && c != nil {
 					err = c.InsertSeccompUnixDevice(fmt.Sprintf("forkmknod.unix.%d", int(cPid)), dev, int(cPid))
 					if err != nil {