Pablo Greco 48fc63
From c211b650ee5cb9934067dbba40718a4a33063e06 Mon Sep 17 00:00:00 2001
Pablo Greco 48fc63
From: David Tardon <dtardon@redhat.com>
Pablo Greco 48fc63
Date: Thu, 3 Jan 2019 14:44:36 +0100
Pablo Greco 48fc63
Subject: [PATCH] fs-util: add new chase_symlinks() flag CHASE_OPEN
Pablo Greco 48fc63
Pablo Greco 48fc63
The new flag returns the O_PATH fd of the final component, which may be
Pablo Greco 48fc63
converted into a proper fd by open()ing it again through the
Pablo Greco 48fc63
/proc/self/fd/xyz path.
Pablo Greco 48fc63
Pablo Greco 48fc63
Together with O_SAFE this provides us with a somewhat safe way to open()
Pablo Greco 48fc63
files in directories potentially owned by unprivileged code, where we
Pablo Greco 48fc63
want to refuse operation if any symlink tricks are played pointing to
Pablo Greco 48fc63
privileged files.
Pablo Greco 48fc63
Pablo Greco 48fc63
(cherry picked from commit 1ed34d75d4f21d2335c5625261954c848d176ae6)
Pablo Greco 48fc63
Pablo Greco 48fc63
Related: #1663143
Pablo Greco 48fc63
---
Pablo Greco 48fc63
 Makefile.am          |  1 +
Pablo Greco 48fc63
 src/shared/util.c    | 17 +++++++++++++
Pablo Greco 48fc63
 src/shared/util.h    |  1 +
Pablo Greco 48fc63
 src/test/test-util.c | 59 +++++++++++++++++++++++++++++++++++++++++++-
Pablo Greco 48fc63
 4 files changed, 77 insertions(+), 1 deletion(-)
Pablo Greco 48fc63
Pablo Greco 48fc63
diff --git a/Makefile.am b/Makefile.am
Pablo Greco 48fc63
index 995c421b8b..648f54b957 100644
Pablo Greco 48fc63
--- a/Makefile.am
Pablo Greco 48fc63
+++ b/Makefile.am
Pablo Greco 48fc63
@@ -1675,6 +1675,7 @@ test_util_SOURCES = \
Pablo Greco 48fc63
 	src/test/test-util.c
Pablo Greco 48fc63
 
Pablo Greco 48fc63
 test_util_LDADD = \
Pablo Greco 48fc63
+	libsystemd-internal.la \
Pablo Greco 48fc63
 	libsystemd-shared.la
Pablo Greco 48fc63
 
Pablo Greco 48fc63
 test_path_lookup_SOURCES = \
Pablo Greco 48fc63
diff --git a/src/shared/util.c b/src/shared/util.c
Pablo Greco 48fc63
index fc4887920f..354d15ff18 100644
Pablo Greco 48fc63
--- a/src/shared/util.c
Pablo Greco 48fc63
+++ b/src/shared/util.c
Pablo Greco 48fc63
@@ -9225,6 +9225,10 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         assert(path);
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+        /* Either the file may be missing, or we return an fd to the final object, but both make no sense */
Pablo Greco 48fc63
+        if ((flags & (CHASE_NONEXISTENT|CHASE_OPEN)) == (CHASE_NONEXISTENT|CHASE_OPEN))
Pablo Greco 48fc63
+                return -EINVAL;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
         /* This is a lot like canonicalize_file_name(), but takes an additional "root" parameter, that allows following
Pablo Greco 48fc63
          * symlinks relative to a root directory, instead of the root of the host.
Pablo Greco 48fc63
          *
Pablo Greco 48fc63
@@ -9476,5 +9480,18 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
Pablo Greco 48fc63
                 done = NULL;
Pablo Greco 48fc63
         }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+        if (flags & CHASE_OPEN) {
Pablo Greco 48fc63
+                int q;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                /* Return the O_PATH fd we currently are looking to the caller. It can translate it to a proper fd by
Pablo Greco 48fc63
+                 * opening /proc/self/fd/xyz. */
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                assert(fd >= 0);
Pablo Greco 48fc63
+                q = fd;
Pablo Greco 48fc63
+                fd = -1;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                return q;
Pablo Greco 48fc63
+        }
Pablo Greco 48fc63
+
Pablo Greco 48fc63
         return exists;
Pablo Greco 48fc63
 }
