Hello, During a recent security audit of LXC, Roman Fiedler identified a security vulnerability in LXC.
CVE 2015-1335:
When a container starts up, lxc sets up the container's inital fstree
by doing a bunch of mounting, guided by the container configuration
file. The container config is owned by the admin or user on the host,
so we do not try to guard against bad entries. However, since the
mount target is in the container, it's possible that the container admin
could divert the mount with symbolic links. This could bypass proper
container startup (i.e. confinement of a root-owned container by the
restrictive apparmor policy, by diverting the required write to
/proc/self/attr/current), or bypass the (path-based) apparmor policy
by diverting, say, /proc to /mnt in the container.
To prevent this,
1. do not allow mounts to paths containing symbolic links
2. do not allow bind mounts from relative paths containing symbolic
links.
The fix for LXC 1.0 is:
https://github.com/lxc/lxc/commit/6bbb8100c4dec4b1c71758c27104985a694a4eac
The fix for LXC 1.1 is:
https://github.com/lxc/lxc/commit/6de26af93d3dd87c8b21a42fdf20f30fa1c1948d
The fix for LXC master is:
https://github.com/lxc/lxc/commit/592fd47a6245508b79fe6ac819fe6d3b2c1289be
Patches for a few recent LXC releases are also attached to this e-mail.
The fix will be included in the upcoming stable releases for both
branches. That will be LXC 1.1.4 and LXC 1.0.8 which will both be
released very soon.
The security teams from the various Linux distributions have been
informed of those security issues ahead of time and so should have or
soon will be pushing security updates to their supported releases.
I'd like to thank Roman for his great work at finding and responsibly
disclosing those issues to us.
The fix for this issue has been developed by Serge Hallyn with the help
of Tyler Hicks and myself.
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
From 557d4b31a797603bed52ccc23a2ad11fe2bce910 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hal...@ubuntu.com>
Date: Mon, 31 Aug 2015 12:57:20 -0500
Subject: [PATCH 1/1] Protect container mounts against symlinks
When a container starts up, lxc sets up the container's inital fstree
by doing a bunch of mounting, guided by the container configuration
file. The container config is owned by the admin or user on the host,
so we do not try to guard against bad entries. However, since the
mount target is in the container, it's possible that the container admin
could divert the mount with symbolic links. This could bypass proper
container startup (i.e. confinement of a root-owned container by the
restrictive apparmor policy, by diverting the required write to
/proc/self/attr/current), or bypass the (path-based) apparmor policy
by diverting, say, /proc to /mnt in the container.
To prevent this,
1. do not allow mounts to paths containing symbolic links
2. do not allow bind mounts from relative paths containing symbolic
links.
Details:
Define safe_mount which ensures that the container has not inserted any
symbolic links into any mount targets for mounts to be done during
container setup.
The host's mount path may contain symbolic links. As it is under the
control of the administrator, that's ok. So safe_mount begins the check
for symbolic links after the rootfs->mount, by opening that directory.
It opens each directory along the path using openat() relative to the
parent directory using O_NOFOLLOW. When the target is reached, it
mounts onto /proc/self/fd/<targetfd>.
Use safe_mount() in mount_entry(), when mounting container proc,
and when needed. In particular, safe_mount() need not be used in
any case where:
1. the mount is done in the container's namespace
2. the mount is for the container's rootfs
3. the mount is relative to a tmpfs or proc/sysfs which we have
just safe_mount()ed ourselves
Since we were using proc/net as a temporary placeholder for /proc/sys/net
during container startup, and proc/net is a symbolic link, use proc/tty
instead.
Update the lxc.container.conf manpage with details about the new
restrictions.
Finally, add a testcase to test some symbolic link possibilities.
Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
---
doc/lxc.container.conf.sgml.in | 12 +++
src/lxc/cgfs.c | 5 +-
src/lxc/cgmanager.c | 4 +-
src/lxc/conf.c | 30 +++---
src/lxc/utils.c | 233 +++++++++++++++++++++++++++++++++++++++++
src/lxc/utils.h | 2 +
src/tests/Makefile.am | 3 +-
src/tests/lxc-test-symlink | 88 ++++++++++++++++
8 files changed, 359 insertions(+), 18 deletions(-)
create mode 100644 src/tests/lxc-test-symlink
diff --git a/doc/lxc.container.conf.sgml.in b/doc/lxc.container.conf.sgml.in
index 9cd0c57..6f06762 100644
--- a/doc/lxc.container.conf.sgml.in
+++ b/doc/lxc.container.conf.sgml.in
@@ -676,6 +676,18 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
container. This is useful to mount /etc, /var or /home for
examples.
</para>
+ <para>
+ NOTE - LXC will generally ensure that mount targets and relative
+ bind-mount sources are properly confined under the container
+ root, to avoid attacks involving over-mounting host directories
+ and files. (Symbolic links in absolute mount sources are ignored)
+ However, if the container configuration first mounts a directory which
+ is under the control of the container user, such as /home/joe, into
+ the container at some <filename>path</filename>, and then mounts
+ under <filename>path</filename>, then a TOCTTOU attack would be
+ possible where the container user modifies a symbolic link under
+ his home directory at just the right time.
+ </para>
<variablelist>
<varlistentry>
<term>
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index 15346dc..df4ad46 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -1363,7 +1363,10 @@ static bool cgroupfs_mount_cgroup(void *hdata, const char *root, int type)
if (!path)
return false;
snprintf(path, bufsz, "%s/sys/fs/cgroup", root);
- r = mount("cgroup_root", path, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME, "size=10240k,mode=755");
+ r = safe_mount("cgroup_root", path, "tmpfs",
+ MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME,
+ "size=10240k,mode=755",
+ root);
if (r < 0) {
SYSERROR("could not mount tmpfs to /sys/fs/cgroup in the container");
return false;
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 1872f03..79af751 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -1332,7 +1332,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
}
/* mount a tmpfs there so we can create subdirs */
- if (mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755")) {
+ if (safe_mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755", root)) {
SYSERROR("Failed to mount tmpfs at %s", cgpath);
return false;
}
@@ -1343,7 +1343,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
return false;
}
- if (mount(dirname, cgpath, "none", MS_BIND, 0)) {
+ if (safe_mount(dirname, cgpath, "none", MS_BIND, 0, root)) {
SYSERROR("Failed to bind mount %s to %s", dirname, cgpath);
return false;
}
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 320b6c9..8acee5a 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -795,7 +795,7 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
}
mflags = add_required_remount_flags(source, destination,
default_mounts[i].flags);
- r = mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options);
+ r = safe_mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options, conf->rootfs.path ? conf->rootfs.mount : NULL);
saved_errno = errno;
if (r < 0)
SYSERROR("error mounting %s on %s flags %lu", source, destination, mflags);
@@ -989,7 +989,8 @@ static int setup_tty(const struct lxc_rootfs *rootfs,
return -1;
}
- if (mount(pty_info->name, lxcpath, "none", MS_BIND, 0)) {
+ if (safe_mount(pty_info->name, lxcpath, "none", MS_BIND, 0,
+ rootfs->mount)) {
WARN("failed to mount '%s'->'%s'",
pty_info->name, path);
continue;
@@ -1016,7 +1017,8 @@ static int setup_tty(const struct lxc_rootfs *rootfs,
close(ret);
}
}
- if (mount(pty_info->name, path, "none", MS_BIND, 0)) {
+ if (safe_mount(pty_info->name, path, "none", MS_BIND, 0,
+ rootfs->mount)) {
WARN("failed to mount '%s'->'%s'",
pty_info->name, path);
continue;
@@ -1442,16 +1444,16 @@ static int mount_autodev(const char *name, char *root, const char *lxcpath)
SYSERROR("WARNING: Failed to create symlink '%s'->'%s'", host_path, devtmpfs_path);
}
DEBUG("Bind mounting %s to %s", devtmpfs_path , path );
- ret = mount(devtmpfs_path, path, NULL, MS_BIND, 0 );
+ ret = safe_mount(devtmpfs_path, path, NULL, MS_BIND, 0, root );
} else {
/* Only mount a tmpfs on here if we don't already a mount */
if ( ! mount_check_fs( host_path, NULL ) ) {
DEBUG("Mounting tmpfs to %s", host_path );
- ret = mount("none", path, "tmpfs", 0, "size=100000,mode=755");
+ ret = safe_mount("none", path, "tmpfs", 0, "size=100000,mode=755", root);
} else {
/* This allows someone to manually set up a mount */
DEBUG("Bind mounting %s to %s", host_path, path );
- ret = mount(host_path , path, NULL, MS_BIND, 0 );
+ ret = safe_mount(host_path , path, NULL, MS_BIND, 0, root );
}
}
if (ret) {
@@ -1828,7 +1830,7 @@ static int setup_dev_console(const struct lxc_rootfs *rootfs,
return -1;
}
- if (mount(console->name, path, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, path, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, path);
return -1;
}
@@ -1883,7 +1885,7 @@ static int setup_ttydir_console(const struct lxc_rootfs *rootfs,
return 0;
}
- if (mount(console->name, lxcpath, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, lxcpath);
return -1;
}
@@ -2033,13 +2035,13 @@ static char *get_field(char *src, int nfields)
static int mount_entry(const char *fsname, const char *target,
const char *fstype, unsigned long mountflags,
- const char *data, int optional)
+ const char *data, int optional, const char *rootfs)
{
#ifdef HAVE_STATVFS
struct statvfs sb;
#endif
- if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) {
+ if (safe_mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data, rootfs)) {
if (optional) {
INFO("failed to mount '%s' on '%s' (optional): %s", fsname,
target, strerror(errno));
@@ -2172,7 +2174,7 @@ static inline int mount_entry_on_systemfs(struct mntent *mntent)
}
ret = mount_entry(mntent->mnt_fsname, mntent->mnt_dir,
- mntent->mnt_type, mntflags, mntdata, optional);
+ mntent->mnt_type, mntflags, mntdata, optional, NULL);
free(pathdirname);
free(mntdata);
@@ -2259,7 +2261,7 @@ skipabs:
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs->mount);
free(mntdata);
@@ -2315,7 +2317,7 @@ static int mount_entry_on_relative_rootfs(struct mntent *mntent,
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs);
free(pathdirname);
free(mntdata);
@@ -3981,7 +3983,7 @@ static int do_tmp_proc_mount(const char *rootfs)
return 0;
domount:
- if (mount("proc", path, "proc", 0, NULL))
+ if (safe_mount("proc", path, "proc", 0, NULL, rootfs))
return -1;
INFO("Mounted /proc in container for security transition");
return 1;
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 5ef04fc..cfd4092 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1322,3 +1322,236 @@ next_loop:
free(path);
return NULL;
}
+
+/*
+ * @path: a pathname where / replaced with '\0'.
+ * @offsetp: pointer to int showing which path segment was last seen.
+ * Updated on return to reflect the next segment.
+ * @fulllen: full original path length.
+ * Returns a pointer to the next path segment, or NULL if done.
+ */
+static char *get_nextpath(char *path, int *offsetp, int fulllen)
+{
+ int offset = *offsetp;
+
+ if (offset >= fulllen)
+ return NULL;
+
+ while (path[offset] != '\0' && offset < fulllen)
+ offset++;
+ while (path[offset] == '\0' && offset < fulllen)
+ offset++;
+
+ *offsetp = offset;
+ return (offset < fulllen) ? &path[offset] : NULL;
+}
+
+/*
+ * Check that @subdir is a subdir of @dir. @len is the length of
+ * @dir (to avoid having to recalculate it).
+ */
+static bool is_subdir(const char *subdir, const char *dir, size_t len)
+{
+ size_t subdirlen = strlen(subdir);
+
+ if (subdirlen < len)
+ return false;
+ if (strncmp(subdir, dir, len) != 0)
+ return false;
+ if (dir[len-1] == '/')
+ return true;
+ if (subdir[len] == '/' || subdirlen == len)
+ return true;
+ return false;
+}
+
+/*
+ * Check if the open fd is a symlink. Return -ELOOP if it is. Return
+ * -ENOENT if we couldn't fstat. Return 0 if the fd is ok.
+ */
+static int check_symlink(int fd)
+{
+ struct stat sb;
+ int ret = fstat(fd, &sb);
+ if (ret < 0)
+ return -ENOENT;
+ if (S_ISLNK(sb.st_mode))
+ return -ELOOP;
+ return 0;
+}
+
+/*
+ * Open a file or directory, provided that it contains no symlinks.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+static int open_if_safe(int dirfd, const char *nextpath)
+{
+ int newfd = openat(dirfd, nextpath, O_RDONLY | O_NOFOLLOW);
+ if (newfd >= 0) // was not a symlink, all good
+ return newfd;
+
+ if (errno == ELOOP)
+ return newfd;
+
+ if (errno == EPERM || errno == EACCES) {
+ /* we're not root (cause we got EPERM) so
+ try opening with O_PATH */
+ newfd = openat(dirfd, nextpath, O_PATH | O_NOFOLLOW);
+ if (newfd >= 0) {
+ /* O_PATH will return an fd for symlinks. We know
+ * nextpath wasn't a symlink at last openat, so if fd
+ * is now a link, then something * fishy is going on
+ */
+ int ret = check_symlink(newfd);
+ if (ret < 0) {
+ close(newfd);
+ newfd = ret;
+ }
+ }
+ }
+
+ return newfd;
+}
+
+/*
+ * Open a path intending for mounting, ensuring that the final path
+ * is inside the container's rootfs.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ *
+ * @target: path to be opened
+ * @prefix_skip: a part of @target in which to ignore symbolic links. This
+ * would be the container's rootfs.
+ *
+ * Return an open fd for the path, or <0 on error.
+ */
+static int open_without_symlink(const char *target, const char *prefix_skip)
+{
+ int curlen = 0, dirfd, fulllen, i;
+ char *dup = NULL;
+
+ fulllen = strlen(target);
+
+ /* make sure prefix-skip makes sense */
+ if (prefix_skip) {
+ curlen = strlen(prefix_skip);
+ if (!is_subdir(target, prefix_skip, curlen)) {
+ ERROR("WHOA there - target '%s' didn't start with prefix '%s'",
+ target, prefix_skip);
+ return -EINVAL;
+ }
+ /*
+ * get_nextpath() expects the curlen argument to be
+ * on a (turned into \0) / or before it, so decrement
+ * curlen to make sure that happens
+ */
+ if (curlen)
+ curlen--;
+ } else {
+ prefix_skip = "/";
+ curlen = 0;
+ }
+
+ /* Make a copy of target which we can hack up, and tokenize it */
+ if ((dup = strdup(target)) == NULL) {
+ SYSERROR("Out of memory checking for symbolic link");
+ return -ENOMEM;
+ }
+ for (i = 0; i < fulllen; i++) {
+ if (dup[i] == '/')
+ dup[i] = '\0';
+ }
+
+ dirfd = open(prefix_skip, O_RDONLY);
+ if (dirfd < 0)
+ goto out;
+ while (1) {
+ int newfd, saved_errno;
+ char *nextpath;
+
+ if ((nextpath = get_nextpath(dup, &curlen, fulllen)) == NULL)
+ goto out;
+ newfd = open_if_safe(dirfd, nextpath);
+ saved_errno = errno;
+ close(dirfd);
+ dirfd = newfd;
+ if (newfd < 0) {
+ errno = saved_errno;
+ if (errno == ELOOP)
+ SYSERROR("%s in %s was a symbolic link!", nextpath, target);
+ else
+ SYSERROR("Error examining %s in %s", nextpath, target);
+ goto out;
+ }
+ }
+
+out:
+ free(dup);
+ return dirfd;
+}
+
+/*
+ * Safely mount a path into a container, ensuring that the mount target
+ * is under the container's @rootfs. (If @rootfs is NULL, then the container
+ * uses the host's /)
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs)
+{
+ int srcfd = -1, destfd, ret, saved_errno;
+ char srcbuf[50], destbuf[50]; // only needs enough for /proc/self/fd/<fd>
+ const char *mntsrc = src;
+
+ if (!rootfs)
+ rootfs = "";
+
+ /* todo - allow symlinks for relative paths if 'allowsymlinks' option is passed */
+ if (flags & MS_BIND && src && src[0] != '/') {
+ INFO("this is a relative bind mount");
+ srcfd = open_without_symlink(src, NULL);
+ if (srcfd < 0)
+ return srcfd;
+ ret = snprintf(srcbuf, 50, "/proc/self/fd/%d", srcfd);
+ if (ret < 0 || ret > 50) {
+ close(srcfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+ mntsrc = srcbuf;
+ }
+
+ destfd = open_without_symlink(dest, rootfs);
+ if (destfd < 0) {
+ if (srcfd != -1)
+ close(srcfd);
+ return destfd;
+ }
+
+ ret = snprintf(destbuf, 50, "/proc/self/fd/%d", destfd);
+ if (ret < 0 || ret > 50) {
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+
+ ret = mount(mntsrc, destbuf, fstype, flags, data);
+ saved_errno = errno;
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ if (ret < 0) {
+ errno = saved_errno;
+ SYSERROR("Failed to mount %s onto %s", src, dest);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index f48f403..30e8a98 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -280,3 +280,5 @@ uint64_t fnv_64a_buf(void *buf, size_t len, uint64_t hval);
int detect_shared_rootfs(void);
int detect_ramfs_rootfs(void);
char *on_path(char *cmd);
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs);
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 19a4205..edb62d6 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -48,7 +48,7 @@ bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
lxc-test-reboot lxc-test-list lxc-test-attach lxc-test-device-add-remove \
lxc-test-apparmor
-bin_SCRIPTS = lxc-test-autostart
+bin_SCRIPTS = lxc-test-autostart lxc-test-symlink
if DISTRO_UBUNTU
bin_SCRIPTS += lxc-test-usernic lxc-test-ubuntu lxc-test-unpriv
@@ -71,6 +71,7 @@ EXTRA_DIST = \
locktests.c \
lxcpath.c \
lxc-test-autostart \
+ lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
may_control.c \
diff --git a/src/tests/lxc-test-symlink b/src/tests/lxc-test-symlink
new file mode 100644
index 0000000..37320f0
--- /dev/null
+++ b/src/tests/lxc-test-symlink
@@ -0,0 +1,88 @@
+#!/bin/bash
+
+set -ex
+
+# lxc: linux Container library
+
+# Authors:
+# Serge Hallyn <serge.hal...@ubuntu.com>
+#
+# This is a regression test for symbolic links
+
+dirname=`mktemp -d`
+fname=`mktemp`
+fname2=`mktemp`
+
+lxcpath=/var/lib/lxcsym1
+
+cleanup() {
+ lxc-destroy -P $lxcpath -f -n symtest1 || true
+ rm -f $lxcpath
+ rmdir $dirname || true
+ rm -f $fname || true
+ rm -f $fname2 || true
+}
+
+trap cleanup EXIT SIGHUP SIGINT SIGTERM
+
+testrun() {
+ expected=$1
+ run=$2
+ pass="pass"
+ lxc-start -P $lxcpath -n symtest1 -l trace -o $lxcpath/log || pass="fail"
+ [ $pass = "pass" ] && lxc-wait -P $lxcpath -n symtest1 -t 10 -s RUNNING || pass="fail"
+ if [ "$pass" != "$expected" ]; then
+ echo "Test $run: expected $expected but container did not. Start log:"
+ cat $lxcpath/log
+ echo "FAIL: Test $run: expected $expected but container did not."
+ false
+ fi
+ lxc-stop -P $lxcpath -n symtest1 -k || true
+}
+
+# make lxcpath a symlink - this should NOT cause failure
+ln -s /var/lib/lxc $lxcpath
+
+lxc-destroy -P $lxcpath -f -n symtest1 || true
+lxc-create -P $lxcpath -t busybox -n symtest1
+
+cat >> /var/lib/lxc/symtest1/config << EOF
+lxc.mount.entry = $dirname opt/xxx/dir none bind,create=dir
+lxc.mount.entry = $fname opt/xxx/file none bind,create=file
+lxc.mount.entry = $fname2 opt/xxx/file2 none bind
+EOF
+
+# Regular - should succeed
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun pass 1
+
+# symlink - should fail
+rm -rf /var/lib/lxc/symtest1/rootfs/opt/xxx
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx2
+ln -s /var/lib/lxc/symtest1/rootfs/opt/xxx2 /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun fail 2
+
+# final final symlink - should fail
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx/dir
+touch $lxcpath/symtest1/rootfs/opt/xxx/file
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2src
+ln -s $lxcpath/symtest1/rootfs/opt/xxx/file2src $lxcpath/symtest1/rootfs/opt/xxx/file2
+testrun fail 3
+
+# Ideally we'd also try a loop device, but that won't work in nested containers
+# anyway - TODO
+
+# what about /proc itself
+
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2
+mv $lxcpath/symtest1/rootfs/proc $lxcpath/symtest1/rootfs/proc1
+ln -s $lxcpath/symtest1/rootfs/proc1 $lxcpath/symtest1/rootfs/proc
+testrun fail 4
+
+echo "all tests passed"
--
2.5.0
From 13e0520e8cee9b1ff40d9e546b2f3865cacbf5b4 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hal...@ubuntu.com>
Date: Mon, 31 Aug 2015 12:57:20 -0500
Subject: [PATCH 1/1] Protect container mounts against symlinks
When a container starts up, lxc sets up the container's inital fstree
by doing a bunch of mounting, guided by the container configuration
file. The container config is owned by the admin or user on the host,
so we do not try to guard against bad entries. However, since the
mount target is in the container, it's possible that the container admin
could divert the mount with symbolic links. This could bypass proper
container startup (i.e. confinement of a root-owned container by the
restrictive apparmor policy, by diverting the required write to
/proc/self/attr/current), or bypass the (path-based) apparmor policy
by diverting, say, /proc to /mnt in the container.
To prevent this,
1. do not allow mounts to paths containing symbolic links
2. do not allow bind mounts from relative paths containing symbolic
links.
Details:
Define safe_mount which ensures that the container has not inserted any
symbolic links into any mount targets for mounts to be done during
container setup.
The host's mount path may contain symbolic links. As it is under the
control of the administrator, that's ok. So safe_mount begins the check
for symbolic links after the rootfs->mount, by opening that directory.
It opens each directory along the path using openat() relative to the
parent directory using O_NOFOLLOW. When the target is reached, it
mounts onto /proc/self/fd/<targetfd>.
Use safe_mount() in mount_entry(), when mounting container proc,
and when needed. In particular, safe_mount() need not be used in
any case where:
1. the mount is done in the container's namespace
2. the mount is for the container's rootfs
3. the mount is relative to a tmpfs or proc/sysfs which we have
just safe_mount()ed ourselves
Since we were using proc/net as a temporary placeholder for /proc/sys/net
during container startup, and proc/net is a symbolic link, use proc/tty
instead.
Update the lxc.container.conf manpage with details about the new
restrictions.
Finally, add a testcase to test some symbolic link possibilities.
Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
---
doc/lxc.container.conf.sgml.in | 12 +++
src/lxc/cgfs.c | 5 +-
src/lxc/cgmanager.c | 4 +-
src/lxc/conf.c | 29 ++---
src/lxc/utils.c | 233 +++++++++++++++++++++++++++++++++++++++++
src/lxc/utils.h | 2 +
src/tests/Makefile.am | 2 +
src/tests/lxc-test-symlink | 88 ++++++++++++++++
8 files changed, 358 insertions(+), 17 deletions(-)
create mode 100644 src/tests/lxc-test-symlink
diff --git a/doc/lxc.container.conf.sgml.in b/doc/lxc.container.conf.sgml.in
index 50c6a2a..0a1ec5f 100644
--- a/doc/lxc.container.conf.sgml.in
+++ b/doc/lxc.container.conf.sgml.in
@@ -699,6 +699,18 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
container. This is useful to mount /etc, /var or /home for
examples.
</para>
+ <para>
+ NOTE - LXC will generally ensure that mount targets and relative
+ bind-mount sources are properly confined under the container
+ root, to avoid attacks involving over-mounting host directories
+ and files. (Symbolic links in absolute mount sources are ignored)
+ However, if the container configuration first mounts a directory which
+ is under the control of the container user, such as /home/joe, into
+ the container at some <filename>path</filename>, and then mounts
+ under <filename>path</filename>, then a TOCTTOU attack would be
+ possible where the container user modifies a symbolic link under
+ his home directory at just the right time.
+ </para>
<variablelist>
<varlistentry>
<term>
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index 11a5925..1fb1771 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -1363,7 +1363,10 @@ static bool cgroupfs_mount_cgroup(void *hdata, const char *root, int type)
if (!path)
return false;
snprintf(path, bufsz, "%s/sys/fs/cgroup", root);
- r = mount("cgroup_root", path, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME, "size=10240k,mode=755");
+ r = safe_mount("cgroup_root", path, "tmpfs",
+ MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME,
+ "size=10240k,mode=755",
+ root);
if (r < 0) {
SYSERROR("could not mount tmpfs to /sys/fs/cgroup in the container");
return false;
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index fe8913e..84457f2 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -1341,7 +1341,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
}
/* mount a tmpfs there so we can create subdirs */
- if (mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755")) {
+ if (safe_mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755", root)) {
SYSERROR("Failed to mount tmpfs at %s", cgpath);
return false;
}
@@ -1352,7 +1352,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
return false;
}
- if (mount(dirname, cgpath, "none", MS_BIND, 0)) {
+ if (safe_mount(dirname, cgpath, "none", MS_BIND, 0, root)) {
SYSERROR("Failed to bind mount %s to %s", dirname, cgpath);
return false;
}
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 4249c6c..0cd1de9 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -763,10 +763,11 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
* 2.6.32...
*/
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "proc", "%r/proc", "proc", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL },
- { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net", "%r/proc/net", NULL, MS_BIND, NULL },
+ /* proc/tty is used as a temporary placeholder for proc/sys/net which we'll move back in a few steps */
+ { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net", "%r/proc/tty", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys", "%r/proc/sys", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, NULL, "%r/proc/sys", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL },
- { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/net", "%r/proc/sys/net", NULL, MS_MOVE, NULL },
+ { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/tty", "%r/proc/sys/net", NULL, MS_MOVE, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sysrq-trigger", "%r/proc/sysrq-trigger", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, NULL, "%r/proc/sysrq-trigger", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_RW, "proc", "%r/proc", "proc", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL },
@@ -809,7 +810,7 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
}
mflags = add_required_remount_flags(source, destination,
default_mounts[i].flags);
- r = mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options);
+ r = safe_mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options, conf->rootfs.path ? conf->rootfs.mount : NULL);
saved_errno = errno;
if (r < 0 && errno == ENOENT) {
INFO("Mount source or target for %s on %s doesn't exist. Skipping.", source, destination);
@@ -1161,7 +1162,7 @@ static int mount_autodev(const char *name, char *root, const char *lxcpath)
return 0;
}
- if (mount("none", path, "tmpfs", 0, "size=100000,mode=755")) {
+ if (safe_mount("none", path, "tmpfs", 0, "size=100000,mode=755", root)) {
SYSERROR("Failed mounting tmpfs onto %s\n", path);
return false;
}
@@ -1246,7 +1247,7 @@ static int fill_autodev(const char *root)
return -1;
}
fclose(pathfile);
- if (mount(hostpath, path, 0, MS_BIND, NULL) != 0) {
+ if (safe_mount(hostpath, path, 0, MS_BIND, NULL, root) != 0) {
SYSERROR("Failed bind mounting device %s from host into container",
d->name);
return -1;
@@ -1501,7 +1502,7 @@ static int setup_dev_console(const struct lxc_rootfs *rootfs,
return -1;
}
- if (mount(console->name, path, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, path, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, path);
return -1;
}
@@ -1556,7 +1557,7 @@ static int setup_ttydir_console(const struct lxc_rootfs *rootfs,
return 0;
}
- if (mount(console->name, lxcpath, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, lxcpath);
return -1;
}
@@ -1706,13 +1707,13 @@ static char *get_field(char *src, int nfields)
static int mount_entry(const char *fsname, const char *target,
const char *fstype, unsigned long mountflags,
- const char *data, int optional)
+ const char *data, int optional, const char *rootfs)
{
#ifdef HAVE_STATVFS
struct statvfs sb;
#endif
- if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) {
+ if (safe_mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data, rootfs)) {
if (optional) {
INFO("failed to mount '%s' on '%s' (optional): %s", fsname,
target, strerror(errno));
@@ -1759,7 +1760,7 @@ static int mount_entry(const char *fsname, const char *target,
#endif
if (mount(fsname, target, fstype,
- mountflags | MS_REMOUNT, data)) {
+ mountflags | MS_REMOUNT, data) < 0) {
if (optional) {
INFO("failed to mount '%s' on '%s' (optional): %s",
fsname, target, strerror(errno));
@@ -1845,7 +1846,7 @@ static inline int mount_entry_on_systemfs(struct mntent *mntent)
}
ret = mount_entry(mntent->mnt_fsname, mntent->mnt_dir,
- mntent->mnt_type, mntflags, mntdata, optional);
+ mntent->mnt_type, mntflags, mntdata, optional, NULL);
free(pathdirname);
free(mntdata);
@@ -1932,7 +1933,7 @@ skipabs:
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs->mount);
free(mntdata);
@@ -1988,7 +1989,7 @@ static int mount_entry_on_relative_rootfs(struct mntent *mntent,
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs);
free(pathdirname);
free(mntdata);
@@ -3688,7 +3689,7 @@ void lxc_execute_bind_init(struct lxc_conf *conf)
fclose(pathfile);
}
- ret = mount(path, destpath, "none", MS_BIND, NULL);
+ ret = safe_mount(path, destpath, "none", MS_BIND, NULL, conf->rootfs.mount);
if (ret < 0)
SYSERROR("Failed to bind lxc.init.static into container");
INFO("lxc.init.static bound into container at %s", path);
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 1df6e8f..68081b5 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1656,3 +1656,236 @@ int setproctitle(char *title)
return ret;
}
+
+/*
+ * @path: a pathname where / replaced with '\0'.
+ * @offsetp: pointer to int showing which path segment was last seen.
+ * Updated on return to reflect the next segment.
+ * @fulllen: full original path length.
+ * Returns a pointer to the next path segment, or NULL if done.
+ */
+static char *get_nextpath(char *path, int *offsetp, int fulllen)
+{
+ int offset = *offsetp;
+
+ if (offset >= fulllen)
+ return NULL;
+
+ while (path[offset] != '\0' && offset < fulllen)
+ offset++;
+ while (path[offset] == '\0' && offset < fulllen)
+ offset++;
+
+ *offsetp = offset;
+ return (offset < fulllen) ? &path[offset] : NULL;
+}
+
+/*
+ * Check that @subdir is a subdir of @dir. @len is the length of
+ * @dir (to avoid having to recalculate it).
+ */
+static bool is_subdir(const char *subdir, const char *dir, size_t len)
+{
+ size_t subdirlen = strlen(subdir);
+
+ if (subdirlen < len)
+ return false;
+ if (strncmp(subdir, dir, len) != 0)
+ return false;
+ if (dir[len-1] == '/')
+ return true;
+ if (subdir[len] == '/' || subdirlen == len)
+ return true;
+ return false;
+}
+
+/*
+ * Check if the open fd is a symlink. Return -ELOOP if it is. Return
+ * -ENOENT if we couldn't fstat. Return 0 if the fd is ok.
+ */
+static int check_symlink(int fd)
+{
+ struct stat sb;
+ int ret = fstat(fd, &sb);
+ if (ret < 0)
+ return -ENOENT;
+ if (S_ISLNK(sb.st_mode))
+ return -ELOOP;
+ return 0;
+}
+
+/*
+ * Open a file or directory, provided that it contains no symlinks.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+static int open_if_safe(int dirfd, const char *nextpath)
+{
+ int newfd = openat(dirfd, nextpath, O_RDONLY | O_NOFOLLOW);
+ if (newfd >= 0) // was not a symlink, all good
+ return newfd;
+
+ if (errno == ELOOP)
+ return newfd;
+
+ if (errno == EPERM || errno == EACCES) {
+ /* we're not root (cause we got EPERM) so
+ try opening with O_PATH */
+ newfd = openat(dirfd, nextpath, O_PATH | O_NOFOLLOW);
+ if (newfd >= 0) {
+ /* O_PATH will return an fd for symlinks. We know
+ * nextpath wasn't a symlink at last openat, so if fd
+ * is now a link, then something * fishy is going on
+ */
+ int ret = check_symlink(newfd);
+ if (ret < 0) {
+ close(newfd);
+ newfd = ret;
+ }
+ }
+ }
+
+ return newfd;
+}
+
+/*
+ * Open a path intending for mounting, ensuring that the final path
+ * is inside the container's rootfs.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ *
+ * @target: path to be opened
+ * @prefix_skip: a part of @target in which to ignore symbolic links. This
+ * would be the container's rootfs.
+ *
+ * Return an open fd for the path, or <0 on error.
+ */
+static int open_without_symlink(const char *target, const char *prefix_skip)
+{
+ int curlen = 0, dirfd, fulllen, i;
+ char *dup = NULL;
+
+ fulllen = strlen(target);
+
+ /* make sure prefix-skip makes sense */
+ if (prefix_skip) {
+ curlen = strlen(prefix_skip);
+ if (!is_subdir(target, prefix_skip, curlen)) {
+ ERROR("WHOA there - target '%s' didn't start with prefix '%s'",
+ target, prefix_skip);
+ return -EINVAL;
+ }
+ /*
+ * get_nextpath() expects the curlen argument to be
+ * on a (turned into \0) / or before it, so decrement
+ * curlen to make sure that happens
+ */
+ if (curlen)
+ curlen--;
+ } else {
+ prefix_skip = "/";
+ curlen = 0;
+ }
+
+ /* Make a copy of target which we can hack up, and tokenize it */
+ if ((dup = strdup(target)) == NULL) {
+ SYSERROR("Out of memory checking for symbolic link");
+ return -ENOMEM;
+ }
+ for (i = 0; i < fulllen; i++) {
+ if (dup[i] == '/')
+ dup[i] = '\0';
+ }
+
+ dirfd = open(prefix_skip, O_RDONLY);
+ if (dirfd < 0)
+ goto out;
+ while (1) {
+ int newfd, saved_errno;
+ char *nextpath;
+
+ if ((nextpath = get_nextpath(dup, &curlen, fulllen)) == NULL)
+ goto out;
+ newfd = open_if_safe(dirfd, nextpath);
+ saved_errno = errno;
+ close(dirfd);
+ dirfd = newfd;
+ if (newfd < 0) {
+ errno = saved_errno;
+ if (errno == ELOOP)
+ SYSERROR("%s in %s was a symbolic link!", nextpath, target);
+ else
+ SYSERROR("Error examining %s in %s", nextpath, target);
+ goto out;
+ }
+ }
+
+out:
+ free(dup);
+ return dirfd;
+}
+
+/*
+ * Safely mount a path into a container, ensuring that the mount target
+ * is under the container's @rootfs. (If @rootfs is NULL, then the container
+ * uses the host's /)
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs)
+{
+ int srcfd = -1, destfd, ret, saved_errno;
+ char srcbuf[50], destbuf[50]; // only needs enough for /proc/self/fd/<fd>
+ const char *mntsrc = src;
+
+ if (!rootfs)
+ rootfs = "";
+
+ /* todo - allow symlinks for relative paths if 'allowsymlinks' option is passed */
+ if (flags & MS_BIND && src && src[0] != '/') {
+ INFO("this is a relative bind mount");
+ srcfd = open_without_symlink(src, NULL);
+ if (srcfd < 0)
+ return srcfd;
+ ret = snprintf(srcbuf, 50, "/proc/self/fd/%d", srcfd);
+ if (ret < 0 || ret > 50) {
+ close(srcfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+ mntsrc = srcbuf;
+ }
+
+ destfd = open_without_symlink(dest, rootfs);
+ if (destfd < 0) {
+ if (srcfd != -1)
+ close(srcfd);
+ return destfd;
+ }
+
+ ret = snprintf(destbuf, 50, "/proc/self/fd/%d", destfd);
+ if (ret < 0 || ret > 50) {
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+
+ ret = mount(mntsrc, destbuf, fstype, flags, data);
+ saved_errno = errno;
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ if (ret < 0) {
+ errno = saved_errno;
+ SYSERROR("Failed to mount %s onto %s", src, dest);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index cc18906..34ff5b0 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -287,3 +287,5 @@ bool switch_to_ns(pid_t pid, const char *ns);
int is_dir(const char *path);
char *get_template_path(const char *t);
int setproctitle(char *title);
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs);
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 461d869..8af9baa 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -54,6 +54,7 @@ if DISTRO_UBUNTU
bin_SCRIPTS += \
lxc-test-apparmor-mount \
lxc-test-checkpoint-restore \
+ lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
lxc-test-usernic
@@ -80,6 +81,7 @@ EXTRA_DIST = \
lxc-test-checkpoint-restore \
lxc-test-cloneconfig \
lxc-test-createconfig \
+ lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
may_control.c \
diff --git a/src/tests/lxc-test-symlink b/src/tests/lxc-test-symlink
new file mode 100644
index 0000000..37320f0
--- /dev/null
+++ b/src/tests/lxc-test-symlink
@@ -0,0 +1,88 @@
+#!/bin/bash
+
+set -ex
+
+# lxc: linux Container library
+
+# Authors:
+# Serge Hallyn <serge.hal...@ubuntu.com>
+#
+# This is a regression test for symbolic links
+
+dirname=`mktemp -d`
+fname=`mktemp`
+fname2=`mktemp`
+
+lxcpath=/var/lib/lxcsym1
+
+cleanup() {
+ lxc-destroy -P $lxcpath -f -n symtest1 || true
+ rm -f $lxcpath
+ rmdir $dirname || true
+ rm -f $fname || true
+ rm -f $fname2 || true
+}
+
+trap cleanup EXIT SIGHUP SIGINT SIGTERM
+
+testrun() {
+ expected=$1
+ run=$2
+ pass="pass"
+ lxc-start -P $lxcpath -n symtest1 -l trace -o $lxcpath/log || pass="fail"
+ [ $pass = "pass" ] && lxc-wait -P $lxcpath -n symtest1 -t 10 -s RUNNING || pass="fail"
+ if [ "$pass" != "$expected" ]; then
+ echo "Test $run: expected $expected but container did not. Start log:"
+ cat $lxcpath/log
+ echo "FAIL: Test $run: expected $expected but container did not."
+ false
+ fi
+ lxc-stop -P $lxcpath -n symtest1 -k || true
+}
+
+# make lxcpath a symlink - this should NOT cause failure
+ln -s /var/lib/lxc $lxcpath
+
+lxc-destroy -P $lxcpath -f -n symtest1 || true
+lxc-create -P $lxcpath -t busybox -n symtest1
+
+cat >> /var/lib/lxc/symtest1/config << EOF
+lxc.mount.entry = $dirname opt/xxx/dir none bind,create=dir
+lxc.mount.entry = $fname opt/xxx/file none bind,create=file
+lxc.mount.entry = $fname2 opt/xxx/file2 none bind
+EOF
+
+# Regular - should succeed
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun pass 1
+
+# symlink - should fail
+rm -rf /var/lib/lxc/symtest1/rootfs/opt/xxx
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx2
+ln -s /var/lib/lxc/symtest1/rootfs/opt/xxx2 /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun fail 2
+
+# final final symlink - should fail
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx/dir
+touch $lxcpath/symtest1/rootfs/opt/xxx/file
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2src
+ln -s $lxcpath/symtest1/rootfs/opt/xxx/file2src $lxcpath/symtest1/rootfs/opt/xxx/file2
+testrun fail 3
+
+# Ideally we'd also try a loop device, but that won't work in nested containers
+# anyway - TODO
+
+# what about /proc itself
+
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2
+mv $lxcpath/symtest1/rootfs/proc $lxcpath/symtest1/rootfs/proc1
+ln -s $lxcpath/symtest1/rootfs/proc1 $lxcpath/symtest1/rootfs/proc
+testrun fail 4
+
+echo "all tests passed"
--
2.5.0
From 1f9cc4943b640d9355709432a705e5fa6e9ad4df Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hal...@ubuntu.com>
Date: Mon, 31 Aug 2015 12:57:20 -0500
Subject: [PATCH 1/1] Protect container mounts against symlinks
When a container starts up, lxc sets up the container's inital fstree
by doing a bunch of mounting, guided by the container configuration
file. The container config is owned by the admin or user on the host,
so we do not try to guard against bad entries. However, since the
mount target is in the container, it's possible that the container admin
could divert the mount with symbolic links. This could bypass proper
container startup (i.e. confinement of a root-owned container by the
restrictive apparmor policy, by diverting the required write to
/proc/self/attr/current), or bypass the (path-based) apparmor policy
by diverting, say, /proc to /mnt in the container.
To prevent this,
1. do not allow mounts to paths containing symbolic links
2. do not allow bind mounts from relative paths containing symbolic
links.
Details:
Define safe_mount which ensures that the container has not inserted any
symbolic links into any mount targets for mounts to be done during
container setup.
The host's mount path may contain symbolic links. As it is under the
control of the administrator, that's ok. So safe_mount begins the check
for symbolic links after the rootfs->mount, by opening that directory.
It opens each directory along the path using openat() relative to the
parent directory using O_NOFOLLOW. When the target is reached, it
mounts onto /proc/self/fd/<targetfd>.
Use safe_mount() in mount_entry(), when mounting container proc,
and when needed. In particular, safe_mount() need not be used in
any case where:
1. the mount is done in the container's namespace
2. the mount is for the container's rootfs
3. the mount is relative to a tmpfs or proc/sysfs which we have
just safe_mount()ed ourselves
Since we were using proc/net as a temporary placeholder for /proc/sys/net
during container startup, and proc/net is a symbolic link, use proc/tty
instead.
Update the lxc.container.conf manpage with details about the new
restrictions.
Finally, add a testcase to test some symbolic link possibilities.
Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
---
doc/lxc.container.conf.sgml.in | 12 +++
src/lxc/cgfs.c | 5 +-
src/lxc/cgmanager.c | 4 +-
src/lxc/conf.c | 29 ++---
src/lxc/utils.c | 235 ++++++++++++++++++++++++++++++++++++++++-
src/lxc/utils.h | 2 +
src/tests/Makefile.am | 2 +
src/tests/lxc-test-symlink | 88 +++++++++++++++
8 files changed, 359 insertions(+), 18 deletions(-)
create mode 100644 src/tests/lxc-test-symlink
diff --git a/doc/lxc.container.conf.sgml.in b/doc/lxc.container.conf.sgml.in
index 50c6a2a..0a1ec5f 100644
--- a/doc/lxc.container.conf.sgml.in
+++ b/doc/lxc.container.conf.sgml.in
@@ -699,6 +699,18 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
container. This is useful to mount /etc, /var or /home for
examples.
</para>
+ <para>
+ NOTE - LXC will generally ensure that mount targets and relative
+ bind-mount sources are properly confined under the container
+ root, to avoid attacks involving over-mounting host directories
+ and files. (Symbolic links in absolute mount sources are ignored)
+ However, if the container configuration first mounts a directory which
+ is under the control of the container user, such as /home/joe, into
+ the container at some <filename>path</filename>, and then mounts
+ under <filename>path</filename>, then a TOCTTOU attack would be
+ possible where the container user modifies a symbolic link under
+ his home directory at just the right time.
+ </para>
<variablelist>
<varlistentry>
<term>
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index fcb3cde..df2e6b2 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -1363,7 +1363,10 @@ static bool cgroupfs_mount_cgroup(void *hdata, const char *root, int type)
if (!path)
return false;
snprintf(path, bufsz, "%s/sys/fs/cgroup", root);
- r = mount("cgroup_root", path, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME, "size=10240k,mode=755");
+ r = safe_mount("cgroup_root", path, "tmpfs",
+ MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME,
+ "size=10240k,mode=755",
+ root);
if (r < 0) {
SYSERROR("could not mount tmpfs to /sys/fs/cgroup in the container");
return false;
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index c143bea..779a1d8 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -1477,7 +1477,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
}
/* mount a tmpfs there so we can create subdirs */
- if (mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755")) {
+ if (safe_mount("cgroup", cgpath, "tmpfs", 0, "size=10000,mode=755", root)) {
SYSERROR("Failed to mount tmpfs at %s", cgpath);
return false;
}
@@ -1488,7 +1488,7 @@ static bool cgm_bind_dir(const char *root, const char *dirname)
return false;
}
- if (mount(dirname, cgpath, "none", MS_BIND, 0)) {
+ if (safe_mount(dirname, cgpath, "none", MS_BIND, 0, root)) {
SYSERROR("Failed to bind mount %s to %s", dirname, cgpath);
return false;
}
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index d37112b..8cff919 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -763,10 +763,11 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
* 2.6.32...
*/
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "proc", "%r/proc", "proc", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL },
- { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net", "%r/proc/net", NULL, MS_BIND, NULL },
+ /* proc/tty is used as a temporary placeholder for proc/sys/net which we'll move back in a few steps */
+ { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys/net", "%r/proc/tty", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sys", "%r/proc/sys", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, NULL, "%r/proc/sys", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL },
- { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/net", "%r/proc/sys/net", NULL, MS_MOVE, NULL },
+ { LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/tty", "%r/proc/sys/net", NULL, MS_MOVE, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, "%r/proc/sysrq-trigger", "%r/proc/sysrq-trigger", NULL, MS_BIND, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_MIXED, NULL, "%r/proc/sysrq-trigger", NULL, MS_REMOUNT|MS_BIND|MS_RDONLY, NULL },
{ LXC_AUTO_PROC_MASK, LXC_AUTO_PROC_RW, "proc", "%r/proc", "proc", MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL },
@@ -809,7 +810,7 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
}
mflags = add_required_remount_flags(source, destination,
default_mounts[i].flags);
- r = mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options);
+ r = safe_mount(source, destination, default_mounts[i].fstype, mflags, default_mounts[i].options, conf->rootfs.path ? conf->rootfs.mount : NULL);
saved_errno = errno;
if (r < 0 && errno == ENOENT) {
INFO("Mount source or target for %s on %s doesn't exist. Skipping.", source, destination);
@@ -1161,7 +1162,7 @@ static int mount_autodev(const char *name, char *root, const char *lxcpath)
return 0;
}
- if (mount("none", path, "tmpfs", 0, "size=100000,mode=755")) {
+ if (safe_mount("none", path, "tmpfs", 0, "size=100000,mode=755", root)) {
SYSERROR("Failed mounting tmpfs onto %s\n", path);
return false;
}
@@ -1246,7 +1247,7 @@ static int fill_autodev(const char *root)
return -1;
}
fclose(pathfile);
- if (mount(hostpath, path, 0, MS_BIND, NULL) != 0) {
+ if (safe_mount(hostpath, path, 0, MS_BIND, NULL, root) != 0) {
SYSERROR("Failed bind mounting device %s from host into container",
d->name);
return -1;
@@ -1499,7 +1500,7 @@ static int setup_dev_console(const struct lxc_rootfs *rootfs,
return -1;
}
- if (mount(console->name, path, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, path, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, path);
return -1;
}
@@ -1554,7 +1555,7 @@ static int setup_ttydir_console(const struct lxc_rootfs *rootfs,
return 0;
}
- if (mount(console->name, lxcpath, "none", MS_BIND, 0)) {
+ if (safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs->mount)) {
ERROR("failed to mount '%s' on '%s'", console->name, lxcpath);
return -1;
}
@@ -1704,13 +1705,13 @@ static char *get_field(char *src, int nfields)
static int mount_entry(const char *fsname, const char *target,
const char *fstype, unsigned long mountflags,
- const char *data, int optional)
+ const char *data, int optional, const char *rootfs)
{
#ifdef HAVE_STATVFS
struct statvfs sb;
#endif
- if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) {
+ if (safe_mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data, rootfs)) {
if (optional) {
INFO("failed to mount '%s' on '%s' (optional): %s", fsname,
target, strerror(errno));
@@ -1757,7 +1758,7 @@ static int mount_entry(const char *fsname, const char *target,
#endif
if (mount(fsname, target, fstype,
- mountflags | MS_REMOUNT, data)) {
+ mountflags | MS_REMOUNT, data) < 0) {
if (optional) {
INFO("failed to mount '%s' on '%s' (optional): %s",
fsname, target, strerror(errno));
@@ -1843,7 +1844,7 @@ static inline int mount_entry_on_systemfs(struct mntent *mntent)
}
ret = mount_entry(mntent->mnt_fsname, mntent->mnt_dir,
- mntent->mnt_type, mntflags, mntdata, optional);
+ mntent->mnt_type, mntflags, mntdata, optional, NULL);
free(pathdirname);
free(mntdata);
@@ -1930,7 +1931,7 @@ skipabs:
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs->mount);
free(mntdata);
@@ -1986,7 +1987,7 @@ static int mount_entry_on_relative_rootfs(struct mntent *mntent,
}
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type,
- mntflags, mntdata, optional);
+ mntflags, mntdata, optional, rootfs);
free(pathdirname);
free(mntdata);
@@ -3646,7 +3647,7 @@ void lxc_execute_bind_init(struct lxc_conf *conf)
fclose(pathfile);
}
- ret = mount(path, destpath, "none", MS_BIND, NULL);
+ ret = safe_mount(path, destpath, "none", MS_BIND, NULL, conf->rootfs.mount);
if (ret < 0)
SYSERROR("Failed to bind lxc.init.static into container");
INFO("lxc.init.static bound into container at %s", path);
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 7ced314..70d12d5 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1403,6 +1403,239 @@ int setproctitle(char *title)
}
/*
+ * @path: a pathname where / replaced with '\0'.
+ * @offsetp: pointer to int showing which path segment was last seen.
+ * Updated on return to reflect the next segment.
+ * @fulllen: full original path length.
+ * Returns a pointer to the next path segment, or NULL if done.
+ */
+static char *get_nextpath(char *path, int *offsetp, int fulllen)
+{
+ int offset = *offsetp;
+
+ if (offset >= fulllen)
+ return NULL;
+
+ while (path[offset] != '\0' && offset < fulllen)
+ offset++;
+ while (path[offset] == '\0' && offset < fulllen)
+ offset++;
+
+ *offsetp = offset;
+ return (offset < fulllen) ? &path[offset] : NULL;
+}
+
+/*
+ * Check that @subdir is a subdir of @dir. @len is the length of
+ * @dir (to avoid having to recalculate it).
+ */
+static bool is_subdir(const char *subdir, const char *dir, size_t len)
+{
+ size_t subdirlen = strlen(subdir);
+
+ if (subdirlen < len)
+ return false;
+ if (strncmp(subdir, dir, len) != 0)
+ return false;
+ if (dir[len-1] == '/')
+ return true;
+ if (subdir[len] == '/' || subdirlen == len)
+ return true;
+ return false;
+}
+
+/*
+ * Check if the open fd is a symlink. Return -ELOOP if it is. Return
+ * -ENOENT if we couldn't fstat. Return 0 if the fd is ok.
+ */
+static int check_symlink(int fd)
+{
+ struct stat sb;
+ int ret = fstat(fd, &sb);
+ if (ret < 0)
+ return -ENOENT;
+ if (S_ISLNK(sb.st_mode))
+ return -ELOOP;
+ return 0;
+}
+
+/*
+ * Open a file or directory, provided that it contains no symlinks.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+static int open_if_safe(int dirfd, const char *nextpath)
+{
+ int newfd = openat(dirfd, nextpath, O_RDONLY | O_NOFOLLOW);
+ if (newfd >= 0) // was not a symlink, all good
+ return newfd;
+
+ if (errno == ELOOP)
+ return newfd;
+
+ if (errno == EPERM || errno == EACCES) {
+ /* we're not root (cause we got EPERM) so
+ try opening with O_PATH */
+ newfd = openat(dirfd, nextpath, O_PATH | O_NOFOLLOW);
+ if (newfd >= 0) {
+ /* O_PATH will return an fd for symlinks. We know
+ * nextpath wasn't a symlink at last openat, so if fd
+ * is now a link, then something * fishy is going on
+ */
+ int ret = check_symlink(newfd);
+ if (ret < 0) {
+ close(newfd);
+ newfd = ret;
+ }
+ }
+ }
+
+ return newfd;
+}
+
+/*
+ * Open a path intending for mounting, ensuring that the final path
+ * is inside the container's rootfs.
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ *
+ * @target: path to be opened
+ * @prefix_skip: a part of @target in which to ignore symbolic links. This
+ * would be the container's rootfs.
+ *
+ * Return an open fd for the path, or <0 on error.
+ */
+static int open_without_symlink(const char *target, const char *prefix_skip)
+{
+ int curlen = 0, dirfd, fulllen, i;
+ char *dup = NULL;
+
+ fulllen = strlen(target);
+
+ /* make sure prefix-skip makes sense */
+ if (prefix_skip) {
+ curlen = strlen(prefix_skip);
+ if (!is_subdir(target, prefix_skip, curlen)) {
+ ERROR("WHOA there - target '%s' didn't start with prefix '%s'",
+ target, prefix_skip);
+ return -EINVAL;
+ }
+ /*
+ * get_nextpath() expects the curlen argument to be
+ * on a (turned into \0) / or before it, so decrement
+ * curlen to make sure that happens
+ */
+ if (curlen)
+ curlen--;
+ } else {
+ prefix_skip = "/";
+ curlen = 0;
+ }
+
+ /* Make a copy of target which we can hack up, and tokenize it */
+ if ((dup = strdup(target)) == NULL) {
+ SYSERROR("Out of memory checking for symbolic link");
+ return -ENOMEM;
+ }
+ for (i = 0; i < fulllen; i++) {
+ if (dup[i] == '/')
+ dup[i] = '\0';
+ }
+
+ dirfd = open(prefix_skip, O_RDONLY);
+ if (dirfd < 0)
+ goto out;
+ while (1) {
+ int newfd, saved_errno;
+ char *nextpath;
+
+ if ((nextpath = get_nextpath(dup, &curlen, fulllen)) == NULL)
+ goto out;
+ newfd = open_if_safe(dirfd, nextpath);
+ saved_errno = errno;
+ close(dirfd);
+ dirfd = newfd;
+ if (newfd < 0) {
+ errno = saved_errno;
+ if (errno == ELOOP)
+ SYSERROR("%s in %s was a symbolic link!", nextpath, target);
+ else
+ SYSERROR("Error examining %s in %s", nextpath, target);
+ goto out;
+ }
+ }
+
+out:
+ free(dup);
+ return dirfd;
+}
+
+/*
+ * Safely mount a path into a container, ensuring that the mount target
+ * is under the container's @rootfs. (If @rootfs is NULL, then the container
+ * uses the host's /)
+ *
+ * CAVEAT: This function must not be used for other purposes than container
+ * setup before executing the container's init
+ */
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs)
+{
+ int srcfd = -1, destfd, ret, saved_errno;
+ char srcbuf[50], destbuf[50]; // only needs enough for /proc/self/fd/<fd>
+ const char *mntsrc = src;
+
+ if (!rootfs)
+ rootfs = "";
+
+ /* todo - allow symlinks for relative paths if 'allowsymlinks' option is passed */
+ if (flags & MS_BIND && src && src[0] != '/') {
+ INFO("this is a relative bind mount");
+ srcfd = open_without_symlink(src, NULL);
+ if (srcfd < 0)
+ return srcfd;
+ ret = snprintf(srcbuf, 50, "/proc/self/fd/%d", srcfd);
+ if (ret < 0 || ret > 50) {
+ close(srcfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+ mntsrc = srcbuf;
+ }
+
+ destfd = open_without_symlink(dest, rootfs);
+ if (destfd < 0) {
+ if (srcfd != -1)
+ close(srcfd);
+ return destfd;
+ }
+
+ ret = snprintf(destbuf, 50, "/proc/self/fd/%d", destfd);
+ if (ret < 0 || ret > 50) {
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ ERROR("Out of memory");
+ return -EINVAL;
+ }
+
+ ret = mount(mntsrc, destbuf, fstype, flags, data);
+ saved_errno = errno;
+ if (srcfd != -1)
+ close(srcfd);
+ close(destfd);
+ if (ret < 0) {
+ errno = saved_errno;
+ SYSERROR("Failed to mount %s onto %s", src, dest);
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
* Mount a proc under @rootfs if proc self points to a pid other than
* my own. This is needed to have a known-good proc mount for setting
* up LSMs both at container startup and attach.
@@ -1440,7 +1673,7 @@ int mount_proc_if_needed(const char *rootfs)
return 0;
domount:
- if (mount("proc", path, "proc", 0, NULL))
+ if (safe_mount("proc", path, "proc", 0, NULL, rootfs) < 0)
return -1;
INFO("Mounted /proc in container for security transition");
return 1;
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index ee12dde..059026f 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -279,6 +279,8 @@ bool switch_to_ns(pid_t pid, const char *ns);
int is_dir(const char *path);
char *get_template_path(const char *t);
int setproctitle(char *title);
+int safe_mount(const char *src, const char *dest, const char *fstype,
+ unsigned long flags, const void *data, const char *rootfs);
int mount_proc_if_needed(const char *rootfs);
int null_stdfds(void);
#endif /* __LXC_UTILS_H */
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 461d869..8af9baa 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -54,6 +54,7 @@ if DISTRO_UBUNTU
bin_SCRIPTS += \
lxc-test-apparmor-mount \
lxc-test-checkpoint-restore \
+ lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
lxc-test-usernic
@@ -80,6 +81,7 @@ EXTRA_DIST = \
lxc-test-checkpoint-restore \
lxc-test-cloneconfig \
lxc-test-createconfig \
+ lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
may_control.c \
diff --git a/src/tests/lxc-test-symlink b/src/tests/lxc-test-symlink
new file mode 100644
index 0000000..37320f0
--- /dev/null
+++ b/src/tests/lxc-test-symlink
@@ -0,0 +1,88 @@
+#!/bin/bash
+
+set -ex
+
+# lxc: linux Container library
+
+# Authors:
+# Serge Hallyn <serge.hal...@ubuntu.com>
+#
+# This is a regression test for symbolic links
+
+dirname=`mktemp -d`
+fname=`mktemp`
+fname2=`mktemp`
+
+lxcpath=/var/lib/lxcsym1
+
+cleanup() {
+ lxc-destroy -P $lxcpath -f -n symtest1 || true
+ rm -f $lxcpath
+ rmdir $dirname || true
+ rm -f $fname || true
+ rm -f $fname2 || true
+}
+
+trap cleanup EXIT SIGHUP SIGINT SIGTERM
+
+testrun() {
+ expected=$1
+ run=$2
+ pass="pass"
+ lxc-start -P $lxcpath -n symtest1 -l trace -o $lxcpath/log || pass="fail"
+ [ $pass = "pass" ] && lxc-wait -P $lxcpath -n symtest1 -t 10 -s RUNNING || pass="fail"
+ if [ "$pass" != "$expected" ]; then
+ echo "Test $run: expected $expected but container did not. Start log:"
+ cat $lxcpath/log
+ echo "FAIL: Test $run: expected $expected but container did not."
+ false
+ fi
+ lxc-stop -P $lxcpath -n symtest1 -k || true
+}
+
+# make lxcpath a symlink - this should NOT cause failure
+ln -s /var/lib/lxc $lxcpath
+
+lxc-destroy -P $lxcpath -f -n symtest1 || true
+lxc-create -P $lxcpath -t busybox -n symtest1
+
+cat >> /var/lib/lxc/symtest1/config << EOF
+lxc.mount.entry = $dirname opt/xxx/dir none bind,create=dir
+lxc.mount.entry = $fname opt/xxx/file none bind,create=file
+lxc.mount.entry = $fname2 opt/xxx/file2 none bind
+EOF
+
+# Regular - should succeed
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun pass 1
+
+# symlink - should fail
+rm -rf /var/lib/lxc/symtest1/rootfs/opt/xxx
+mkdir -p /var/lib/lxc/symtest1/rootfs/opt/xxx2
+ln -s /var/lib/lxc/symtest1/rootfs/opt/xxx2 /var/lib/lxc/symtest1/rootfs/opt/xxx
+touch /var/lib/lxc/symtest1/rootfs/opt/xxx/file2
+testrun fail 2
+
+# final final symlink - should fail
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx/dir
+touch $lxcpath/symtest1/rootfs/opt/xxx/file
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2src
+ln -s $lxcpath/symtest1/rootfs/opt/xxx/file2src $lxcpath/symtest1/rootfs/opt/xxx/file2
+testrun fail 3
+
+# Ideally we'd also try a loop device, but that won't work in nested containers
+# anyway - TODO
+
+# what about /proc itself
+
+rm -rf $lxcpath/symtest1/rootfs/opt/xxx
+mkdir -p $lxcpath/symtest1/rootfs/opt/xxx
+touch $lxcpath/symtest1/rootfs/opt/xxx/file2
+mv $lxcpath/symtest1/rootfs/proc $lxcpath/symtest1/rootfs/proc1
+ln -s $lxcpath/symtest1/rootfs/proc1 $lxcpath/symtest1/rootfs/proc
+testrun fail 4
+
+echo "all tests passed"
--
2.5.0
signature.asc
Description: Digital signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
