terminal: harden terminal allocation

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,

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);
 		}
 

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);
+}

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 */

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");

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;
-}

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 */