[lxc-devel] [lxc/master] lsm: rewrite
brauner on Github
lxc-bot at linuxcontainers.org
Tue Aug 11 08:33:33 UTC 2020
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/20200811/222781de/attachment-0001.bin>
-------------- next part --------------
From d701d729f68b05469e672b1e64be078a09448268 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 10 Aug 2020 23:55:13 +0200
Subject: [PATCH 1/2] lsm: rework lsm handling
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/attach.c | 10 ++-
src/lxc/attach.h | 1 +
src/lxc/conf.c | 48 ++++++++--
src/lxc/lsm/apparmor.c | 76 +++++++++++++---
src/lxc/lsm/lsm.c | 193 +++--------------------------------------
src/lxc/lsm/lsm.h | 20 ++---
src/lxc/lsm/nop.c | 43 +++++++--
src/lxc/lsm/selinux.c | 60 +++++++++++--
src/lxc/start.c | 8 +-
src/lxc/start.h | 2 +
src/lxc/utils.c | 35 --------
src/lxc/utils.h | 1 -
src/tests/attach.c | 18 ++--
13 files changed, 236 insertions(+), 279 deletions(-)
diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 336f54e787..94a80c2bd6 100644
--- a/src/lxc/attach.c
+++ b/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;
diff --git a/src/lxc/attach.h b/src/lxc/attach.h
index 54fa0c73af..bc9a81376c 100644
--- a/src/lxc/attach.h
+++ b/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,
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 4425694e19..2067b03c89 100644
--- a/src/lxc/conf.c
+++ b/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) {
diff --git a/src/lxc/lsm/apparmor.c b/src/lxc/lsm/apparmor.c
index 02f824f975..533c043c02 100644
--- a/src/lxc/lsm/apparmor.c
+++ b/src/lxc/lsm/apparmor.c
@@ -24,7 +24,7 @@
lxc_log_define(apparmor, lsm);
-/* set by lsm_apparmor_drv_init if true */
+/* set by lsm_apparmor_ops_init if true */
static int aa_enabled = 0;
static bool aa_parser_available = false;
static bool aa_supports_unix = false;
@@ -1128,6 +1128,55 @@ static int apparmor_prepare(struct lxc_conf *conf, const char *lxcpath)
return ret;
}
+static int apparmor_keyring_label_set(const char *label)
+{
+ return 0;
+}
+
+static int apparmor_process_label_fd_get(pid_t pid, bool on_exec)
+{
+ int ret = -1;
+ int labelfd;
+ char path[LXC_LSMATTRLEN];
+
+ if (on_exec)
+ TRACE("On-exec not supported with AppArmor");
+
+ 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 AppArmor LSM label file descriptor");
+
+ return labelfd;
+}
+
+static int apparmor_process_label_set_at(int label_fd, const char *label, bool on_exec)
+{
+ int ret = -1;
+ size_t len;
+ __do_free char *command = NULL;
+
+ if (on_exec)
+ log_trace(0, "Changing AppArmor profile on exec not supported");
+
+ len = strlen(label) + strlen("changeprofile ") + 1;
+ command = malloc(len);
+ if (!command)
+ return ret_errno(ENOMEM);
+
+ ret = snprintf(command, len, "changeprofile %s", label);
+ if (ret < 0 || (size_t)ret >= len)
+ return -EFBIG;
+
+ ret = lxc_write_nointr(label_fd, command, len - 1);
+
+ INFO("Set AppArmor label to \"%s\"", label);
+ return 0;
+}
+
/*
* apparmor_process_label_set: Set AppArmor process profile
*
@@ -1169,13 +1218,13 @@ static int apparmor_process_label_set(const char *inlabel, struct lxc_conf *conf
return 0;
}
tid = lxc_raw_gettid();
- label_fd = lsm_process_label_fd_get(tid, on_exec);
+ label_fd = apparmor_process_label_fd_get(tid, on_exec);
if (label_fd < 0) {
SYSERROR("Failed to change AppArmor profile to %s", label);
return -1;
}
- ret = lsm_process_label_set_at(label_fd, label, on_exec);
+ ret = apparmor_process_label_set_at(label_fd, label, on_exec);
close(label_fd);
if (ret < 0) {
ERROR("Failed to change AppArmor profile to %s", label);
@@ -1186,16 +1235,19 @@ static int apparmor_process_label_set(const char *inlabel, struct lxc_conf *conf
return 0;
}
-static struct lsm_drv apparmor_drv = {
- .name = "AppArmor",
- .enabled = apparmor_enabled,
- .process_label_get = apparmor_process_label_get,
- .process_label_set = apparmor_process_label_set,
- .prepare = apparmor_prepare,
- .cleanup = apparmor_cleanup,
+static struct lsm_ops apparmor_ops = {
+ .name = "AppArmor",
+ .cleanup = apparmor_cleanup,
+ .enabled = apparmor_enabled,
+ .keyring_label_set = apparmor_keyring_label_set,
+ .prepare = apparmor_prepare,
+ .process_label_fd_get = apparmor_process_label_fd_get,
+ .process_label_get = apparmor_process_label_get,
+ .process_label_set = apparmor_process_label_set,
+ .process_label_set_at = apparmor_process_label_set_at,
};
-struct lsm_drv *lsm_apparmor_drv_init(void)
+const struct lsm_ops *lsm_apparmor_ops_init(void)
{
bool have_mac_admin = false;
@@ -1225,5 +1277,5 @@ struct lsm_drv *lsm_apparmor_drv_init(void)
out:
aa_enabled = 1;
- return &apparmor_drv;
+ return &apparmor_ops;
}
diff --git a/src/lxc/lsm/lsm.c b/src/lxc/lsm/lsm.c
index cd1f4696c6..93c3aca820 100644
--- a/src/lxc/lsm/lsm.c
+++ b/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;
}
diff --git a/src/lxc/lsm/lsm.h b/src/lxc/lsm/lsm.h
index 8c7b4661c8..c035db29d1 100644
--- a/src/lxc/lsm/lsm.h
+++ b/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 */
diff --git a/src/lxc/lsm/nop.c b/src/lxc/lsm/nop.c
index 5b345b9a25..f5f08425f0 100644
--- a/src/lxc/lsm/nop.c
+++ b/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;
}
diff --git a/src/lxc/lsm/selinux.c b/src/lxc/lsm/selinux.c
index e28731e8fd..8fa8c2e3f1 100644
--- a/src/lxc/lsm/selinux.c
+++ b/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;
}
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 994b1c8cbf..732e8d18f6 100644
--- a/src/lxc/start.c
+++ b/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;
diff --git a/src/lxc/start.h b/src/lxc/start.h
index 69b7362f3c..3c9fa04b58 100644
--- a/src/lxc/start.h
+++ b/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 {
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 9dd34050d2..9971aed165 100644
--- a/src/lxc/utils.c
+++ b/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;
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index dd34f1a2ff..ddd1cbe620 100644
--- a/src/lxc/utils.h
+++ b/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);
diff --git a/src/tests/attach.c b/src/tests/attach.c
index 07e641d56d..79fd790e7e 100644
--- a/src/tests/attach.c
+++ b/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);
From 3bb6ff017b944693d9de53b44b26f67c01cf7fe4 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 11 Aug 2020 10:32:01 +0200
Subject: [PATCH 2/2] lsm: use atomic in ase we're used multi-threaded
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/lsm/apparmor.c | 91 ++++++++++++++++++++++--------------------
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/src/lxc/lsm/apparmor.c b/src/lxc/lsm/apparmor.c
index 533c043c02..c4fce1ee6c 100644
--- a/src/lxc/lsm/apparmor.c
+++ b/src/lxc/lsm/apparmor.c
@@ -4,6 +4,7 @@
#define _GNU_SOURCE 1
#endif
#include <errno.h>
+#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mount.h>
@@ -25,14 +26,13 @@
lxc_log_define(apparmor, lsm);
/* set by lsm_apparmor_ops_init if true */
-static int aa_enabled = 0;
-static bool aa_parser_available = false;
-static bool aa_supports_unix = false;
-static bool aa_can_stack = false;
-static bool aa_is_stacked = false;
-static bool aa_admin = false;
-
-static int mount_features_enabled = 0;
+static atomic_int aa_enabled ;
+static atomic_int aa_parser_available = -1;
+static atomic_int aa_supports_unix;
+static atomic_int aa_can_stack;
+static atomic_int aa_is_stacked;
+static atomic_int aa_admin;
+static atomic_int mount_features_enabled;
#define AA_DEF_PROFILE "lxc-container-default"
#define AA_DEF_PROFILE_CGNS "lxc-container-default-cgns"
@@ -375,7 +375,7 @@ static const char AA_PROFILE_UNPRIVILEGED[] =
static bool check_mount_feature_enabled(void)
{
- return mount_features_enabled == 1;
+ return atomic_load(&mount_features_enabled);
}
static void load_mount_features_enabled(void)
@@ -385,13 +385,13 @@ static void load_mount_features_enabled(void)
ret = stat(AA_MOUNT_RESTR, &statbuf);
if (ret == 0)
- mount_features_enabled = 1;
+ atomic_store(&mount_features_enabled, 1);
}
/* aa_getcon is not working right now. Use our hand-rolled version below */
static int apparmor_enabled(void)
{
- FILE *fin;
+ __do_fclose FILE *fin = NULL;
char e;
int ret;
@@ -399,7 +399,6 @@ static int apparmor_enabled(void)
if (!fin)
return 0;
ret = fscanf(fin, "%c", &e);
- fclose(fin);
if (ret == 1 && e == 'Y') {
load_mount_features_enabled();
return 1;
@@ -545,14 +544,17 @@ static inline char *apparmor_namespace(const char *ctname, const char *lxcpath)
*/
static bool check_apparmor_parser_version()
{
+ int major = 0, minor = 0, micro = 0, ret = 0;
struct lxc_popen_FILE *parserpipe;
int rc;
- int major = 0, minor = 0, micro = 0;
+
+ if (atomic_load(&aa_parser_available) >= 0)
+ return false;
parserpipe = lxc_popen("apparmor_parser --version");
if (!parserpipe) {
fprintf(stderr, "Failed to run check for apparmor_parser\n");
- return false;
+ goto out;
}
rc = fscanf(parserpipe->f, "AppArmor parser version %d.%d.%d", &major, &minor, µ);
@@ -562,24 +564,27 @@ static bool check_apparmor_parser_version()
* lxc_popen executed failed to find the apparmor_parser binary.
* See the TODO comment above for details.
*/
- return false;
+ goto out;
}
rc = lxc_pclose(parserpipe);
if (rc < 0) {
fprintf(stderr, "Error waiting for child process\n");
- return false;
+ goto out;
}
if (rc != 0) {
fprintf(stderr, "'apparmor_parser --version' executed with an error status\n");
- return false;
+ goto out;
}
- aa_supports_unix = (major > 2) ||
- (major == 2 && minor > 10) ||
- (major == 2 && minor == 10 && micro >= 95);
+ if ((major > 2) || (major == 2 && minor > 10) || (major == 2 && minor == 10 && micro >= 95))
+ atomic_store(&aa_supports_unix, 1);
- return true;
+ ret = 1;
+
+out:
+ atomic_store(&aa_parser_available, ret);
+ return ret == 1;
}
static bool file_is_yes(const char *path)
@@ -744,7 +749,7 @@ static char *get_apparmor_profile_content(struct lxc_conf *conf, const char *lxc
append_all_remount_rules(&profile, &size);
- if (aa_supports_unix)
+ if (atomic_load(&aa_supports_unix))
must_append_sized(&profile, &size, AA_PROFILE_UNIX_SOCKETS,
STRARRAYLEN(AA_PROFILE_UNIX_SOCKETS));
@@ -752,7 +757,7 @@ static char *get_apparmor_profile_content(struct lxc_conf *conf, const char *lxc
must_append_sized(&profile, &size, AA_PROFILE_CGROUP_NAMESPACES,
STRARRAYLEN(AA_PROFILE_CGROUP_NAMESPACES));
- if (aa_can_stack && !aa_is_stacked) {
+ if (atomic_load(&aa_can_stack) && !atomic_load(&aa_is_stacked)) {
char *namespace, *temp;
must_append_sized(&profile, &size, AA_PROFILE_STACKING_BASE,
@@ -775,7 +780,7 @@ static char *get_apparmor_profile_content(struct lxc_conf *conf, const char *lxc
must_append_sized(&profile, &size, AA_PROFILE_NESTING_BASE,
STRARRAYLEN(AA_PROFILE_NESTING_BASE));
- if (!aa_can_stack || aa_is_stacked) {
+ if (!atomic_load(&aa_can_stack) || atomic_load(&aa_is_stacked)) {
char *temp;
temp = must_concat(NULL, " change_profile -> \"",
@@ -836,7 +841,7 @@ static bool make_apparmor_namespace(struct lxc_conf *conf, const char *lxcpath)
{
char *path;
- if (!aa_can_stack || aa_is_stacked)
+ if (!atomic_load(&aa_can_stack) || atomic_load(&aa_is_stacked))
return true;
path = make_apparmor_namespace_path(conf->name, lxcpath);
@@ -1021,7 +1026,7 @@ static int load_apparmor_profile(struct lxc_conf *conf, const char *lxcpath)
*/
static void apparmor_cleanup(struct lxc_conf *conf, const char *lxcpath)
{
- if (!aa_admin)
+ if (!atomic_load(&aa_admin))
return;
if (!conf->lsm_aa_profile_created)
@@ -1039,7 +1044,7 @@ static int apparmor_prepare(struct lxc_conf *conf, const char *lxcpath)
const char *label;
char *curlabel = NULL, *genlabel = NULL;
- if (!aa_enabled) {
+ if (!atomic_load(&aa_enabled)) {
ERROR("AppArmor not enabled");
return -1;
}
@@ -1054,7 +1059,7 @@ static int apparmor_prepare(struct lxc_conf *conf, const char *lxcpath)
}
if (label && strcmp(label, AA_GENERATED) == 0) {
- if (!aa_parser_available) {
+ if (!check_apparmor_parser_version()) {
ERROR("Cannot use generated profile: apparmor_parser not available");
goto out;
}
@@ -1071,7 +1076,7 @@ static int apparmor_prepare(struct lxc_conf *conf, const char *lxcpath)
goto out;
}
- if (aa_can_stack && !aa_is_stacked) {
+ if (atomic_load(&aa_can_stack) && !atomic_load(&aa_is_stacked)) {
char *namespace = apparmor_namespace(conf->name, lxcpath);
size_t llen = strlen(genlabel);
must_append_sized(&genlabel, &llen, "//&:", STRARRAYLEN("//&:"));
@@ -1085,7 +1090,7 @@ static int apparmor_prepare(struct lxc_conf *conf, const char *lxcpath)
curlabel = apparmor_process_label_get(lxc_raw_getpid());
- if (!aa_can_stack && aa_needs_transition(curlabel)) {
+ if (!atomic_load(&aa_can_stack) && aa_needs_transition(curlabel)) {
/* we're already confined, and stacking isn't supported */
if (!label || strcmp(curlabel, label) == 0) {
@@ -1196,7 +1201,7 @@ static int apparmor_process_label_set(const char *inlabel, struct lxc_conf *conf
pid_t tid;
const char *label;
- if (!aa_enabled) {
+ if (!atomic_load(&aa_enabled)) {
ERROR("AppArmor not enabled");
return -1;
}
@@ -1250,19 +1255,18 @@ static struct lsm_ops apparmor_ops = {
const struct lsm_ops *lsm_apparmor_ops_init(void)
{
bool have_mac_admin = false;
+ bool can_stack, is_stacked;
if (!apparmor_enabled())
return NULL;
- /* We only support generated profiles when apparmor_parser is usable */
- if (!check_apparmor_parser_version())
- goto out;
-
- aa_parser_available = true;
-
- aa_can_stack = apparmor_can_stack();
- if (aa_can_stack)
- aa_is_stacked = file_is_yes("/sys/kernel/security/apparmor/.ns_stacked");
+ can_stack = apparmor_can_stack();
+ if (can_stack) {
+ atomic_store(&aa_can_stack, 1);
+ is_stacked = file_is_yes("/sys/kernel/security/apparmor/.ns_stacked");
+ if (is_stacked)
+ atomic_store(&aa_is_stacked, 1);
+ }
#if HAVE_LIBCAP
have_mac_admin = lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE);
@@ -1270,12 +1274,11 @@ const struct lsm_ops *lsm_apparmor_ops_init(void)
if (!have_mac_admin)
WARN("Per-container AppArmor profiles are disabled because the mac_admin capability is missing");
- else if (am_host_unpriv() && !aa_is_stacked)
+ else if (am_host_unpriv() && !atomic_load(&aa_is_stacked))
WARN("Per-container AppArmor profiles are disabled because LXC is running in an unprivileged container without stacking");
else
- aa_admin = true;
+ atomic_store(&aa_admin, 1);
-out:
- aa_enabled = 1;
+ atomic_store(&aa_enabled, 1);
return &apparmor_ops;
}
More information about the lxc-devel
mailing list