[lxc-devel] [PATCH 1/1] lxc-clone: support unprivileged use
Stéphane Graber
stgraber at ubuntu.com
Thu Jan 23 00:44:25 UTC 2014
On Wed, Jan 22, 2014 at 06:18:04PM -0600, Serge Hallyn wrote:
> This also fixes unprivileged use of lxc-snapshot and lxc-rename.
>
Seems to be working fine here and not spotting any obvious problem with
the code.
However, it'd be nice to show an appropriate error message when trying
to clone in snapshot mode, as that's not supported for unprivileged
containers and isn't currently handled.
With that fixed:
Acked-by: Stéphane Graber <stgraber at ubuntu.com
And thanks for fixing the last feature that was broken for unprivileged
containers and didn't have to be (as opposed to snapshots who simply
can't be supported).
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> src/lxc/bdev.c | 100 +++++++++++++++++++++++++++++----------
> src/lxc/bdev.h | 4 +-
> src/lxc/lxccontainer.c | 126 +++++++++++++++++++++++++++----------------------
> src/lxc/utils.h | 3 ++
> 4 files changed, 150 insertions(+), 83 deletions(-)
>
> diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
> index 5cd7340..2c3388f 100644
> --- a/src/lxc/bdev.c
> +++ b/src/lxc/bdev.c
> @@ -2003,20 +2003,70 @@ struct bdev *bdev_init(const char *src, const char *dst, const char *data)
> return bdev;
> }
>
> +struct rsync_data {
> + struct bdev *orig;
> + struct bdev *new;
> +};
> +
> +static int rsync_rootfs(struct rsync_data *data)
> +{
> + struct bdev *orig = data->orig,
> + *new = data->new;
> +
> + if (unshare(CLONE_NEWNS) < 0) {
> + SYSERROR("unshare CLONE_NEWNS");
> + return -1;
> + }
> +
> + // If not a snapshot, copy the fs.
> + if (orig->ops->mount(orig) < 0) {
> + ERROR("failed mounting %s onto %s\n", orig->src, orig->dest);
> + return -1;
> + }
> + if (new->ops->mount(new) < 0) {
> + ERROR("failed mounting %s onto %s\n", new->src, new->dest);
> + return -1;
> + }
> + if (setgid(0) < 0) {
> + ERROR("Failed to setgid to 0");
> + return -1;
> + }
> + if (setuid(0) < 0) {
> + ERROR("Failed to setuid to 0");
> + return -1;
> + }
> + if (do_rsync(orig->dest, new->dest) < 0) {
> + ERROR("rsyncing %s to %s\n", orig->src, new->src);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int rsync_rootfs_wrapper(void *data)
> +{
> + struct rsync_data *arg = data;
> + return rsync_rootfs(arg);
> +}
> /*
> * If we're not snaphotting, then bdev_copy becomes a simple case of mount
> * the original, mount the new, and rsync the contents.
> */
> -struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
> - const char *oldpath, const char *lxcpath, const char *bdevtype,
> +struct bdev *bdev_copy(struct lxc_container *c0, const char *cname,
> + const char *lxcpath, const char *bdevtype,
> int flags, const char *bdevdata, uint64_t newsize,
> int *needs_rdep)
> {
> struct bdev *orig, *new;
> pid_t pid;
> + int ret;
> bool snap = flags & LXC_CLONE_SNAPSHOT;
> bool maybe_snap = flags & LXC_CLONE_MAYBE_SNAPSHOT;
> bool keepbdevtype = flags & LXC_CLONE_KEEPBDEVTYPE;
> + const char *src = c0->lxc_conf->rootfs.path;
> + const char *oldname = c0->name;
> + const char *oldpath = c0->config_path;
> + struct rsync_data data;
>
> /* if the container name doesn't show up in the rootfs path, then
> * we don't know how to come up with a new name
> @@ -2049,6 +2099,21 @@ struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
> }
> }
>
> + /* check for privilege */
> + if (am_unpriv()) {
> + if (bdevtype && strcmp(bdevtype, "dir") != 0) {
> + ERROR("Unprivileged users can only make dir copy-clones");
> + bdev_put(orig);
> + return NULL;
> + }
> + if (strcmp(orig->type, "dir") != 0) {
> + ERROR("Unprivileged users can only make dir copy-clones");
> + bdev_put(orig);
> + return NULL;
> + }
> + }
> +
> +
> /*
> * special case for snapshot - if caller requested maybe_snapshot and
> * keepbdevtype and backing store is directory, then proceed with a copy
> @@ -2081,6 +2146,8 @@ struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
> bdev_put(new);
> return NULL;
> }
> + if (snap)
> + return new;
>
> pid = fork();
> if (pid < 0) {
> @@ -2100,29 +2167,14 @@ struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
> return new;
> }
>
> - if (unshare(CLONE_NEWNS) < 0) {
> - SYSERROR("unshare CLONE_NEWNS");
> - exit(1);
> - }
> - if (snap)
> - exit(0);
> -
> - // If not a snapshot, copy the fs.
> - if (orig->ops->mount(orig) < 0) {
> - ERROR("failed mounting %s onto %s\n", src, orig->dest);
> - exit(1);
> - }
> - if (new->ops->mount(new) < 0) {
> - ERROR("failed mounting %s onto %s\n", new->src, new->dest);
> - exit(1);
> - }
> - if (do_rsync(orig->dest, new->dest) < 0) {
> - ERROR("rsyncing %s to %s\n", orig->src, new->src);
> - exit(1);
> - }
> - // don't bother umounting, ns exit will do that
> + data.orig = orig;
> + data.new = new;
> + if (am_unpriv())
> + ret = userns_exec_1(c0->lxc_conf, rsync_rootfs_wrapper, &data);
> + else
> + ret = rsync_rootfs(&data);
>
> - exit(0);
> + exit(ret == 0 ? 0 : 1);
> }
>
> static struct bdev * do_bdev_create(const char *dest, const char *type,
> diff --git a/src/lxc/bdev.h b/src/lxc/bdev.h
> index c45f086..cc7b59e 100644
> --- a/src/lxc/bdev.h
> +++ b/src/lxc/bdev.h
> @@ -99,8 +99,8 @@ char *overlayfs_getlower(char *p);
> */
> struct bdev *bdev_init(const char *src, const char *dst, const char *data);
>
> -struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
> - const char *oldpath, const char *lxcpath, const char *bdevtype,
> +struct bdev *bdev_copy(struct lxc_container *c0, const char *cname,
> + const char *lxcpath, const char *bdevtype,
> int flags, const char *bdevdata, uint64_t newsize,
> int *needs_rdep);
> struct bdev *bdev_create(const char *dest, const char *type,
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 3723997..5a3802a 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -66,10 +66,6 @@
>
> lxc_log_define(lxc_container, lxc);
>
> -inline static bool am_unpriv() {
> - return geteuid() != 0;
> -}
> -
> static bool file_exists(const char *f)
> {
> struct stat statbuf;
> @@ -2348,8 +2344,7 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c,
> struct bdev *bdev;
> int need_rdep;
>
> - bdev = bdev_copy(c0->lxc_conf->rootfs.path, c0->name, c->name,
> - c0->config_path, c->config_path, newtype, flags,
> + bdev = bdev_copy(c0, c->name, c->config_path, newtype, flags,
> bdevdata, newsize, &need_rdep);
> if (!bdev) {
> ERROR("Error copying storage");
> @@ -2375,36 +2370,49 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c,
> return 0;
> }
>
> -static int clone_update_rootfs(struct lxc_container *c0,
> - struct lxc_container *c, int flags,
> - char **hookargs)
> +struct clone_update_data {
> + struct lxc_container *c0;
> + struct lxc_container *c1;
> + int flags;
> + char **hookargs;
> +};
> +
> +static int clone_update_rootfs(struct clone_update_data *data)
> {
> + struct lxc_container *c0 = data->c0;
> + struct lxc_container *c = data->c1;
> + int flags = data->flags;
> + char **hookargs = data->hookargs;
> int ret = -1;
> char path[MAXPATHLEN];
> struct bdev *bdev;
> FILE *fout;
> - pid_t pid;
> struct lxc_conf *conf = c->lxc_conf;
>
> /* update hostname in rootfs */
> /* we're going to mount, so run in a clean namespace to simplify cleanup */
>
> - pid = fork();
> - if (pid < 0)
> + if (setgid(0) < 0) {
> + ERROR("Failed to setgid to 0");
> return -1;
> - if (pid > 0)
> - return wait_for_pid(pid);
> + }
> + if (setuid(0) < 0) {
> + ERROR("Failed to setuid to 0");
> + return -1;
> + }
>
> + if (unshare(CLONE_NEWNS) < 0)
> + return -1;
> bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
> if (!bdev)
> - exit(1);
> + return -1;
> if (strcmp(bdev->type, "dir") != 0) {
> if (unshare(CLONE_NEWNS) < 0) {
> ERROR("error unsharing mounts");
> - exit(1);
> + return -1;
> }
> if (bdev->ops->mount(bdev) < 0)
> - exit(1);
> + return -1;
> } else { // TODO come up with a better way
> if (bdev->dest)
> free(bdev->dest);
> @@ -2431,26 +2439,32 @@ static int clone_update_rootfs(struct lxc_container *c0,
>
> if (run_lxc_hooks(c->name, "clone", conf, c->get_config_path(c), hookargs)) {
> ERROR("Error executing clone hook for %s", c->name);
> - exit(1);
> + return -1;
> }
> }
>
> if (!(flags & LXC_CLONE_KEEPNAME)) {
> ret = snprintf(path, MAXPATHLEN, "%s/etc/hostname", bdev->dest);
> if (ret < 0 || ret >= MAXPATHLEN)
> - exit(1);
> + return -1;
> if (!file_exists(path))
> - exit(0);
> + return 0;
> if (!(fout = fopen(path, "w"))) {
> SYSERROR("unable to open %s: ignoring\n", path);
> - exit(0);
> + return 0;
> }
> if (fprintf(fout, "%s", c->name) < 0)
> - exit(1);
> + return -1;
> if (fclose(fout) < 0)
> - exit(1);
> + return -1;
> }
> - exit(0);
> + return 0;
> +}
> +
> +static int clone_update_rootfs_wrapper(void *data)
> +{
> + struct clone_update_data *arg = (struct clone_update_data *) data;
> + return clone_update_rootfs(arg);
> }
>
> /*
> @@ -2489,16 +2503,13 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
> char newpath[MAXPATHLEN];
> int ret, storage_copied = 0;
> const char *n, *l;
> + struct clone_update_data data;
> FILE *fout;
> + pid_t pid;
>
> 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;
>
> @@ -2541,6 +2552,13 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
> goto out;
> }
>
> + if (am_unpriv()) {
> + if (chown_mapped_root(newpath, c->lxc_conf) < 0) {
> + ERROR("Error chowning %s to container root\n", newpath);
> + goto out;
> + }
> + }
> +
> c2 = lxc_container_new(n, l);
> if (!c2) {
> ERROR("clone: failed to create new container (%s %s)", n, l);
> @@ -2581,12 +2599,31 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
> if (!c2->save_config(c2, NULL))
> goto out;
>
> - if (clone_update_rootfs(c, c2, flags, hookargs) < 0)
> + if ((pid = fork()) < 0) {
> + SYSERROR("fork");
> goto out;
> + }
> + if (pid > 0) {
> + ret = wait_for_pid(pid);
> + if (ret)
> + goto out;
> + container_mem_unlock(c);
> + return c2;
> + }
> + data.c0 = c;
> + data.c1 = c2;
> + data.flags = flags;
> + data.hookargs = hookargs;
> + if (am_unpriv())
> + ret = userns_exec_1(c->lxc_conf, clone_update_rootfs_wrapper,
> + &data);
> + else
> + ret = clone_update_rootfs(&data);
> + if (ret < 0)
> + exit(1);
>
> - // TODO: update c's lxc.snapshot = count
> container_mem_unlock(c);
> - return c2;
> + exit(0);
>
> out:
> container_mem_unlock(c);
> @@ -2608,11 +2645,6 @@ 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");
> @@ -2685,11 +2717,6 @@ 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)
> @@ -2828,11 +2855,6 @@ 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) {
> @@ -2911,11 +2933,6 @@ 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");
> @@ -2965,11 +2982,6 @@ 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;
> diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> index ab2bd84..dd81c62 100644
> --- a/src/lxc/utils.h
> +++ b/src/lxc/utils.h
> @@ -260,4 +260,7 @@ extern void **lxc_append_null_to_array(void **array, size_t count);
> //initialize rand with urandom
> extern int randseed(bool);
>
> +inline static bool am_unpriv(void) {
> + return geteuid() != 0;
> +}
> #endif
> --
> 1.8.5.3
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140122/789dffcf/attachment.pgp>
More information about the lxc-devel
mailing list