[lxc-devel] [lxc/master] terminal: security fixes

brauner on Github lxc-bot at linuxcontainers.org
Sat Jun 30 09:21:55 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180630/7e6cf9a3/attachment.bin>
-------------- next part --------------
From 23e70c1e33e701bccbef40415ee368fe82456338 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 30 Jun 2018 11:10:12 +0200
Subject: [PATCH 1/2] conf: simplify lxc_setup_dev_console()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/conf.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index ebf32eac1..bb483522d 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -1238,7 +1238,6 @@ struct lxc_device_node {
 };
 
 static const struct lxc_device_node lxc_devices[] = {
-	{ "console", S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 5 },
 	{ "full",    S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 7 },
 	{ "null",    S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 3 },
 	{ "random",  S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 8 },
@@ -1648,7 +1647,7 @@ static int setup_personality(int persona)
 static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 				 const struct lxc_terminal *console)
 {
-	int fd, ret;
+	int ret;
 	char path[MAXPATHLEN];
 	char *rootfs_path = rootfs->path ? rootfs->mount : "";
 
@@ -1675,17 +1674,15 @@ static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 	/* For unprivileged containers autodev or automounts will already have
 	 * taken care of creating /dev/console.
 	 */
-	fd = open(path, O_CREAT | O_EXCL, S_IXUSR | S_IXGRP | S_IXOTH);
-	if (fd < 0) {
+	ret = mknod(path, S_IFREG | 0000, 0);
+	if (ret < 0) {
 		if (errno != EEXIST) {
 			SYSERROR("Failed to create console");
 			return -errno;
 		}
-	} else {
-		close(fd);
 	}
 
-	ret = chmod(console->name, S_IXUSR | S_IXGRP | S_IXOTH);
+	ret = fchmod(console->slave, S_IXUSR | S_IXGRP | S_IXOTH);
 	if (ret < 0) {
 		SYSERROR("Failed to set mode \"0%o\" to \"%s\"",
 			 S_IXUSR | S_IXGRP | S_IXOTH, console->name);

From 56517cfd8b1fcd291a44679a8d420c614b1bd177 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 30 Jun 2018 11:15:36 +0200
Subject: [PATCH 2/2] terminal: safely retrieve path of slave device

openpty() is a horrible function that uses strcpy() into the char *name
argument if name != NULL. We can't rely on the path being sane in all cases so
let's split out the name retrieval to ttyname_r().

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/terminal.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/lxc/terminal.c b/src/lxc/terminal.c
index 614c07a13..04c7dabad 100644
--- a/src/lxc/terminal.c
+++ b/src/lxc/terminal.c
@@ -570,13 +570,20 @@ static int lxc_terminal_peer_proxy_alloc(struct lxc_terminal *terminal,
 	/* This is the proxy terminal that will be given to the client, and
 	 * that the real terminal master will send to / recv from.
 	 */
-	ret = openpty(&terminal->proxy.master, &terminal->proxy.slave,
-		      terminal->proxy.name, NULL, NULL);
+	ret = openpty(&terminal->proxy.master, &terminal->proxy.slave, NULL,
+		      NULL, NULL);
 	if (ret < 0) {
 		SYSERROR("Failed to open proxy terminal");
 		return -1;
 	}
 
+	ret = ttyname_r(terminal->proxy.slave, terminal->proxy.name,
+			sizeof(terminal->proxy.name));
+	if (ret < 0) {
+		SYSERROR("Failed to retrieve name of proxy terminal slave");
+		return -1;
+	}
+
 	ret = lxc_setup_tios(terminal->proxy.slave, &oldtermio);
 	if (ret < 0)
 		goto on_error;
@@ -862,12 +869,18 @@ int lxc_terminal_create(struct lxc_terminal *terminal)
 {
 	int ret;
 
-	ret = openpty(&terminal->master, &terminal->slave, terminal->name, NULL, NULL);
+	ret = openpty(&terminal->master, &terminal->slave, NULL, NULL, NULL);
 	if (ret < 0) {
 		SYSERROR("Failed to open terminal");
 		return -1;
 	}
 
+	ret = ttyname_r(terminal->slave, terminal->name, sizeof(terminal->name));
+	if (ret < 0) {
+		SYSERROR("Failed to retrieve name of terminal slave");
+		return -1;
+	}
+
 	ret = fcntl(terminal->master, F_SETFD, FD_CLOEXEC);
 	if (ret < 0) {
 		SYSERROR("Failed to set FD_CLOEXEC flag on terminal master");


More information about the lxc-devel mailing list