The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2046
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Prior versions of attach used to do quite some IPC to do secure LSM
confinement. Especially since we merged the following commit to fix
CVE-2016-8649:
commit 81f466d05f2a89cb4f122ef7f593ff3f279b165c
Author: Christian Brauner <christian.brau...@canonical.com>
Date: Tue Nov 8 19:21:19 2016 +0100
attach: do not send procfd to attached process
So far, we opened a file descriptor refering to proc on the host inside the
host namespace and handed that fd to the attached process in
attach_child_main(). This was done to ensure that LSM labels were correctly
setup. However, by exploiting a potential kernel bug, ptrace could be used to
prevent the file descriptor from being closed which in turn could be used by an
unprivileged container to gain access to the host namespace. Aside from this
needing an upstream kernel fix, we should make sure that we don't pass the fd
for proc itself to the attached process. However, we cannot completely prevent
this, as the attached process needs to be able to change its apparmor profile
by writing to /proc/self/attr/exec or /proc/self/attr/current. To minimize the
attack surface, we only send the fd for /proc/self/attr/exec or
/proc/self/attr/current to the attached process. To do this we introduce a
little more IPC between the child and parent:
* IPC mechanism: (X is receiver)
* initial process intermediate attached
* X <--- send pid of
* attached proc,
* then exit
* send 0 ------------------------------------> X
* [do initialization]
* X <------------------------------------ send 1
* [add to cgroup, ...]
* send 2 ------------------------------------> X
* [set LXC_ATTACH_NO_NEW_PRIVS]
* X <------------------------------------ send 3
* [open LSM label fd]
* send 4 ------------------------------------> X
* [set LSM label]
* close socket close socket
* run program
The attached child tells the parent when it is ready to have its LSM labels set
up. The parent then opens an approriate fd for the child PID to
/proc/<pid>/attr/exec or /proc/<pid>/attr/current and sends it via SCM_RIGHTS
to the child. The child can then set its LSM laben. Both sides then close the
I actually think this is not needed if we just defer to our lsm_init() and
lsm_process_label_set() routines. This has several advantages:
- A dirfd for /proc isn't even needed in the parent anymore.
- When /proc is mounted hidepid=2 we can still attach and do proper LSM
confinement.
Closes #2045.
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 5910ab2b5a151812047e69ecf34ed9adbf10a6c2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Sun, 17 Dec 2017 14:27:21 +0100
Subject: [PATCH] attach: rework LSM handling
Prior versions of attach used to do quite some IPC to do secure LSM
confinement. Especially since we merged the following commit to fix
CVE-2016-8649:
commit 81f466d05f2a89cb4f122ef7f593ff3f279b165c
Author: Christian Brauner <christian.brau...@canonical.com>
Date: Tue Nov 8 19:21:19 2016 +0100
attach: do not send procfd to attached process
So far, we opened a file descriptor refering to proc on the host inside the
host namespace and handed that fd to the attached process in
attach_child_main(). This was done to ensure that LSM labels were correctly
setup. However, by exploiting a potential kernel bug, ptrace could be used
to
prevent the file descriptor from being closed which in turn could be used
by an
unprivileged container to gain access to the host namespace. Aside from this
needing an upstream kernel fix, we should make sure that we don't pass the
fd
for proc itself to the attached process. However, we cannot completely
prevent
this, as the attached process needs to be able to change its apparmor
profile
by writing to /proc/self/attr/exec or /proc/self/attr/current. To minimize
the
attack surface, we only send the fd for /proc/self/attr/exec or
/proc/self/attr/current to the attached process. To do this we introduce a
little more IPC between the child and parent:
* IPC mechanism: (X is receiver)
* initial process intermediate attached
* X <--- send pid of
* attached proc,
* then exit
* send 0 ------------------------------------> X
* [do initialization]
* X <------------------------------------ send 1
* [add to cgroup, ...]
* send 2 ------------------------------------> X
* [set
LXC_ATTACH_NO_NEW_PRIVS]
* X <------------------------------------ send 3
* [open LSM label fd]
* send 4 ------------------------------------> X
* [set LSM label]
* close socket close socket
* run program
The attached child tells the parent when it is ready to have its LSM labels
set
up. The parent then opens an approriate fd for the child PID to
/proc/<pid>/attr/exec or /proc/<pid>/attr/current and sends it via
SCM_RIGHTS
to the child. The child can then set its LSM laben. Both sides then close
the
I actually think this is not needed if we just defer to our lsm_init() and
lsm_process_label_set() routines. This has several advantages:
- A dirfd for /proc isn't even needed in the parent anymore.
- When /proc is mounted hidepid=2 we can still attach and do proper LSM
confinement.
Closes #2045.
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
src/lxc/attach.c | 212 ++++++++-----------------------------------------------
1 file changed, 28 insertions(+), 184 deletions(-)
diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index d22716199..5e6e5bbbd 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -32,6 +32,7 @@
#include <string.h>
#include <unistd.h>
#include <linux/unistd.h>
+#include <sys/apparmor.h>
#include <sys/mount.h>
#include <sys/param.h>
#include <sys/prctl.h>
@@ -86,103 +87,6 @@
lxc_log_define(lxc_attach, lxc);
-/* /proc/pid-to-str/current\0 = (5 + 21 + 7 + 1) */
-#define __LSMATTRLEN (5 + (LXC_NUMSTRLEN64) + 7 + 1)
-static int lsm_openat(int procfd, pid_t pid, int on_exec)
-{
- int ret = -1;
- int labelfd = -1;
- const char *name;
- char path[__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, __LSMATTRLEN, "%d/attr/exec", pid);
- else
- ret = snprintf(path, __LSMATTRLEN, "%d/attr/current", pid);
- if (ret < 0 || ret >= __LSMATTRLEN)
- return -1;
-
- labelfd = openat(procfd, path, O_RDWR);
- if (labelfd < 0) {
- SYSERROR("Unable to open file descriptor to set LSM label.");
- return -1;
- }
-
- return labelfd;
-}
-
-static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
-{
- int fret = -1;
- const char *name;
- char *command = NULL;
-
- 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 (strcmp(name, "AppArmor") == 0) {
- int size;
-
- command =
- malloc(strlen(lsm_label) + strlen("changeprofile ") + 1);
- if (!command) {
- SYSERROR("Failed to write apparmor profile.");
- goto out;
- }
-
- size = sprintf(command, "changeprofile %s", lsm_label);
- if (size < 0) {
- SYSERROR("Failed to write apparmor profile.");
- goto out;
- }
-
- if (write(lsm_labelfd, command, size + 1) < 0) {
- SYSERROR("Unable to set LSM label: %s.", command);
- goto out;
- }
- INFO("Set LSM label to: %s.", command);
- } else if (strcmp(name, "SELinux") == 0) {
- if (write(lsm_labelfd, lsm_label, strlen(lsm_label) + 1) < 0) {
- SYSERROR("Unable to set LSM label: %s.", lsm_label);
- goto out;
- }
- INFO("Set LSM label to: %s.", lsm_label);
- } else {
- ERROR("Unable to restore label for unknown LSM: %s.", name);
- goto out;
- }
- fret = 0;
-
-out:
- free(command);
-
- if (lsm_labelfd != -1)
- close(lsm_labelfd);
-
- return fret;
-}
-
/* /proc/pid-to-str/status\0 = (5 + 21 + 7 + 1) */
#define __PROC_STATUS_LEN (5 + (LXC_NUMSTRLEN64) + 7 + 1)
static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
@@ -958,23 +862,23 @@ int lxc_attach(const char *name, const char *lxcpath,
* closed and cannot assume that the child exiting will automatically do
* that.
*
- * IPC mechanism: (X is receiver)
- * initial process intermediate attached
- * X <--- send pid of
- * attached proc,
- * then exit
- * send 0 ------------------------------------> X
- * [do initialization]
- * X <------------------------------------ send 1
+ * IPC mechanism:
+ *
+ * initial process intermediate process attached process
+ *
+ * <------ send pid of attached
+ * process
+ * exit(intermediate process)
+ *
+ * send 0 ------------------------------------------>
+ * [initialize]
+ *
+ * <------------------------------------------ send 1
* [add to cgroup, ...]
- * send 2 ------------------------------------> X
- * [set
LXC_ATTACH_NO_NEW_PRIVS]
- * X <------------------------------------ send 3
- * [open LSM label fd]
- * send 4 ------------------------------------> X
- * [set LSM label]
- * close socket close socket
- * run program
+ *
+ * send 2 ------------------------------------------>
+ * [close socket] [close socket]
+ * [run program]
*/
ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
if (ret < 0) {
@@ -1003,7 +907,6 @@ int lxc_attach(const char *name, const char *lxcpath,
}
if (pid) {
- int procfd = -1;
pid_t to_cleanup_pid = pid;
/* close file namespace descriptors */
@@ -1026,15 +929,6 @@ int lxc_attach(const char *name, const char *lxcpath,
if
(setup_resource_limits(&init_ctx->container->lxc_conf->limits, pid) < 0)
goto on_error;
- /* Open /proc before setns() to the containers namespace so we
- * don't rely on any information from inside the container.
- */
- procfd = open("/proc", O_DIRECTORY | O_RDONLY | O_CLOEXEC);
- if (procfd < 0) {
- SYSERROR("Unable to open /proc.");
- goto on_error;
- }
-
/* Let the child process know to go ahead. */
status = 0;
ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status));
@@ -1093,43 +987,6 @@ int lxc_attach(const char *name, const char *lxcpath,
goto on_error;
}
- /* Wait for the (grand)child to tell us that it's ready to set
- * up its LSM labels.
- */
- expected = 3;
- ret = lxc_read_nointr_expect(ipc_sockets[0], &status,
- sizeof(status), &expected);
- if (ret <= 0) {
- ERROR("Expected to receive sequence number 3: %s.",
- strerror(errno));
- goto on_error;
- }
-
- /* Open LSM fd and send it to child. */
- if ((options->namespaces & CLONE_NEWNS) &&
- (options->attach_flags & LXC_ATTACH_LSM) &&
- init_ctx->lsm_label) {
- int on_exec, saved_errno;
- int labelfd = -1;
-
- on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ?
1 : 0;
- /* Open fd for the LSM security module. */
- labelfd = lsm_openat(procfd, attached_pid, on_exec);
- if (labelfd < 0)
- goto on_error;
-
- /* Send child fd of the LSM security module to write
to. */
- ret = lxc_abstract_unix_send_fds(ipc_sockets[0],
&labelfd, 1, NULL, 0);
- saved_errno = errno;
- close(labelfd);
- if (ret <= 0) {
- ERROR("Intended to send file descriptor %d:
%s.", labelfd, strerror(saved_errno));
- goto on_error;
- }
- }
-
- if (procfd >= 0)
- close(procfd);
/* Now shut down communication with child, we're done. */
shutdown(ipc_sockets[0], SHUT_RDWR);
close(ipc_sockets[0]);
@@ -1147,8 +1004,6 @@ int lxc_attach(const char *name, const char *lxcpath,
/* First shut down the socket, then wait for the pid, otherwise
* the pid we're waiting for may never exit.
*/
- if (procfd >= 0)
- close(procfd);
shutdown(ipc_sockets[0], SHUT_RDWR);
close(ipc_sockets[0]);
if (to_cleanup_pid)
@@ -1246,7 +1101,7 @@ int lxc_attach(const char *name, const char *lxcpath,
static int attach_child_main(void* data)
{
- int expected, fd, lsm_labelfd, ret, status;
+ int expected, fd, ret, status;
long flags;
#if HAVE_SYS_PERSONALITY_H
long new_personality;
@@ -1401,35 +1256,24 @@ static int attach_child_main(void* data)
"gainable privileges.");
}
- /* Tell the (grand)parent to send us LSM label fd. */
- status = 3;
- ret = lxc_write_nointr(ipc_socket, &status, sizeof(status));
- if (ret <= 0) {
- ERROR("Intended to send sequence number 3: %s.",
strerror(errno));
- shutdown(ipc_socket, SHUT_RDWR);
- rexit(-1);
- }
-
if ((options->namespaces & CLONE_NEWNS) &&
(options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
int on_exec;
- /* Receive fd for LSM security module. */
- ret = lxc_abstract_unix_recv_fds(ipc_socket, &lsm_labelfd, 1,
NULL, 0);
- if (ret <= 0) {
- ERROR("Expected to receive file descriptor: %s.",
strerror(errno));
- shutdown(ipc_socket, SHUT_RDWR);
- rexit(-1);
- }
- /* Change into our new LSM profile. */
+ lsm_init();
+
on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
- if (lsm_set_label_at(lsm_labelfd, on_exec, init_ctx->lsm_label)
< 0) {
- SYSERROR("Failed to set LSM label.");
+ ret = lsm_process_label_set(init_ctx->lsm_label,
+ init_ctx->container->lxc_conf, 1,
+ on_exec);
+ if (ret < 0) {
+ SYSERROR("Failed to set LSM label to \"%s\"",
+ init_ctx->lsm_label);
shutdown(ipc_socket, SHUT_RDWR);
- close(lsm_labelfd);
rexit(-1);
}
- close(lsm_labelfd);
+ TRACE("Successfully changed LSM label to \"%s\"",
+ init_ctx->lsm_label);
}
if (init_ctx->container && init_ctx->container->lxc_conf &&
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
