Skip to content

Commit a8b6c3e

Browse files
committed
Fix checking of parent directories
Taken from the justification in the launchpad bug: To a task in freezer cgroup /a/b/c/d, it should appear that there are no cgroups other than its descendents. Since this is a filesystem, we must have the parent directories, but each parent cgroup should only contain the child which the task can see. So, when this task looks at /a/b, it should see only directory 'c' and no files. Attempt to create /a/b/x should result in -EPERM, whether /a/b/x already exists or not. Attempts to query /a/b/x should result in -ENOENT whether /a/b/x exists or not. Opening /a/b/tasks should result in -ENOENT. The caller_may_see_dir checks specifically whether a task may see a cgroup directory - i.e. /a/b/x if opening /a/b/x/tasks, and /a/b/c/d if doing opendir('/a/b/c/d'). caller_is_in_ancestor() will return true if the caller in /a/b/c/d looks at /a/b/c/d/e. If the caller is in a child cgroup of the queried one - i.e. if the task in /a/b/c/d queries /a/b, then *nextcg will container the next (the only) directory which he can see in the path - 'c'. Beyond this, regular DAC permissions should apply, with the root-in-user-namespace privilege over its mapped uids being respected. The fc_may_access check does this check for both directories and files. This is CVE-2015-1342 (LP: #1508481) Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
1 parent d09b7ff commit a8b6c3e

File tree

4 files changed

+646
-46
lines changed

4 files changed

+646
-46
lines changed

Diff for: Makefile.am

+5-3
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ endif
2929

3030
TEST_READ: tests/test-read.c
3131
$(CC) -o tests/test-read tests/test-read.c
32-
3332
TEST_CPUSET: tests/cpusetrange.c cpuset.c
3433
$(CC) -o tests/cpusetrange tests/cpusetrange.c cpuset.c
34+
TEST_SYSCALLS: tests/test_syscalls.c
35+
$(CC) -o tests/test_syscalls tests/test_syscalls.c
3536

36-
tests: TEST_READ TEST_CPUSET
37+
tests: TEST_READ TEST_CPUSET TEST_SYSCALLS
3738

3839
distclean:
3940
rm -rf .deps/ \
@@ -60,4 +61,5 @@ distclean:
6061
lxcfs.o \
6162
m4/ \
6263
missing \
63-
stamp-h1
64+
stamp-h1 \
65+
tests/test_syscalls

Diff for: lxcfs.c

+94-43
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,12 @@ static bool perms_include(int fmode, mode_t req_mode)
235235
return ((fmode & r) == r);
236236
}
237237

