From 8ea93a0fa7b6a26894af61a56251e0698d1c3f54 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 10 Aug 2020 11:13:53 +0200 Subject: [PATCH] terminal: harden terminal allocation Signed-off-by: Christian Brauner --- src/lxc/attach.c | 16 +------- src/lxc/conf.c | 2 +- src/lxc/file_utils.c | 19 +++++++++ src/lxc/file_utils.h | 1 + src/lxc/start.c | 7 ---- src/lxc/terminal.c | 98 +++++++++++++++++++++++++------------------- src/lxc/terminal.h | 2 +- 7 files changed, 79 insertions(+), 66 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 654a7d8fd..336f54e78 100644 --- 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 0a1bb6d75..4425694e1 100644 --- 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 75bbc2eb2..4a8c7a8d9 100644 --- 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 2455feb7f..df3a00d4d 100644 --- 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 49de132f9..994b1c8cb 100644 --- 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 aeb243279..2a1925baf 100644 --- 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 838993d37..7d402a264 100644 --- 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 */ -- 2.47.2