On Fri, Jan 17, 2014 at 11:34:50PM -0500, S.Çağlar Onur wrote:
> 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 v1:
> - am_unpriv is now a simple geteuid check,
> - API functions are now providing error messages,
> - lxc-info, lxc-attach and lxc-execute are now checking geteuid
> 

That looks much better to me, though I disagree that lxc-execute doesn't
work. It certainly is a bit restricted compared ot its system
equivalent, but all of those limitations will be the same as with
lxc-start.

stgraber@castiana:~$ lxc-execute -n p1 -- /bin/bash
lxc-init: failed to mount /proc : Device or resource busy
lxc-init: failed to mount /dev/shm : No such file or directory
root@p1:/# 

It may be worth teaching lxc_init a bit about unprivileged containers
but besides that, if all I wanted to do was run a command using that
container rootfs, it'd work.

The rest I think looks good and hopefully we can drop quite a few of
those before the 1.0 release. It shouldn't be too hard to get basic dir
cloning working and a good chunk of the rest would get resolved if we
had attach working.

Ideally, we'd release with only the snapshot functions and the
add/remove device function disabled as those simply aren't doable at
this point.

> Signed-off-by: S.Çağlar Onur <[email protected]>
> ---
>  src/lxc/lxc_attach.c   |  5 ++++
>  src/lxc/lxc_execute.c  |  5 ++++
>  src/lxc/lxc_info.c     | 20 +++++++------
>  src/lxc/lxccontainer.c | 78 
> ++++++++++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 95 insertions(+), 13 deletions(-)
> 
> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> index 6744c05..1159d09 100644
> --- 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_execute.c b/src/lxc/lxc_execute.c
> index 6a54bf6..8177ed3 100644
> --- a/src/lxc/lxc_execute.c
> +++ b/src/lxc/lxc_execute.c
> @@ -92,6 +92,11 @@ int main(int argc, char *argv[])
>       char *rcfile;
>       struct lxc_conf *conf;
>  
> +        if (geteuid() != 0) {
> +                ERROR("lxc-execute is not currently supported with 
> unprivileged containers");
> +                return -1;
> +        }
> +
>       lxc_list_init(&defines);
>  
>       if (lxc_caps_init())
> diff --git a/src/lxc/lxc_info.c b/src/lxc/lxc_info.c
> index b515087..0990036 100644
> --- 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 0bebdff..368cb46 100644
> --- 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);
> -- 
> 1.8.3.2
> 
> _______________________________________________
> lxc-devel mailing list
> [email protected]
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

Attachment: signature.asc
Description: Digital signature

_______________________________________________
lxc-devel mailing list
[email protected]
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to