Turn DBus profile errors into warnings

This patch also allows setting the DBus policies to filter even if
xdg-dbus-proxy is not installed. In that case, unrestricted access to the bus is
allowed, but a warning is emitted.

2 files changed, 42 insertions, 50 deletions

diff --git a/src/firejail/dbus.c b/src/firejail/dbus.c
index e358aaa49..0f4f18c57 100644
--- a/src/firejail/dbus.c
+++ b/src/firejail/dbus.c
@@ -41,6 +41,7 @@
 #define DBUS_USER_PROXY_SOCKET_FORMAT DBUS_USER_DIR_FORMAT "/%d-user"
 #define DBUS_SYSTEM_PROXY_SOCKET_FORMAT DBUS_USER_DIR_FORMAT "/%d-system"
 #define DBUS_MAX_NAME_LENGTH 255
+#define XDG_DBUS_PROXY_PATH "/usr/bin/xdg-dbus-proxy"
 
 static pid_t dbus_proxy_pid = 0;
 static int dbus_proxy_status_fd = -1;
@@ -78,42 +79,57 @@ int dbus_check_name(const char *name) {
         return in_segment && segments >= 2;
 }
 
-static void dbus_check_bus_profile(char const *prefix, DbusPolicy policy) {
+static void dbus_check_bus_profile(char const *prefix, DbusPolicy *policy) {
+        if (*policy == DBUS_POLICY_FILTER) {
+                struct stat s;
+                if (stat(XDG_DBUS_PROXY_PATH, &s) == -1) {
+                        if (errno == ENOENT) {
+                                fprintf(stderr,
+                                                "Warning: " XDG_DBUS_PROXY_PATH
+                                                " was not found, downgrading %s policy to allow.\n"
+                                                "To enable DBus filtering, install the xdg-dbus-proxy "
+                                                "program.\n", prefix);
+                                *policy = DBUS_POLICY_ALLOW;
+                        } else {
+                                errExit("stat");
+                        }
+                } else {
+                        // No need to warn on profile entries.
+                        return;
+                }
+        }
+
         size_t prefix_length = strlen(prefix);
         ProfileEntry *it = cfg.profile;
+        int num_matches = 0;
+        const char *first_match = NULL;
         while (it) {
                 char *data = it->data;
                 it = it->next;
                 if (strncmp(prefix, data, prefix_length) == 0) {
-                        switch (policy) {
-                        case DBUS_POLICY_ALLOW:
-                                // We should never get here, because profile parsing will fail earlier.
-                                fprintf(stderr,
-                                                "Error: %s filter rule configured, but the bus is not "
-                                                "set to filter.\n",
-                                                prefix);
-                                exit(1);
-                                break;
-                        case DBUS_POLICY_FILTER:
-                                // All good.
-                                break;
-                        case DBUS_POLICY_BLOCK:
-                                fwarning("%s filter rule configured, but the bus is blocked.\n", prefix);
-                                fwarning("Ignoring \"%s\" and any other %s filter rules.\n", data, prefix);
-                                break;
-                        default:
-                                fprintf(stderr, "Error: Unknown %s policy.\n", prefix);
-                                exit(1);
-                                break;
-                        }
-                        break;
+                        ++num_matches;
+                        if (first_match == NULL)
+                                first_match = data;
+                }
+        }
+
+        if (num_matches > 0) {
+                assert(first_match != NULL);
+                if (num_matches == 1) {
+                        fprintf(stderr, "Ignoring \"%s\".\n", first_match);
+                } else if (num_matches == 2) {
+                        fprintf(stderr, "Ignoring \"%s\" and 1 other %s filter rule.\n",
+                                        first_match, prefix);
+                } else {
+                        fprintf(stderr, "Ignoring \"%s\" and %d other %s filter rules.\n",
+                                        first_match, num_matches - 1, prefix);
                 }
         }
 }
 
 void dbus_check_profile(void) {
-        dbus_check_bus_profile("dbus-user", arg_dbus_user);
-        dbus_check_bus_profile("dbus-system", arg_dbus_system);
+        dbus_check_bus_profile("dbus-user", &arg_dbus_user);
+        dbus_check_bus_profile("dbus-system", &arg_dbus_system);
 }
 
 static void write_arg(int fd, char const *format, ...) {
@@ -221,7 +237,7 @@ void dbus_proxy_start(void) {
                         if (i != status_pipe[1] && i != args_pipe[0])
                                 close(i); // close open files
                 }
-                char *args[4] = {"/usr/bin/xdg-dbus-proxy", NULL, NULL, NULL};
+                char *args[4] = {XDG_DBUS_PROXY_PATH, NULL, NULL, NULL};
                 if (asprintf(&args[1], "--fd=%d", status_pipe[1]) == -1
                         || asprintf(&args[2], "--args=%d", args_pipe[0]) == -1)
                         errExit("asprintf");
diff --git a/src/firejail/profile.c b/src/firejail/profile.c
index 9bfd2ff1c..0be119903 100644
--- a/src/firejail/profile.c
+++ b/src/firejail/profile.c
@@ -453,12 +453,6 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 return 0;
         }
         else if (strncmp(ptr, "dbus-user.talk ", 15) == 0) {
-                if (arg_dbus_user == DBUS_POLICY_ALLOW) {
-                        fprintf(stderr, "Session DBus filtering (dbus-user filter) is "
-                                                        "required for dbus-user.talk rules\n");
-                        exit(1);
-                }
-
                 if (!dbus_check_name(ptr + 15)) {
                         printf("Invalid dbus-user.talk name: %s\n", ptr + 15);
                         exit(1);
@@ -466,12 +460,6 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 return 1;
         }
         else if (strncmp(ptr, "dbus-user.own ", 14) == 0) {
-                if (arg_dbus_user == DBUS_POLICY_ALLOW) {
-                        fprintf(stderr, "Session DBus filtering (dbus-user filter) is "
-                                                        "required for dbus-user.own rules\n");
-                        exit(1);
-                }
-
                 if (!dbus_check_name(ptr + 14)) {
                         fprintf(stderr, "Invalid dbus-user.own name: %s\n", ptr + 14);
                         exit(1);
@@ -495,12 +483,6 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 return 0;
         }
         else if (strncmp(ptr, "dbus-system.talk ", 17) == 0) {
-                if (arg_dbus_system == DBUS_POLICY_ALLOW) {
-                        fprintf(stderr, "System DBus filtering (dbus-system filter) is "
-                                                        "required for dbus-system.talk rules\n");
-                        exit(1);
-                }
-
                 if (!dbus_check_name(ptr + 17)) {
                         fprintf(stderr, "Invalid dbus-system.talk name: %s\n", ptr + 17);
                         exit(1);
@@ -508,12 +490,6 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 return 1;
         }
         else if (strncmp(ptr, "dbus-system.own ", 16) == 0) {
-                if (arg_dbus_system == DBUS_POLICY_ALLOW) {
-                        fprintf(stderr, "System DBus filtering (dbus-system filter) is "
-                                                        "required for dbus-system.own rules\n");
-                        exit(1);
-                }
-
                 if (!dbus_check_name(ptr + 16)) {
                         fprintf(stderr, "Invalid dbus-system.own name: %s\n", ptr + 16);
                         exit(1);