238+
239+
/*
240+
* taskcg is a/b/c
241+
* querycg is /a/b/c/d/e
242+
* we return 'd'
243+
*/
238244
static char *get_next_cgroup_dir(const char *taskcg, const char *querycg)
239245
{
240246
char *start, *end;
@@ -378,53 +384,71 @@ static void prune_init_slice(char *cg)
378384
*/
379385
static bool caller_is_in_ancestor(pid_t pid, const char *contrl, const char *cg, char **nextcg)
380386
{
381-
char fnam[PROCLEN];
382-
FILE *f;
383387
bool answer = false;
384-
char *line = NULL;
385-
size_t len = 0;
386-
int ret;
388+
char *c2 = get_pid_cgroup(pid, contrl);
389+
char *linecmp;
387390

388-
ret = snprintf(fnam, PROCLEN, "/proc/%d/cgroup", pid);
389-
if (ret < 0 || ret >= PROCLEN)
390-
return false;
391-
if (!(f = fopen(fnam, "r")))
391+
if (!c2)
392392
return false;
393+
prune_init_slice(c2);
393394

394-
while (getline(&line, &len, f) != -1) {
395-
char *c1, *c2, *linecmp;
396-
if (!line[0])
397-
continue;
398-
c1 = strchr(line, ':');
399-
if (!c1)
400-
goto out;
401-
c1++;
402-
c2 = strchr(c1, ':');
403-
if (!c2)
404-
goto out;
405-
*c2 = '\0';
406-
if (strcmp(c1, contrl) != 0)
407-
continue;
408-
c2++;
409-
stripnewline(c2);
410-
prune_init_slice(c2);
411-
/*
412-
* callers pass in '/' for root cgroup, otherwise they pass
413-
* in a cgroup without leading '/'
414-
*/
415-
linecmp = *cg == '/' ? c2 : c2+1;
416-
if (strncmp(linecmp, cg, strlen(linecmp)) != 0) {
417-
if (nextcg)
418-
*nextcg = get_next_cgroup_dir(linecmp, cg);
419-
goto out;
395+
/*
396+
* callers pass in '/' for root cgroup, otherwise they pass
397+
* in a cgroup without leading '/'
398+
*/
399+
linecmp = *cg == '/' ? c2 : c2+1;
400+
if (strncmp(linecmp, cg, strlen(linecmp)) != 0) {
401+
if (nextcg) {
402+
*nextcg = get_next_cgroup_dir(linecmp, cg);
420403
}
404+
goto out;
405+
}
406+
answer = true;
407+
408+
out:
409+
free(c2);
410+
return answer;
411+
}
412+
413+
/*
414+
* If caller is in /a/b/c, he may see that /a exists, but not /b or /a/c.
415+
*/
416+
static bool caller_may_see_dir(pid_t pid, const char *contrl, const char *cg)
417+
{
418+
bool answer = false;
419+
char *c2, *task_cg;
420+
size_t target_len, task_len;
421+
422+
if (strcmp(cg, "/") == 0)
423+
return true;
424+
425+
c2 = get_pid_cgroup(pid, contrl);
426+
427+
if (!c2)
428+
return false;
429+
430+
task_cg = c2 + 1;
431+
target_len = strlen(cg);
432+
task_len = strlen(task_cg);
433+
if (strcmp(cg, task_cg) == 0) {
421434
answer = true;
422435
goto out;
423436
}
437+
if (target_len < task_len) {
438+
/* looking up a parent dir */
439+
if (strncmp(task_cg, cg, target_len) == 0 && task_cg[target_len] == '/')
440+
answer = true;
441+
goto out;
442+
}
443+
if (target_len > task_len) {
444+
/* looking up a child dir */
445+
if (strncmp(task_cg, cg, task_len) == 0 && cg[task_len] == '/')
446+
answer = true;
447+
goto out;
448+
}
424449

425450
out:
426-
fclose(f);
427-
free(line);
451+
free(c2);
428452
return answer;
429453
}
430454

@@ -552,6 +576,10 @@ static int cg_getattr(const char *path, struct stat *sb)
552576
* cgroup, or cgdir if fpath is a file */
553577

554578
if (is_child_cgroup(controller, path1, path2)) {
579+
if (!caller_may_see_dir(fc->pid, controller, cgroup)) {
580+
ret = -ENOENT;
581+
goto out;
582+
}
555583
if (!caller_is_in_ancestor(fc->pid, controller, cgroup, NULL)) {
556584
/* this is just /cgroup/controller, return it as a dir */
557585
sb->st_mode = S_IFDIR | 00555;
@@ -630,8 +658,11 @@ static int cg_opendir(const char *path, struct fuse_file_info *fi)
630658
}
631659
}
632660

633-
if (cgroup && !fc_may_access(fc, controller, cgroup, NULL, O_RDONLY)) {
634-
return -EACCES;
661+
if (cgroup) {
662+
if (!caller_may_see_dir(fc->pid, controller, cgroup))
663+
return -ENOENT;
664+
if (!fc_may_access(fc, controller, cgroup, NULL, O_RDONLY))
665+
return -EACCES;
635666
}
636667

637668
/* we'll free this at cg_releasedir */
@@ -780,6 +811,10 @@ static int cg_open(const char *path, struct fuse_file_info *fi)
780811
}
781812
free_key(k);
782813

814+
if (!caller_may_see_dir(fc->pid, controller, path1)) {
815+
ret = -ENOENT;
816+
goto out;
817+
}
783818
if (!fc_may_access(fc, controller, path1, path2, fi->flags)) {
784819
// should never get here
785820
ret = -EACCES;
@@ -1563,7 +1598,7 @@ int cg_chmod(const char *path, mode_t mode)
15631598
int cg_mkdir(const char *path, mode_t mode)
15641599
{
15651600
struct fuse_context *fc = fuse_get_context();
1566-
char *fpath = NULL, *path1, *cgdir = NULL, *controller;
1601+
char *fpath = NULL, *path1, *cgdir = NULL, *controller, *next = NULL;
15671602
const char *cgroup;
15681603
int ret;
15691604

@@ -1585,6 +1620,14 @@ int cg_mkdir(const char *path, mode_t mode)
15851620
else
15861621
path1 = cgdir;
15871622

1623+
if (!caller_is_in_ancestor(fc->pid, controller, path1, &next)) {
1624+
if (fpath && strcmp(next, fpath) == 0)
1625+
ret = -EEXIST;
1626+
else
1627+
ret = -ENOENT;
1628+
goto out;
1629+
}
1630+
15881631
if (!fc_may_access(fc, controller, path1, NULL, O_RDWR)) {
15891632
ret = -EACCES;
15901633
goto out;
@@ -1599,13 +1642,14 @@ int cg_mkdir(const char *path, mode_t mode)
15991642

16001643
out:
16011644
free(cgdir);
1645+
free(next);
16021646
return ret;
16031647
}
16041648

16051649
static int cg_rmdir(const char *path)
16061650
{
16071651
struct fuse_context *fc = fuse_get_context();
1608-
char *fpath = NULL, *cgdir = NULL, *controller;
1652+
char *fpath = NULL, *cgdir = NULL, *controller, *next = NULL;
16091653
const char *cgroup;
16101654
int ret;
16111655

@@ -1626,8 +1670,14 @@ static int cg_rmdir(const char *path)
16261670
goto out;
16271671
}
16281672

1629-
fprintf(stderr, "rmdir: verifying access to %s:%s (req path %s)\n",
1630-
controller, cgdir, path);
1673+
if (!caller_is_in_ancestor(fc->pid, controller, cgroup, &next)) {
1674+
if (!fpath || strcmp(next, fpath) == 0)
1675+
ret = -EBUSY;
1676+
else
1677+
ret = -ENOENT;
1678+
goto out;
1679+
}
1680+
16311681
if (!fc_may_access(fc, controller, cgdir, NULL, O_WRONLY)) {
16321682
ret = -EACCES;
16331683
goto out;
@@ -1646,6 +1696,7 @@ static int cg_rmdir(const char *path)
16461696

16471697
out:
16481698
free(cgdir);
1699+
free(next);
16491700
return ret;
16501701
}
16511702

Diff for: tests/test_confinement.sh

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#!/bin/bash
2+
3+
set -ex
4+
5+
[ $(id -u) -eq 0 ]
6+
7+
d=$(mktemp -t -d tmp.XXX)
8+
d2=$(mktemp -t -d tmp.XXX)
9+
10+
pid=-1
11+
cleanup() {
12+
[ $pid -ne -1 ] && kill -9 $pid
13+
umount -l $d || true
14+
umount -l $d2 || true
15+
rm -rf $d $d2
16+
}
17+
18+
cmdline=$(realpath $0)
19+
dirname=$(dirname ${cmdline})
20+
topdir=$(dirname ${dirname})
21+
22+
trap cleanup EXIT HUP INT TERM
23+
24+
${topdir}/lxcfs $d &
25+
pid=$!
26+
27+
# put ourselves into x1
28+
cgm movepidabs freezer / $$
29+
cgm create freezer x1
30+
cgm movepid freezer x1 $$
31+
32+
mount -t cgroup -o freezer freezer $d2
33+
sudo rmdir $d2/lxcfs_test_a1/lxcfs_test_a2 || true
34+
sudo rmdir $d2/lxcfs_test_a1 || true
35+
36+
echo "Making sure root cannot mkdir"
37+
bad=0
38+
mkdir $d/cgroup/freezer/lxcfs_test_a1 && bad=1
39+
if [ "${bad}" -eq 1 ]; then
40+
false
41+
fi
42+
43+
echo "Making sure root cannot rmdir"
44+
mkdir $d2/lxcfs_test_a1
45+
mkdir $d2/lxcfs_test_a1/lxcfs_test_a2
46+
rmdir $d/cgroup/freezer/lxcfs_test_a1 && bad=1
47+
if [ "${bad}" -eq 1 ]; then
48+
false
49+
fi
50+
[ -d $d2/lxcfs_test_a1 ]
51+
rmdir $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2 && bad=1
52+
if [ "${bad}" -eq 1 ]; then
53+
false
54+
fi
55+
[ -d $d2/lxcfs_test_a1/lxcfs_test_a2 ]
56+
57+
echo "Making sure root cannot read/write"
58+
sleep 200 &
59+
p=$!
60+
echo $p > $d/cgroup/freezer/lxcfs_test_a1/tasks && bad=1
61+
if [ "${bad}" -eq 1 ]; then
62+
false
63+
fi
64+
cat $d/cgroup/freezer/lxcfs_test_a1/tasks && bad=1
65+
if [ "${bad}" -eq 1 ]; then
66+
false
67+
fi
68+
echo $p > $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2/tasks && bad=1
69+
if [ "${bad}" -eq 1 ]; then
70+
false
71+
fi
72+
cat $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2/tasks && bad=1
73+
if [ "${bad}" -eq 1 ]; then
74+
false
75+
fi
76+
77+
# make sure things like truncate and access don't leak info about
78+
# the /lxcfs_test_a1 cgroup which we shouldn't be able to reach
79+
echo "Testing other system calls"
80+
${dirname}/test_syscalls $d/cgroup/freezer/lxcfs_test_a1
81+
${dirname}/test_syscalls $d/cgroup/freezer/lxcfs_test_a1/lxcfs_test_a2
82+
83+
echo "Making sure root can act on descendents"
84+
mycg=$(cgm getpidcgroupabs freezer $$)
85+
newcg=${mycg}/lxcfs_test_a1
86+
rmdir $d2/$newcg || true # cleanup previosu run
87+
mkdir $d/cgroup/freezer/$newcg
88+
echo $p > $d/cgroup/freezer/$newcg/tasks
89+
cat $d/cgroup/freezer/$newcg/tasks
90+
kill -9 $p
91+
while [ `wc -l $d/cgroup/freezer/$newcg/tasks | awk '{ print $1 }'` -ne 0 ]; do
92+
sleep 1
93+
done
94+
rmdir $d/cgroup/freezer/$newcg
95+
96+
echo "All tests passed!"

0 commit comments

Comments
 (0)