Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Maintenance:3954
lxc.openSUSE_13.2_Update
CVE-2015-1335-Protect-container-mounts-against-...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2015-1335-Protect-container-mounts-against-symlinks.patch of Package lxc.openSUSE_13.2_Update
From cd8a89eb7e991442afb4cfb879fb2eb59ffd6f86 Mon Sep 17 00:00:00 2001 From: Serge Hallyn <serge.hallyn@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.hallyn@ubuntu.com> --- doc/lxc.container.conf.sgml.in | 12 +++ src/lxc/cgfs.c | 5 +- src/lxc/cgmanager.c | 4 +- src/lxc/conf.c | 35 +++--- src/lxc/utils.c | 235 ++++++++++++++++++++++++++++++++++++++++- src/lxc/utils.h | 2 + src/tests/Makefile.am | 2 + src/tests/lxc-test-symlink | 88 +++++++++++++++ 8 files changed, 363 insertions(+), 20 deletions(-) create mode 100755 src/tests/lxc-test-symlink Index: lxc-1.0.6/doc/lxc.container.conf.sgml.in =================================================================== --- lxc-1.0.6.orig/doc/lxc.container.conf.sgml.in +++ lxc-1.0.6/doc/lxc.container.conf.sgml.in @@ -676,6 +676,18 @@ Foundation, Inc., 51 Franklin Street, Fi 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> Index: lxc-1.0.6/src/lxc/cgfs.c =================================================================== --- lxc-1.0.6.orig/src/lxc/cgfs.c +++ lxc-1.0.6/src/lxc/cgfs.c @@ -1363,7 +1363,10 @@ static bool cgroupfs_mount_cgroup(void * 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; Index: lxc-1.0.6/src/lxc/cgmanager.c =================================================================== --- lxc-1.0.6.orig/src/lxc/cgmanager.c +++ lxc-1.0.6/src/lxc/cgmanager.c @@ -1262,7 +1262,7 @@ static bool cgm_bind_dir(const char *roo } /* 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; } @@ -1273,7 +1273,7 @@ static bool cgm_bind_dir(const char *roo 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; } Index: lxc-1.0.6/src/lxc/conf.c =================================================================== --- lxc-1.0.6.orig/src/lxc/conf.c +++ lxc-1.0.6/src/lxc/conf.c @@ -791,7 +791,7 @@ static int lxc_mount_auto_mounts(struct } 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); @@ -1438,16 +1438,16 @@ static int mount_autodev(const char *nam 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) { @@ -1792,7 +1792,7 @@ static int setup_dev_console(const struc 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; } @@ -1847,7 +1847,7 @@ static int setup_ttydir_console(const st 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; } @@ -1997,13 +1997,13 @@ static char *get_field(char *src, int nf 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)); @@ -2048,7 +2048,7 @@ static int mount_entry(const char *fsnam #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)); @@ -2134,7 +2134,7 @@ static inline int mount_entry_on_systemf } 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); @@ -2221,7 +2221,7 @@ skipabs: } ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, - mntflags, mntdata, optional); + mntflags, mntdata, optional, rootfs->mount); free(mntdata); @@ -2277,7 +2277,7 @@ static int mount_entry_on_relative_rootf } ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, - mntflags, mntdata, optional); + mntflags, mntdata, optional, rootfs); free(pathdirname); free(mntdata); Index: lxc-1.0.6/src/lxc/utils.c =================================================================== --- lxc-1.0.6.orig/src/lxc/utils.c +++ lxc-1.0.6/src/lxc/utils.c @@ -1169,6 +1169,239 @@ bool dir_exists(const char *path) return S_ISDIR(sb.st_mode); } + /* + * @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; +} + /* Note we don't use SHA-1 here as we don't want to depend on HAVE_GNUTLS. * FNV has good anti collision properties and we're not worried * about pre-image resistance or one-way-ness, we're just trying to make Index: lxc-1.0.6/src/lxc/utils.h =================================================================== --- lxc-1.0.6.orig/src/lxc/utils.h +++ lxc-1.0.6/src/lxc/utils.h @@ -273,6 +273,9 @@ extern uid_t get_ns_uid(uid_t orig); extern bool dir_exists(const char *path); +int safe_mount(const char *src, const char *dest, const char *fstype, + unsigned long flags, const void *data, const char *rootfs); + #define FNV1A_64_INIT ((uint64_t)0xcbf29ce484222325ULL) uint64_t fnv_64a_buf(void *buf, size_t len, uint64_t hval); #endif Index: lxc-1.0.6/src/tests/Makefile.am =================================================================== --- lxc-1.0.6.orig/src/tests/Makefile.am +++ lxc-1.0.6/src/tests/Makefile.am @@ -71,6 +71,7 @@ EXTRA_DIST = \ locktests.c \ lxcpath.c \ lxc-test-autostart \ + lxc-test-symlink \ lxc-test-ubuntu \ lxc-test-unpriv \ lxc-test-usernic \ Index: lxc-1.0.6/src/tests/lxc-test-symlink =================================================================== --- /dev/null +++ lxc-1.0.6/src/tests/lxc-test-symlink @@ -0,0 +1,88 @@ +#!/bin/bash + +set -ex + +# lxc: linux Container library + +# Authors: +# Serge Hallyn <serge.hallyn@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" Index: lxc-1.0.6/src/tests/Makefile.in =================================================================== --- lxc-1.0.6.orig/src/tests/Makefile.in +++ lxc-1.0.6/src/tests/Makefile.in @@ -563,6 +563,7 @@ EXTRA_DIST = \ locktests.c \ lxcpath.c \ lxc-test-autostart \ + lxc-test-symlink \ lxc-test-ubuntu \ lxc-test-unpriv \ lxc-test-usernic \
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
