Handle symlinks as IPC security targets

- Security policy files name a "policy target" path but
  the policy object itself is keyed by a canonical
  "program target" which is the result of resolving symlinks
  for the "policy target", and also matches the target
  of the /proc/<pid>/exe symlink for a running process.
- policy lookup functions resolve their argument to its
  program target before looking up the policy. If they
  need to create a new one, they pass a "program target",
  not a "policy target" as an argument.
- Renamed validate_ipc_target to validate_ipc_program_target
  to emphasize the fact that it should be called with
  a "program target" not a "policy target". This has
  the unfortunate side-effect that errors report the "program
  target" at fault, not the "policy target" as it appears in
  the config file. This is necessary in order to avoid the
  possible race condition, described below.

Originally, validate_ipc_target() always tried to resolve
its argument for symlinks, and returned a parogram target string
if it validates. This created a possible race condition with
security implications. The problem is that get_feature_policy()
first independently resolved the policy target in order to check
whether a policy already exists. If it didn't find any, it called
alloc_feature_policy() which called validate_ipc_target() which
resolved the policy target again. In the time between the two
checks, the symlink could be altered, and a lucky attacker could
fool the program into thinking that a policy doesn't exist
for a target, and then switch the symlink to point at another file.
At the very least this could allow him to create two policies
for the same program target, and possibly to bypass security
by associating the permissions for one target with another,
or force default permissions to apply to a target for which
a more specific rule has been configured. So we don't that.

Instead, the policy target is resolved once and that result is
used for the rest of the lookup/creation process.

sway/commands/ipc.c

@@ -18,8 +18,11 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
 		return error;
 	}
 
-	const char *program = argv[0];
+	char *program = NULL;
 
+	if (!(program = resolve_path(argv[0]))) {
+		return NULL;
+	}
 	if (config->reading && strcmp("{", argv[1]) != 0) {
 		return cmd_results_new(CMD_INVALID, "ipc",
 				"Expected '{' at start of IPC config definition.");
@@ -32,6 +35,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);
 }
 

sway/security.c

@@ -6,29 +6,30 @@
 #include <stdio.h>
 #include "sway/config.h"
 #include "sway/security.h"
+#include "util.h"
 #include "log.h"
 
-static bool validate_ipc_target(const char *name) {
+static bool validate_ipc_program_target(const char *program) {
 	struct stat sb;
 
-	if (!strcmp(name, "*")) {
+	if (!strcmp(program, "*")) {
 		return true;
 	}
-	if (lstat(name, &sb) == -1) {
+	if (lstat(program, &sb) == -1) {
 		goto failed;
 	}
 	if (!S_ISREG(sb.st_mode)) {
 		sway_log(L_ERROR,
 				"IPC target '%s' MUST be/point at an existing regular file",
-				name);
+				program);
 		goto failed;
 	}
 	if (sb.st_uid != 0) {
-		sway_log(L_ERROR, "IPC target '%s' MUST be owned by root", name);
+		sway_log(L_ERROR, "IPC target '%s' MUST be owned by root", program);
 		goto failed;
 	}
 	if (sb.st_mode & S_IWOTH) {
-		sway_log(L_ERROR, "IPC target '%s' MUST NOT be world writable", name);
+		sway_log(L_ERROR, "IPC target '%s' MUST NOT be world writable", program);
 		goto failed;
 	}
 
@@ -38,11 +39,10 @@ static bool validate_ipc_target(const char *name) {
 	return false;
 }
 
-struct feature_policy *alloc_feature_policy(const char *name) {
+struct feature_policy *alloc_feature_policy(const char *program) {
 	uint32_t default_policy = 0;
-	char *program = NULL;
 
-	if (!validate_ipc_target(name)) {
+	if (!validate_ipc_program_target(program)) {
 		return NULL;
 	}
 	for (int i = 0; i < config->feature_policies->length; ++i) {
@@ -62,20 +62,17 @@ struct feature_policy *alloc_feature_policy(const char *name) {
 		goto failed;
 	}
 	policy->features = default_policy;
-	free(program);
 	return policy;
 
 failed:
-	free(program);
 	free(policy);
 	return NULL;
 }
 
-struct ipc_policy *alloc_ipc_policy(const char *name) {
+struct ipc_policy *alloc_ipc_policy(const char *program) {
 	uint32_t default_policy = 0;
-	char *program = NULL;
 
-	if (!validate_ipc_target(name)) {
+	if (!validate_ipc_program_target(program)) {
 		return NULL;
 	}
 	for (int i = 0; i < config->ipc_policies->length; ++i) {
@@ -95,11 +92,9 @@ struct ipc_policy *alloc_ipc_policy(const char *name) {
 		goto failed;
 	}
 	policy->features = default_policy;
-	free(program);
 	return policy;
 
 failed:
-	free(program);
 	free(policy);
 	return NULL;
 }
@@ -148,20 +143,26 @@ static const char *get_pid_exe(pid_t pid) {
 
 struct feature_policy *get_feature_policy(const char *name) {
 	struct feature_policy *policy = NULL;
+	char *program = NULL;
+
+	if (!(program = resolve_path (name))) {
+		sway_abort ("Unable to allocate security policy");
+	}
 	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) {
+		if (strcmp (p->program, program) == 0) {
 			policy = p;
 			break;
 		}
 	}
 	if (!policy) {
-		policy = alloc_feature_policy (name);
+		policy = alloc_feature_policy (program);
 		if (!policy) {
 			sway_abort ("Unable to allocate security policy");
 		}
 		list_add (config->feature_policies, policy);
 	}
+	free(program);
 	return policy;
 }