Refactor IPC target validation

- `ipc` config option shouldn't fail if target doesn't exist
- Added resolve_ipc_path() to reduce boilerplate
- Policy allocators now store the result of validate_ipc_target()
  under policy->validated.
- cmd_permit/_reject use policy->validated to make decisions.
- sway_log instead of sway_assert on unvalidated target, we don't want
  to abort, even in debug builds.

include/sway/config.h

@@ -208,6 +208,7 @@ enum secure_feature {
 struct feature_policy {
 	char *program;
 	uint32_t features;
+	bool validated;
 };
 
 enum ipc_feature {
@@ -235,6 +236,7 @@ enum ipc_feature {
 struct ipc_policy {
 	char *program;
 	uint32_t features;
+	bool validated;
 };
 
 /**

include/sway/security.h

@@ -15,4 +15,5 @@ struct feature_policy *alloc_feature_policy(const char *program);
 struct ipc_policy *alloc_ipc_policy(const char *program);
 struct command_policy *alloc_command_policy(const char *command);
 
+char *resolve_ipc_path(const char* program);
 #endif

sway/commands/ipc.c

@@ -21,11 +21,9 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
 
 	char *program = NULL;
 
-	if (!strcmp(argv[0], "*")) {
-		program = strdup(argv[0]);
-	} else if (!(program = resolve_path(argv[0]))) {
+	if (!(program = resolve_ipc_path(argv[0]))) {
 		return cmd_results_new(
-				CMD_INVALID, "ipc", "Unable to resolve IPC Policy target.");
+				CMD_FAILURE, "ipc", "Memory Allocation error occured.");
 	}
 	if (config->reading && strcmp("{", argv[1]) != 0) {
 		return cmd_results_new(CMD_INVALID, "ipc",

sway/commands/permit.c

@@ -49,27 +49,20 @@ struct cmd_results *cmd_permit(int argc, char **argv) {
 		return 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]);
+	if (!(program = resolve_ipc_path(argv[0]))) {
+		sway_abort("memory allocation failed");
 	}
 
 	struct feature_policy *policy = get_feature_policy(program);
-	if (assign_perms) {
+	if (policy->validated) {
 		policy->features |= get_features(argc, argv, &error);
+		sway_log(L_DEBUG, "Permissions granted to %s for features %d",
+				policy->program, policy->features);
+	} else {
+		sway_log(L_ERROR, "Unable to validate IPC permit target '%s'."
+			" will issue empty policy", argv[0]);
 	}
-	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);
@@ -85,19 +78,15 @@ struct cmd_results *cmd_reject(int argc, char **argv) {
 	}
 
 	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]);
+	if (!(program = resolve_ipc_path(argv[0]))) {
+		sway_abort("memory allocation failed");
 	}
 
 	struct feature_policy *policy = get_feature_policy(program);
+	if (!policy->validated) {
+		sway_log(L_ERROR, "Unable to validate IPC reject target '%s'."
+			" Allowing `reject` directive anyway", argv[0]);
+	}
 	policy->features &= ~get_features(argc, argv, &error);
 
 	sway_log(L_DEBUG, "Permissions granted to %s for features %d",

sway/security.c

@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include "sway/config.h"
 #include "sway/security.h"
+#include "util.h"
 #include "log.h"
 
 static bool validate_ipc_target(const char *program) {
@@ -21,7 +22,7 @@ static bool validate_ipc_target(const char *program) {
 	}
 	if (!S_ISREG(sb.st_mode)) {
 		sway_log(L_ERROR,
-				"IPC target '%s' MUST be/point at an existing regular file",
+				"IPC target '%s' MUST point at an existing file",
 				program);
 		return false;
 	}
@@ -30,7 +31,7 @@ static bool validate_ipc_target(const char *program) {
 		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);
+		sway_log(L_WARN, "IPC target '%s' MUST be owned by root (waived for debug build)", program);
 		return true;
 #endif
 	}
@@ -45,9 +46,6 @@ static bool validate_ipc_target(const char *program) {
 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) {
@@ -60,6 +58,7 @@ struct feature_policy *alloc_feature_policy(const char *program) {
 	if (!policy) {
 		return NULL;
 	}
+	policy->validated = validate_ipc_target	(program);
 	policy->program = strdup(program);
 	if (!policy->program) {
 		free(policy);
@@ -73,9 +72,6 @@ struct feature_policy *alloc_feature_policy(const char *program) {
 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) {
@@ -88,6 +84,7 @@ struct ipc_policy *alloc_ipc_policy(const char *program) {
 	if (!policy) {
 		return NULL;
 	}
+	policy->validated = validate_ipc_target	(program);
 	policy->program = strdup(program);
 	if (!policy->program) {
 		free(policy);
@@ -226,3 +223,21 @@ const char *command_policy_str(enum command_context context) {
 			return "unknown";
 	}
 }
+
+/**
+ * An IPC-specific version of util.c:resolve_path()
+ * Always returns the "best" path It can unless an ENOMEM occurs ,
+ * in which case it returns NULL.
+ */
+char *resolve_ipc_path(const char* name) {
+	char *program = NULL;
+	if (!strcmp(name, "*")) {
+		program = strdup(name);
+	} else {
+		program = resolve_path(name);
+		if (!program) {
+			program = strdup(name);
+		}
+	}
+	return program;
+}