Merge pull request #3406 from kris7t/dbus-proxy

DBus filtering enhancements

5 files changed, 242 insertions, 14 deletions

diff --git a/src/firejail/dbus.c b/src/firejail/dbus.c
index 0f4f18c57..18576612d 100644
--- a/src/firejail/dbus.c
+++ b/src/firejail/dbus.c
@@ -48,7 +48,7 @@ static int dbus_proxy_status_fd = -1;
 static char *dbus_user_proxy_socket = NULL;
 static char *dbus_system_proxy_socket = NULL;
 
-int dbus_check_name(const char *name) {
+static int check_bus_or_interface_name(const char *name, int hyphens_allowed) {
         unsigned long length = strlen(name);
         if (length == 0 || length > DBUS_MAX_NAME_LENGTH)
                 return 0;
@@ -62,14 +62,48 @@ int dbus_check_name(const char *name) {
                         if (*p == '.') {
                                 ++segments;
                                 in_segment = 0;
-                        } else if (!alpha && !digit && *p != '_' && *p != '-') {
+                        } else if (!alpha && !digit && *p != '_' && (!hyphens_allowed || *p != '-')) {
+                                return 0;
+                        }
+                }
+                else {
+                        if (*p == '*') {
+                                return *(p + 1) == '\0';
+                        } else if (!alpha && *p != '_' && (!hyphens_allowed || *p != '-')) {
+                                return 0;
+                        }
+                        in_segment = 1;
+                }
+                ++p;
+        }
+        return in_segment && segments >= 2;
+}
+
+static int check_object_path(const char *path) {
+        unsigned long length = strlen(path);
+        if (length == 0 || path[0] != '/')
+                return 0;
+        // The root path "/" is the only path allowed to have a trailing slash.
+        if (length == 1)
+                return 1;
+        const char *p = path + 1;
+        int segments = 1;
+        int in_segment = 0;
+        while (*p) {
+                int alpha = (*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z');
+                int digit = *p >= '0' && *p <= '9';
+                if (in_segment) {
+                        if (*p == '/') {
+                                ++segments;
+                                in_segment = 0;
+                        } else if (!alpha && !digit && *p != '_') {
                                 return 0;
                         }
                 }
                 else {
                         if (*p == '*') {
                                 return *(p + 1) == '\0';
-                        } else if (!alpha && *p != '_' && *p != '-') {
+                        } else if (!alpha && *p != '_') {
                                 return 0;
                         }
                         in_segment = 1;
@@ -79,6 +113,38 @@ int dbus_check_name(const char *name) {
         return in_segment && segments >= 2;
 }
 
+int dbus_check_name(const char *name) {
+        return check_bus_or_interface_name(name, 1);
+}
+
+int dbus_check_call_rule(const char *rule) {
+        char buf[DBUS_MAX_NAME_LENGTH + 1];
+        char *name_end = strchr(rule, '=');
+        if (name_end == NULL)
+                return 0;
+        size_t name_length = (size_t) (name_end - rule);
+        if (name_length > DBUS_MAX_NAME_LENGTH)
+                return 0;
+        strncpy(buf, rule, (size_t) name_length);
+        buf[name_length] = '\0';
+        if (!dbus_check_name(buf))
+                return 0;
+        ++name_end;
+        char *interface_end = strchr(name_end, '@');
+        if (interface_end == NULL)
+                return check_bus_or_interface_name(name_end, 0);
+        size_t interface_length = (size_t) (interface_end - name_end);
+        if (interface_length > DBUS_MAX_NAME_LENGTH)
+                return 0;
+        if (interface_length > 0) {
+                strncpy(buf, name_end, interface_length);
+                buf[interface_length] = '\0';
+                if (!check_bus_or_interface_name(buf, 0))
+                        return 0;
+        }
+        return check_object_path(interface_end + 1);
+}
+
 static void dbus_check_bus_profile(char const *prefix, DbusPolicy *policy) {
         if (*policy == DBUS_POLICY_FILTER) {
                 struct stat s;
@@ -219,6 +285,8 @@ static char *find_user_socket(void) {
 void dbus_proxy_start(void) {
         dbus_create_user_dir();
 
+        EUID_USER();
+
         int status_pipe[2];
         if (pipe(status_pipe) == -1)
                 errExit("pipe");
@@ -233,10 +301,21 @@ void dbus_proxy_start(void) {
                 errExit("fork");
         if (dbus_proxy_pid == 0) {
                 int i;
-                for (i = 3; i < FIREJAIL_MAX_FD; i++) {
+                for (i = STDERR_FILENO + 1; i < FIREJAIL_MAX_FD; i++) {
                         if (i != status_pipe[1] && i != args_pipe[0])
                                 close(i); // close open files
                 }
+                if (arg_dbus_log_file != NULL) {
+                        int output_fd = creat(arg_dbus_log_file, 0666);
+                        if (output_fd < 0)
+                                errExit("creat");
+                        if (output_fd != STDOUT_FILENO) {
+                                if (dup2(output_fd, STDOUT_FILENO) != STDOUT_FILENO)
+                                        errExit("dup2");
+                                close(output_fd);
+                        }
+                }
+                close(STDIN_FILENO);
                 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)
@@ -262,6 +341,9 @@ void dbus_proxy_start(void) {
                                                  (int) getuid(), (int) getpid()) == -1)
                                 errExit("asprintf");
                         write_arg(args_pipe[1], "%s", dbus_user_proxy_socket);
+                        if (arg_dbus_log_user) {
+                                write_arg(args_pipe[1], "--log");
+                        }
                         write_arg(args_pipe[1], "--filter");
                         write_profile(args_pipe[1], "dbus-user.");
                 }
@@ -278,6 +360,9 @@ void dbus_proxy_start(void) {
                                                  (int) getuid(), (int) getpid()) == -1)
                                 errExit("asprintf");
                         write_arg(args_pipe[1], "%s", dbus_system_proxy_socket);
+                        if (arg_dbus_log_system) {
+                                write_arg(args_pipe[1], "--log");
+                        }
                         write_arg(args_pipe[1], "--filter");
                         write_profile(args_pipe[1], "dbus-system.");
                 }
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 9b288b1b9..1ef4887ea 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -349,6 +349,9 @@ typedef enum {
 } DbusPolicy;
 extern DbusPolicy arg_dbus_user; // --dbus-user
 extern DbusPolicy arg_dbus_system; // --dbus-system
+extern int arg_dbus_log_user;
+extern int arg_dbus_log_system;
+extern const char *arg_dbus_log_file;
 
 extern int login_shell;
 extern int parent_to_child_fds[2];
@@ -847,6 +850,7 @@ void set_profile_run_file(pid_t pid, const char *fname);
 
 // dbus.c
 int dbus_check_name(const char *name);
+int dbus_check_call_rule(const char *name);
 void dbus_check_profile(void);
 void dbus_proxy_start(void);
 void dbus_proxy_stop(void);
diff --git a/src/firejail/main.c b/src/firejail/main.c
index dc213b988..958374c43 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -148,6 +148,9 @@ int arg_nou2f = 0; // --nou2f
 int arg_deterministic_exit_code = 0;    // always exit with first child's exit status
 DbusPolicy arg_dbus_user = DBUS_POLICY_ALLOW;   // --dbus-user
 DbusPolicy arg_dbus_system = DBUS_POLICY_ALLOW; // --dbus-system
+const char *arg_dbus_log_file = NULL;
+int arg_dbus_log_user = 0;
+int arg_dbus_log_system = 0;
 int login_shell = 0;
 
 //**********************************************************************************
@@ -2062,17 +2065,29 @@ int main(int argc, char **argv, char **envp) {
                 else if (strncmp("--dbus-user=", argv[i], 12) == 0) {
                         if (strcmp("filter", argv[i] + 12) == 0) {
                                 if (arg_dbus_user == DBUS_POLICY_BLOCK) {
-                                        fprintf(stderr, "Error: Cannot relax --dbus-user policy, it is already set to block\n");
-                                        exit(1);
+                                        fprintf(stderr, "Warning: Cannot relax --dbus-user policy, it is already set to block\n");
+                                } else {
+                                        arg_dbus_user = DBUS_POLICY_FILTER;
                                 }
-                                arg_dbus_user = DBUS_POLICY_FILTER;
                         } else if (strcmp("none", argv[i] + 12) == 0) {
+                                if (arg_dbus_log_user) {
+                                        fprintf(stderr, "Error: --dbus-user.log requires --dbus-user=filter\n");
+                                        exit(1);
+                                }
                                 arg_dbus_user = DBUS_POLICY_BLOCK;
                         } else {
                                 fprintf(stderr, "Unknown dbus-user policy: %s\n", argv[i] + 12);
                                 exit(1);
                         }
                 }
+                else if (strncmp(argv[i], "--dbus-user.see=", 16) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-user.see %s", argv[i] + 16) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
                 else if (strncmp(argv[i], "--dbus-user.talk=", 17) == 0) {
                         char *line;
                         if (asprintf(&line, "dbus-user.talk %s", argv[i] + 17) == -1)
@@ -2089,20 +2104,48 @@ int main(int argc, char **argv, char **envp) {
                         profile_check_line(line, 0, NULL); // will exit if something wrong
                         profile_add(line);
                 }
+                else if (strncmp(argv[i], "--dbus-user.call=", 17) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-user.call %s", argv[i] + 17) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
+                else if (strncmp(argv[i], "--dbus-user.broadcast=", 22) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-user.broadcast %s", argv[i] + 22) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
                 else if (strncmp("--dbus-system=", argv[i], 14) == 0) {
                         if (strcmp("filter", argv[i] + 14) == 0) {
                                 if (arg_dbus_system == DBUS_POLICY_BLOCK) {
-                                        fprintf(stderr, "Error: Cannot relax --dbus-system policy, it is already set to block\n");
-                                        exit(1);
+                                        fprintf(stderr, "Warning: Cannot relax --dbus-system policy, it is already set to block\n");
+                                } else {
+                                        arg_dbus_system = DBUS_POLICY_FILTER;
                                 }
-                                arg_dbus_system = DBUS_POLICY_FILTER;
                         } else if (strcmp("none", argv[i] + 14) == 0) {
+                                if (arg_dbus_log_system) {
+                                        fprintf(stderr, "Error: --dbus-system.log requires --dbus-system=filter\n");
+                                        exit(1);
+                                }
                                 arg_dbus_system = DBUS_POLICY_BLOCK;
                         } else {
                                 fprintf(stderr, "Unknown dbus-system policy: %s\n", argv[i] + 14);
                                 exit(1);
                         }
                 }
+                else if (strncmp(argv[i], "--dbus-system.see=", 18) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-system.see %s", argv[i] + 18) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
                 else if (strncmp(argv[i], "--dbus-system.talk=", 19) == 0) {
                         char *line;
                         if (asprintf(&line, "dbus-system.talk %s", argv[i] + 19) == -1)
@@ -2119,6 +2162,43 @@ int main(int argc, char **argv, char **envp) {
                         profile_check_line(line, 0, NULL); // will exit if something wrong
                         profile_add(line);
                 }
+                else if (strncmp(argv[i], "--dbus-system.call=", 19) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-system.call %s", argv[i] + 19) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
+                else if (strncmp(argv[i], "--dbus-system.broadcast=", 24) == 0) {
+                        char *line;
+                        if (asprintf(&line, "dbus-system.broadcast %s", argv[i] + 24) == -1)
+                                errExit("asprintf");
+
+                        profile_check_line(line, 0, NULL); // will exit if something wrong
+                        profile_add(line);
+                }
+                else if (strncmp(argv[i], "--dbus-log=", 11) == 0) {
+                        if (arg_dbus_log_file != NULL) {
+                                fprintf(stderr, "Error: --dbus-log option already specified\n");
+                                exit(1);
+                        }
+                        arg_dbus_log_file = argv[i] + 11;
+                }
+                else if (strcmp(argv[i], "--dbus-user.log") == 0) {
+                        if (arg_dbus_user != DBUS_POLICY_FILTER) {
+                                fprintf(stderr, "Error: --dbus-user.log requires --dbus-user=filter\n");
+                                exit(1);
+                        }
+                        arg_dbus_log_user = 1;
+                }
+                else if (strcmp(argv[i], "--dbus-system.log") == 0) {
+                        if (arg_dbus_system != DBUS_POLICY_FILTER) {
+                                fprintf(stderr, "Error: --dbus-system.log requires --dbus-system=filter\n");
+                                exit(1);
+                        }
+                        arg_dbus_log_system = 1;
+                }
 
                 //*************************************
                 // network
diff --git a/src/firejail/profile.c b/src/firejail/profile.c
index 0be119903..a87222824 100644
--- a/src/firejail/profile.c
+++ b/src/firejail/profile.c
@@ -441,10 +441,14 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 if (strcmp("filter", ptr) == 0) {
                         if (arg_dbus_user == DBUS_POLICY_BLOCK) {
                                 fprintf(stderr, "Error: Cannot relax dbus-user policy, it is already set to block\n");
-                                exit(1);
+                        } else {
+                                arg_dbus_user = DBUS_POLICY_FILTER;
                         }
-                        arg_dbus_user = DBUS_POLICY_FILTER;
                 } else if (strcmp("none", ptr) == 0) {
+                        if (arg_dbus_log_user) {
+                                fprintf(stderr, "Error: --dbus-user.log requires --dbus-user=filter\n");
+                                exit(1);
+                        }
                         arg_dbus_user = DBUS_POLICY_BLOCK;
                 } else {
                         fprintf(stderr, "Unknown dbus-user policy: %s\n", ptr);
@@ -452,6 +456,13 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 }
                 return 0;
         }
+        else if (strncmp(ptr, "dbus-user.see ", 14) == 0) {
+                if (!dbus_check_name(ptr + 14)) {
+                        printf("Invalid dbus-user.see name: %s\n", ptr + 15);
+                        exit(1);
+                }
+                return 1;
+        }
         else if (strncmp(ptr, "dbus-user.talk ", 15) == 0) {
                 if (!dbus_check_name(ptr + 15)) {
                         printf("Invalid dbus-user.talk name: %s\n", ptr + 15);
@@ -466,15 +477,33 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 }
                 return 1;
         }
+        else if (strncmp(ptr, "dbus-user.call ", 15) == 0) {
+                if (!dbus_check_call_rule(ptr + 15)) {
+                        fprintf(stderr, "Invalid dbus-user.call rule: %s\n", ptr + 15);
+                        exit(1);
+                }
+                return 1;
+        }
+        else if (strncmp(ptr, "dbus-user.broadcast ", 20) == 0) {
+                if (!dbus_check_call_rule(ptr + 20)) {
+                        fprintf(stderr, "Invalid dbus-user.broadcast rule: %s\n", ptr + 20);
+                        exit(1);
+                }
+                return 1;
+        }
         else if (strncmp("dbus-system ", ptr, 12) == 0) {
                 ptr += 12;
                 if (strcmp("filter", ptr) == 0) {
                         if (arg_dbus_system == DBUS_POLICY_BLOCK) {
                                 fprintf(stderr, "Error: Cannot relax dbus-system policy, it is already set to block\n");
-                                exit(1);
+                        } else {
+                                arg_dbus_system = DBUS_POLICY_FILTER;
                         }
-                        arg_dbus_system = DBUS_POLICY_FILTER;
                 } else if (strcmp("none", ptr) == 0) {
+                        if (arg_dbus_log_system) {
+                                fprintf(stderr, "Error: --dbus-system.log requires --dbus-system=filter\n");
+                                exit(1);
+                        }
                         arg_dbus_system = DBUS_POLICY_BLOCK;
                 } else {
                         fprintf(stderr, "Unknown dbus-system policy: %s\n", ptr);
@@ -482,6 +511,13 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 }
                 return 0;
         }
+        else if (strncmp(ptr, "dbus-system.see ", 16) == 0) {
+                if (!dbus_check_name(ptr + 16)) {
+                        fprintf(stderr, "Invalid dbus-system.see name: %s\n", ptr + 17);
+                        exit(1);
+                }
+                return 1;
+        }
         else if (strncmp(ptr, "dbus-system.talk ", 17) == 0) {
                 if (!dbus_check_name(ptr + 17)) {
                         fprintf(stderr, "Invalid dbus-system.talk name: %s\n", ptr + 17);
@@ -496,6 +532,20 @@ int profile_check_line(char *ptr, int lineno, const char *fname) {
                 }
                 return 1;
         }
+        else if (strncmp(ptr, "dbus-system.call ", 17) == 0) {
+                if (!dbus_check_call_rule(ptr + 17)) {
+                        fprintf(stderr, "Invalid dbus-system.call rule: %s\n", ptr + 17);
+                        exit(1);
+                }
+                return 1;
+        }
+        else if (strncmp(ptr, "dbus-system.broadcast ", 22) == 0) {
+                if (!dbus_check_call_rule(ptr + 22)) {
+                        fprintf(stderr, "Invalid dbus-system.broadcast rule: %s\n", ptr + 22);
+                        exit(1);
+                }
+                return 1;
+        }
         else if (strcmp(ptr, "nou2f") == 0) {
                 arg_nou2f = 1;
                 return 0;
diff --git a/src/firejail/usage.c b/src/firejail/usage.c
index 08546fa51..4ab464289 100644
--- a/src/firejail/usage.c
+++ b/src/firejail/usage.c
@@ -53,11 +53,20 @@ static char *usage_str =
 #endif
         "    --cpu=cpu-number,cpu-number - set cpu affinity.\n"
         "    --cpu.print=name|pid - print the cpus in use.\n"
+        "    --dbus-log=file - set DBus log file location.\n"
         "    --dbus-system=filter|none - set system DBus access policy.\n"
+        "    --dbus-system.broadcast=rule - allow signals on the system DBus according to rule.\n"
+        "    --dbus-system.call=rule - allow calls on the system DBus according to rule.\n"
+        "    --dbus-system.log - turn on logging for the system DBus."
         "    --dbus-system.own=name - allow ownership of name on the system DBus.\n"
+        "    --dbus-system.see=name - allow seeing name on the system DBus.\n"
         "    --dbus-system.talk=name - allow talking to name on the system DBus.\n"
         "    --dbus-user=filter|none - set session DBus access policy.\n"
+        "    --dbus-user.broadcast=rule - allow signals on the session DBus according to rule.\n"
+        "    --dbus-user.call=rule - allow calls on the session DBus according to rule.\n"
+        "    --dbus-user.log - turn on logging for the user DBus."
         "    --dbus-user.own=name - allow ownership of name on the session DBus.\n"
+        "    --dbus-user.see=name - allow seeing name on the session DBus.\n"
         "    --dbus-user.talk=name - allow talking to name on the session DBus.\n"
         "    --debug - print sandbox debug messages.\n"
         "    --debug-blacklists - debug blacklisting.\n"