attach: move to file descriptor-only interactions

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index d8f609b986730594bca70e225d9e6d5776f84c54..4ac4979b6b34aed9ac63830235e7e4d6f0b255d8 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -58,6 +58,7 @@ static lxc_attach_options_t attach_static_default_options = LXC_ATTACH_OPTIONS_D
 
 struct attach_context {
        int init_pid;
+       int dfd_pid;
        char *lsm_label;
        struct lxc_container *container;
        signed long personality;
@@ -115,11 +116,12 @@ static int get_personality(const char *name, const char *lxcpath,
 static int get_attach_context(struct attach_context *ctx,
                              struct lxc_container *container)
 {
+       __do_close int fd_status = -EBADF;
        __do_free char *line = NULL;
-       __do_fclose FILE *proc_file = NULL;
+       __do_fclose FILE *f_status = NULL;
        int ret;
        bool found;
-       char proc_fn[LXC_PROC_STATUS_LEN];
+       char path[LXC_PROC_PID_LEN];
        size_t line_bufsz = 0;
 
        ctx->container = container;
@@ -128,18 +130,26 @@ static int get_attach_context(struct attach_context *ctx,
        if (ctx->init_pid < 0)
                return log_error(-1, "Failed to get init pid");
 
-       /* Read capabilities. */
-       ret = snprintf(proc_fn, LXC_PROC_STATUS_LEN, "/proc/%d/status", ctx->init_pid);
-       if (ret < 0 || ret >= LXC_PROC_STATUS_LEN)
-               return -EIO;
+       ret = snprintf(path, sizeof(path), "/proc/%d", ctx->init_pid);
+       if (ret < 0 || ret >= sizeof(path))
+               return ret_errno(EIO);
 
-       proc_file = fopen(proc_fn, "re");
-       if (!proc_file)
-               return log_error_errno(-errno, errno, "Failed to open %s", proc_fn);
+       ctx->dfd_pid = openat(-EBADF, path, O_CLOEXEC | O_NOCTTY | O_NOFOLLOW | O_PATH | O_DIRECTORY);
+       if (ctx->dfd_pid < 0)
+               return -errno;
+
+       fd_status = openat(ctx->dfd_pid, "status", O_CLOEXEC | O_NOCTTY | O_NOFOLLOW | O_RDONLY);
+       if (fd_status < 0)
+               return -errno;
+
+       f_status = fdopen(fd_status, "re");
+       if (!f_status)
+               return log_error_errno(-errno, errno, "Failed to open file descriptor %d", fd_status);
+       move_fd(fd_status);
 
        found = false;
 
-       while (getline(&line, &line_bufsz, proc_file) != -1) {
+       while (getline(&line, &line_bufsz, f_status) != -1) {
                ret = sscanf(line, "CapBnd: %llx", &ctx->capability_mask);
                if (ret != EOF && ret == 1) {
                        found = true;
@@ -148,7 +158,7 @@ static int get_attach_context(struct attach_context *ctx,
        }
 
        if (!found)
-               return log_error_errno(-ENOENT, ENOENT, "Failed to read capability bounding set from %s", proc_fn);
+               return log_error_errno(-ENOENT, ENOENT, "Failed to read capability bounding set from %s/status", path);
 
        ctx->lsm_ops = lsm_init_static();
 
@@ -266,18 +276,21 @@ static inline void close_nsfds(struct attach_context *ctx)
 
 static void put_attach_context(struct attach_context *ctx)
 {
-       free_disarm(ctx->lsm_label);
+       if (ctx) {
+               free_disarm(ctx->lsm_label);
+               close_prot_errno_disarm(ctx->dfd_pid);
 
-       if (ctx->container) {
-               lxc_container_put(ctx->container);
-               ctx->container = NULL;
-       }
+               if (ctx->container) {
+                       lxc_container_put(ctx->container);
+                       ctx->container = NULL;
+               }
 
-       close_nsfds(ctx);
-       free(ctx);
+               close_nsfds(ctx);
+               free(ctx);
+       }
 }
 
-static int attach_nsfds(struct attach_context *ctx)
+static int attach_context_container(struct attach_context *ctx)
 {
        for (int i = 0; i < LXC_NS_MAX; i++) {
                int ret;
@@ -298,6 +311,21 @@ static int attach_nsfds(struct attach_context *ctx)
        return 0;
 }
 
+/*
+ * Place anything in here that needs to be get rid of before we move into the
+ * container's context and fail hard if we can't.
+ */
+static bool attach_context_security_barrier(struct attach_context *ctx)
+{
+       if (ctx) {
+               if (close(ctx->dfd_pid))
+                       return false;
+               ctx->dfd_pid = -EBADF;
+       }
+
+       return true;
+}
+
 int lxc_attach_remount_sys_proc(void)
 {
        int ret;
@@ -1201,14 +1229,27 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
                        _exit(EXIT_FAILURE);
                }
 
+               if (!attach_context_security_barrier(ctx)) {
+                       shutdown(ipc_sockets[1], SHUT_RDWR);
+                       put_attach_context(ctx);
+                       _exit(EXIT_FAILURE);
+               }
+
                TRACE("Intermediate process starting to initialize");
 
                cwd = getcwd(NULL, 0);
 
-               /* Attach now, create another subprocess later, since pid namespaces
-                * only really affect the children of the current process.
+               /*
+                * Attach now, create another subprocess later, since pid
+                * namespaces only really affect the children of the current
+                * process.
+                *
+                * Note that this is a crucial barrier. We're no moving into
+                * the container's context so we need to make sure to not leak
+                * anything sensitive. That especially means things such as
+                * open file descriptors!
                 */
-               ret = attach_nsfds(ctx);
+               ret = attach_context_container(ctx);
                if (ret < 0) {
                        ERROR("Failed to enter namespaces");
                        shutdown(ipc_sockets[1], SHUT_RDWR);

diff --git a/src/lxc/macro.h b/src/lxc/macro.h
index 092782aab8e3d256a26da4c1658edf9727338af4..11da99118eddf115466fa7dc1874397bd2daf0a3 100644 (file)
--- a/src/lxc/macro.h
+++ b/src/lxc/macro.h
@@ -283,6 +283,19 @@
 #define LXC_MAX_BUFFER 4096
 #define LXC_NAMESPACE_NAME_MAX 256
 
+/* /proc/       =    6
+ *                +
+ * <pid-as-str> =   INTTYPE_TO_STRLEN(pid_t)
+ *                +
+ * /fd/         =    4
+ *                +
+ * <fd-as-str>  =   INTTYPE_TO_STRLEN(int)
+ *                +
+ * \0           =    1
+ */
+#define LXC_PROC_PID_LEN \
+       (6 + INTTYPE_TO_STRLEN(pid_t) + 1)
+
 /* /proc/       =    6
  *                +
  * <pid-as-str> =   INTTYPE_TO_STRLEN(pid_t)