The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2996
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
lxc-ls without root privileges on privileged containers should not display
information. In lxc_container_new(), ongoing_create()'s result is not checked
for all possible returned values. Hence, an unprivileged user can send command
messages to the container's monitor. For example:
$ lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr - 0 - - - false
$ sudo lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr RUNNING 0 - 10.0.3.51 - false
After this change:
$ lxc-ls -P /.../tests -f <-------- No more display without root privileges
$ sudo lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr RUNNING 0 - 10.0.3.37 - false
$
Signed-off-by: Rachid Koucha <rachid.kou...@gmail.com>
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 9fbe07f68da62c90ff849eb1e2d59396d2a9672f Mon Sep 17 00:00:00 2001
From: Rachid Koucha <47061324+rachid-kou...@users.noreply.github.com>
Date: Fri, 10 May 2019 18:56:12 +0200
Subject: [PATCH] lxccontainer: do not display if missing privileges
lxc-ls without root privileges on privileged containers should not display
information. In lxc_container_new(), ongoing_create()'s result is not checked
for all possible returned values. Hence, an unprivileged user can send command
messages to the container's monitor. For example:
$ lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr - 0 - - - false
$ sudo lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr RUNNING 0 - 10.0.3.51 - false
After this change:
$ lxc-ls -P /.../tests -f <-------- No more display without root privileges
$ sudo lxc-ls -P /.../tests -f
NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED
ctr RUNNING 0 - 10.0.3.37 - false
$
Signed-off-by: Rachid Koucha <rachid.kou...@gmail.com>
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
src/lxc/lxccontainer.c | 56 ++++++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 98f86a24e4..cea8aa5d7b 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -135,7 +135,8 @@ static bool config_file_exists(const char *lxcpath, const
char *cname)
return file_exists(fname);
}
-/* A few functions to help detect when a container creation failed. If a
+/*
+ * A few functions to help detect when a container creation failed. If a
* container creation was killed partway through, then trying to actually start
* that container could harm the host. We detect this by creating a 'partial'
* file under the container directory, and keeping an advisory lock. When
@@ -143,30 +144,39 @@ static bool config_file_exists(const char *lxcpath, const
char *cname)
* start a container, if we find that file, without a flock, we remove the
* container.
*/
+enum {
+ LXC_CREATE_FAILED = -1,
+ LXC_CREATE_SUCCESS = 0,
+ LXC_CREATE_ONGOING = 1,
+ LXC_CREATE_INCOMPLETE = 2,
+};
+
static int ongoing_create(struct lxc_container *c)
{
+ __do_close_prot_errno int fd = -EBADF;
__do_free char *path = NULL;
- int fd, ret;
- size_t len;
struct flock lk = {0};
+ int ret;
+ size_t len;
len = strlen(c->config_path) + strlen(c->name) + 10;
path = must_realloc(NULL, len);
ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name);
if (ret < 0 || (size_t)ret >= len)
- return -1;
+ return LXC_CREATE_FAILED;
fd = open(path, O_RDWR | O_CLOEXEC);
if (fd < 0) {
if (errno != ENOENT)
- return -1;
+ return LXC_CREATE_FAILED;
- return 0;
+ return LXC_CREATE_SUCCESS;
}
lk.l_type = F_WRLCK;
lk.l_whence = SEEK_SET;
- /* F_OFD_GETLK requires that l_pid be set to 0 otherwise the kernel
+ /*
+ * F_OFD_GETLK requires that l_pid be set to 0 otherwise the kernel
* will EINVAL us.
*/
lk.l_pid = 0;
@@ -178,15 +188,13 @@ static int ongoing_create(struct lxc_container *c)
ret = 0;
}
- close(fd);
-
/* F_OFD_GETLK will not send us back a pid so don't check it. */
if (ret == 0)
/* Create is still ongoing. */
- return 1;
+ return LXC_CREATE_ONGOING;
/* Create completed but partial is still there. */
- return 2;
+ return LXC_CREATE_INCOMPLETE;
}
static int create_partial(struct lxc_container *c)
@@ -891,13 +899,14 @@ static bool do_lxcapi_start(struct lxc_container *c, int
useinit, char * const a
return false;
ret = ongoing_create(c);
- if (ret < 0) {
+ switch (ret) {
+ case LXC_CREATE_FAILED:
ERROR("Failed checking for incomplete container creation");
return false;
- } else if (ret == 1) {
+ case LXC_CREATE_ONGOING:
ERROR("Ongoing container creation detected");
return false;
- } else if (ret == 2) {
+ case LXC_CREATE_INCOMPLETE:
ERROR("Failed to create container");
do_lxcapi_destroy(c);
return false;
@@ -5249,6 +5258,7 @@ struct lxc_container *lxc_container_new(const char *name,
const char *configpath
{
struct lxc_container *c;
size_t len;
+ int rc;
if (!name)
return NULL;
@@ -5302,10 +5312,24 @@ struct lxc_container *lxc_container_new(const char
*name, const char *configpath
goto err;
}
- if (ongoing_create(c) == 2) {
- ERROR("Failed to complete container creation for %s", c->name);
+ rc = ongoing_create(c);
+ switch (rc) {
+ case LXC_CREATE_INCOMPLETE:
+ SYSERROR("Failed to complete container creation for %s",
c->name);
container_destroy(c, NULL);
lxcapi_clear_config(c);
+ break;
+ case LXC_CREATE_ONGOING:
+ /* container creation going on */
+ break;
+ case LXC_CREATE_FAILED:
+ /* container creation failed */
+ if (errno != EACCES && errno != EPERM) {
+ /* insufficient privileges */
+ SYSERROR("Failed checking for incomplete container %s
creation", c->name);
+ goto err;
+ }
+ break;
}
c->daemonize = true;
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
