handle unprivileged user calls more gracefully (v3)

Return an error if the function is not supposed to be called by an unprivileged user.
Otherwise those calls fail in the middle of their execution with different reasons.

changes since v2:
- am_unpriv is now a simple geteuid check,
- API functions are now providing error messages,
- lxc-info, lxc-attach are now checking geteuidi,
- lxc-ls is now calling get_ips only if the container is running

Signed-off-by: S.Çağlar Onur <caglar@10ur.org>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/src/lxc/lxc-ls b/src/lxc/lxc-ls
index 1ed7da4fbe60b1664ea903d66f0585b1aa96d9a2..68c0b4118457c0034719f5b98b91f09a6bc93761 100755 (executable)
--- a/src/lxc/lxc-ls
+++ b/src/lxc/lxc-ls
@@ -264,14 +264,14 @@ for container_name in lxc.list_containers(config_path=nest_lxcpath):
                 continue
 
             # FIXME: We should get get_ips working as non-root
-            if container.running and (not SUPPORT_SETNS_NET
-                                      or os.geteuid() != 0):
-                entry[protocol] = 'UNKNOWN'
-                continue
-
-            ips = container.get_ips(family=family)
-            if ips:
-                entry[protocol] = ", ".join(ips)
+            if container.running:
+                if not SUPPORT_SETNS_NET or os.geteuid() != 0:
+                    entry[protocol] = 'UNKNOWN'
+                    continue
+
+                ips = container.get_ips(family=family)
+                if ips:
+                    entry[protocol] = ", ".join(ips)
 
     # Append the container
     containers.append(entry)

diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
index 6744c058726feb52cdedbec38b0d2d74f263052b..1159d09580534fc9c2b3117d7e1c5e1517c5c899 100644 (file)
--- a/src/lxc/lxc_attach.c
+++ b/src/lxc/lxc_attach.c
@@ -188,6 +188,11 @@ int main(int argc, char *argv[])
        lxc_attach_options_t attach_options = LXC_ATTACH_OPTIONS_DEFAULT;
        lxc_attach_command_t command;
 
+        if (geteuid() != 0) {
+                ERROR("lxc-attach is not currently supported with unprivileged containers");
+                return -1;
+        }
+
        ret = lxc_caps_init();
        if (ret)
                return ret;

