[lxc-devel] [lxc/master] RFC: Seccomp notify api update

Blub on Github lxc-bot at linuxcontainers.org
Fri Jul 5 08:53:29 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1109 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190705/be2dc87d/attachment.bin>
-------------- next part --------------
From d475aa8e69c59fc75e7e497a4db507369896e057 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Thu, 4 Jul 2019 09:17:04 +0200
Subject: [PATCH 1/9] af_unix: add lxc_abstract_unix_send_fds_iov

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/af_unix.c | 25 ++++++++++++++++---------
 src/lxc/af_unix.h |  3 +++
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index c688a8746f..7ab499a4f4 100644
--- a/src/lxc/af_unix.c
+++ b/src/lxc/af_unix.c
@@ -153,19 +153,16 @@ int lxc_abstract_unix_connect(const char *path)
 	return fd;
 }
 
-int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
-			       void *data, size_t size)
+int lxc_abstract_unix_send_fds_iov(int fd, int *sendfds, int num_sendfds,
+				   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(num_sendfds * sizeof(int));
 
 	memset(&msg, 0, sizeof(msg));
-	memset(&iov, 0, sizeof(iov));
 
 	cmsgbuf = malloc(cmsgbufsize);
 	if (!cmsgbuf) {
@@ -185,10 +182,8 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
 
 	memcpy(CMSG_DATA(cmsg), sendfds, num_sendfds * sizeof(int));
 
-	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 = sendmsg(fd, &msg, MSG_NOSIGNAL);
@@ -199,6 +194,18 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
 	return ret;
 }
 
+int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
+			       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_send_fds_iov(fd, sendfds, num_sendfds, &iov,
+					      1);
+}
+
 int lxc_unix_send_fds(int fd, int *sendfds, int num_sendfds, void *data,
 		      size_t size)
 {
diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h
index 8a068d920f..9f4729c0b9 100644
--- a/src/lxc/af_unix.h
+++ b/src/lxc/af_unix.h
@@ -35,6 +35,9 @@ extern void lxc_abstract_unix_close(int fd);
 extern int lxc_abstract_unix_connect(const char *path);
 extern int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds,
 				      void *data, size_t size);
+extern int lxc_abstract_unix_send_fds_iov(int fd, int *sendfds,
+					  int num_sendfds, struct iovec *iov,
+					  size_t iovlen);
 extern int lxc_unix_send_fds(int fd, int *sendfds, int num_sendfds, void *data,
 			     size_t size);
 extern int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds,

From 0f7c4226bc9b8a8ddc9be45e7c5c80d12dc12f45 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Thu, 4 Jul 2019 14:34:01 +0200
Subject: [PATCH 2/9] af_unix: add lxc_unix_connect_type

we want to use SOCK_SEQPACKET and in the future perhaps
SOCK_DATAGRAM as well

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/af_unix.c | 9 +++++++--
 src/lxc/af_unix.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index 7ab499a4f4..4829e14afa 100644
--- a/src/lxc/af_unix.c
+++ b/src/lxc/af_unix.c
@@ -372,13 +372,13 @@ int lxc_unix_sockaddr(struct sockaddr_un *ret, const char *path)
 	return (int)(offsetof(struct sockaddr_un, sun_path) + len + 1);
 }
 
-int lxc_unix_connect(struct sockaddr_un *addr)
+int lxc_unix_connect_type(struct sockaddr_un *addr, int type)
 {
 	__do_close_prot_errno int fd = -EBADF;
 	int ret;
 	ssize_t len;
 
-	fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+	fd = socket(AF_UNIX, type | SOCK_CLOEXEC, 0);
 	if (fd < 0) {
 		SYSERROR("Failed to open new AF_UNIX socket");
 		return -1;
@@ -399,6 +399,11 @@ int lxc_unix_connect(struct sockaddr_un *addr)
 	return move_fd(fd);
 }
 
+int lxc_unix_connect(struct sockaddr_un *addr, int type)
+{
+	return lxc_unix_connect_type(addr, SOCK_STREAM);
+}
+
 int lxc_socket_set_timeout(int fd, int rcv_timeout, int snd_timeout)
 {
 	struct timeval out = {0};
diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h
index 9f4729c0b9..eee9f4c275 100644
--- a/src/lxc/af_unix.h
+++ b/src/lxc/af_unix.h
@@ -46,6 +46,7 @@ extern int lxc_abstract_unix_send_credential(int fd, void *data, size_t size);
 extern int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size);
 extern int lxc_unix_sockaddr(struct sockaddr_un *ret, const char *path);
 extern int lxc_unix_connect(struct sockaddr_un *addr);
+extern int lxc_unix_connect_type(struct sockaddr_un *addr, int type);
 extern int lxc_socket_set_timeout(int fd, int rcv_timeout, int snd_timeout);
 
 #endif /* __LXC_AF_UNIX_H */

From 6751b537d3207cfd819ad06507df9271634b5b14 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Thu, 4 Jul 2019 14:25:02 +0200
Subject: [PATCH 3/9] file_utils: add lxc_recvmsg_nointr_iov

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/file_utils.c | 18 ++++++++++++++++++
 src/lxc/file_utils.h |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c
index 31411d712e..9cbe6c2753 100644
--- a/src/lxc/file_utils.c
+++ b/src/lxc/file_utils.c
@@ -142,6 +142,24 @@ ssize_t lxc_recv_nointr(int sockfd, void *buf, size_t len, int flags)
 	return ret;
 }
 
