From cc1d05c12af0ec93392671a97815ac64ab935f25 Mon Sep 17 00:00:00 2001 From: Serge Hallyn Date: Wed, 21 Oct 2015 05:26:17 +0000 Subject: [PATCH 1/1] fix checking of parent dirs Signed-off-by: Serge Hallyn --- lxcfs.c | 128 ++++++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 43 deletions(-) Index: lxcfs-0.10/lxcfs.c =================================================================== --- lxcfs-0.10.orig/lxcfs.c +++ lxcfs-0.10/lxcfs.c @@ -209,6 +209,12 @@ static bool perms_include(int fmode, mod return ((fmode & r) == r); } + +/* + * taskcg is a/b/c + * querycg is /a/b/c/d/e + * we return 'd' + */ static char *get_next_cgroup_dir(const char *taskcg, const char *querycg) { char *start, *end; @@ -292,17 +298,11 @@ static void prune_init_slice(char *cg) } } -/* - * If caller is in /a/b/c/d, he may only act on things under cg=/a/b/c/d. - * If caller is in /a, he may act on /a/b, but not on /b. - * if the answer is false and nextcg is not NULL, then *nextcg will point - * to a nih_alloc'd string containing the next cgroup directory under cg - */ -static bool caller_is_in_ancestor(pid_t pid, const char *contrl, const char *cg, char **nextcg) +static char *get_pid_cgroup(pid_t pid, const char *contrl) { nih_local char *fnam = NULL; FILE *f; - bool answer = false; + char *answer = NULL; char *line = NULL; size_t len = 0; @@ -311,7 +311,7 @@ static bool caller_is_in_ancestor(pid_t return false; while (getline(&line, &len, f) != -1) { - char *c1, *c2, *linecmp; + char *c1, *c2; if (!line[0]) continue; c1 = strchr(line, ':'); @@ -326,18 +326,7 @@ static bool caller_is_in_ancestor(pid_t continue; c2++; stripnewline(c2); - prune_init_slice(c2); - /* - * callers pass in '/' for root cgroup, otherwise they pass - * in a cgroup without leading '/' - */ - linecmp = *cg == '/' ? c2 : c2+1; - if (strncmp(linecmp, cg, strlen(linecmp)) != 0) { - if (nextcg) - *nextcg = get_next_cgroup_dir(linecmp, cg); - goto out; - } - answer = true; + answer = NIH_MUST( nih_strdup(NULL, c2) ); goto out; } @@ -348,6 +337,73 @@ out: } /* + * If caller is in /a/b/c/d, he may only act on things under cg=/a/b/c/d. + * If caller is in /a, he may act on /a/b, but not on /b. + * if the answer is false and nextcg is not NULL, then *nextcg will point + * to a nih_alloc'd string containing the next cgroup directory under cg + */ +static bool caller_is_in_ancestor(pid_t pid, const char *contrl, const char *cg, char **nextcg) +{ + nih_local char *c2 = get_pid_cgroup(pid, contrl); + char *linecmp; + + if (!c2) + return false; + prune_init_slice(c2); + /* + * callers pass in '/' for root cgroup, otherwise they pass + * in a cgroup without leading '/' + */ + linecmp = *cg == '/' ? c2 : c2+1; + if (strncmp(linecmp, cg, strlen(linecmp)) != 0) { + if (nextcg) { + *nextcg = get_next_cgroup_dir(linecmp, cg); + } + return false; + } + return true; + +} + +/* + * If caller is in /a/b/c, he may see that /a exists, but not /b or /a/c. + */ +static bool caller_may_see_dir(pid_t pid, const char *contrl, const char *cg) +{ + nih_local char *c2 = NULL; + size_t l1, l2; + + if (strcmp(cg, "/") == 0) + return true; + + c2 = get_pid_cgroup(pid, contrl); + + if (!c2) + return false; + + char *tcg = c2 + 1; + l1 = strlen(cg); + l2 = strlen(tcg); + if (strcmp(cg, tcg) == 0) + return true; + + if (l1 < l2) { + /* looking up a parent dir */ + if (strncmp(tcg, cg, l1) == 0 && tcg[l1] == '/') + return true; + return false; + } + if (l1 > l2) { + /* looking up a child dir */ + if (strncmp(tcg, cg, l2) == 0 && cg[l2] == '/') + return true; + return false; + } + + return false; + } + +/* * given /cgroup/freezer/a/b, return "freezer". this will be nih-allocated * and needs to be nih_freed. */ @@ -518,6 +574,8 @@ static int cg_getattr(const char *path, * cgroup, or cgdir if fpath is a file */ if (is_child_cgroup(controller, path1, path2)) { + if (!caller_may_see_dir(fc->pid, controller, cgroup)) + return -ENOENT; if (!caller_is_in_ancestor(fc->pid, controller, cgroup, NULL)) { /* this is just /cgroup/controller, return it as a dir */ sb->st_mode = S_IFDIR | 00555; @@ -589,8 +647,12 @@ static int cg_opendir(const char *path, } } - if (cgroup && !fc_may_access(fc, controller, cgroup, NULL, O_RDONLY)) - return -EACCES; + if (cgroup) { + if (!caller_may_see_dir(fc->pid, controller, cgroup)) + return -ENOENT; + if (!fc_may_access(fc, controller, cgroup, NULL, O_RDONLY)) + return -EACCES; + } /* we'll free this at cg_releasedir */ dir_info = NIH_MUST( nih_alloc(NULL, sizeof(*dir_info)) ); @@ -717,6 +779,9 @@ static int cg_open(const char *path, str if (!k) return -EINVAL; + if (!caller_may_see_dir(fc->pid, controller, path1)) + return -ENOENT; + if (!fc_may_access(fc, controller, path1, path2, fi->flags)) // should never get here return -EACCES; @@ -1470,6 +1535,15 @@ int cg_mkdir(const char *path, mode_t mo else path1 = cgdir; + nih_local char *next = NULL; + bool ret = caller_is_in_ancestor(fc->pid, controller, path1, &next); + if (!ret ) { + //if (!fpath || strcmp(next, fpath) == 0) + if (fpath && strcmp(next, fpath) == 0) + return -EEXIST; + return -ENOENT; + } + if (!fc_may_access(fc, controller, path1, NULL, O_RDWR)) return -EACCES; @@ -1505,6 +1579,14 @@ static int cg_rmdir(const char *path) if (!fpath) return -EINVAL; + nih_local char *next = NULL; + bool ret = caller_is_in_ancestor(fc->pid, controller, cgroup, &next); + if (!ret ) { + if (!fpath || strcmp(next, fpath) == 0) + return -EBUSY; + return -ENOENT; + } + if (!fc_may_access(fc, controller, cgdir, NULL, O_WRONLY)) return -EACCES; @@ -1562,44 +1644,6 @@ static void get_blkio_io_value(char *str } } -static char *get_pid_cgroup(pid_t pid, const char *contrl) -{ - nih_local char *fnam = NULL; - FILE *f; - char *answer = NULL; - char *line = NULL; - size_t len = 0; - - fnam = NIH_MUST( nih_sprintf(NULL, "/proc/%d/cgroup", pid) ); - if (!(f = fopen(fnam, "r"))) - return false; - - while (getline(&line, &len, f) != -1) { - char *c1, *c2; - if (!line[0]) - continue; - c1 = strchr(line, ':'); - if (!c1) - goto out; - c1++; - c2 = strchr(c1, ':'); - if (!c2) - goto out; - *c2 = '\0'; - if (strcmp(c1, contrl) != 0) - continue; - c2++; - stripnewline(c2); - answer = NIH_MUST( nih_strdup(NULL, c2) ); - goto out; - } - -out: - fclose(f); - free(line); - return answer; -} - static int read_file(const char *path, char *buf, size_t size, struct file_info *d) { Index: lxcfs-0.10/tests/test_confinement.sh =================================================================== --- /dev/null +++ lxcfs-0.10/tests/test_confinement.sh @@ -0,0 +1,95 @@ +#!/bin/bash + +set -ex + +[ $(id -u) -eq 0 ] + +d=$(mktemp -t -d tmp.XXX) +d2=$(mktemp -t -d tmp.XXX) + +pid=-1 +cleanup() { + [ $pid -ne -1 ] && kill -9 $pid + umount -l $d || true + umount -l $d2 || true + rm -rf $d $d2 +} + +cmdline=$(realpath $0) +dirname=$(dirname ${cmdline}) +topdir=$(dirname ${dirname}) + +trap cleanup EXIT HUP INT TERM + +${topdir}/lxcfs $d & +pid=$! + +# put ourselves into x1 +cgm movepidabs freezer / $$ +cgm create freezer x1 +cgm movepid freezer x1 $$ + +mount -t cgroup -o freezer freezer $d2 +sudo rmdir $d2/lxcfs_test_a1/lxcfs_test_a2 || true +sudo rmdir $d2/lxcfs_test_a1 || true + +echo "Making sure root cannot mkdir" +bad=0 +mkdir $d/cgroup/freezer/lxcfs_test_a1 && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi + +echo "Making sure root cannot rmdir" +mkdir $d2/lxcfs_test_a1 +mkdir $d2/lxcfs_test_a1/lxcfs_test_a2 +rmdir $d/cgroup/freezer/lxcfs_test_a1 && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi +[ -d $d2/lxcfs_test_a1 ] +rmdir $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2 && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi +[ -d $d2/lxcfs_test_a1/lxcfs_test_a2 ] + +echo "Making sure root cannot read/write" +sleep 200 & +p=$! +echo $p > $d/cgroup/freezer/lxcfs_test_a1/tasks && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi +cat $d/cgroup/freezer/lxcfs_test_a1/tasks && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi +echo $p > $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2/tasks && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi +cat $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2/tasks && bad=1 +if [ "${bad}" -eq 1 ]; then + false +fi + +# make sure things like truncate and access don't leak info about +# the /lxcfs_test_a1 cgroup which we shouldn't be able to reach +echo "Testing other system calls" +${dirname}/test_syscalls $d/cgroup/freezer/lxcfs_test_a1 + +echo "Making sure root can act on descendents" +mycg=$(cgm getpidcgroupabs freezer $$) +newcg=${mycg}/lxcfs_test_a1 +rmdir $d2/$newcg || true # cleanup previosu run +mkdir $d/cgroup/freezer/$newcg +echo $p > $d/cgroup/freezer/$newcg/tasks +cat $d/cgroup/freezer/$newcg/tasks +kill -9 $p +while [ `wc -l $d/cgroup/freezer/$newcg/tasks | awk '{ print $1 }'` -ne 0 ]; do + sleep 1 +done +rmdir $d/cgroup/freezer/$newcg + +echo "All tests passed!" Index: lxcfs-0.10/Makefile.am =================================================================== --- lxcfs-0.10.orig/Makefile.am +++ lxcfs-0.10/Makefile.am @@ -25,13 +25,10 @@ lxcfs.1: lxcfs lxcfs.man.add $(HELP2MAN) -n "Set up cgroup fs for containers" --no-discard-stderr -s 1 -I lxcfs.man.add -N ./lxcfs > lxcfs.1 endif -TEST_READ: tests/test-read.c - $(CC) -o tests/test-read tests/test-read.c +TEST_SYSCALLS: tests/test_syscalls.c + $(CC) -o tests/test_syscalls tests/test_syscalls.c -TEST_CPUSET: tests/cpusetrange.c cpuset.c - $(CC) -o tests/cpusetrange tests/cpusetrange.c cpuset.c - -tests: TEST_READ TEST_CPUSET +tests: TEST_SYSCALLS distclean: rm -rf .deps/ \ Index: lxcfs-0.10/tests/test_syscalls.c =================================================================== --- /dev/null +++ lxcfs-0.10/tests/test_syscalls.c @@ -0,0 +1,402 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +void test_open(const char *path) +{ + int fd = open(path, O_RDONLY); + if (fd >= 0) { + fprintf(stderr, "leak at open of %s\n", path); + exit(1); + } + if (errno != ENOENT) { + fprintf(stderr, "leak at open of %s: errno was %d\n", path, errno); + exit(1); + } +} + +void test_stat(const char *path) +{ + struct stat sb; + if (stat(path, &sb) >= 0) { + fprintf(stderr, "leak at stat of %s\n", path); + exit(1); + } + if (errno != ENOENT) { + fprintf(stderr, "leak at stat of %s: errno was %d\n", path, errno); + exit(1); + } +} + +void test_access(const char *path) +{ + if (access(path, O_RDONLY) >= 0) { + fprintf(stderr, "leak at access of %s\n", path); + exit(1); + } + if (errno != ENOENT) { + fprintf(stderr, "leak at access of %s: errno was %d\n", path, errno); + exit(1); + } +} + +void test_bind(const char *path) +{ + if (mount(path, path, "none", MS_BIND, NULL) == 0) { + fprintf(stderr, "leak at bind mount of %s\n", path); + exit(1); + } +} + +void test_truncate(const char *path) +{ + if (truncate(path, 0) == 0) { + fprintf(stderr, "leak at truncate of %s\n", path); + exit(1); + } +} + +void test_chdir(const char *path) +{ + if (chdir(path) == 0) { + fprintf(stderr, "leak at chdir to %s\n", path); + exit(1); + } +} + +void test_rename(const char *path) +{ + char *d = strdupa(path), *tmpname; + d = dirname(d); + size_t len = strlen(path) + 30; + + tmpname = alloca(len); + snprintf(tmpname, len, "%s/%d", d, (int)getpid()); + if (rename(path, tmpname) == 0 || errno != ENOENT) { + fprintf(stderr, "leak at rename of %s\n", path); + exit(1); + } +} + +void test_mkdir(const char *path) +{ + size_t len = strlen(path) + 30; + char *tmpname = alloca(len); + snprintf(tmpname, len, "%s/%d", path, (int)getpid()); + + if (mkdir(path, 0755) == 0) { + fprintf(stderr, "leak at mkdir of %s\n", path); + exit(1); + } + if (errno != ENOENT) { + fprintf(stderr, "leak at mkdir of %s, errno was %s\n", path, strerror(errno)); + exit(1); + } + if (mkdir(tmpname, 0755) == 0) { + fprintf(stderr, "leak at mkdir of %s\n", tmpname); + exit(1); + } + if (errno != ENOENT) { + fprintf(stderr, "leak at mkdir of %s, errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_rmdir(const char *path) +{ + size_t len = strlen(path) + 30; + char *tmpname = alloca(len); + snprintf(tmpname, len, "%s/%d", path, (int)getpid()); + + if (rmdir(path) == 0 || errno != ENOENT) { + fprintf(stderr, "leak at rmdir of %s\n", path); + exit(1); + } + if (rmdir(tmpname) == 0 || errno != ENOENT) { + fprintf(stderr, "leak at rmdir of %s\n", tmpname); + exit(1); + } +} + +void test_creat(const char *path) +{ + if (creat(path, 0755) >= 0) { + fprintf(stderr, "leak at creat of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at creat of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_link(const char *path) +{ + char *d = strdupa(path), *tmpname; + d = dirname(d); + size_t len = strlen(path) + 30; + tmpname = alloca(len); + snprintf(tmpname, len, "%s/%d", d, (int)getpid()); + + if (link(path, tmpname) == 0) { + fprintf(stderr, "leak at link of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at link of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_unlink(const char *path) +{ + if (unlink(path) == 0) { + fprintf(stderr, "leak at unlink of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at unlink of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_symlink(const char *path) +{ + char *d = strdupa(path), *tmpname; + d = dirname(d); + size_t len = strlen(path) + 30; + tmpname = alloca(len); + snprintf(tmpname, len, "%s/%d", d, (int)getpid()); + + if (symlink(tmpname, path) == 0) { + fprintf(stderr, "leak at symlink of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at symlink of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_readlink(const char *path) +{ + char *dest = alloca(2 * strlen(path)); + + if (readlink(path, dest, 2 * strlen(path)) >= 0) { + fprintf(stderr, "leak at readlink of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at readlink of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_chmod(const char *path) +{ + if (chmod(path, 0755) == 0) { + fprintf(stderr, "leak at chmod of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at chmod of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_chown(const char *path) +{ + if (chown(path, 0, 0) == 0) { + fprintf(stderr, "leak at chown of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at chown of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_lchown(const char *path) +{ + if (lchown(path, 0, 0) == 0) { + fprintf(stderr, "leak at lchown of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at lchown of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_mknod(const char *path) +{ + if (mknod(path, 0755, makedev(0, 0)) == 0) { + fprintf(stderr, "leak at mknod of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at mknod of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_chroot(const char *path) +{ + if (chroot(path) == 0) { + fprintf(stderr, "leak at chroot of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at chroot of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_xattrs(const char *path) +{ + /* + * might consider doing all of: + * setxattr + * lsetxattr + * getxattr + * lgetxattr + * listxattr + * llistxattr + * removexattr + * lremovexattr + */ + char value[200]; + if (getxattr(path, "security.selinux", value, 200) >= 0) { + fprintf(stderr, "leak at getxattr of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at getxattr of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_utimes(const char *path) +{ + struct utimbuf times; + times.actime = 0; + times.modtime = 0; + + if (utime(path, ×) == 0) { + fprintf(stderr, "leak at utime of %s\n", path); + exit(1); + } + if (errno != ENOENT && errno != ENOSYS) { + fprintf(stderr, "leak at utime of %s: errno was %s\n", path, strerror(errno)); + exit(1); + } +} + +void test_openat(const char *path) +{ + char *d = strdupa(path), *f, *tmpname; + int fd, fd2; + f = basename(d); + d = dirname(d); + fd = open(d, O_RDONLY); + if (fd < 0) { + fprintf(stderr, "Error in openat test: could not open parent dir\n"); + return; + } + fd2 = openat(fd, f, O_RDONLY); + if (fd2 >= 0 || errno != ENOENT) { + fprintf(stderr, "leak at openat of %s\n", f); + exit(1); + } + size_t len = strlen(path) + strlen("/cgroup.procs") + 1; + tmpname = alloca(len); + snprintf(tmpname, len, "%s/cgroup.procs", f); + fd2 = openat(fd, tmpname, O_RDONLY); + if (fd2 >= 0 || errno != ENOENT) { + fprintf(stderr, "leak at openat of %s\n", tmpname); + exit(1); + } + close(fd); +} + +int main(int argc, char *argv[]) +{ + char *procspath; + size_t len; + + if (geteuid() != 0) { + fprintf(stderr, "Run me as root\n"); + exit(1); + } + + if (argc != 2) { + fprintf(stderr, "Usage: %s [lxcfs_test_cgroup_path]\n", argv[0]); + exit(1); + } + + /* Try syscalls on the directory and on $directory/cgroup.procs */ + len = strlen(argv[1]) + strlen("/cgroup.procs") + 1; + procspath = alloca(len); + snprintf(procspath, len, "%s/cgroup.procs", argv[1]); + + test_open(argv[1]); + test_open(procspath); + test_stat(argv[1]); + test_stat(procspath); + test_access(argv[1]); + test_access(procspath); + + test_bind(argv[1]); + test_bind(procspath); + test_truncate(argv[1]); + test_truncate(procspath); + test_chdir(argv[1]); + test_chdir(procspath); + test_rename(argv[1]); + test_rename(procspath); + test_mkdir(argv[1]); + test_mkdir(procspath); + test_rmdir(argv[1]); + test_rmdir(procspath); + test_creat(argv[1]); + test_creat(procspath); + test_link(argv[1]); + test_link(procspath); + test_unlink(argv[1]); + test_unlink(procspath); + test_symlink(argv[1]); + test_symlink(procspath); + test_readlink(argv[1]); + test_readlink(procspath); + test_chmod(argv[1]); + test_chmod(procspath); + test_chown(argv[1]); + test_chown(procspath); + test_lchown(argv[1]); + test_lchown(procspath); + test_mknod(argv[1]); + test_mknod(procspath); + test_chroot(argv[1]); + test_chroot(procspath); + test_xattrs(argv[1]); + test_xattrs(procspath); + test_utimes(argv[1]); + test_utimes(procspath); + + test_openat(argv[1]); + // meh... linkat etc? + + printf("All tests passed\n"); + return 0; +}