[lxc-devel] [PATCH RFC] chroot_into_slave: move tmp-/ before chrooting to it

[Andrey Wagin]

2014-10-05 8:21 GMT+04:00 Serge Hallyn <serge.hallyn at ubuntu.com>:
> First, make sure to remount MS_SHARED mounts as MS_SLAVE
> before doing chroot_into_slave.
>
> Then, move-mount our tmp-/ to / before chrooting to it.  This
> ensures that the new root is mounted on top of "/" rather than
> on top of our old chroot, which allows chroot escape.

Looks like this scheme is insecure too. I added chrootbreak2.c to your scripts:
https://github.com/avagin/pivot_root-vs-rootfs

>
> Without this, lxc users who have / on a ramfs are able to
> break out of the container roots (unless otherwise protected,
> i.e. by apparmor/selinux).  This does not affect users who
> do not have / on ramfs.
>
> Note 1: This has only been compile-tested.
>
> Note 2: As I mentioned we may want to do this in all cases, not
> only when we detect we are on ramfs.  In a case where the user
> is already running out of a chroot (which could be the case in some
> weird rootfs setups) container users may otherwise be able to break
> out of the container chroot.
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  src/lxc/conf.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index e8979c9..dbd09bd 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1499,12 +1499,16 @@ static int chroot_into_slave(struct lxc_conf *conf)
>                 SYSERROR("Failed to make tmp-/ at %s rslave", path);
>                 return -1;
>         }
> -       if (chroot(path)) {
> -               SYSERROR("Failed to chroot into tmp-/");
> +       if (chdir(path) < 0) {
> +               SYSERROR("Failed to chdir into tmp-/");
>                 return -1;
>         }
> -       if (chdir("/")) {
> -               SYSERROR("Failed to chdir into tmp-/");
> +       if (mount(".", "/", NULL, MS_MOVE, 0) < 0) {
> +               SYSERROR("Failed to move-mount tmp-/");
> +               return -1;
> +       }
> +       if (chroot(".")) {
> +               SYSERROR("Failed to chroot into tmp-/");
>                 return -1;
>         }
>         INFO("Chrooted into tmp-/ at %s", path);
> @@ -3952,6 +3956,8 @@ int do_rootfs_setup(struct lxc_conf *conf, const char *name, const char *lxcpath
>                 }
>         }
>
> +       remount_all_slave();
> +
>         if (detect_ramfs_rootfs()) {
>                 if (chroot_into_slave(conf)) {
>                         ERROR("Failed to chroot into slave /");
> @@ -3959,8 +3965,6 @@ int do_rootfs_setup(struct lxc_conf *conf, const char *name, const char *lxcpath
>                 }
>         }
>
> -       remount_all_slave();
> -
>         if (run_lxc_hooks(name, "pre-mount", conf, lxcpath, NULL)) {
>                 ERROR("failed to run pre-mount hooks for container '%s'.", name);
>                 return -1;
> --
> 2.1.0
>