fd-util: introduce fd_verify_safe_flags

As per https://github.com/systemd/systemd/pull/31419#discussion_r1496921074

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index fa78284100f92b47509fb6f245cb185e3eeccd2b..371547facb9e692a57a756cba9132af19ec9d44c 100644 (file)
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -913,6 +913,36 @@ int fd_is_opath(int fd) {
         return FLAGS_SET(r, O_PATH);
 }
 
+int fd_verify_safe_flags(int fd) {
+        int flags, unexpected_flags;
+
+        /* Check if an extrinsic fd is safe to work on (by a privileged service). This ensures that clients
+         * can't trick a privileged service into giving access to a file the client doesn't already have
+         * access to (especially via something like O_PATH).
+         *
+         * O_NOFOLLOW: For some reason the kernel will return this flag from fcntl; it doesn't go away
+         *             immediately after open(). It should have no effect whatsoever to an already-opened FD,
+         *             and since we refuse O_PATH it should be safe.
+         *
+         * RAW_O_LARGEFILE: glibc secretly sets this and neglects to hide it from us if we call fcntl.
+         *                  See comment in missing_fcntl.h for more details about this.
+         */
+
+        assert(fd >= 0);
+
+        flags = fcntl(fd, F_GETFL);
+        if (flags < 0)
+                return -errno;
+
+        unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE);
+        if (unexpected_flags != 0)
+                return log_debug_errno(SYNTHETIC_ERRNO(EREMOTEIO),
+                                       "Unexpected flags set for extrinsic fd: 0%o",
+                                       (unsigned) unexpected_flags);
+
+        return 0;
+}
+
 int read_nr_open(void) {
         _cleanup_free_ char *nr_open = NULL;
         int r;

diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 044811443b69ceee4b1249975e59d9a2bc3dfd60..f549831090a3ff3dca15d406d7bc1de7c4deb368 100644 (file)
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -22,20 +22,6 @@
 #define EBADF_PAIR { -EBADF, -EBADF }
 #define EBADF_TRIPLET { -EBADF, -EBADF, -EBADF }
 
-/* Flags that are safe to have set on an FD given to a privileged service to operate on.
- * This ensures that clients can't trick a privileged service into giving access to a file the client
- * doesn't already have access to (especially via something like O_PATH).
- *
- * O_NOFOLLOW: For some reason the kernel will return this flag from fcntl; it doesn't go away immediately
- *             after open(). It should have no effect whatsoever to an already-opened FD, but if it does
- *             it's decreasing the risk to a privileged service since it disables symlink following.
- *
- * RAW_O_LARGEFILE: glibc secretly sets this and neglects to hide it from us if we call fcntl. See comment
- *                  in missing_fcntl.h for more details about this.
- */
-#define SAFE_FD_FLAGS (O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE)
-#define UNSAFE_FD_FLAGS(flags) ((unsigned)(flags) & ~SAFE_FD_FLAGS)
-
 int close_nointr(int fd);
 int safe_close(int fd);
 void safe_close_pair(int p[static 2]);
@@ -126,7 +112,10 @@ static inline int make_null_stdio(void) {
 
 int fd_reopen(int fd, int flags);
 int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd);
+
 int fd_is_opath(int fd);
+int fd_verify_safe_flags(int fd);
+
 int read_nr_open(void);
 int fd_get_diskseq(int fd, uint64_t *ret);
 

diff --git a/src/home/homed-bus.c b/src/home/homed-bus.c
index bfe23ceb126319a588742bd93c5b394e77db7111..a6f26fea66d93b612100a2453190956c90b556b1 100644 (file)
--- a/src/home/homed-bus.c
+++ b/src/home/homed-bus.c
@@ -89,7 +89,7 @@ int bus_message_read_blobs(sd_bus_message *m, Hashmap **ret, sd_bus_error *error
                 _cleanup_free_ char *filename = NULL;
                 _cleanup_close_ int fd = -EBADF;
                 const char *_filename = NULL;
-                int _fd, flags;
+                int _fd;
 
                 r = sd_bus_message_read(m, "{sh}", &_filename, &_fd);
                 if (r < 0)
@@ -111,18 +111,14 @@ int bus_message_read_blobs(sd_bus_message *m, Hashmap **ret, sd_bus_error *error
 
                 r = fd_verify_regular(fd);
                 if (r < 0)
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "FD for %s is not a regular file", filename);
+                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "FD for '%s' is not a regular file", filename);
 
-                flags = fcntl(fd, F_GETFL);
-                if (flags < 0)
-                        return -errno;
-
-                /* Refuse fds w/ unexpected flags set. In particular, we don't want to permit O_PATH FDs, since
-                 * those don't actually guarantee that the client has access to the file. */
-                if (UNSAFE_FD_FLAGS(flags) != 0)
+                r = fd_verify_safe_flags(fd);
+                if (r == -EREMOTEIO)
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS,
-                                                 "FD for %s has unexpected flags set: 0%o",
-                                                 filename, UNSAFE_FD_FLAGS(flags));
+                                                 "FD for '%s' has unexpected flags set", filename);
+                if (r < 0)
+                        return r;
 
                 r = hashmap_put(blobs, filename, FD_TO_PTR(fd));
                 if (r < 0)

diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index 648d2254fd2f190d76746cb2c9c9c27ad155d189..579a03811b9f6f463422432b3257cb1ba08c7a39 100644 (file)
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -356,18 +356,13 @@ void server_process_native_file(
         if (st.st_size <= 0)
                 return;
 
-        int flags = fcntl(fd, F_GETFL);
-        if (flags < 0) {
-                log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to get flags of passed file, ignoring: %m");
-                return;
-        }
-
-        if (UNSAFE_FD_FLAGS(flags) != 0) {
-                log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
-                                    "Unexpected flags of passed memory fd (0%o), ignoring message: %m",
-                                    UNSAFE_FD_FLAGS(flags));
-                return;
-        }
+        r = fd_verify_safe_flags(fd);
+        if (r == -EREMOTEIO)
+                return (void) log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                        "Unexpected flags of passed memory fd, ignoring message.");
+        if (r < 0)
+                return (void) log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                        "Failed to get flags of passed file: %m");
 
         /* If it's a memfd, check if it is sealed. If so, we can just mmap it and use it, and do not need to
          * copy the data out. */