[lxc-devel] [lxd/master] seccomp: handle new liblxc seccomp notify updates

brauner on Github lxc-bot at linuxcontainers.org
Tue Jul 9 15:10:07 UTC 2019


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/20190709/5d2cca7c/attachment.bin>
-------------- next part --------------
From 0e730605db9415f07508292743e5501e20a10e95 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 9 Jul 2019 12:28:51 +0200
Subject: [PATCH 1/2] unixfd: add lxc_abstract_unix_recv_fds_iov()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 shared/netutils/unixfd.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/shared/netutils/unixfd.c b/shared/netutils/unixfd.c
index 6aac9da772..0c150551bb 100644
--- a/shared/netutils/unixfd.c
+++ b/shared/netutils/unixfd.c
@@ -56,20 +56,18 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
 	return sendmsg(fd, &msg, MSG_NOSIGNAL);
 }
 
-int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
-			       void *data, size_t size)
+static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
+					  struct iovec *iov, size_t iovlen)
 {
 	__do_free char *cmsgbuf = NULL;
 	int ret;
 	struct msghdr msg;
-	struct iovec iov;
 	struct cmsghdr *cmsg = NULL;
 	char buf[1] = {0};
 	size_t cmsgbufsize = CMSG_SPACE(sizeof(struct ucred)) +
 			     CMSG_SPACE(num_recvfds * sizeof(int));
 
 	memset(&msg, 0, sizeof(msg));
-	memset(&iov, 0, sizeof(iov));
 
 	cmsgbuf = malloc(cmsgbufsize);
 	if (!cmsgbuf) {
@@ -80,10 +78,8 @@ int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
 	msg.msg_control = cmsgbuf;
 	msg.msg_controllen = cmsgbufsize;
 
-	iov.iov_base = data ? data : buf;
-	iov.iov_len = data ? size : sizeof(buf);
-	msg.msg_iov = &iov;
-	msg.msg_iovlen = 1;
+	msg.msg_iov = iov;
+	msg.msg_iovlen = iovlen;
 
 again:
 	ret = recvmsg(fd, &msg, 0);
@@ -112,3 +108,14 @@ int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
 out:
 	return ret;
 }
+
+int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
+			       void *data, size_t size)
+{
+	char buf[1] = {0};
+	struct iovec iov = {
+		.iov_base = data ? data : buf,
+		.iov_len = data ? size : sizeof(buf),
+	};
+	return lxc_abstract_unix_recv_fds_iov(fd, recvfds, num_recvfds, &iov, 1);
+}

From eedbaf4fa4af2994ecaf7f9e2ba2749f7fa836b6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 9 Jul 2019 17:09:14 +0200
Subject: [PATCH 2/2] seccomp: handle new liblxc seccomp notify updates

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/seccomp.go                   | 186 ++++++++++++++++++++++++++-----
 shared/netutils/network_linux.go |  10 +-
 shared/netutils/unixfd.c         |  20 ++--
 3 files changed, 174 insertions(+), 42 deletions(-)