+ssize_t lxc_recvmsg_nointr_iov(int sockfd, struct iovec *iov, size_t iovlen,
+			       int flags)
+{
+	ssize_t ret;
+	struct msghdr msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg_iov = iov;
+	msg.msg_iovlen = iovlen;
+
+again:
+	ret = recvmsg(sockfd, &msg, flags);
+	if (ret < 0 && errno == EINTR)
+		goto again;
+
+	return ret;
+}
+
 ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count, const void *expected_buf)
 {
 	ssize_t ret;
diff --git a/src/lxc/file_utils.h b/src/lxc/file_utils.h
index 1b8033d69b..a087147e11 100644
--- a/src/lxc/file_utils.h
+++ b/src/lxc/file_utils.h
@@ -26,6 +26,7 @@
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/uio.h>
 #include <sys/vfs.h>
 #include <unistd.h>
 
@@ -43,6 +44,8 @@ extern ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count,
 extern ssize_t lxc_read_file_expect(const char *path, void *buf, size_t count,
 				      const void *expected_buf);
 extern ssize_t lxc_recv_nointr(int sockfd, void *buf, size_t len, int flags);
+ssize_t lxc_recvmsg_nointr_iov(int sockfd, struct iovec *iov, size_t iovlen,
+			       int flags);
 
 extern bool file_exists(const char *f);
 extern int print_to_file(const char *file, const char *content);

From f6deac453fbe3b46d01df1e54995547abe787851 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Wed, 3 Jul 2019 17:30:49 +0200
Subject: [PATCH 4/9] conf: add lxc.seccomp.notify.cookie

This is an arbitrary string to to be included in proxied
seccomp notification messages.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/confile.c    | 34 ++++++++++++++++++++++++++++++++++
 src/lxc/lxcseccomp.h |  1 +
 2 files changed, 35 insertions(+)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index bb1edc7cb9..b08aa0174f 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -153,6 +153,7 @@ lxc_config_define(rootfs_options);
 lxc_config_define(rootfs_path);
 lxc_config_define(seccomp_profile);
 lxc_config_define(seccomp_allow_nesting);
+lxc_config_define(seccomp_notify_cookie);
 lxc_config_define(seccomp_notify_proxy);
 lxc_config_define(selinux_context);
 lxc_config_define(signal_halt);
@@ -246,6 +247,7 @@ static struct lxc_config_t config_jump_table[] = {
 	{ "lxc.rootfs.options",            set_config_rootfs_options,              get_config_rootfs_options,              clr_config_rootfs_options,            },
 	{ "lxc.rootfs.path",               set_config_rootfs_path,                 get_config_rootfs_path,                 clr_config_rootfs_path,               },
 	{ "lxc.seccomp.allow_nesting",     set_config_seccomp_allow_nesting,       get_config_seccomp_allow_nesting,       clr_config_seccomp_allow_nesting,     },
+	{ "lxc.seccomp.notify.cookie",     set_config_seccomp_notify_cookie,       get_config_seccomp_notify_cookie,       clr_config_seccomp_notify_cookie,     },
 	{ "lxc.seccomp.notify.proxy",      set_config_seccomp_notify_proxy,        get_config_seccomp_notify_proxy,        clr_config_seccomp_notify_proxy,      },
 	{ "lxc.seccomp.profile",           set_config_seccomp_profile,             get_config_seccomp_profile,             clr_config_seccomp_profile,           },
 	{ "lxc.selinux.context",           set_config_selinux_context,             get_config_selinux_context,             clr_config_selinux_context,           },
@@ -1013,6 +1015,16 @@ static int set_config_seccomp_allow_nesting(const char *key, const char *value,
 #endif
 }
 
+static int set_config_seccomp_notify_cookie(const char *key, const char *value,
+					    struct lxc_conf *lxc_conf, void *data)
+{
+#ifdef HAVE_SECCOMP_NOTIFY
+	return set_config_string_item(&lxc_conf->seccomp.notifier.cookie, value);
+#else
+	return minus_one_set_errno(ENOSYS);
+#endif
+}
+
 static int set_config_seccomp_notify_proxy(const char *key, const char *value,
 					   struct lxc_conf *lxc_conf, void *data)
 {
@@ -3955,6 +3967,16 @@ static int get_config_seccomp_allow_nesting(const char *key, char *retv,
 #endif
 }
 
+static int get_config_seccomp_notify_cookie(const char *key, char *retv, int inlen,
+					    struct lxc_conf *c, void *data)
+{
+#ifdef HAVE_SECCOMP_NOTIFY
+	return lxc_get_conf_str(retv, inlen, c->seccomp.notifier.cookie);
+#else
+	return minus_one_set_errno(ENOSYS);
+#endif
+}
+
 static int get_config_seccomp_notify_proxy(const char *key, char *retv, int inlen,
 					   struct lxc_conf *c, void *data)
 {
@@ -4563,6 +4585,18 @@ static inline int clr_config_seccomp_allow_nesting(const char *key,
 #endif
 }
 
+static inline int clr_config_seccomp_notify_cookie(const char *key,
+						   struct lxc_conf *c, void *data)
+{
+#ifdef HAVE_SECCOMP_NOTIFY
+	free(c->seccomp.notifier.cookie);
+	c->seccomp.notifier.cookie = NULL;
+	return 0;
+#else
+	return minus_one_set_errno(ENOSYS);
+#endif
+}
+
 static inline int clr_config_seccomp_notify_proxy(const char *key,
 						   struct lxc_conf *c, void *data)
 {
diff --git a/src/lxc/lxcseccomp.h b/src/lxc/lxcseccomp.h
index 121aa4e7a7..f819436724 100644
--- a/src/lxc/lxcseccomp.h
+++ b/src/lxc/lxcseccomp.h
@@ -69,6 +69,7 @@ struct seccomp_notify {
 	struct sockaddr_un proxy_addr;
 	struct seccomp_notif *req_buf;
 	struct seccomp_notif_resp *rsp_buf;
+	char *cookie;
 };
 
 #define HAVE_SECCOMP_NOTIFY 1

From 6d96d2e3a4530ad55a05fc753b826b73576e920f Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Jul 2019 09:22:11 +0200
Subject: [PATCH 5/9] seccomp: update notify api

The previous API doesn't reflect the fact that
`seccomp_notif` and `seccomp_notif_resp` are allocatd
dynamically with sizes figured out at runtime.

We now query the sizes via the seccomp(2) syscall and change
`struct seccomp_notify_proxy_msg` to contain the sizes
instead of the data, with the data following afterwards.

Additionally it did not provide a convenient way to identify
the container the message originated from, for which we now
include a cookie configured via `lxc.seccomp.notify.cookie`.

Also verify the `id` in the client's response.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/lxcseccomp.h | 13 ++++++--
 src/lxc/seccomp.c    | 73 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/src/lxc/lxcseccomp.h b/src/lxc/lxcseccomp.h
index f819436724..c13307f871 100644
--- a/src/lxc/lxcseccomp.h
+++ b/src/lxc/lxcseccomp.h
@@ -54,12 +54,20 @@ struct lxc_handler;
 
 #if HAVE_DECL_SECCOMP_NOTIFY_FD
 
+/* this is not exposed by libseccomp */
+struct seccomp_notif_sizes {
+	__u16 seccomp_notif;
+	__u16 seccomp_notif_resp;
+	__u16 seccomp_data;
+};
+
 struct seccomp_notify_proxy_msg {
 	uint32_t version;
-	struct seccomp_notif req;
-	struct seccomp_notif_resp resp;
 	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 */
 };
 
 struct seccomp_notify {
@@ -67,6 +75,7 @@ struct seccomp_notify {
 	int notify_fd;
 	int proxy_fd;
 	struct sockaddr_un proxy_addr;
+	struct seccomp_notif_sizes sizes;
 	struct seccomp_notif *req_buf;
 	struct seccomp_notif_resp *rsp_buf;
 	char *cookie;
diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 1e688a4517..2c35ff989c 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -49,8 +49,23 @@
 #define MIPS_ARCH_N64 lxc_seccomp_arch_mips64
 #endif
 
+#ifndef SECCOMP_GET_NOTIF_SIZES
+#define SECCOMP_GET_NOTIF_SIZES 3
+#endif
+
 lxc_log_define(seccomp, lxc);
 
+static inline int __seccomp(unsigned int operation, unsigned int flags,
+			  void *args)
+{
+#ifdef __NR_seccomp
+	return syscall(__NR_seccomp, operation, flags, args);
+#else
+	errno = ENOSYS;
+	return -1;
+#endif
+}
+
 static int parse_config_v1(FILE *f, char *line, size_t *line_bufsz, struct lxc_conf *conf)
 {
 	int ret = 0;
@@ -1333,6 +1348,8 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 	__do_close_prot_errno int fd_mem = -EBADF;
 	int reconnect_count, ret;
 	ssize_t bytes;
+	struct iovec iov[4];
+	size_t iov_len, msg_base_size, msg_full_size;
 	char mem_path[6 /* /proc/ */
 		      + INTTYPE_TO_STRLEN(int64_t)
 		      + 3 /* mem */
@@ -1343,6 +1360,8 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 	struct seccomp_notif_resp *resp = conf->seccomp.notifier.rsp_buf;
 	int listener_proxy_fd = conf->seccomp.notifier.proxy_fd;
 	struct seccomp_notify_proxy_msg msg = {0};
+	char *cookie = conf->seccomp.notifier.cookie;
+	uint64_t req_id;
 
 	if (listener_proxy_fd < 0) {
 		ERROR("No seccomp proxy registered");
@@ -1355,6 +1374,9 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		goto out;
 	}
 
+	/* remember the ID in case we receive garbage from the proxy */
+	resp->id = req_id = req->id;
+
 	snprintf(mem_path, sizeof(mem_path), "/proc/%d/mem", req->pid);
 	fd_mem = open(mem_path, O_RDONLY | O_CLOEXEC);
 	if (fd_mem < 0) {
@@ -1374,15 +1396,38 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		goto out;
 	}
 
-	memcpy(&msg.req, req, sizeof(msg.req));
 	msg.monitor_pid = hdlr->monitor_pid;
 	msg.init_pid = hdlr->pid;
+	memcpy(&msg.sizes, &conf->seccomp.notifier.sizes, sizeof(msg.sizes));
+
+	msg_base_size = 0;
+	iov[0].iov_base = &msg;
+	msg_base_size += (iov[0].iov_len = sizeof(msg));
+	iov[1].iov_base = req;
+	msg_base_size += (iov[1].iov_len = msg.sizes.seccomp_notif);
+	iov[2].iov_base = resp;
+	msg_base_size += (iov[2].iov_len = msg.sizes.seccomp_notif_resp);
+	msg_full_size = msg_base_size;
+
+	if (cookie) {
+		size_t len = strlen(cookie);
+
+		msg.cookie_len = (uint64_t)len;
+
+		iov[3].iov_base = cookie;
+		msg_full_size += (iov[3].iov_len = len);
+
+		iov_len = 4;
+	} else {
+		iov_len = 3;
+	}
 
 	reconnect_count = 0;
 	do {
-		bytes = lxc_unix_send_fds(listener_proxy_fd, &fd_mem, 1, &msg,
-					  sizeof(msg));
-		if (bytes != (ssize_t)sizeof(msg)) {
+		bytes = lxc_abstract_unix_send_fds_iov(listener_proxy_fd,
+						       &fd_mem, 1, iov,
+						       iov_len);
+		if (bytes != (ssize_t)msg_full_size) {
 			SYSERROR("Failed to forward message to seccomp proxy");
 			if (seccomp_notify_default_answer(fd, req, resp, hdlr))
 				goto out;
@@ -1391,17 +1436,24 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 
 	close_prot_errno_disarm(fd_mem);
 
+	if (resp->id != req_id) {
+		resp->id = req_id;
+		ERROR("Proxy returned response with illegal id");
+		(void)seccomp_notify_default_answer(fd, req, resp, hdlr);
+		goto out;
+	}
+
 	reconnect_count = 0;
 	do {
-		bytes = lxc_recv_nointr(listener_proxy_fd, &msg, sizeof(msg), 0);
-		if (bytes != (ssize_t)sizeof(msg)) {
+		bytes = lxc_recvmsg_nointr_iov(listener_proxy_fd, iov,iov_len,
+		                               0);
+		if (bytes != (ssize_t)msg_base_size) {
 			SYSERROR("Failed to receive message from seccomp proxy");
 			if (seccomp_notify_default_answer(fd, req, resp, hdlr))
 				goto out;
 		}
 	} while (reconnect_count++);
 
-	memcpy(resp, &msg.resp, sizeof(*resp));
 	ret = seccomp_notify_respond(fd, resp);
 	if (ret)
 		SYSERROR("Failed to send seccomp notification");
@@ -1454,6 +1506,13 @@ int lxc_seccomp_setup_proxy(struct lxc_seccomp *seccomp,
 			return -1;
 		}
 
+		ret = __seccomp(SECCOMP_GET_NOTIF_SIZES, 0,
+				&seccomp->notifier.sizes);
+		if (ret) {
+			SYSERROR("Failed to query seccomp notify struct sizes");
+			return -1;
+		}
+
 		ret = seccomp_notify_alloc(&seccomp->notifier.req_buf,
 					  &seccomp->notifier.rsp_buf);
 		if (ret) {

From 9b665d23ccfd6b834e1682e355ade89af13e072d Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Jul 2019 09:31:09 +0200
Subject: [PATCH 6/9] seccomp: use SOCK_SEQPACKET for the notify proxy

The seccomp notify API has a few variables: The struct sizes
are queried at runtime, and we now also have a user
configured cookie.
This means that with a SOCK_STREAM connection the proxy
needs to carefully read() the right amount of data based on
the contents of our proxy message struct to avoid ending up
in the middle of a packet.
While for now this may not be too tragic, since we currently
only ever send a single packet and then wait for the
response, we may at some point want to be able to handle
multiple processes simultaneously, hence it makes sense to
switch to a packet based connection.

So switch to using SOCK_SEQPACKET which is packet based,
(and also guarantees ordering). The `MSG_PEEK` flag can be
used with `recvmsg()` to figure out a packet's size on the
other end, and usually the size *should* not change after
that for an existing connection from a running container.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/seccomp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 2c35ff989c..707e2a73a4 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -1309,7 +1309,8 @@ static int seccomp_notify_reconnect(struct lxc_handler *handler)
 
 	close_prot_errno_disarm(handler->conf->seccomp.notifier.proxy_fd);
 
-	notify_fd = lxc_unix_connect(&handler->conf->seccomp.notifier.proxy_addr);
+	notify_fd = lxc_unix_connect_type(
+		&handler->conf->seccomp.notifier.proxy_addr, SOCK_SEQPACKET);
 	if (notify_fd < 0) {
 		SYSERROR("Failed to reconnect to seccomp proxy");
 		return -1;
@@ -1493,7 +1494,8 @@ int lxc_seccomp_setup_proxy(struct lxc_seccomp *seccomp,
 		__do_close_prot_errno int notify_fd = -EBADF;
 		int ret;
 
-		notify_fd = lxc_unix_connect(&seccomp->notifier.proxy_addr);
+		notify_fd = lxc_unix_connect_type(&seccomp->notifier.proxy_addr,
+					     SOCK_SEQPACKET);
 		if (notify_fd < 0) {
 			SYSERROR("Failed to connect to seccomp proxy");
 			return -1;

From 77bbccf0f478f154aec9ac76fd06c58577bbe2bc Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Jul 2019 09:40:04 +0200
Subject: [PATCH 7/9] seccomp: remove reconnect-loop

When we fail to send a message, we send a default seccomp
response and try to reconnect to the proxy. It doesn't
really make much sense to retry to send the request over the
new connection as the syscall has already been answered. The
same goes for receiving the response - after reconnecting to
the proxy, we're a new client to a potentially new proxy
process, so awaiting a response without having sent a
request doesn't make all too much sense either.

In the future we should probably have a timeout or retry
count for the entire proxy _transaction_ before sending a
response to seccomp at all (and probably handle requests
asynchronously).

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/seccomp.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 707e2a73a4..1ea5319007 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -1347,7 +1347,7 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 
 #if HAVE_DECL_SECCOMP_NOTIFY_FD
 	__do_close_prot_errno int fd_mem = -EBADF;
-	int reconnect_count, ret;
+	int ret;
 	ssize_t bytes;
 	struct iovec iov[4];
 	size_t iov_len, msg_base_size, msg_full_size;
@@ -1423,17 +1423,13 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		iov_len = 3;
 	}
 
-	reconnect_count = 0;
-	do {
-		bytes = lxc_abstract_unix_send_fds_iov(listener_proxy_fd,
-						       &fd_mem, 1, iov,
-						       iov_len);
-		if (bytes != (ssize_t)msg_full_size) {
-			SYSERROR("Failed to forward message to seccomp proxy");
-			if (seccomp_notify_default_answer(fd, req, resp, hdlr))
-				goto out;
-		}
-	} while (reconnect_count++);
+	bytes = lxc_abstract_unix_send_fds_iov(listener_proxy_fd, &fd_mem, 1,
+					       iov, iov_len);
+	if (bytes != (ssize_t)msg_full_size) {
+		SYSERROR("Failed to forward message to seccomp proxy");
+		(void)seccomp_notify_default_answer(fd, req, resp, hdlr);
+		goto out;
+	}
 
 	close_prot_errno_disarm(fd_mem);
 
@@ -1444,16 +1440,12 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		goto out;
 	}
 
-	reconnect_count = 0;
-	do {
-		bytes = lxc_recvmsg_nointr_iov(listener_proxy_fd, iov,iov_len,
-		                               0);
-		if (bytes != (ssize_t)msg_base_size) {
-			SYSERROR("Failed to receive message from seccomp proxy");
-			if (seccomp_notify_default_answer(fd, req, resp, hdlr))
-				goto out;
-		}
-	} while (reconnect_count++);
+	bytes = lxc_recvmsg_nointr_iov(listener_proxy_fd, iov,iov_len, 0);
+	if (bytes != (ssize_t)msg_base_size) {
+		SYSERROR("Failed to receive message from seccomp proxy");
+		(void)seccomp_notify_default_answer(fd, req, resp, hdlr);
+		goto out;
+	}
 
 	ret = seccomp_notify_respond(fd, resp);
 	if (ret)

From 7930491603b41ca5c1f274266b74adfa04b8bd17 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Jul 2019 09:44:17 +0200
Subject: [PATCH 8/9] seccomp: don't ignore syscalls when there's no proxy

The container process would just hang.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/seccomp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 1ea5319007..923aafc544 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -1364,17 +1364,17 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 	char *cookie = conf->seccomp.notifier.cookie;
 	uint64_t req_id;
 
-	if (listener_proxy_fd < 0) {
-		ERROR("No seccomp proxy registered");
-		return minus_one_set_errno(EINVAL);
-	}
-
 	ret = seccomp_notify_receive(fd, req);
 	if (ret) {
 		SYSERROR("Failed to read seccomp notification");
 		goto out;
 	}
 
+	if (listener_proxy_fd < 0) {
+		ERROR("No seccomp proxy registered");
+		return minus_one_set_errno(EINVAL);
+	}
+
 	/* remember the ID in case we receive garbage from the proxy */
 	resp->id = req_id = req->id;
 

From 1eb2bf063a4fc943c1fa26250cd9f165aabf0d23 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 5 Jul 2019 10:41:19 +0200
Subject: [PATCH 9/9] seccomp: retry connecting to the proxy once

If the first sendmsg() fails, try to reconnect once before
failing. Otherwise if a proxy restarts while no syscall
happens, the next syscall always fails with ENOSYS.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/lxc/seccomp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 923aafc544..95db4db4cb 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -1355,6 +1355,7 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		      + INTTYPE_TO_STRLEN(int64_t)
 		      + 3 /* mem */
 		      + 1 /* \0 */];
+	bool reconnected = false;
 	struct lxc_handler *hdlr = data;
 	struct lxc_conf *conf = hdlr->conf;
 	struct seccomp_notif *req = conf->seccomp.notifier.req_buf;
@@ -1423,10 +1424,19 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
 		iov_len = 3;
 	}
 
+retry:
 	bytes = lxc_abstract_unix_send_fds_iov(listener_proxy_fd, &fd_mem, 1,
 					       iov, iov_len);
 	if (bytes != (ssize_t)msg_full_size) {
 		SYSERROR("Failed to forward message to seccomp proxy");
+		if (!reconnected) {
+			ret = seccomp_notify_reconnect(hdlr);
+			if (ret == 0) {
+				reconnected = true;
+				goto retry;
+			}
+		}
+
 		(void)seccomp_notify_default_answer(fd, req, resp, hdlr);
 		goto out;
 	}


More information about the lxc-devel mailing list