Merge pull request #3517 from brauner/2020-08-10/fixes_2

src/lxc/attach.c

@@ -91,7 +91,9 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 	if (!found)
 		return log_error_errno(NULL, ENOENT, "Failed to read capability bounding set from %s", proc_fn);
 
-	info->lsm_label = lsm_process_label_get(pid);
+	info->lsm_ops = lsm_init();
+
+	info->lsm_label = info->lsm_ops->process_label_get(pid);
 	info->ns_inherited = 0;
 	for (int i = 0; i < LXC_NS_MAX; i++)
 		info->ns_fd[i] = -EBADF;
@@ -777,12 +779,12 @@ static int attach_child_main(struct attach_clone_payload *payload)
 		/* Change into our new LSM profile. */
 		on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? true : false;
 
-		ret = lsm_process_label_set_at(lsm_fd, init_ctx->lsm_label, on_exec);
+		ret = init_ctx->lsm_ops->process_label_set_at(lsm_fd, init_ctx->lsm_label, on_exec);
 		close(lsm_fd);
 		if (ret < 0)
 			goto on_error;
 
-		TRACE("Set %s LSM label to \"%s\"", lsm_name(), init_ctx->lsm_label);
+		TRACE("Set %s LSM label to \"%s\"", init_ctx->lsm_ops->name, init_ctx->lsm_label);
 	}
 
 	if ((init_ctx->container && init_ctx->container->lxc_conf &&
@@ -1242,7 +1244,7 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 
 			ret = -1;
 			on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? true : false;
-			labelfd = lsm_process_label_fd_get(attached_pid, on_exec);
+			labelfd = init_ctx->lsm_ops->process_label_fd_get(attached_pid, on_exec);
 			if (labelfd < 0)
 				goto close_mainloop;
 

src/lxc/attach.h

@@ -19,6 +19,7 @@ struct lxc_proc_context_info {
 	unsigned long long capability_mask;
 	int ns_inherited;
 	int ns_fd[LXC_NS_MAX];
+	const struct lsm_ops *lsm_ops;
 };
 
 __hidden extern int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,

src/lxc/conf.c

@@ -3211,13 +3211,49 @@ static int lxc_setup_boot_id(void)
 	return 0;
 }
 
+static int lxc_setup_keyring(const struct lsm_ops *lsm_ops, const struct lxc_conf *conf)
+{
+	key_serial_t keyring;
+	int ret = 0;
+
+	if (conf->lsm_se_keyring_context)
+		ret = lsm_ops->keyring_label_set(conf->lsm_se_keyring_context);
+	else if (conf->lsm_se_context)
+		ret = lsm_ops->keyring_label_set(conf->lsm_se_context);
+	if (ret < 0)
+		return log_error_errno(-1, errno, "Failed to set keyring context");
+
+	/*
+	 * Try to allocate a new session keyring for the container to prevent
+	 * information leaks.
+	 */
+	keyring = keyctl(KEYCTL_JOIN_SESSION_KEYRING, prctl_arg(0),
+			 prctl_arg(0), prctl_arg(0), prctl_arg(0));
+	if (keyring < 0) {
+		switch (errno) {
+		case ENOSYS:
+			DEBUG("The keyctl() syscall is not supported or blocked");
+			break;
+		case EACCES:
+			__fallthrough;
+		case EPERM:
+			DEBUG("Failed to access kernel keyring. Continuing...");
+			break;
+		default:
+			SYSERROR("Failed to create kernel keyring");
+			break;
+		}
+	}
+
+	return ret;
+}
+
 int lxc_setup(struct lxc_handler *handler)
 {
 	__do_close int pty_mnt_fd = -EBADF;
 	int ret;
 	const char *lxcpath = handler->lxcpath, *name = handler->name;
 	struct lxc_conf *lxc_conf = handler->conf;
-	char *keyring_context = NULL;
 
 	ret = lxc_setup_rootfs_prepare_root(lxc_conf, name, lxcpath);
 	if (ret < 0)
@@ -3230,15 +3266,9 @@ int lxc_setup(struct lxc_handler *handler)
 	}
 
 	if (!lxc_conf->keyring_disable_session) {
-		if (lxc_conf->lsm_se_keyring_context) {
-			keyring_context = lxc_conf->lsm_se_keyring_context;
-		} else if (lxc_conf->lsm_se_context) {
-			keyring_context = lxc_conf->lsm_se_context;
-		}
-
-		ret = lxc_setup_keyring(keyring_context);
+		ret = lxc_setup_keyring(handler->lsm_ops, lxc_conf);
 		if (ret < 0)
-			return -1;
+			return log_error(-1, "Failed to setup container keyring");
 	}
 
 	if (handler->ns_clone_flags & CLONE_NEWNET) {

src/lxc/lsm/lsm.c

@@ -17,193 +17,26 @@
 
 lxc_log_define(lsm, lxc);
 
-static struct lsm_drv *drv = NULL;
+__hidden extern struct lsm_ops *lsm_apparmor_ops_init(void);
+__hidden extern struct lsm_ops *lsm_selinux_ops_init(void);
+__hidden extern struct lsm_ops *lsm_nop_ops_init(void);
 
-__hidden extern struct lsm_drv *lsm_apparmor_drv_init(void);
-__hidden extern struct lsm_drv *lsm_selinux_drv_init(void);
-__hidden extern struct lsm_drv *lsm_nop_drv_init(void);
-
-__attribute__((constructor))
-void lsm_init(void)
+const struct lsm_ops *lsm_init(void)
 {
-	if (drv) {
-		INFO("LSM security driver %s", drv->name);
-		return;
-	}
+	const struct lsm_ops *ops = NULL;
 
 	#if HAVE_APPARMOR
-	drv = lsm_apparmor_drv_init();
+	ops = lsm_apparmor_ops_init();
 	#endif
+
 	#if HAVE_SELINUX
-	if (!drv)
-		drv = lsm_selinux_drv_init();
+	if (!ops)
+		ops = lsm_selinux_ops_init();
 	#endif
 
-	if (!drv)
-		drv = lsm_nop_drv_init();
-	INFO("Initialized LSM security driver %s", drv->name);
-}
-
-int lsm_enabled(void)
-{
-	if (drv)
-		return drv->enabled();
-	return 0;
-}
-
-const char *lsm_name(void)
-{
-	if (drv)
-		return drv->name;
-	return "none";
-}
-
-char *lsm_process_label_get(pid_t pid)
-{
-	if (!drv) {
-		ERROR("LSM driver not inited");
-		return NULL;
-	}
-	return drv->process_label_get(pid);
-}
-
-int lsm_process_label_fd_get(pid_t pid, bool on_exec)
-{
-	int ret = -1;
-	int labelfd = -1;
-	const char *name;
-	char path[LXC_LSMATTRLEN];
-
-	name = lsm_name();
-
-	if (strcmp(name, "nop") == 0)
-		return 0;
-
-	if (strcmp(name, "none") == 0)
-		return 0;
-
-	/* We don't support on-exec with AppArmor */
-	if (strcmp(name, "AppArmor") == 0)
-		on_exec = 0;
-
-	if (on_exec)
-		ret = snprintf(path, LXC_LSMATTRLEN, "/proc/%d/attr/exec", pid);
-	else
-		ret = snprintf(path, LXC_LSMATTRLEN, "/proc/%d/attr/current", pid);
-	if (ret < 0 || ret >= LXC_LSMATTRLEN)
-		return -1;
-
-	labelfd = open(path, O_RDWR);
-	if (labelfd < 0) {
-		SYSERROR("Unable to %s LSM label file descriptor", name);
-		return -1;
-	}
-
-	return labelfd;
-}
-
-int lsm_process_label_set_at(int label_fd, const char *label, bool on_exec)
-{
-	int ret = -1;
-	const char *name;
-
-	name = lsm_name();
-
-	if (strcmp(name, "nop") == 0)
-		return 0;
-
-	if (strcmp(name, "none") == 0)
-		return 0;
-
-	/* We don't support on-exec with AppArmor */
-	if (strcmp(name, "AppArmor") == 0)
-		on_exec = false;
-
-	if (strcmp(name, "AppArmor") == 0) {
-		size_t len;
-		char *command;
-
-		if (on_exec) {
-			ERROR("Changing AppArmor profile on exec not supported");
-			return -1;
-		}
-
-		len = strlen(label) + strlen("changeprofile ") + 1;
-		command = malloc(len);
-		if (!command)
-			goto on_error;
-
-		ret = snprintf(command, len, "changeprofile %s", label);
-		if (ret < 0 || (size_t)ret >= len) {
-			int saved_errno = errno;
-			free(command);
-			errno = saved_errno;
-			goto on_error;
-		}
-
-		ret = lxc_write_nointr(label_fd, command, len - 1);
-		free(command);
-	} else if (strcmp(name, "SELinux") == 0) {
-		ret = lxc_write_nointr(label_fd, label, strlen(label));
-	} else {
-		errno = EINVAL;
-		ret = -1;
-	}
-	if (ret < 0) {
-on_error:
-		SYSERROR("Failed to set %s label \"%s\"", name, label);
-		return -1;
-	}
-
-	INFO("Set %s label to \"%s\"", name, label);
-	return 0;
-}
-
-int lsm_process_label_set(const char *label, struct lxc_conf *conf,
-			  bool on_exec)
-{
-	if (!drv) {
-		ERROR("LSM driver not inited");
-		return -1;
-	}
-	return drv->process_label_set(label, conf, on_exec);
-}
-
-int lsm_process_prepare(struct lxc_conf *conf, const char *lxcpath)
-{
-	if (!drv) {
-		ERROR("LSM driver not inited");
-		return 0;
-	}
-
-	if (!drv->prepare)
-		return 0;
-
-	return drv->prepare(conf, lxcpath);
-}
-
-void lsm_process_cleanup(struct lxc_conf *conf, const char *lxcpath)
-{
-	if (!drv) {
-		ERROR("LSM driver not inited");
-		return;
-	}
-
-	if (!drv->cleanup)
-		return;
-
-	drv->cleanup(conf, lxcpath);
-}
-
-int lsm_keyring_label_set(char *label) {
-
-	if (!drv) {
-		ERROR("LSM driver not inited");
-		return -1;
-	}
-
-	if (!drv->keyring_label_set)
-		return 0;
+	if (!ops)
+		ops = lsm_nop_ops_init();
 
-	return drv->keyring_label_set(label);
+	INFO("Initialized LSM security driver %s", ops->name);
+	return ops;
 }

src/lxc/lsm/lsm.h

@@ -11,27 +11,19 @@ struct lxc_conf;
 #include "macro.h"
 #include "utils.h"
 
-struct lsm_drv {
+struct lsm_ops {
 	const char *name;
 
 	int (*enabled)(void);
 	char *(*process_label_get)(pid_t pid);
-	int (*process_label_set)(const char *label, struct lxc_conf *conf,
-				 bool on_exec);
-	int (*keyring_label_set)(char* label);
+	int (*process_label_set)(const char *label, struct lxc_conf *conf, bool on_exec);
+	int (*keyring_label_set)(const char *label);
 	int (*prepare)(struct lxc_conf *conf, const char *lxcpath);
 	void (*cleanup)(struct lxc_conf *conf, const char *lxcpath);
+	int (*process_label_fd_get)(pid_t pid, bool on_exec);
+	int (*process_label_set_at)(int label_fd, const char *label, bool on_exec);
 };
 
-__hidden extern void lsm_init(void);
-__hidden extern int lsm_enabled(void);
-__hidden extern const char *lsm_name(void);
-__hidden extern char *lsm_process_label_get(pid_t pid);
-__hidden extern int lsm_process_prepare(struct lxc_conf *conf, const char *lxcpath);
-__hidden extern int lsm_process_label_set(const char *label, struct lxc_conf *conf, bool on_exec);
-__hidden extern int lsm_process_label_fd_get(pid_t pid, bool on_exec);
-__hidden extern int lsm_process_label_set_at(int label_fd, const char *label, bool on_exec);
-__hidden extern void lsm_process_cleanup(struct lxc_conf *conf, const char *lxcpath);
-__hidden extern int lsm_keyring_label_set(char *label);
+__hidden extern const struct lsm_ops *lsm_init(void);
 
 #endif /* __LXC_LSM_H */

src/lxc/lsm/nop.c

@@ -24,14 +24,43 @@ static int nop_enabled(void)
 	return 0;
 }
 
-static struct lsm_drv nop_drv = {
-	.name = "nop",
-	.enabled           = nop_enabled,
-	.process_label_get = nop_process_label_get,
-	.process_label_set = nop_process_label_set,
+static int nop_keyring_label_set(const char *label)
+{
+	return 0;
+}
+
+static int nop_prepare(struct lxc_conf *conf, const char *lxcpath)
+{
+	return 0;
+}
+
+static void nop_cleanup(struct lxc_conf *conf, const char *lxcpath)
+{
+}
+
+static int nop_process_label_fd_get(pid_t pid, bool on_exec)
+{
+	return 0;
+}
+
+static int nop_process_label_set_at(int label_fd, const char *label, bool on_exec)
+{
+	return 0;
+}
+
+static struct lsm_ops nop_ops = {
+	.name			= "nop",
+	.cleanup		= nop_cleanup,
+	.enabled		= nop_enabled,
+	.keyring_label_set	= nop_keyring_label_set,
+	.prepare		= nop_prepare,
+	.process_label_fd_get	= nop_process_label_fd_get,
+	.process_label_get	= nop_process_label_get,
+	.process_label_set	= nop_process_label_set,
+	.process_label_set_at	= nop_process_label_set_at,
 };
 
-struct lsm_drv *lsm_nop_drv_init(void)
+const struct lsm_ops *lsm_nop_ops_init(void)
 {
-	return &nop_drv;
+	return &nop_ops;
 }

src/lxc/lsm/selinux.c

@@ -84,23 +84,71 @@ static int selinux_process_label_set(const char *inlabel, struct lxc_conf *conf,
  *
  * Returns 0 on success, < 0 on failure
  */
-static int selinux_keyring_label_set(char *label)
+static int selinux_keyring_label_set(const char *label)
 {
 	return setkeycreatecon_raw(label);
-};
+}
+
+static int selinux_prepare(struct lxc_conf *conf, const char *lxcpath)
+{
+	return 0;
+}
+
+static void selinux_cleanup(struct lxc_conf *conf, const char *lxcpath)
+{
+}
+
+static int selinux_process_label_fd_get(pid_t pid, bool on_exec)
+{
+	int ret = -1;
+	int labelfd;
+	char path[LXC_LSMATTRLEN];
+
+	if (on_exec)
+		ret = snprintf(path, LXC_LSMATTRLEN, "/proc/%d/attr/exec", pid);
+	else
+		ret = snprintf(path, LXC_LSMATTRLEN, "/proc/%d/attr/current", pid);
+	if (ret < 0 || ret >= LXC_LSMATTRLEN)
+		return -1;
+
+	labelfd = open(path, O_RDWR);
+	if (labelfd < 0)
+		return log_error_errno(-errno, errno, "Unable to open SELinux LSM label file descriptor");
 
-static struct lsm_drv selinux_drv = {
+	return labelfd;
+}
+
+static int selinux_process_label_set_at(int label_fd, const char *label, bool on_exec)
+{
+	int ret;
+
+	if (!label)
+		return 0;
+
+	ret = lxc_write_nointr(label_fd, label, strlen(label));
+	if (ret < 0)
+		return log_error_errno(-errno, errno, "Failed to set AppArmor SELinux label to \"%s\"", label);
+
+	INFO("Set SELinux label to \"%s\"", label);
+	return 0;
+}
+
+static struct lsm_ops selinux_ops = {
 	.name			= "SELinux",
+	.cleanup		= selinux_cleanup,
 	.enabled		= is_selinux_enabled,
+	.keyring_label_set	= selinux_keyring_label_set,
+	.prepare		= selinux_prepare,
+	.process_label_fd_get	= selinux_process_label_fd_get,
 	.process_label_get	= selinux_process_label_get,
 	.process_label_set	= selinux_process_label_set,
-	.keyring_label_set	= selinux_keyring_label_set,
+	.process_label_set_at	= selinux_process_label_set_at,
 };
 
-struct lsm_drv *lsm_selinux_drv_init(void)
+const struct lsm_ops *lsm_selinux_ops_init(void)
 {
 	if (!is_selinux_enabled())
 		return NULL;
 
-	return &selinux_drv;
+	return &selinux_ops;
 }

src/lxc/start.c

@@ -728,7 +728,7 @@ int lxc_init(const char *name, struct lxc_handler *handler)
 	if (status_fd < 0)
 		return log_error_errno(-1, errno, "Failed to open monitor status fd");
 
-	lsm_init();
+	handler->lsm_ops = lsm_init();
 	TRACE("Initialized LSM");
 
 	/* Begin by setting the state to STARTING. */
@@ -827,7 +827,7 @@ int lxc_init(const char *name, struct lxc_handler *handler)
 		return log_error(-1, "Failed loading seccomp policy");
 	TRACE("Read seccomp policy");
 
-	ret = lsm_process_prepare(conf, handler->lxcpath);
+	ret = handler->lsm_ops->prepare(conf, handler->lxcpath);
 	if (ret < 0) {
 		ERROR("Failed to initialize LSM");
 		goto out_delete_terminal;
@@ -918,7 +918,7 @@ void lxc_end(struct lxc_handler *handler)
 	while (namespace_count--)
 		free(namespaces[namespace_count]);
 
-	lsm_process_cleanup(handler->conf, handler->lxcpath);
+	handler->lsm_ops->cleanup(handler->conf, handler->lxcpath);
 
 	if (cgroup_ops) {
 		cgroup_ops->payload_destroy(cgroup_ops, handler);
@@ -1269,7 +1269,7 @@ static int do_start(void *data)
 	}
 
 	/* Set the label to change to when we exec(2) the container's init. */
-	ret = lsm_process_label_set(NULL, handler->conf, true);
+	ret = handler->lsm_ops->process_label_set(NULL, handler->conf, true);
 	if (ret < 0)
 		goto out_warn_father;
 

src/lxc/start.h

@@ -127,6 +127,8 @@ struct lxc_handler {
 
 	/* Internal fds that always need to stay open. */
 	int keep_fds[3];
+
+	const struct lsm_ops *lsm_ops;
 };
 
 struct execute_args {

src/lxc/utils.c

@@ -1838,41 +1838,6 @@ int lxc_rm_rf(const char *dirname)
 	return fret;
 }
 
-int lxc_setup_keyring(char *keyring_label)
-{
-	key_serial_t keyring;
-	int ret = 0;
-
-	if (keyring_label) {
-		if (lsm_keyring_label_set(keyring_label) < 0) {
-			ERROR("Couldn't set keyring label");
-		}
-	}
-
-	/* Try to allocate a new session keyring for the container to prevent
-	 * information leaks.
-	 */
-	keyring = keyctl(KEYCTL_JOIN_SESSION_KEYRING, prctl_arg(0),
-			 prctl_arg(0), prctl_arg(0), prctl_arg(0));
-	if (keyring < 0) {
-		switch (errno) {
-		case ENOSYS:
-			DEBUG("The keyctl() syscall is not supported or blocked");
-			break;
-		case EACCES:
-			__fallthrough;
-		case EPERM:
-			DEBUG("Failed to access kernel keyring. Continuing...");
-			break;
-		default:
-			SYSERROR("Failed to create kernel keyring");
-			break;
-		}
-	}
-
-	return ret;
-}
-
 bool lxc_can_use_pidfd(int pidfd)
 {
 	int ret;

src/lxc/utils.h

@@ -229,7 +229,6 @@ __hidden extern uint64_t lxc_find_next_power2(uint64_t n);
 __hidden extern int lxc_set_death_signal(int signal, pid_t parent, int parent_status_fd);
 __hidden extern int fd_cloexec(int fd, bool cloexec);
 __hidden extern int lxc_rm_rf(const char *dirname);
-__hidden extern int lxc_setup_keyring(char *keyring_label);
 __hidden extern bool lxc_can_use_pidfd(int pidfd);
 
 __hidden extern int fix_stdio_permissions(uid_t uid);

src/tests/attach.c

@@ -47,14 +47,16 @@
 static const char *lsm_config_key = NULL;
 static const char *lsm_label = NULL;
 
+const struct lsm_ops *lsm_ops;
+
 static void test_lsm_detect(void)
 {
-	if (lsm_enabled()) {
-		if (!strcmp(lsm_name(), "SELinux")) {
+	if (lsm_ops->enabled()) {
+		if (!strcmp(lsm_ops->name, "SELinux")) {
 			lsm_config_key = "lxc.selinux.context";
 			lsm_label      = "unconfined_u:unconfined_r:lxc_t:s0-s0:c0.c1023";
 		}
-		else if (!strcmp(lsm_name(), "AppArmor")) {
+		else if (!strcmp(lsm_ops->name, "AppArmor")) {
 			lsm_config_key = "lxc.apparmor.profile";
 			if (file_exists("/proc/self/ns/cgroup"))
 				lsm_label      = "lxc-container-default-cgns";
@@ -62,7 +64,7 @@ static void test_lsm_detect(void)
 				lsm_label      = "lxc-container-default";
 		}
 		else {
-			TSTERR("unknown lsm %s enabled, add test code here", lsm_name());
+			TSTERR("unknown lsm %s enabled, add test code here", lsm_ops->name);
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -78,7 +80,7 @@ static void test_attach_lsm_set_config(struct lxc_container *ct)
 
 static int test_attach_lsm_func_func(void* payload)
 {
-	TSTOUT("%s", lsm_process_label_get(syscall(SYS_getpid)));
+	TSTOUT("%s", lsm_ops->process_label_get(syscall(SYS_getpid)));
 	return 0;
 }
 
@@ -328,7 +330,7 @@ static struct lxc_container *test_ct_create(const char *lxcpath,
 		goto out2;
 	}
 
-	if (lsm_enabled())
+	if (lsm_ops->enabled())
 		test_attach_lsm_set_config(ct);
 
 	ct->want_daemonize(ct, true);
@@ -368,7 +370,7 @@ static int test_attach(const char *lxcpath, const char *name, const char *templa
 		goto err2;
 	}
 
-	if (lsm_enabled()) {
+	if (lsm_ops->enabled()) {
 		ret = test_attach_lsm_cmd(ct);
 		if (ret < 0) {
 			TSTERR("attach lsm cmd test failed");
@@ -398,6 +400,8 @@ int main(int argc, char *argv[])
 
 	(void)strlcpy(template, P_tmpdir"/attach_XXXXXX", sizeof(template));
 
+	lsm_ops = lsm_init();
+
 	i = lxc_make_tmpfile(template, false);
 	if (i < 0) {
 		lxc_error("Failed to create temporary log file for container %s\n", TSTNAME);