summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLibravatar Jerzi Kaminsky <JerziKaminsky@users.noreply.github.com>2017-04-16 09:30:14 +0300
committerLibravatar Jerzi Kaminsky <JerziKaminsky@users.noreply.github.com>2017-04-16 17:09:53 +0300
commit2ad8850398693cb572152e6d97c59de371996273 (patch)
tree21abe14fe200099fffe5de9b7770cf2ca921e371
parentAdd resolve_path() to utils (diff)
downloadsway-2ad8850398693cb572152e6d97c59de371996273.tar.gz
sway-2ad8850398693cb572152e6d97c59de371996273.tar.zst
sway-2ad8850398693cb572152e6d97c59de371996273.zip
Handle symlinks as IPC security targets
- When policies are allocated, the ipc target path goes through symlink resolution. The result is used as the canonical for matching pids to policies at runtime. In particular, this matches up with the target of the `/proc/<pid>/exe`. - There's a possible race condition if this isn't done correctly, read 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.
-rw-r--r--sway/commands/ipc.c10
-rw-r--r--sway/commands/permit.c39
2 files changed, 45 insertions, 4 deletions
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 @@
1#define _XOPEN_SOURCE 500
1#include <stdio.h> 2#include <stdio.h>
2#include <string.h> 3#include <string.h>
3#include "sway/security.h" 4#include "sway/security.h"
@@ -18,8 +19,14 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
18 return error; 19 return error;
19 } 20 }
20 21
21 const char *program = argv[0]; 22 char *program = NULL;
22 23
24 if (!strcmp(argv[0], "*")) {
25 program = strdup(argv[0]);
26 } else if (!(program = resolve_path(argv[0]))) {
27 return cmd_results_new(
28 CMD_INVALID, "ipc", "Unable to resolve IPC Policy target.");
29 }
23 if (config->reading && strcmp("{", argv[1]) != 0) { 30 if (config->reading && strcmp("{", argv[1]) != 0) {
24 return cmd_results_new(CMD_INVALID, "ipc", 31 return cmd_results_new(CMD_INVALID, "ipc",
25 "Expected '{' at start of IPC config definition."); 32 "Expected '{' at start of IPC config definition.");
@@ -32,6 +39,7 @@ struct cmd_results *cmd_ipc(int argc, char **argv) {
32 current_policy = alloc_ipc_policy(program); 39 current_policy = alloc_ipc_policy(program);
33 list_add(config->ipc_policies, current_policy); 40 list_add(config->ipc_policies, current_policy);
34 41
42 free(program);
35 return cmd_results_new(CMD_BLOCK_IPC, NULL, NULL); 43 return cmd_results_new(CMD_BLOCK_IPC, NULL, NULL);
36} 44}
37 45
diff --git a/sway/commands/permit.c b/sway/commands/permit.c
index c55f46d8..66fa4e2a 100644
--- a/sway/commands/permit.c
+++ b/sway/commands/permit.c
@@ -1,7 +1,9 @@
1#define _XOPEN_SOURCE 500
1#include <string.h> 2#include <string.h>
2#include "sway/commands.h" 3#include "sway/commands.h"
3#include "sway/config.h" 4#include "sway/config.h"
4#include "sway/security.h" 5#include "sway/security.h"
6#include "util.h"
5#include "log.h" 7#include "log.h"
6 8
7static enum secure_feature get_features(int argc, char **argv, 9static enum secure_feature get_features(int argc, char **argv,
@@ -47,12 +49,29 @@ struct cmd_results *cmd_permit(int argc, char **argv) {
47 return error; 49 return error;
48 } 50 }
49 51
50 struct feature_policy *policy = get_feature_policy(argv[0]); 52 bool assign_perms = true;
51 policy->features |= get_features(argc, argv, &error); 53 char *program = NULL;
52 54
55 if (!strcmp(argv[0], "*")) {
56 program = strdup(argv[0]);
57 } else {
58 program = resolve_path(argv[0]);
59 }
60 if (!program) {
61 sway_assert(program, "Unable to resolve IPC permit target '%s'."
62 " will issue empty policy", argv[0]);
63 assign_perms = false;
64 program = strdup(argv[0]);
65 }
66
67 struct feature_policy *policy = get_feature_policy(program);
68 if (assign_perms) {
69 policy->features |= get_features(argc, argv, &error);
70 }
53 sway_log(L_DEBUG, "Permissions granted to %s for features %d", 71 sway_log(L_DEBUG, "Permissions granted to %s for features %d",
54 policy->program, policy->features); 72 policy->program, policy->features);
55 73
74 free(program);
56 return cmd_results_new(CMD_SUCCESS, NULL, NULL); 75 return cmd_results_new(CMD_SUCCESS, NULL, NULL);
57} 76}
58 77
@@ -65,11 +84,25 @@ struct cmd_results *cmd_reject(int argc, char **argv) {
65 return error; 84 return error;
66 } 85 }
67 86
68 struct feature_policy *policy = get_feature_policy(argv[0]); 87 char *program = NULL;
88 if (!strcmp(argv[0], "*")) {
89 program = strdup(argv[0]);
90 } else {
91 program = resolve_path(argv[0]);
92 }
93 if (!program) {
94 // Punt
95 sway_log(L_INFO, "Unable to resolve IPC reject target '%s'."
96 " Will use provided path", argv[0]);
97 program = strdup(argv[0]);
98 }
99
100 struct feature_policy *policy = get_feature_policy(program);
69 policy->features &= ~get_features(argc, argv, &error); 101 policy->features &= ~get_features(argc, argv, &error);
70 102
71 sway_log(L_DEBUG, "Permissions granted to %s for features %d", 103 sway_log(L_DEBUG, "Permissions granted to %s for features %d",
72 policy->program, policy->features); 104 policy->program, policy->features);
73 105
106 free(program);
74 return cmd_results_new(CMD_SUCCESS, NULL, NULL); 107 return cmd_results_new(CMD_SUCCESS, NULL, NULL);
75} 108}