Merge pull request #1173 from JerziKaminsky/security_resolve_symlink

FOR_REVIEW: IPC security - Allow policy targets to be symlinks

10 files changed, 179 insertions, 40 deletions

diff --git a/common/util.c b/common/util.c
index 12ed0cdc..a9e6a9c2 100644
--- a/common/util.c
+++ b/common/util.c
@@ -1,3 +1,7 @@
+#define _XOPEN_SOURCE 500
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
 #include <math.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -118,3 +122,40 @@ uint32_t parse_color(const char *color) {
 	}
 	return res;
 }
+
+char* resolve_path(const char* path) {
+    struct stat sb;
+    ssize_t r;
+    int i;
+    char *current = NULL;
+    char *resolved = NULL;
+
+    if(!(current = strdup(path))) {
+           return NULL;
+    }
+    for (i = 0; i < 16; ++i) {
+        if (lstat(current, &sb) == -1) {
+           goto failed;
+        }
+        if((sb.st_mode & S_IFMT) != S_IFLNK) {
+            return current;
+        }
+        if (!(resolved = malloc(sb.st_size + 1))) {
+           goto failed;
+        }
+        r = readlink(current, resolved, sb.st_size);
+        if (r == -1 || r > sb.st_size) {
+           goto failed;
+        }
+        resolved[r] = '\0';
+        free(current);
+        current = strdup(resolved);
+        free(resolved);
+        resolved = NULL;
+    }
+
+failed:
+    free(resolved);
+    free(current);
+    return NULL;
+}
\ No newline at end of file
diff --git a/include/sway/security.h b/include/sway/security.h
index c3a5cfd4..0edffdfa 100644
--- a/include/sway/security.h
+++ b/include/sway/security.h
@@ -3,9 +3,11 @@
 #include <unistd.h>
 #include "sway/config.h"
 
-uint32_t get_feature_policy(pid_t pid);
-uint32_t get_ipc_policy(pid_t pid);
-uint32_t get_command_policy(const char *cmd);
+uint32_t get_feature_policy_mask(pid_t pid);
+uint32_t get_ipc_policy_mask(pid_t pid);
+uint32_t get_command_policy_mask(const char *cmd);
+
+struct feature_policy *get_feature_policy(const char *name);
 
 const char *command_policy_str(enum command_context context);
 
diff --git a/include/util.h b/include/util.h
index 839af265..e5365458 100644
--- a/include/util.h
+++ b/include/util.h
@@ -49,4 +49,12 @@ pid_t get_parent_pid(pid_t pid);
  */
 uint32_t parse_color(const char *color);
 
+/**
+ * Given a path string, recurseively resolves any symlinks to their targets
+ * (which may be a file, directory) and returns the result.
+ * argument is returned. Caller must free the returned buffer.
+ * If an error occures, if the path does not exist or if the path corresponds
+ * to a dangling symlink, NULL is returned.
+ */
+char* resolve_path(const char* path);
 #endif
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) {