235 changes: 234 additions & 1 deletion src/lxc/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,239 @@ 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;
}

/*
* 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
Expand Down Expand Up @@ -1446,7 +1679,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;
Expand Down
2 changes: 2 additions & 0 deletions src/lxc/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
2 changes: 2 additions & 0 deletions src/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ bin_SCRIPTS += \
lxc-test-apparmor-mount \
lxc-test-checkpoint-restore \
lxc-test-snapdeps \
lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
lxc-test-usernic
Expand Down Expand Up @@ -82,6 +83,7 @@ EXTRA_DIST = \
lxc-test-cloneconfig \
lxc-test-createconfig \
lxc-test-snapdeps \
lxc-test-symlink \
lxc-test-ubuntu \
lxc-test-unpriv \
may_control.c \
Expand Down
88 changes: 88 additions & 0 deletions src/tests/lxc-test-symlink
Original file line number Diff line number Diff line change
@@ -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"