Merge pull request #1173 from JerziKaminsky/security_resolve_symlink

FOR_REVIEW: IPC security - Allow policy targets to be symlinks

7 files changed, 125 insertions, 37 deletions

diff --git a/sway/commands.c b/sway/commands.c
index 17c7d717..4d7af301 100644
--- a/sway/commands.c
+++ b/sway/commands.c
@@ -437,7 +437,7 @@ struct cmd_results *handle_command(char *_exec, enum command_context context) {
                                 free_argv(argc, argv);
                                 goto cleanup;
                         }
-                        if (!(get_command_policy(argv[0]) & context)) {
+                        if (!(get_command_policy_mask(argv[0]) & context)) {
                                 if (results) {
                                         free_cmd_results(results);
                                 }
diff --git a/sway/commands/ipc.c b/sway/commands/ipc.c
index 8a7b849f..f0b3035a 100644
--- a/sway/commands/ipc.c
+++ b/sway/commands/ipc.c
@@ -1,3 +1,4 @@
+#define _XOPEN_SOURCE 500
 #include <stdio.h>
 #include <string.h>
 #include "sway/security.h"
@@ -18,8 +19,14 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
                 return error;
         }
 
-        const char *program = argv[0];
+        char *program = NULL;
 
+        if (!strcmp(argv[0], "*")) {
+                program = strdup(argv[0]);
+        } else if (!(program = resolve_path(argv[0]))) {
+                return cmd_results_new(
+                                CMD_INVALID, "ipc", "Unable to resolve IPC Policy target.");
+        }
         if (config->reading && strcmp("{", argv[1]) != 0) {
                 return cmd_results_new(CMD_INVALID, "ipc",
                                 "Expected '{' at start of IPC config definition.");
@@ -32,6 +39,7 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
         current_policy = alloc_ipc_policy(program);
         list_add(config->ipc_policies, current_policy);
 
+        free(program);
         return cmd_results_new(CMD_BLOCK_IPC, NULL, NULL);
 }
 
diff --git a/sway/commands/permit.c b/sway/commands/permit.c
index e2bec2e2..66fa4e2a 100644
--- a/sway/commands/permit.c
+++ b/sway/commands/permit.c
@@ -1,7 +1,9 @@
+#define _XOPEN_SOURCE 500
 #include <string.h>
 #include "sway/commands.h"
 #include "sway/config.h"
 #include "sway/security.h"
+#include "util.h"
 #include "log.h"
 
 static enum secure_feature get_features(int argc, char **argv,
@@ -38,25 +40,6 @@ static enum secure_feature get_features(int argc, char **argv,
         return features;
 }
 
-static struct feature_policy *get_policy(const char *name) {
-        struct feature_policy *policy = NULL;
-        for (int i = 0; i < config->feature_policies->length; ++i) {
-                struct feature_policy *p = config->feature_policies->items[i];
-                if (strcmp(p->program, name) == 0) {
-                        policy = p;
-                        break;
-                }
-        }
-        if (!policy) {
-                policy = alloc_feature_policy(name);
-                if (!policy) {
-                        sway_abort("Unable to allocate security policy");
-                }
-                list_add(config->feature_policies, policy);
-        }
-        return policy;
-}
-
 struct cmd_results *cmd_permit(int argc, char **argv) {
         struct cmd_results *error = NULL;
         if ((error = checkarg(argc, "permit", EXPECTED_MORE_THAN, 1))) {
@@ -66,12 +49,29 @@ struct cmd_results *cmd_permit(int argc, char **argv) {
                 return error;
         }
 
-        struct feature_policy *policy = get_policy(argv[0]);
-        policy->features |= get_features(argc, argv, &error);
+        bool assign_perms = true;
+        char *program = NULL;
 
+        if (!strcmp(argv[0], "*")) {
+                program = strdup(argv[0]);
+        } else {
+                program = resolve_path(argv[0]);
+        }
+        if (!program) {
+                sway_assert(program, "Unable to resolve IPC permit target '%s'."
+                        " will issue empty policy", argv[0]);
+                assign_perms = false;
+                program = strdup(argv[0]);
+        }
+
+        struct feature_policy *policy = get_feature_policy(program);
+        if (assign_perms) {
+                policy->features |= get_features(argc, argv, &error);
+        }
         sway_log(L_DEBUG, "Permissions granted to %s for features %d",
                         policy->program, policy->features);
 
+        free(program);
         return cmd_results_new(CMD_SUCCESS, NULL, NULL);
 }
 
@@ -84,11 +84,25 @@ struct cmd_results *cmd_reject(int argc, char **argv) {
                 return error;
         }
 
-        struct feature_policy *policy = get_policy(argv[0]);
+        char *program = NULL;
+        if (!strcmp(argv[0], "*")) {
+                program = strdup(argv[0]);
+        } else {
+                program = resolve_path(argv[0]);
+        }
+        if (!program) {
+                // Punt
+                sway_log(L_INFO, "Unable to resolve IPC reject target '%s'."
+                        " Will use provided path", argv[0]);
+                program = strdup(argv[0]);
+        }
+
+        struct feature_policy *policy = get_feature_policy(program);
         policy->features &= ~get_features(argc, argv, &error);
 
         sway_log(L_DEBUG, "Permissions granted to %s for features %d",
                         policy->program, policy->features);
 
+        free(program);
         return cmd_results_new(CMD_SUCCESS, NULL, NULL);
 }
diff --git a/sway/extensions.c b/sway/extensions.c
index 15d2f971..96957dbf 100644
--- a/sway/extensions.c
+++ b/sway/extensions.c
@@ -86,7 +86,7 @@ static void set_background(struct wl_client *client, struct wl_resource *resourc
                 struct wl_resource *_output, struct wl_resource *surface) {
         pid_t pid;
         wl_client_get_credentials(client, &pid, NULL, NULL);
-        if (!(get_feature_policy(pid) & FEATURE_BACKGROUND)) {
+        if (!(get_feature_policy_mask(pid) & FEATURE_BACKGROUND)) {
                 sway_log(L_INFO, "Denying background feature to %d", pid);
                 return;
         }
@@ -114,7 +114,7 @@ static void set_panel(struct wl_client *client, struct wl_resource *resource,
                 struct wl_resource *_output, struct wl_resource *surface) {
         pid_t pid;
         wl_client_get_credentials(client, &pid, NULL, NULL);
-        if (!(get_feature_policy(pid) & FEATURE_PANEL)) {
+        if (!(get_feature_policy_mask(pid) & FEATURE_PANEL)) {
                 sway_log(L_INFO, "Denying panel feature to %d", pid);
                 return;
         }
@@ -152,7 +152,7 @@ static void desktop_ready(struct wl_client *client, struct wl_resource *resource
 static void set_panel_position(struct wl_client *client, struct wl_resource *resource, uint32_t position) {
         pid_t pid;
         wl_client_get_credentials(client, &pid, NULL, NULL);
-        if (!(get_feature_policy(pid) & FEATURE_PANEL)) {
+        if (!(get_feature_policy_mask(pid) & FEATURE_PANEL)) {
                 sway_log(L_INFO, "Denying panel feature to %d", pid);
                 return;
         }
@@ -191,7 +191,7 @@ static void set_lock_surface(struct wl_client *client, struct wl_resource *resou
                 struct wl_resource *_output, struct wl_resource *surface) {
         pid_t pid;
         wl_client_get_credentials(client, &pid, NULL, NULL);
-        if (!(get_feature_policy(pid) & FEATURE_LOCK)) {
+        if (!(get_feature_policy_mask(pid) & FEATURE_LOCK)) {
                 sway_log(L_INFO, "Denying lock feature to %d", pid);
                 return;
         }
diff --git a/sway/handlers.c b/sway/handlers.c
index b61c0a19..a8de135f 100644
--- a/sway/handlers.c
+++ b/sway/handlers.c
@@ -595,7 +595,7 @@ static void handle_view_state_request(wlc_handle view, enum wlc_view_state_bit s
         pid_t pid = wlc_view_get_pid(view);
         switch (state) {
         case WLC_BIT_FULLSCREEN:
-                if (!(get_feature_policy(pid) & FEATURE_FULLSCREEN)) {
+                if (!(get_feature_policy_mask(pid) & FEATURE_FULLSCREEN)) {
                         sway_log(L_INFO, "Denying fullscreen to %d (%s)", pid, c->name);
                         break;
                 }
@@ -811,7 +811,7 @@ static bool handle_key(wlc_handle view, uint32_t time, const struct wlc_modifier
         swayc_t *focused = get_focused_container(&root_container);
         if (focused->type == C_VIEW) {
                 pid_t pid = wlc_view_get_pid(focused->handle);
-                if (!(get_feature_policy(pid) & FEATURE_KEYBOARD)) {
+                if (!(get_feature_policy_mask(pid) & FEATURE_KEYBOARD)) {
                         return EVENT_HANDLED;
                 }
         }
@@ -875,7 +875,7 @@ static bool handle_pointer_motion(wlc_handle handle, uint32_t time, const struct
         swayc_t *focused = get_focused_container(&root_container);
         if (focused->type == C_VIEW) {
                 pid_t pid = wlc_view_get_pid(focused->handle);
-                if (!(get_feature_policy(pid) & FEATURE_MOUSE)) {
+                if (!(get_feature_policy_mask(pid) & FEATURE_MOUSE)) {
                         return EVENT_HANDLED;
                 }
         }
@@ -953,7 +953,7 @@ static bool handle_pointer_button(wlc_handle view, uint32_t time, const struct w
         if (swayc_is_fullscreen(focused)) {
                 if (focused->type == C_VIEW) {
                         pid_t pid = wlc_view_get_pid(focused->handle);
-                        if (!(get_feature_policy(pid) & FEATURE_MOUSE)) {
+                        if (!(get_feature_policy_mask(pid) & FEATURE_MOUSE)) {
                                 return EVENT_HANDLED;
                         }
                 }
@@ -1001,7 +1001,7 @@ static bool handle_pointer_button(wlc_handle view, uint32_t time, const struct w
 
         if (focused->type == C_VIEW) {
                 pid_t pid = wlc_view_get_pid(focused->handle);
-                if (!(get_feature_policy(pid) & FEATURE_MOUSE)) {
+                if (!(get_feature_policy_mask(pid) & FEATURE_MOUSE)) {
                         return EVENT_HANDLED;
                 }
         }
diff --git a/sway/ipc-server.c b/sway/ipc-server.c
index 67a3cdc8..dca881fa 100644
--- a/sway/ipc-server.c
+++ b/sway/ipc-server.c
@@ -181,7 +181,7 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) {
         client->event_source = wlc_event_loop_add_fd(client_fd, WLC_EVENT_READABLE, ipc_client_handle_readable, client);
 
         pid_t pid = get_client_pid(client->fd);
-        client->security_policy = get_ipc_policy(pid);
+        client->security_policy = get_ipc_policy_mask(pid);
 
         list_add(ipc_client_list, client);
 
diff --git a/sway/security.c b/sway/security.c
index f8a96ba7..8eab6126 100644
--- a/sway/security.c
+++ b/sway/security.c
@@ -1,4 +1,6 @@
 #define _XOPEN_SOURCE 500
+#include <sys/types.h>
+#include <sys/stat.h>
 #include <string.h>
 #include <unistd.h>
 #include <stdio.h>
@@ -6,8 +8,46 @@
 #include "sway/security.h"
 #include "log.h"
 
+static bool validate_ipc_target(const char *program) {
+        struct stat sb;
+
+        sway_log(L_DEBUG, "Validating IPC target '%s'", program);
+
+        if (!strcmp(program, "*")) {
+                return true;
+        }
+        if (lstat(program, &sb) == -1) {
+                return false;
+        }
+        if (!S_ISREG(sb.st_mode)) {
+                sway_log(L_ERROR,
+                                "IPC target '%s' MUST be/point at an existing regular file",
+                                program);
+                return false;
+        }
+        if (sb.st_uid != 0) {
+#ifdef NDEBUG
+                sway_log(L_ERROR, "IPC target '%s' MUST be owned by root", program);
+                return false;
+#else
+                sway_log(L_INFO, "IPC target '%s' MUST be owned by root (waived for debug build)", program);
+                return true;
+#endif
+        }
+        if (sb.st_mode & S_IWOTH) {
+                sway_log(L_ERROR, "IPC target '%s' MUST NOT be world writable", program);
+                return false;
+        }
+
+        return true;
+}
+
 struct feature_policy *alloc_feature_policy(const char *program) {
         uint32_t default_policy = 0;
+
+        if (!validate_ipc_target(program)) {
+                return NULL;
+        }
         for (int i = 0; i < config->feature_policies->length; ++i) {
                 struct feature_policy *policy = config->feature_policies->items[i];
                 if (strcmp(policy->program, "*") == 0) {
@@ -26,11 +66,16 @@ struct feature_policy *alloc_feature_policy(const char *program) {
                 return NULL;
         }
         policy->features = default_policy;
+
         return policy;
 }
 
 struct ipc_policy *alloc_ipc_policy(const char *program) {
         uint32_t default_policy = 0;
+
+        if (!validate_ipc_target(program)) {
+                return NULL;
+        }
         for (int i = 0; i < config->ipc_policies->length; ++i) {
                 struct ipc_policy *policy = config->ipc_policies->items[i];
                 if (strcmp(policy->program, "*") == 0) {
@@ -49,6 +94,7 @@ struct ipc_policy *alloc_ipc_policy(const char *program) {
                 return NULL;
         }
         policy->features = default_policy;
+
         return policy;
 }
 
@@ -94,7 +140,27 @@ static const char *get_pid_exe(pid_t pid) {
         return link;
 }
 
-uint32_t get_feature_policy(pid_t pid) {
+struct feature_policy *get_feature_policy(const char *name) {
+        struct feature_policy *policy = NULL;
+
+        for (int i = 0; i < config->feature_policies->length; ++i) {
+                struct feature_policy *p = config->feature_policies->items[i];
+                if (strcmp(p->program, name) == 0) {
+                        policy = p;
+                        break;
+                }
+        }
+        if (!policy) {
+                policy = alloc_feature_policy(name);
+                if (!policy) {
+                        sway_abort("Unable to allocate security policy");
+                }
+                list_add(config->feature_policies, policy);
+        }
+        return policy;
+}
+
+uint32_t get_feature_policy_mask(pid_t pid) {
         uint32_t default_policy = 0;
         const char *link = get_pid_exe(pid);
 
@@ -111,7 +177,7 @@ uint32_t get_feature_policy(pid_t pid) {
         return default_policy;
 }
 
-uint32_t get_ipc_policy(pid_t pid) {
+uint32_t get_ipc_policy_mask(pid_t pid) {
         uint32_t default_policy = 0;
         const char *link = get_pid_exe(pid);
 
@@ -128,7 +194,7 @@ uint32_t get_ipc_policy(pid_t pid) {
         return default_policy;
 }
 
-uint32_t get_command_policy(const char *cmd) {
+uint32_t get_command_policy_mask(const char *cmd) {
         uint32_t default_policy = 0;
 
         for (int i = 0; i < config->command_policies->length; ++i) {