From cc14a6c011604c3b598247c35fea78d11c8d15ab Mon Sep 17 00:00:00 2001 From: David Michael Date: Mon, 25 Feb 2019 13:18:30 -0500 Subject: [PATCH 1/4] fs-util: exempt root prefix directories from UID checks When chase_symlinks is given a root path, it is assumed that all processed symlinks are restricted under that path. It should not be necessary to verify components of that prefix path since they are not relevant to the symlinks. This change skips unsafe UID transitions in this root prefix, i.e. it now ignores when an unprivileged user's directory contains a root-owned directory above the symlink root. --- src/basic/fs-util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 3ff8615797..281b85d900 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -926,6 +926,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (fstat(child, &st) < 0) return -errno; if ((flags & CHASE_SAFE) && + (empty_or_root(root) || (size_t)(todo - buffer) > strlen(root)) && unsafe_transition(&previous_stat, &st)) return log_unsafe_transition(fd, child, path, flags); From c3aa4adcafa046da7c41ef2e5411fc8dd8feecaa Mon Sep 17 00:00:00 2001 From: David Michael Date: Mon, 25 Feb 2019 13:26:07 -0500 Subject: [PATCH 2/4] tmpfiles: pass arg_root to chase_symlinks as the root prefix This informs chase_symlinks that symlinks should be treated as if the path given by --root= is the root of their file system. With the parent commit, this allows tmpfiles to create files as the root user under a prefix that may be owned by an unprivileged user. In particular, this fixes the case where tmpfiles generates initial files in a staging root directory for packaging under a directory owned by the unprivileged packager user (e.g. in Gentoo). --- src/tmpfiles/tmpfiles.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 6296a8acdf..801e79b01d 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -856,7 +856,7 @@ static int path_open_parent_safe(const char *path) { if (!dn) return log_oom(); - fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_WARN, NULL); + fd = chase_symlinks(dn, arg_root, CHASE_OPEN|CHASE_SAFE|CHASE_WARN, NULL); if (fd < 0 && fd != -ENOLINK) return log_error_errno(fd, "Failed to validate path %s: %m", path); @@ -877,7 +877,7 @@ static int path_open_safe(const char *path) { "Failed to open invalid path '%s'.", path); - fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_WARN|CHASE_NOFOLLOW, NULL); + fd = chase_symlinks(path, arg_root, CHASE_OPEN|CHASE_SAFE|CHASE_WARN|CHASE_NOFOLLOW, NULL); if (fd < 0 && fd != -ENOLINK) return log_error_errno(fd, "Failed to validate path %s: %m", path); @@ -2256,7 +2256,7 @@ static int process_item(Item *i, OperationMask operation) { i->done |= operation; - r = chase_symlinks(i->path, NULL, CHASE_NO_AUTOFS|CHASE_WARN, NULL); + r = chase_symlinks(i->path, arg_root, CHASE_NO_AUTOFS|CHASE_WARN, NULL); if (r == -EREMOTE) { log_notice_errno(r, "Skipping %s", i->path); return 0; From 8595c4588c0ed8e61745fc6a4bd0c7798ee6d10f Mon Sep 17 00:00:00 2001 From: David Michael Date: Tue, 26 Feb 2019 17:25:48 +0000 Subject: [PATCH 3/4] TEST-22: add test for unprivileged dirs in root prefixes This verifies the fix for the issue described in: https://github.com/systemd/systemd/pull/11820 --- test/TEST-22-TMPFILES/test-08.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100755 test/TEST-22-TMPFILES/test-08.sh diff --git a/test/TEST-22-TMPFILES/test-08.sh b/test/TEST-22-TMPFILES/test-08.sh new file mode 100755 index 0000000000..e7bf044783 --- /dev/null +++ b/test/TEST-22-TMPFILES/test-08.sh @@ -0,0 +1,32 @@ +#! /bin/bash +# +# Verify tmpfiles can run in a root directory under a path prefix that contains +# directories owned by unprivileged users, for example when a root file system +# is mounted in a regular user's home directory. +# +# https://github.com/systemd/systemd/pull/11820 +# + +set -e + +rm -fr /tmp/root /tmp/user +mkdir -p /tmp/root /tmp/user/root +chown daemon:daemon /tmp/user + +# Verify the command works as expected with no prefix or a root-owned prefix. +echo 'd /tmp/root/test1' | systemd-tmpfiles --create - +test -d /tmp/root/test1 +echo 'd /test2' | systemd-tmpfiles --root=/tmp/root --create - +test -d /tmp/root/test2 + +# Verify the command fails to write to a root-owned subdirectory under an +# unprivileged user's directory when it's not part of the prefix, as expected +# by the unsafe_transition function. +! echo 'd /tmp/user/root/test' | systemd-tmpfiles --create - +! test -e /tmp/user/root/test +! echo 'd /user/root/test' | systemd-tmpfiles --root=/tmp --create - +! test -e /tmp/user/root/test + +# Verify the above works when all user-owned directories are in the prefix. +echo 'd /test' | systemd-tmpfiles --root=/tmp/user/root --create - +test -d /tmp/user/root/test From 2a2fe6ed649fc68cb71ba7700e0ca72625ff51cb Mon Sep 17 00:00:00 2001 From: David Michael Date: Tue, 26 Feb 2019 13:31:28 -0500 Subject: [PATCH 4/4] test-fs-util: test chase_symlinks with user-owned dirs This verifies the fix for the issue described in: https://github.com/systemd/systemd/pull/11820 --- src/test/test-fs-util.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index e049abc4a4..f0f015056e 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -154,6 +154,30 @@ static void test_chase_symlinks(void) { assert_se(path_equal(result, q)); result = mfree(result); + /* Paths underneath the "root" with different UIDs while using CHASE_SAFE */ + + if (geteuid() == 0) { + p = strjoina(temp, "/user"); + assert_se(mkdir(p, 0755) >= 0); + assert_se(chown(p, UID_NOBODY, GID_NOBODY) >= 0); + + q = strjoina(temp, "/user/root"); + assert_se(mkdir(q, 0755) >= 0); + + p = strjoina(q, "/link"); + assert_se(symlink("/", p) >= 0); + + /* Fail when user-owned directories contain root-owned subdirectories. */ + r = chase_symlinks(p, temp, CHASE_SAFE, &result); + assert_se(r == -ENOLINK); + result = mfree(result); + + /* Allow this when the user-owned directories are all in the "root". */ + r = chase_symlinks(p, q, CHASE_SAFE, &result); + assert_se(r > 0); + result = mfree(result); + } + /* Paths using . */ r = chase_symlinks("/etc/./.././", NULL, 0, &result);