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

diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 3691f33ffdbf8e94e6153fdb7758475f8b33bb7a..044811443b69ceee4b1249975e59d9a2bc3dfd60 100644 (file)
--- a/src/basic/fd-util.h
+++ b/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]);

diff --git a/src/home/homed-bus.c b/src/home/homed-bus.c
index bc6e8e37c5b5c3f58b54bfa7b5abc109ae743f9a..bfe23ceb126319a588742bd93c5b394e77db7111 100644 (file)
--- a/src/home/homed-bus.c
+++ b/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)

diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index 75d9082abfc014db482a69c45f3985b532c48b5e..648d2254fd2f190d76746cb2c9c9c27ad155d189 100644 (file)
--- a/src/journal/journald-native.c
+++ b/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;
         }