terminal: harden terminal allocation

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

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 654a7d8fd0ffbc27c9441ceecd52c0d2cb8e7051..336f54e787c5558de935665788b64b9859053665 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -876,7 +876,7 @@ static int attach_child_main(struct attach_clone_payload *payload)
        /* Make sure that the processes STDIO is correctly owned by the user that we are switching to */
        ret = fix_stdio_permissions(new_uid);
        if (ret)
-               WARN("Failed to ajust stdio permissions");
+               WARN("Failed to adjust stdio permissions");
 
        if (!lxc_switch_uid_gid(new_uid, new_gid))
                goto on_error;
@@ -896,23 +896,11 @@ static int lxc_attach_terminal(const char *name, const char *lxcpath, struct lxc
 
        lxc_terminal_init(terminal);
 
-       ret = lxc_terminal_create(name, lxcpath, terminal);
+       ret = lxc_terminal_create(name, lxcpath, conf, terminal);
        if (ret < 0)
                return log_error(-1, "Failed to create terminal");
 
-       /* Shift ttys to container. */
-       ret = lxc_terminal_map_ids(conf, terminal);
-       if (ret < 0) {
-               ERROR("Failed to chown terminal");
-               goto on_error;
-       }
-
        return 0;
-
-on_error:
-       lxc_terminal_delete(terminal);
-       lxc_terminal_conf_free(terminal);
-       return -1;
 }
 
 static int lxc_attach_terminal_mainloop_init(struct lxc_terminal *terminal,

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 0a1bb6d75af96df0dc4170939424c685c122c349..4425694e192c5e5e93f1d14da9474b87b5af6038 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -4559,7 +4559,7 @@ int userns_exec_mapped_root(const char *path, int path_fd,
 
                ret = fchown(target_fd, 0, st.st_gid);
                if (ret) {
-                       SYSERROR("Failed to chown %d(%s) to -1:%d", target_fd, path, st.st_gid);
+                       SYSERROR("Failed to chown %d(%s) to 0:%d", target_fd, path, st.st_gid);
                        _exit(EXIT_FAILURE);
                }
 

diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c
index 75bbc2eb25badd7e03c4ca9a94342751264e17ec..4a8c7a8d9903480e34d37a099ec87395b94ca17e 100644 (file)
--- a/src/lxc/file_utils.c
+++ b/src/lxc/file_utils.c
@@ -18,6 +18,7 @@
 #include "macro.h"
 #include "memory_utils.h"
 #include "string_utils.h"
+#include "syscall_wrappers.h"
 #include "utils.h"
 
 int lxc_open_dirfd(const char *dir)
@@ -558,3 +559,21 @@ bool exists_file_at(int dir_fd, const char *path)
 
        return fstatat(dir_fd, path, &sb, 0) == 0;
 }
+
+int open_beneath(int dir_fd, const char *path, unsigned int flags)
+{
+       __do_close int fd = -EBADF;
+       struct lxc_open_how how = {
+               .flags          = flags,
+               .resolve        = RESOLVE_NO_XDEV | RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS | RESOLVE_BENEATH,
+       };
+
+       fd = openat2(dir_fd, path, &how, sizeof(how));
+       if (fd >= 0)
+               return move_fd(fd);
+
+       if (errno != ENOSYS)
+               return -errno;
+
+       return openat(dir_fd, path, O_NOFOLLOW | flags);
+}

diff --git a/src/lxc/file_utils.h b/src/lxc/file_utils.h
index 2455feb7fc54a0f5b44d8e89b279f4c9197687f4..df3a00d4dc2d9c73e772704373e7d5268c5ed354 100644 (file)
--- a/src/lxc/file_utils.h
+++ b/src/lxc/file_utils.h
@@ -75,5 +75,6 @@ __hidden extern FILE *fopen_cached(const char *path, const char *mode, void **ca
 __hidden extern int timens_offset_write(clockid_t clk_id, int64_t s_offset, int64_t ns_offset);
 __hidden extern bool exists_dir_at(int dir_fd, const char *path);
 __hidden extern bool exists_file_at(int dir_fd, const char *path);
+__hidden extern int open_beneath(int dir_fd, const char *path, unsigned int flags);
 
 #endif /* __LXC_FILE_UTILS_H */

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 49de132f9fc57565eb816a92ea546476fe1412c9..994b1c8cbfb26031ae7e88fe3a68506eb9aaf4d6 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -815,13 +815,6 @@ int lxc_init(const char *name, struct lxc_handler *handler)
        }
        TRACE("Created console");
 
-       ret = lxc_terminal_map_ids(conf, &conf->console);
-       if (ret < 0) {
-               ERROR("Failed to chown console");
-               goto out_delete_terminal;
-       }
-       TRACE("Chowned console");
-
        handler->cgroup_ops = cgroup_init(handler->conf);
        if (!handler->cgroup_ops) {
                ERROR("Failed to initialize cgroup driver");

diff --git a/src/lxc/terminal.c b/src/lxc/terminal.c
index aeb243279589f12728798e6c527c672649ba8b7b..2a1925baff934036c136955daed8f2f76a489860 100644 (file)
--- a/src/lxc/terminal.c
+++ b/src/lxc/terminal.c
@@ -828,7 +828,31 @@ int lxc_terminal_create_log_file(struct lxc_terminal *terminal)
        return 0;
 }
 
-static int lxc_terminal_create_foreign(struct lxc_terminal *terminal)
+static int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal)
+{
+       int ret;
+
+       if (lxc_list_empty(&c->id_map))
+               return 0;
+
+       if (is_empty_string(terminal->name) && terminal->pty < 0)
+               return 0;
+
+       if (terminal->pty >= 0)
+               ret = userns_exec_mapped_root(NULL, terminal->pty, c);
+       else
+               ret = userns_exec_mapped_root(terminal->name, -EBADF, c);
+       if (ret < 0)
+               return log_error(-1, "Failed to chown terminal %d(%s)", terminal->pty,
+                                !is_empty_string(terminal->name) ? terminal->name : "(null)");
+
+       TRACE("Chowned terminal %d(%s)", terminal->pty,
+             !is_empty_string(terminal->name) ? terminal->name : "(null)");
+
+       return 0;
+}
+
+static int lxc_terminal_create_foreign(struct lxc_conf *conf, struct lxc_terminal *terminal)
 {
        int ret;
 
@@ -838,6 +862,12 @@ static int lxc_terminal_create_foreign(struct lxc_terminal *terminal)
                return -1;
        }
 