diff --git a/lxd/seccomp.go b/lxd/seccomp.go
index c83f697b3c..3621d1301f 100644
--- a/lxd/seccomp.go
+++ b/lxd/seccomp.go
@@ -40,6 +40,7 @@ import (
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/syscall.h>
 #include <sys/sysmacros.h>
@@ -68,22 +69,42 @@ struct seccomp_notif_resp {
 	__s32 error;
 	__u32 flags;
 };
-#endif
+
+#endif // !SECCOMP_RET_USER_NOTIF
+
+struct seccomp_notif_sizes expected_sizes;
 
 struct seccomp_notify_proxy_msg {
-	uint32_t version;
-#ifdef SECCOMP_RET_USER_NOTIF
-	struct seccomp_notif req;
-	struct seccomp_notif_resp resp;
-#endif // SECCOMP_RET_USER_NOTIF
+	uint64_t __reserved;
 	pid_t monitor_pid;
 	pid_t init_pid;
+	struct seccomp_notif_sizes sizes;
+	uint64_t cookie_len;
+	// followed by: seccomp_notif, seccomp_notif_resp, cookie
 };
 
 #define SECCOMP_PROXY_MSG_SIZE (sizeof(struct seccomp_notify_proxy_msg))
+#define SECCOMP_NOTIFY_SIZE (sizeof(struct seccomp_notif))
+#define SECCOMP_RESPONSE_SIZE (sizeof(struct seccomp_notif_resp))
+#define SECCOMP_MSG_SIZE_MIN (SECCOMP_PROXY_MSG_SIZE + SECCOMP_NOTIFY_SIZE + SECCOMP_RESPONSE_SIZE)
+#define SECCOMP_COOKIE_SIZE (64 * sizeof(char))
+#define SECCOMP_MSG_SIZE_MAX (SECCOMP_MSG_SIZE_MIN + SECCOMP_COOKIE_SIZE)
 
 #ifdef SECCOMP_RET_USER_NOTIF
 
+static int seccomp_notify_get_sizes(struct seccomp_notif_sizes *sizes)
+{
+	if (syscall(SYS_seccomp, SECCOMP_GET_NOTIF_SIZES, 0, sizes) != 0)
+		return -1;
+
+	if (sizes->seccomp_notif != sizeof(struct seccomp_notif) ||
+	    sizes->seccomp_notif_resp != sizeof(struct seccomp_notif_resp) ||
+	    sizes->seccomp_data != sizeof(struct seccomp_data))
+		return -1;
+
+	return 0;
+}
+
 static int device_allowed(dev_t dev, mode_t mode)
 {
 	if ((dev == makedev(0, 0)) && (mode & S_IFCHR)) // whiteout
@@ -121,13 +142,13 @@ static int device_allowed(dev_t dev, mode_t mode)
 	#endif
 #endif
 
-static int seccomp_notify_mknod_set_response(int fd_mem, struct seccomp_notify_proxy_msg *msg,
+static int seccomp_notify_mknod_set_response(int fd_mem,
+					     struct seccomp_notif *req,
+					     struct seccomp_notif_resp *resp,
 					     char *buf, size_t size,
 					     mode_t *mode, dev_t *dev,
 					     pid_t *pid)
 {
-	struct seccomp_notif *req = &msg->req;
-	struct seccomp_notif_resp *resp = &msg->resp;
 	int ret;
 	ssize_t bytes;
 
@@ -183,13 +204,48 @@ static int seccomp_notify_mknod_set_response(int fd_mem, struct seccomp_notify_p
 	return 0;
 }
 
-static void seccomp_notify_mknod_update_response(struct seccomp_notify_proxy_msg *msg,
+static void seccomp_notify_mknod_update_response(struct seccomp_notif_resp *resp,
 						 int new_neg_errno)
 {
-	msg->resp.error = new_neg_errno;
+	resp->error = new_neg_errno;
 }
+
+static void prepare_seccomp_iovec(struct iovec *iov,
+				  struct seccomp_notify_proxy_msg *msg,
+				  struct seccomp_notif *notif,
+				  struct seccomp_notif_resp *resp, char *cookie)
+{
+	iov[0].iov_base = msg;
+	iov[0].iov_len = SECCOMP_PROXY_MSG_SIZE;
+
+	iov[1].iov_base = notif;
+	iov[1].iov_len = SECCOMP_NOTIFY_SIZE;
+
+	iov[2].iov_base = resp;
+	iov[2].iov_len = SECCOMP_RESPONSE_SIZE;
+
+	iov[3].iov_base = cookie;
+	iov[3].iov_len = SECCOMP_COOKIE_SIZE;
+}
+
 #else
-static int seccomp_notify_mknod_set_response(int fd_mem, struct seccomp_notify_proxy_msg *msg,
+
+static void prepare_seccomp_iovec(struct iovec *iov,
+				  struct seccomp_notify_proxy_msg *msg,
+				  struct seccomp_notif *notif,
+				  struct seccomp_notif_resp *resp, char *cookie)
+{
+}
+
+static int seccomp_notify_get_sizes(struct seccomp_notif_sizes *sizes)
+{
+	errno = ENOSYS;
+	return -1;
+}
+
+static int seccomp_notify_mknod_set_response(int fd_mem,
+					     struct seccomp_notif *req,
+					     struct seccomp_notif_resp *resp,
 					     char *buf, size_t size,
 					     mode_t *mode, dev_t *dev,
 					     pid_t *pid)
@@ -380,6 +436,11 @@ type SeccompServer struct {
 }
 
 func NewSeccompServer(d *Daemon, path string) (*SeccompServer, error) {
+	ret := C.seccomp_notify_get_sizes(&C.expected_sizes)
+	if ret < 0 {
+		return nil, fmt.Errorf("Failed to query kernel for seccomp notifier sizes")
+	}
+
 	// Cleanup existing sockets
 	if shared.PathExists(path) {
 		err := os.Remove(path)
@@ -389,7 +450,7 @@ func NewSeccompServer(d *Daemon, path string) (*SeccompServer, error) {
 	}
 
 	// Bind new socket
-	l, err := net.Listen("unix", path)
+	l, err := net.Listen("unixpacket", path)
 	if err != nil {
 		return nil, err
 	}
@@ -429,15 +490,76 @@ func NewSeccompServer(d *Daemon, path string) (*SeccompServer, error) {
 				}
 
 				for {
-					buf := make([]byte, C.SECCOMP_PROXY_MSG_SIZE)
-					fdMem, err := netutils.AbstractUnixReceiveFdData(int(unixFile.Fd()), buf)
+
+					msg_ptr := C.malloc(C.sizeof_struct_seccomp_notify_proxy_msg)
+					msg := (*C.struct_seccomp_notify_proxy_msg)(msg_ptr)
+					C.memset(msg_ptr, 0, C.sizeof_struct_seccomp_notify_proxy_msg)
+
+					req_ptr := C.malloc(C.sizeof_struct_seccomp_notif)
+					req := (*C.struct_seccomp_notif)(req_ptr)
+					C.memset(req_ptr, 0, C.sizeof_struct_seccomp_notif)
+
+					resp_ptr := C.malloc(C.sizeof_struct_seccomp_notif_resp)
+					resp := (*C.struct_seccomp_notif_resp)(resp_ptr)
+					C.memset(resp_ptr, 0, C.sizeof_struct_seccomp_notif_resp)
+
+					cookie_ptr := C.malloc(64 * C.sizeof_char)
+					cookie := (*C.char)(cookie_ptr)
+					C.memset(cookie_ptr, 0, 64*C.sizeof_char)
+
+					iov_unsafe_ptr := C.malloc(4 * C.sizeof_struct_iovec)
+					iov := (*C.struct_iovec)(iov_unsafe_ptr)
+					C.memset(iov_unsafe_ptr, 0, 4*C.sizeof_struct_iovec)
+
+					C.prepare_seccomp_iovec(iov, msg, req, resp, cookie)
+
+					bytes, fdMem, err := netutils.AbstractUnixReceiveFdData(int(unixFile.Fd()), iov_unsafe_ptr, 4)
 					if err != nil || err == io.EOF {
-						logger.Debugf("Disconnected from seccomp socket after receive: pid=%v", ucred.pid)
+						logger.Debugf("Disconnected from seccomp socket after failed receive: pid=%v, err=%s", ucred.pid, err)
 						c.Close()
 						return
 					}
 
-					go s.Handler(c, ucred, buf, fdMem)
+					cleanup := func() {
+						c.Close()
+						unix.Close(fdMem)
+						C.free(unsafe.Pointer(msg))
+						C.free(unsafe.Pointer(req))
+						C.free(unsafe.Pointer(resp))
+						C.free(unsafe.Pointer(cookie))
+					}
+
+					if uint64(bytes) < uint64(C.SECCOMP_MSG_SIZE_MIN) {
+						logger.Debugf("Disconnected from seccomp socket after incomplete receive: pid=%v", ucred.pid)
+						cleanup()
+						return
+					}
+
+					if msg.__reserved != 0 {
+						logger.Debugf("Disconnected from seccomp socket after client sent non-zero reserved field: pid=%v", ucred.pid)
+						cleanup()
+						return
+					}
+
+					if msg.sizes.seccomp_notif != C.expected_sizes.seccomp_notif {
+						logger.Debugf("Disconnected from seccomp socket since client uses different seccomp_notif sizes: %d != %d, pid=%v", msg.sizes.seccomp_notif, C.expected_sizes.seccomp_notif, ucred.pid)
+						cleanup()
+						return
+					}
+
+					if msg.sizes.seccomp_notif_resp != C.expected_sizes.seccomp_notif_resp {
+						logger.Debugf("Disconnected from seccomp socket since client uses different seccomp_notif_resp sizes: %d != %d, pid=%v", msg.sizes.seccomp_notif_resp, C.expected_sizes.seccomp_notif_resp, ucred.pid)
+						cleanup()
+						return
+					}
+
+					if msg.sizes.seccomp_data != C.expected_sizes.seccomp_data {
+						logger.Debugf("Disconnected from seccomp socket since client uses different seccomp_data sizes: %d != %d, pid=%v", msg.sizes.seccomp_data, C.expected_sizes.seccomp_data, ucred.pid)
+						cleanup()
+						return
+					}
+
+					go s.Handler(c, int(unixFile.Fd()), ucred, fdMem, iov, msg, req, resp, cookie)
 				}
 			}()
 		}
@@ -535,12 +657,17 @@ func doMknod(c container, dev types.Device, requestPID int) (error, int) {
 	return nil, 0
 }
 
-func (s *SeccompServer) Handler(c net.Conn, ucred *ucred, buf []byte, fdMem int) error {
+func (s *SeccompServer) Handler(c net.Conn, clientFd int, ucred *ucred,
+	fdMem int, iov *C.struct_iovec, msg *C.struct_seccomp_notify_proxy_msg,
+	req *C.struct_seccomp_notif, resp *C.struct_seccomp_notif_resp,
+	cookie *C.char) error {
 	logger.Debugf("Handling seccomp notification from: %v", ucred.pid)
 
 	defer unix.Close(fdMem)
-	var msg C.struct_seccomp_notify_proxy_msg
-	C.memcpy(unsafe.Pointer(&msg), unsafe.Pointer(&buf[0]), C.SECCOMP_PROXY_MSG_SIZE)
+	defer C.free(unsafe.Pointer(msg))
+	defer C.free(unsafe.Pointer(req))
+	defer C.free(unsafe.Pointer(resp))
+	defer C.free(unsafe.Pointer(cookie)) // currently unused
 
 	var cMode C.mode_t
 	var cDev C.dev_t
@@ -548,7 +675,7 @@ func (s *SeccompServer) Handler(c net.Conn, ucred *ucred, buf []byte, fdMem int)
 	var err error
 	goErrno := 0
 	cPathBuf := [unix.PathMax]C.char{}
-	ret := C.seccomp_notify_mknod_set_response(C.int(fdMem), &msg,
+	ret := C.seccomp_notify_mknod_set_response(C.int(fdMem), req, resp,
 		&cPathBuf[0],
 		unix.PathMax, &cMode,
 		&cDev, &cPid)
@@ -579,12 +706,19 @@ func (s *SeccompServer) Handler(c net.Conn, ucred *ucred, buf []byte, fdMem int)
 		}
 	}
 
-	C.seccomp_notify_mknod_update_response(&msg, C.int(goErrno))
-	C.memcpy(unsafe.Pointer(&buf[0]), unsafe.Pointer(&msg), C.SECCOMP_PROXY_MSG_SIZE)
+	C.seccomp_notify_mknod_update_response(resp, C.int(goErrno))
 
-	_, err = c.Write(buf)
-	if err != nil {
-		logger.Debugf("Disconnected from seccomp socket after write: pid=%v", ucred.pid)
+	msghdr := C.struct_msghdr{}
+	msghdr.msg_iov = iov
+	msghdr.msg_iovlen = 4 - 1 // without cookie
+	bytes := C.sendmsg(C.int(clientFd), &msghdr, C.MSG_NOSIGNAL)
+	if bytes < 0 {
+		logger.Debugf("Disconnected from seccomp socket after failed write: pid=%v", ucred.pid)
+		return err
+	}
+
+	if uint64(bytes) != uint64(C.SECCOMP_MSG_SIZE_MIN) {
+		logger.Debugf("Disconnected from seccomp socket after short write: pid=%v", ucred.pid)
 		return err
 	}
 
diff --git a/shared/netutils/network_linux.go b/shared/netutils/network_linux.go
index 957080a239..599cfebd03 100644
--- a/shared/netutils/network_linux.go
+++ b/shared/netutils/network_linux.go
@@ -234,17 +234,17 @@ func AbstractUnixReceiveFd(sockFD int) (*os.File, error) {
 	return file, nil
 }
 
-func AbstractUnixReceiveFdData(sockFD int, buf []byte) (int, error) {
+func AbstractUnixReceiveFdData(sockFD int, iov unsafe.Pointer, iovLen int32) (int64, int, error) {
 	fd := C.int(-1)
 	sk_fd := C.int(sockFD)
-	ret := C.lxc_abstract_unix_recv_fds(sk_fd, &fd, C.int(1), unsafe.Pointer(&buf[0]), C.size_t(len(buf)))
+	ret, errno := C.lxc_abstract_unix_recv_fds_iov(sk_fd, &fd, C.int(1), (*C.struct_iovec)(iov), C.size_t(iovLen))
 	if ret < 0 {
-		return int(-C.EBADF), fmt.Errorf("Failed to receive file descriptor via abstract unix socket")
+		return -1, int(-C.EBADF), fmt.Errorf("Failed to receive file descriptor via abstract unix socket: errno=%d", errno)
 	}
 
 	if ret == 0 {
-		return int(-C.EBADF), io.EOF
+		return -1, int(-C.EBADF), io.EOF
 	}
 
-	return int(fd), nil
+	return int64(ret), int(fd), nil
 }
diff --git a/shared/netutils/unixfd.c b/shared/netutils/unixfd.c
index 0c150551bb..e2dcc58084 100644
--- a/shared/netutils/unixfd.c
+++ b/shared/netutils/unixfd.c
@@ -56,19 +56,17 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
 	return sendmsg(fd, &msg, MSG_NOSIGNAL);
 }
 
-static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
-					  struct iovec *iov, size_t iovlen)
+static ssize_t lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds,
+					      int num_recvfds,
+					      struct iovec *iov, size_t iovlen)
 {
 	__do_free char *cmsgbuf = NULL;
-	int ret;
-	struct msghdr msg;
+	ssize_t ret;
+	struct msghdr msg = { 0 };
 	struct cmsghdr *cmsg = NULL;
-	char buf[1] = {0};
 	size_t cmsgbufsize = CMSG_SPACE(sizeof(struct ucred)) +
 			     CMSG_SPACE(num_recvfds * sizeof(int));
 
-	memset(&msg, 0, sizeof(msg));
-
 	cmsgbuf = malloc(cmsgbufsize);
 	if (!cmsgbuf) {
 		errno = ENOMEM;
@@ -82,7 +80,7 @@ static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
 	msg.msg_iovlen = iovlen;
 
 again:
-	ret = recvmsg(fd, &msg, 0);
+	ret = recvmsg(fd, &msg, MSG_TRUNC | MSG_CMSG_CLOEXEC | MSG_NOSIGNAL);
 	if (ret < 0) {
 		if (errno == EINTR)
 			goto again;
@@ -109,13 +107,13 @@ static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds,
 	return ret;
 }
 
-int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
-			       void *data, size_t size)
+ssize_t lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,
+				   void *data, size_t size)
 {
 	char buf[1] = {0};
 	struct iovec iov = {
 		.iov_base = data ? data : buf,
 		.iov_len = data ? size : sizeof(buf),
 	};
-	return lxc_abstract_unix_recv_fds_iov(fd, recvfds, num_recvfds, &iov, 1);
+	return lxc_abstract_unix_recv_fds_iov(fd, recvfds, num_recvfds, &iov, iov.iov_len);
 }


More information about the lxc-devel mailing list