Pablo Greco 48fc63
diff --git a/src/shared/util.h b/src/shared/util.h
Pablo Greco 48fc63
index fa3e2e3009..d89f0d34a1 100644
Pablo Greco 48fc63
--- a/src/shared/util.h
Pablo Greco 48fc63
+++ b/src/shared/util.h
Pablo Greco 48fc63
@@ -1161,6 +1161,7 @@ enum {
Pablo Greco 48fc63
         CHASE_NONEXISTENT = 1U << 1,   /* If set, it's OK if the path doesn't actually exist. */
Pablo Greco 48fc63
         CHASE_NO_AUTOFS   = 1U << 2,   /* If set, return -EREMOTE if autofs mount point found */
Pablo Greco 48fc63
         CHASE_SAFE        = 1U << 3,   /* If set, return EPERM if we ever traverse from unprivileged to privileged files or directories */
Pablo Greco 48fc63
+        CHASE_OPEN        = 1U << 4,   /* If set, return an O_PATH object to the final component */
Pablo Greco 48fc63
 };
Pablo Greco 48fc63
 
Pablo Greco 48fc63
 int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret);
Pablo Greco 48fc63
diff --git a/src/test/test-util.c b/src/test/test-util.c
Pablo Greco 48fc63
index e5a646ec20..8ef3850e10 100644
Pablo Greco 48fc63
--- a/src/test/test-util.c
Pablo Greco 48fc63
+++ b/src/test/test-util.c
Pablo Greco 48fc63
@@ -1910,11 +1910,45 @@ static void test_acquire_data_fd(void) {
Pablo Greco 48fc63
         test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE|ACQUIRE_NO_TMPFILE);
Pablo Greco 48fc63
 }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+static int id128_read_fd(int fd, sd_id128_t *ret) {
Pablo Greco 48fc63
+        char buf[33];
Pablo Greco 48fc63
+        ssize_t k;
Pablo Greco 48fc63
+        unsigned j;
Pablo Greco 48fc63
+        sd_id128_t t;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        assert_return(fd >= 0, -EINVAL);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        k = loop_read(fd, buf, 33, false);
Pablo Greco 48fc63
+        if (k < 0)
Pablo Greco 48fc63
+                return (int) k;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        if (k != 33)
Pablo Greco 48fc63
+                return -EIO;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        if (buf[32] !='\n')
Pablo Greco 48fc63
+                return -EIO;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        for (j = 0; j < 16; j++) {
Pablo Greco 48fc63
+                int a, b;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                a = unhexchar(buf[j*2]);
Pablo Greco 48fc63
+                b = unhexchar(buf[j*2+1]);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                if (a < 0 || b < 0)
Pablo Greco 48fc63
+                        return -EIO;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                t.bytes[j] = a << 4 | b;
Pablo Greco 48fc63
+        }
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        *ret = t;
Pablo Greco 48fc63
+        return 0;
Pablo Greco 48fc63
+}
Pablo Greco 48fc63
+
Pablo Greco 48fc63
 static void test_chase_symlinks(void) {
Pablo Greco 48fc63
         _cleanup_free_ char *result = NULL;
Pablo Greco 48fc63
         char temp[] = "/tmp/test-chase.XXXXXX";
Pablo Greco 48fc63
         const char *top, *p, *pslash, *q, *qslash;
Pablo Greco 48fc63
-        int r;
Pablo Greco 48fc63
+        int r, pfd;
Pablo Greco 48fc63
 
Pablo Greco 48fc63
         assert_se(mkdtemp(temp));
Pablo Greco 48fc63
 
Pablo Greco 48fc63
@@ -2139,6 +2173,29 @@ static void test_chase_symlinks(void) {
Pablo Greco 48fc63
                 assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
Pablo Greco 48fc63
         }
Pablo Greco 48fc63
 
Pablo Greco 48fc63
+        p = strjoina(temp, "/machine-id-test");
Pablo Greco 48fc63
+        assert_se(symlink("/usr/../etc/./machine-id", p) >= 0);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+        pfd = chase_symlinks(p, NULL, CHASE_OPEN, NULL);
Pablo Greco 48fc63
+        if (pfd != -ENOENT) {
Pablo Greco 48fc63
+                char procfs[sizeof("/proc/self/fd/") - 1 + DECIMAL_STR_MAX(pfd) + 1];
Pablo Greco 48fc63
+                _cleanup_close_ int fd = -1;
Pablo Greco 48fc63
+                sd_id128_t a, b;
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                assert_se(pfd >= 0);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                xsprintf(procfs, "/proc/self/fd/%i", pfd);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                fd = open(procfs, O_RDONLY|O_CLOEXEC);
Pablo Greco 48fc63
+                assert_se(fd >= 0);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                safe_close(pfd);
Pablo Greco 48fc63
+
Pablo Greco 48fc63
+                assert_se(id128_read_fd(fd, &a) >= 0);
Pablo Greco 48fc63
+                assert_se(sd_id128_get_machine(&b) >= 0);
Pablo Greco 48fc63
+                assert_se(sd_id128_equal(a, b));
Pablo Greco 48fc63
+        }
Pablo Greco 48fc63
+
Pablo Greco 48fc63
         assert_se(rm_rf_dangerous(temp, false, true, false) >= 0);
Pablo Greco 48fc63
 }
Pablo Greco 48fc63