+       ret = lxc_terminal_map_ids(conf, terminal);
+       if (ret < 0) {
+               SYSERROR("Failed to change ownership of terminal multiplexer device");
+               goto err;
+       }
+
        ret = ttyname_r(terminal->pty, terminal->name, sizeof(terminal->name));
        if (ret < 0) {
                SYSERROR("Failed to retrieve name of terminal pty");
@@ -869,38 +899,40 @@ err:
        return -ENODEV;
 }
 
-static int lxc_terminal_create_native(const char *name, const char *lxcpath,
+static int lxc_terminal_create_native(const char *name, const char *lxcpath, struct lxc_conf *conf,
                                      struct lxc_terminal *terminal)
 {
-       __do_close int devpts_fd = -EBADF, ptx_fd = -EBADF, pty_fd = -EBADF;
+       __do_close int devpts_fd = -EBADF;
        int ret;
 
        devpts_fd = lxc_cmd_get_devpts_fd(name, lxcpath);
        if (devpts_fd < 0)
                return log_error_errno(-1, errno, "Failed to receive devpts fd");
 
-       ptx_fd = openat(devpts_fd, "ptmx", O_RDWR | O_NOCTTY | O_CLOEXEC);
-       if (ptx_fd < 0)
+       terminal->ptx = open_beneath(devpts_fd, "ptmx", O_RDWR | O_NOCTTY | O_CLOEXEC);
+       if (terminal->ptx < 0)
                return log_error_errno(-1, errno, "Failed to open terminal multiplexer device");
 
-       ret = grantpt(ptx_fd);
-       if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to grant access to multiplexer device");
+       ret = unlockpt(terminal->ptx);
+       if (ret < 0) {
+               SYSERROR("Failed to unlock multiplexer device device");
+               goto err;
+       }
 
-       ret = unlockpt(ptx_fd);
-       if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to unlock multiplexer device device");
+       terminal->pty = ioctl(terminal->ptx, TIOCGPTPEER, O_RDWR | O_NOCTTY | O_CLOEXEC);
+       if (terminal->pty < 0) {
+               SYSERROR("Failed to allocate new pty device");
+               goto err;
+       }
 
-       pty_fd = ioctl(ptx_fd, TIOCGPTPEER, O_RDWR | O_NOCTTY | O_CLOEXEC);
-       if (pty_fd < 0)
-               return log_error_errno(-1, errno, "Failed to allocate new pty device");
+       // ret = lxc_terminal_map_ids(conf, terminal);
 
        ret = ttyname_r(terminal->pty, terminal->name, sizeof(terminal->name));
-       if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to retrieve name of terminal pty");
+       if (ret < 0) {
+               SYSERROR("Failed to retrieve name of terminal pty");
+               goto err;
+       }
 
-       terminal->ptx = move_fd(ptx_fd);
-       terminal->pty = move_fd(pty_fd);
        ret = lxc_terminal_peer_default(terminal);
        if (ret < 0) {
                ERROR("Failed to allocate proxy terminal");
@@ -914,12 +946,13 @@ err:
        return -ENODEV;
 }
 
-int lxc_terminal_create(const char *name, const char *lxcpath, struct lxc_terminal *terminal)
+int lxc_terminal_create(const char *name, const char *lxcpath, struct lxc_conf *conf,
+                       struct lxc_terminal *terminal)
 {
-       if (!lxc_terminal_create_native(name, lxcpath, terminal))
+       if (!lxc_terminal_create_native(name, lxcpath, conf, terminal))
                return 0;
 
-       return lxc_terminal_create_foreign(terminal);
+       return lxc_terminal_create_foreign(conf, terminal);
 }
 
 int lxc_terminal_setup(struct lxc_conf *conf)
@@ -932,7 +965,7 @@ int lxc_terminal_setup(struct lxc_conf *conf)
                return 0;
        }
 
-       ret = lxc_terminal_create_foreign(terminal);
+       ret = lxc_terminal_create_foreign(conf, terminal);
        if (ret < 0)
                return -1;
 
@@ -1209,24 +1242,3 @@ void lxc_terminal_conf_free(struct lxc_terminal *terminal)
                lxc_ringbuf_release(&terminal->ringbuf);
        lxc_terminal_signal_fini(terminal);
 }
