From: Franck Bui <fbui@suse.com>
Date: Fri, 30 Nov 2018 14:13:44 +0000 (+0100)
Subject: fs-util: make chase_symlink() returns -ENOLINK when unsafe transitions are met
X-Git-Tag: v240~101^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=36c97decbe46a62b6976b8ba7c59e8552f14f935;p=thirdparty%2Fsystemd.git

fs-util: make chase_symlink() returns -ENOLINK when unsafe transitions are met

We previously returned -EPERM but it can be returned for various other reasons
too.

Let's use -ENOLINK instead as this value shouldn't be used currently. This
allows users of CHASE_SAFE to detect without any ambiguities when unsafe
transitions are encountered by chase_symlinks().

All current users of CHASE_SAFE that explicitly reacted on -EPERM have been
converted to react on -ENOLINK.
---

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index e45ad06491a..f91d338507f 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -649,12 +649,12 @@ static int log_unsafe_transition(int a, int b, const char *path, unsigned flags)
         _cleanup_free_ char *n1 = NULL, *n2 = NULL;
 
         if (!FLAGS_SET(flags, CHASE_WARN))
-                return -EPERM;
+                return -ENOLINK;
 
         (void) fd_get_path(a, &n1);
         (void) fd_get_path(b, &n2);
 
-        return log_warning_errno(SYNTHETIC_ERRNO(EPERM),
+        return log_warning_errno(SYNTHETIC_ERRNO(ENOLINK),
                                  "Detected unsafe path transition %s %s %s during canonicalization of %s.",
                                  n1, special_glyph(ARROW), n2, path);
 }
@@ -719,6 +719,10 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
          *    path is fully normalized, and == 0 for each normalization step. This may be combined with
          *    CHASE_NONEXISTENT, in which case 1 is returned when a component is not found.
          *
+         * 4. With CHASE_SAFE: in this case the path must not contain unsafe transitions, i.e. transitions from
+         *    unprivileged to privileged files or directories. In such cases the return value is -ENOLINK. If
+         *    CHASE_WARN is also set a warning describing the unsafe transition is emitted.
+         *
          * */
 
         /* A root directory of "/" or "" is identical to none */
diff --git a/src/core/service.c b/src/core/service.c
index 964a7fd0572..1fafb33f237 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -934,8 +934,8 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         prio = may_warn ? LOG_INFO : LOG_DEBUG;
 
         fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN|CHASE_SAFE, NULL);
-        if (fd == -EPERM) {
-                log_unit_full(UNIT(s), LOG_DEBUG, fd, "Permission denied while opening PID file or potentially unsafe symlink chain, will now retry with relaxed checks: %s", s->pid_file);
+        if (fd == -ENOLINK) {
+                log_unit_full(UNIT(s), LOG_DEBUG, fd, "Potentially unsafe symlink chain, will now retry with relaxed checks: %s", s->pid_file);
 
                 questionable_pid_file = true;
 
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index d5f652c9383..26980d939f4 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -249,11 +249,11 @@ static void test_chase_symlinks(void) {
                 assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
 
                 assert_se(chown(q, 0, 0) >= 0);
-                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -ENOLINK);
 
                 assert_se(rmdir(q) >= 0);
                 assert_se(symlink("/etc/passwd", q) >= 0);
-                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -ENOLINK);
 
                 assert_se(chown(p, 0, 0) >= 0);
                 assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index eeeb1d18506..1f2caf5f738 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -862,7 +862,7 @@ static int path_open_parent_safe(const char *path) {
                 return log_oom();
 
         fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE, NULL);
-        if (fd == -EPERM)
+        if (fd == -ENOLINK)
                 return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path);
         if (fd < 0)
                 return log_error_errno(fd, "Failed to validate path %s: %m", path);
@@ -885,7 +885,7 @@ static int path_open_safe(const char *path) {
                                        path);
 
         fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_NOFOLLOW, NULL);
-        if (fd == -EPERM)
+        if (fd == -ENOLINK)
                 return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path);
         if (fd < 0)
                 return log_error_errno(fd, "Failed to validate path %s: %m", path);