diff --git a/src/lxc/lxc_info.c b/src/lxc/lxc_info.c
index b515087686e955209f3d6a0a5c073968d72c301d..0990036b609bb2be35bdd8628c9c7217c86462b1 100644 (file)
--- a/src/lxc/lxc_info.c
+++ b/src/lxc/lxc_info.c
@@ -310,15 +310,19 @@ static int print_info(const char *name, const char *lxcpath)
        }
 
        if (ips) {
-               char **addresses = c->get_ips(c, NULL, NULL, 0);
-               if (addresses) {
-                       char *address;
-                       i = 0;
-                       while (addresses[i]) {
-                               address = addresses[i];
-                               print_info_msg_str("IP:", address);
-                               i++;
+               if (geteuid() == 0) {
+                       char **addresses = c->get_ips(c, NULL, NULL, 0);
+                       if (addresses) {
+                               char *address;
+                               i = 0;
+                               while (addresses[i]) {
+                                       address = addresses[i];
+                                       print_info_msg_str("IP:", address);
+                                       i++;
+                               }
                        }
+               } else {
+                       print_info_msg_str("IP:", "UNKNOWN");
                }
        }
 

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 0bebdff1b015ae060e413262bacefb8d0d22cacf..368cb46bf2a600d89e6e5e61091bfb70b79407e7 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -62,8 +62,14 @@
 
 #define MAX_BUFFER 4096
 
+#define NOT_SUPPORTED_ERROR "the requested function %s is not currently supported with unprivileged containers"
+
 lxc_log_define(lxc_container, lxc);
 
+inline static bool am_unpriv() {
+       return geteuid() != 0;
+}
+
 static bool file_exists(const char *f)
 {
        struct stat statbuf;
@@ -1489,6 +1495,11 @@ static char** lxcapi_get_interfaces(struct lxc_container *c)
        char **interfaces = NULL;
        int old_netns = -1, new_netns = -1;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               goto out;
+       }
+
        if (!enter_to_ns(c, &old_netns, &new_netns))
                goto out;
 
@@ -1538,6 +1549,11 @@ static char** lxcapi_get_ips(struct lxc_container *c, const char* interface, con
        char *address = NULL;
        int old_netns = -1, new_netns = -1;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               goto out;
+       }
+
        if (!enter_to_ns(c, &old_netns, &new_netns))
                goto out;
 
@@ -1818,7 +1834,7 @@ static int lxc_rmdir_onedev_wrapper(void *data)
 static bool lxcapi_destroy(struct lxc_container *c)
 {
        struct bdev *r = NULL;
-       bool bret = false, am_unpriv;
+       bool bret = false;
        int ret;
 
        if (!c || !lxcapi_is_defined(c))
@@ -1833,14 +1849,12 @@ static bool lxcapi_destroy(struct lxc_container *c)
                goto out;
        }
 
-       am_unpriv = c->lxc_conf && geteuid() != 0 && !lxc_list_empty(&c->lxc_conf->id_map);
-
        if (c->lxc_conf && has_snapshots(c)) {
                ERROR("container %s has dependent snapshots", c->name);
                goto out;
        }
 
-       if (!am_unpriv && c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) {
+       if (!am_unpriv() && c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) {
                r = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
                if (r) {
                        if (r->ops->destroy(r) < 0) {
@@ -1857,7 +1871,7 @@ static bool lxcapi_destroy(struct lxc_container *c)
        const char *p1 = lxcapi_get_config_path(c);
        char *path = alloca(strlen(p1) + strlen(c->name) + 2);
        sprintf(path, "%s/%s", p1, c->name);
-       if (am_unpriv)
+       if (am_unpriv())
                ret = userns_exec_1(c->lxc_conf, lxc_rmdir_onedev_wrapper, path);
        else
                ret = lxc_rmdir_onedev(path);
@@ -2406,6 +2420,11 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
        if (!c || !c->is_defined(c))
                return NULL;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return NULL;
+       }
+
        if (container_mem_lock(c))
                return NULL;
 
@@ -2515,6 +2534,11 @@ static bool lxcapi_rename(struct lxc_container *c, const char *newname)
        if (!c || !c->name || !c->config_path)
                return false;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return false;
+       }
+
        bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
        if (!bdev) {
                ERROR("Failed to find original backing store type");
@@ -2543,6 +2567,11 @@ static int lxcapi_attach(struct lxc_container *c, lxc_attach_exec_t exec_functio
        if (!c)
                return -1;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return -1;
+       }
+
        return lxc_attach(c->name, c->config_path, exec_function, exec_payload, options, attached_process);
 }
 
@@ -2555,6 +2584,11 @@ static int lxcapi_attach_run_wait(struct lxc_container *c, lxc_attach_options_t
        if (!c)
                return -1;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return -1;
+       }
+
        command.program = (char*)program;
        command.argv = (char**)argv;
        r = lxc_attach(c->name, c->config_path, lxc_attach_run_command, &command, options, &pid);
@@ -2587,6 +2621,11 @@ static int lxcapi_snapshot(struct lxc_container *c, const char *commentfile)
        struct lxc_container *c2;
        char snappath[MAXPATHLEN], newname[20];
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return -1;
+       }
+
        // /var/lib/lxc -> /var/lib/lxcsnaps \0
        ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
        if (ret < 0 || ret >= MAXPATHLEN)
@@ -2724,6 +2763,12 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
 
        if (!c || !lxcapi_is_defined(c))
                return -1;
+
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return -1;
+       }
+
        // snappath is ${lxcpath}snaps/${lxcname}/
        dirlen = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
        if (dirlen < 0 || dirlen >= MAXPATHLEN) {
@@ -2802,6 +2847,11 @@ static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapnam
        if (!c || !c->name || !c->config_path)
                return false;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return false;
+       }
+
        bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
        if (!bdev) {
                ERROR("Failed to find original backing store type");
@@ -2851,6 +2901,11 @@ static bool lxcapi_snapshot_destroy(struct lxc_container *c, const char *snapnam
        if (!c || !c->name || !c->config_path)
                return false;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return false;
+       }
+
        ret = snprintf(clonelxcpath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
        if (ret < 0 || ret >= MAXPATHLEN)
                goto err;
@@ -2967,11 +3022,19 @@ out:
 
 static bool lxcapi_add_device_node(struct lxc_container *c, const char *src_path, const char *dest_path)
 {
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return false;
+       }
        return add_remove_device_node(c, src_path, dest_path, true);
 }
 
 static bool lxcapi_remove_device_node(struct lxc_container *c, const char *src_path, const char *dest_path)
 {
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return false;
+       }
        return add_remove_device_node(c, src_path, dest_path, false);
 }
 
@@ -2984,6 +3047,11 @@ static int lxcapi_attach_run_waitl(struct lxc_container *c, lxc_attach_options_t
        if (!c)
                return -1;
 
+       if (am_unpriv()) {
+               ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
+               return -1;
+       }
+
        va_start(ap, arg);
        argv = lxc_va_arg_list_to_argv_const(ap, 1);
        va_end(ap);