-
-int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal)
-{
-       int ret;
-
-       if (lxc_list_empty(&c->id_map))
-               return 0;
-
-       if (strcmp(terminal->name, "") == 0)
-               return 0;
-
-       ret = userns_exec_mapped_root(terminal->name, terminal->pty, c);
-       if (ret < 0) {
-               return log_error(-1, "Failed to chown terminal %d(%s)",
-                                terminal->pty, terminal->name);
-       }
-
-       TRACE("Chowned terminal %d(%s)", terminal->pty, terminal->name);
-
-       return 0;
-}

diff --git a/src/lxc/terminal.h b/src/lxc/terminal.h
index 838993d37fd5591be9a182f384efe3a46d719898..7d402a264ae3e6e0fb2268517e53788553d1191b 100644 (file)
--- a/src/lxc/terminal.h
+++ b/src/lxc/terminal.h
@@ -111,6 +111,7 @@ __hidden extern int  lxc_terminal_allocate(struct lxc_conf *conf, int sockfd, in
  *   (Handlers for SIGWINCH and I/O are not registered in a mainloop.)
  */
 __hidden extern int lxc_terminal_create(const char *name, const char *lxcpath,
+                                       struct lxc_conf *conf,
                                        struct lxc_terminal *console);
 
 /**
@@ -250,6 +251,5 @@ __hidden extern int lxc_terminal_prepare_login(int fd);
 __hidden extern void lxc_terminal_conf_free(struct lxc_terminal *terminal);
 __hidden extern void lxc_terminal_info_init(struct lxc_terminal_info *terminal);
 __hidden extern void lxc_terminal_init(struct lxc_terminal *terminal);
-__hidden extern int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal);
 
 #endif /* __LXC_TERMINAL_H */