fd-util: Add helpers to check if FD flags are safe

Adds a SAFE_FD_FLAGS define to list out all the safe FD flags, and also
an UNSAFE_FD_FLAGS() macro to strip out the safe flags and leave only
the unsafe flags. This can be used to quickly check if any unsafe flags
are set and print them for diagnostic purposes

src/basic/fd-util.h

@@ -8,6 +8,7 @@
 #include <sys/socket.h>
 
 #include "macro.h"
+#include "missing_fcntl.h"
 #include "stdio-util.h"
 
 /* maximum length of fdname */
@@ -21,6 +22,20 @@
 #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]);

src/home/homed-bus.c

@@ -3,7 +3,6 @@
 #include "fd-util.h"
 #include "home-util.h"
 #include "homed-bus.h"
-#include "missing_fcntl.h"
 #include "stat-util.h"
 #include "strv.h"
 
@@ -120,8 +119,10 @@ int bus_message_read_blobs(sd_bus_message *m, Hashmap **ret, sd_bus_error *error
 
                 /* 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 ((flags & ~(O_ACCMODE|RAW_O_LARGEFILE)) != 0)
-                        return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "FD for %s has unexpected flags set", filename);
+                if (UNSAFE_FD_FLAGS(flags) != 0)
+                        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));
 
                 r = hashmap_put(blobs, filename, FD_TO_PTR(fd));
                 if (r < 0)

src/journal/journald-native.c

@@ -22,7 +22,6 @@
 #include "journald-wall.h"
 #include "memfd-util.h"
 #include "memory-util.h"
-#include "missing_fcntl.h"
 #include "parse-util.h"
 #include "path-util.h"
 #include "process-util.h"
@@ -363,8 +362,10 @@ void server_process_native_file(
                 return;
         }
 
-        if ((flags & ~(O_ACCMODE|RAW_O_LARGEFILE)) != 0) {
-                log_ratelimit_error(JOURNAL_LOG_RATELIMIT, "Unexpected flags of passed memory fd, ignoring message: %m");
+        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;
         }