From a067d9467e4530ac0ba5487471944b279dcccca2 Mon Sep 17 00:00:00 2001 From: Serge Hallyn Date: Fri, 24 Jul 2015 23:06:38 +0000 Subject: [PATCH 1/1] Detect when mount entry targets are symbolic links as this can be used as a step in attacking the host. Changelog: Aug 7: make file mounts works Signed-off-by: Serge Hallyn --- src/lxc/conf.c | 257 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 246 insertions(+), 11 deletions(-) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 9870455..dc25732 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -1704,26 +1704,242 @@ static char *get_field(char *src, int nfields) return p; } +/* + * @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; +} + +/* + * Checks whether path intending for mounting is or contains a symlink. + * If so, return -1. + * If there is an error opening it, return -1. + * Otherwise returns an open fd for /a/b/c/d + */ +static int get_dir_fd_without_symlink(const char *target, const char *prefix_skip, bool skiplast) +{ + 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 (curlen) + curlen--; + if (strncmp(target, prefix_skip, curlen) != 0) { + ERROR("WHOA there - target '%s' didn't start with prefix '%s'", + target, prefix_skip); + return -EINVAL; + } + } else { + prefix_skip = "/"; + curlen = 0; + } + + if ((dup = strdup(target)) == NULL) { + SYSERROR("Out of memory checking for symbolic link"); + return -1; + } + + // tokenize + 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; + struct stat sb; + + if ((nextpath = get_nextpath(dup, &curlen, fulllen)) == NULL) + goto out; + if (fstatat(dirfd, nextpath, &sb, AT_SYMLINK_NOFOLLOW) < 0) { + SYSERROR("Error checking file type for %s in %s\n", nextpath, dup); + close(dirfd); + dirfd = -1; + goto out; + } + if (!S_ISDIR(sb.st_mode)) { + if (!skiplast) { + ERROR("race? Mount target switched from dir to non-dir\n"); + close(dirfd); + dirfd = -1; + } + /* this is not a directory so pass back the parent dir */ + goto out; + } + newfd = openat(dirfd, nextpath, O_RDONLY | O_NOFOLLOW); + saved_errno = errno; + close(dirfd); + dirfd = -1; + 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; + } + dirfd = newfd; + } + +out: + free(dup); + return dirfd; +} +/* + * let's say we are passed target = "/lxc/a/b/c". /lxc is a + * symlink to /var/lib/lxc. If /a/b/c does not container any + * links, then we are happy. If a, b, or c is a symlink, return + * an error. + * + * We check this by making sure that /proc/self/mountinfo's last + * entry contains "$lxcpath . /a/b/c". + */ + +static char *next_word(char *ws) { + while (*ws && *ws != ' ') ws++; + while (*ws && *ws == ' ') ws++; + return ws; +} + +static bool ensure_not_symlink(const char *target, const char *prefix_skip) +{ + FILE *f = fopen("/proc/self/mountinfo", "r"); + char *line = NULL, *ws = NULL, *we = NULL; + size_t len = 0, i; + bool ret = false; + + if (!f) { + ERROR("Cannot open /proc/self/mountinfo"); + return false; + } + + while (getline(&line, &len, f) != -1) { + } + fclose(f); + + if (!line) + return false; + ws = line; + for (i = 0; i < 4; i++) + ws = next_word(ws); + if (!*ws) + goto out; + we = ws; + while (*we && *we != ' ') + we++; + if (!*we) + goto out; + *we = '\0'; + + /* now make sure that ws starts with prefix_skip and ends with rest of target */ + if (prefix_skip && strncmp(ws, prefix_skip, strlen(prefix_skip)) != 0) { + ERROR("Mount onto %s resulted in %s\n", target, ws); + goto out; + } + + size_t start = prefix_skip ? strlen(prefix_skip) : 0; + if (strcmp(ws + start, target + start) != 0) { + ERROR("Mount onto %s resulted in %s\n", target, ws); + goto out; + } + + ret = true; + +out: + free(line); + return ret; +} + 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 *prefix_skip) { + char *mounttarget; + int targetfd, oldfd, ret = 0; + bool isdir; #ifdef HAVE_STATVFS struct statvfs sb; #endif - if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) { + isdir = is_dir(target); + targetfd = get_dir_fd_without_symlink(target, prefix_skip, !isdir); + if (targetfd == -2) { + ERROR("error detecting whether mount target '%s' is a symlink", target); + if (optional) + return 0; + return -1; + } else if (targetfd == -1) { + ERROR("mount target '%s' is a symlink: attack by container admin?", target); + return -1; + } + + oldfd = open(".", O_RDONLY); + if (oldfd < 0) { + SYSERROR("Error recording current directory"); + close(targetfd); + return -1; + } + + if (fchdir(targetfd) < 0) { + SYSERROR("Error switching to mount target directory '%s'", target); + close(oldfd); + return -1; + } + + if (isdir) + mounttarget = "."; + else { + mounttarget = rindex(target, '/'); + if (!mounttarget) + return -1; + mounttarget++; + } + + if (mount(fsname, mounttarget, fstype, mountflags & ~MS_REMOUNT, data)) { if (optional) { INFO("failed to mount '%s' on '%s' (optional): %s", fsname, target, strerror(errno)); - return 0; + ret = 0; + goto out; } else { SYSERROR("failed to mount '%s' on '%s'", fsname, target); - return -1; + ret = 0; + goto out; } } + if (!ensure_not_symlink(target, prefix_skip)) { + ERROR("Mounted '%s' onto a symlink in the container: symlink attack?", target); + umount2(target, MNT_DETACH); + return -1; + } + if ((mountflags & MS_REMOUNT) || (mountflags & MS_BIND)) { DEBUG("remounting %s on %s to respect bind or remount options", fsname ? fsname : "(none)", target ? target : "(none)"); @@ -1751,6 +1967,7 @@ static int mount_entry(const char *fsname, const char *target, if (!(required_flags & ~mountflags) && rqd_flags == 0) { DEBUG("mountflags already was %lu, skipping remount", mountflags); + ret = 0; goto skipremount; } } @@ -1758,17 +1975,19 @@ static int mount_entry(const char *fsname, const char *target, } #endif - if (mount(fsname, target, fstype, + if (mount(fsname, mounttarget, fstype, mountflags | MS_REMOUNT, data)) { if (optional) { INFO("failed to mount '%s' on '%s' (optional): %s", fsname, target, strerror(errno)); - return 0; + ret = 0; + goto out; } else { SYSERROR("failed to mount '%s' on '%s'", fsname, target); - return -1; + ret = -1; + goto out; } } } @@ -1776,9 +1995,13 @@ static int mount_entry(const char *fsname, const char *target, #ifdef HAVE_STATVFS skipremount: #endif +out: DEBUG("mounted '%s' on '%s', type '%s'", fsname, target, fstype); + if (fchdir(oldfd) < 0) + SYSERROR("Warning: error returning to old directory"); + close(oldfd); - return 0; + return ret; } /* @@ -1806,6 +2029,10 @@ static void cull_mntent_opt(struct mntent *mntent) } } +/* + * this is called when mounting into a container which doesn't + * have an explicit lxc.rootfs. + */ static inline int mount_entry_on_systemfs(struct mntent *mntent) { unsigned long mntflags; @@ -1845,7 +2072,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); @@ -1853,6 +2080,10 @@ static inline int mount_entry_on_systemfs(struct mntent *mntent) return ret; } +/* + * This is called when a lxc.mount.entry specifies a full path + * (i.e. /var/lib/lxc/u1/rootfs/mnt) as the mount target + */ static int mount_entry_on_absolute_rootfs(struct mntent *mntent, const struct lxc_rootfs *rootfs, const char *lxc_name) @@ -1932,7 +2163,7 @@ skipabs: } ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, - mntflags, mntdata, optional); + mntflags, mntdata, optional, rootfs->mount); free(mntdata); @@ -1941,6 +2172,10 @@ out: return ret; } +/* + * This is called when a lxc.mount.entry specifies a relative + * path (i.e. home/user) as the mount target + */ static int mount_entry_on_relative_rootfs(struct mntent *mntent, const char *rootfs) { @@ -1988,7 +2223,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); -- 2.5.0