From 7dbecdde95d1f309d8fdd02fe480dc3fbef7c7c1 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Sun, 19 Feb 2017 02:36:36 -0500 Subject: [PATCH 1/6] Revise IPC security configuration --- security.in => security.d/00-defaults.in | 37 ++++++++++++------------ sway/CMakeLists.txt | 2 +- sway/sway-security.7.txt | 34 ++++++++++++---------- 3 files changed, 39 insertions(+), 34 deletions(-) rename security.in => security.d/00-defaults.in (69%) diff --git a/security.in b/security.d/00-defaults.in similarity index 69% rename from security.in rename to security.d/00-defaults.in index 16897ade..99859edd 100644 --- a/security.in +++ b/security.d/00-defaults.in @@ -8,33 +8,34 @@ # This file should live at __SYSCONFDIR__/sway/security and will be # automatically read by sway. -# Configures which programs are allowed to use which sway features -permit * fullscreen keyboard mouse ipc +# Configures enabled compositor features for specific programs +permit * fullscreen keyboard mouse permit __PREFIX__/bin/swaylock lock -permit __PREFIX__/bin/swaybar panel permit __PREFIX__/bin/swaybg background permit __PREFIX__/bin/swaygrab screenshot +permit __PREFIX__/bin/swaybar panel -# Configures which IPC features are enabled -ipc { - command enabled - outputs enabled - workspaces enabled - tree enabled - marks enabled - bar-config enabled - inputs enabled +# Configures enabled IPC features for specific programs +ipc __PREFIX__/bin/swaymsg { + * enabled events { - workspace enabled - output enabled - mode enabled - window enabled - input enabled - binding disabled + * disabled } } +ipc __PREFIX__/bin/swaybar { + bar-config enabled + outputs enabled + workspaces enabled + command enabled +} + +ipc __PREFIX__/bin/swaygrab { + outputs enabled + tree enabled +} + # Limits the contexts from which certain commands are permitted commands { * all diff --git a/sway/CMakeLists.txt b/sway/CMakeLists.txt index d5453003..981f8a07 100644 --- a/sway/CMakeLists.txt +++ b/sway/CMakeLists.txt @@ -91,7 +91,7 @@ function(add_config name source destination) endfunction() add_config(config config sway) -add_config(security security sway) +add_config(00-defaults security.d/00-defaults sway/security.d) add_manpage(sway 1) add_manpage(sway 5) diff --git a/sway/sway-security.7.txt b/sway/sway-security.7.txt index 7d8aa4ad..98e3f5ac 100644 --- a/sway/sway-security.7.txt +++ b/sway/sway-security.7.txt @@ -19,8 +19,13 @@ usually best suited to a distro maintainer who wants to ship a secure sway environment in their distro. Sway provides a number of means of securing it but you must make a few changes external to sway first. -Security-related configuration is only valid in /etc/sway/config (or whatever path -is appropriate for your system). +Configuration of security features is limited to files in the security directory +(this is likely /etc/sway/security.d/*, but depends on your installation prefix). +Files in this directory must be owned by root:root and chmod 600. The default +security configuration is installed to /etc/sway/security.d/00-defaults, and +should not be modified - it will be updated with the latest recommended security +defaults between releases. To override the defaults, you should add more files to +this directory. Environment security -------------------- @@ -160,22 +165,20 @@ Setting a command policy overwrites any previous policy that was in place. IPC policies ------------ -You may whitelist IPC access like so: +Disabling IPC access via swaymsg is encouraged if you intend to secure the IPC +socket, because any program that can execute swaymsg could circumvent its own +security policy by simply invoking swaymsg. - permit /usr/bin/swaybar ipc - permit /usr/bin/swaygrab ipc - # etc +You can configure which features of IPC are available for particular clients: -Note that it's suggested you do not enable swaymsg to access IPC if you intend to -secure your IPC socket, because any program could just run swaymsg itself instead -of connecting to IPC directly. - -You can also configure which features of IPC are available with an IPC block: - - ipc { + ipc { ... } +You may use * for to configure the default policy for all clients. +Configuring IPC policies for specific executables is not supported on FreeBSD, and +the default policy will be applied to all IPC connections. + The following commands are available within this block: **bar-config** :: @@ -201,7 +204,7 @@ The following commands are available within this block: You can also control which IPC events can be raised with an events block: - ipc { + ipc { events { ... } @@ -227,7 +230,8 @@ The following commands are vaild within an ipc events block: **workspace** :: Controls workspace notifications. -Disabling some of these may cause swaybar to behave incorrectly. +In each of these blocks, you may use * (as in "* enabled" or "* disabled") to +control access to every feature at once. Authors ------- From b10721b89e3f3992b2476c55237a25dbeb0bce46 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 06:11:43 -0500 Subject: [PATCH 2/6] Add initial support code for new IPC security --- include/sway/config.h | 8 ++++-- include/sway/security.h | 6 +++-- sway/commands/ipc.c | 12 ++++----- sway/commands/permit.c | 1 - sway/config.c | 2 +- sway/ipc-server.c | 45 +++------------------------------- sway/security.c | 54 ++++++++++++++++++++++++++++++++++++++--- 7 files changed, 70 insertions(+), 58 deletions(-) diff --git a/include/sway/config.h b/include/sway/config.h index febde63d..c3a916b1 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -203,7 +203,6 @@ enum secure_feature { FEATURE_FULLSCREEN = 16, FEATURE_KEYBOARD = 32, FEATURE_MOUSE = 64, - FEATURE_IPC = 128, }; struct feature_policy { @@ -228,6 +227,11 @@ enum ipc_feature { IPC_FEATURE_EVENT_INPUT = 8192 }; +struct ipc_policy { + char *program; + uint32_t features; +}; + /** * The configuration struct. The result of loading a config file. */ @@ -300,7 +304,7 @@ struct sway_config { // Security list_t *command_policies; list_t *feature_policies; - uint32_t ipc_policy; + list_t *ipc_policies; }; void pid_workspace_add(struct pid_workspace *pw); diff --git a/include/sway/security.h b/include/sway/security.h index 1cc85bee..c3a5cfd4 100644 --- a/include/sway/security.h +++ b/include/sway/security.h @@ -3,12 +3,14 @@ #include #include "sway/config.h" -enum secure_feature get_feature_policy(pid_t pid); -enum command_context get_command_policy(const char *cmd); +uint32_t get_feature_policy(pid_t pid); +uint32_t get_ipc_policy(pid_t pid); +uint32_t get_command_policy(const char *cmd); const char *command_policy_str(enum command_context context); 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); #endif diff --git a/sway/commands/ipc.c b/sway/commands/ipc.c index 113a975b..44d7a010 100644 --- a/sway/commands/ipc.c +++ b/sway/commands/ipc.c @@ -86,10 +86,10 @@ struct cmd_results *cmd_ipc_cmd(int argc, char **argv) { } if (enabled) { - config->ipc_policy |= type; - sway_log(L_DEBUG, "Enabled IPC %s feature", argv[-1]); + //config->ipc_policy |= type; + sway_log(L_DEBUG, "Enabled IPC %s feature %d", argv[-1], (int)type); } else { - config->ipc_policy &= ~type; + //config->ipc_policy &= ~type; sway_log(L_DEBUG, "Disabled IPC %s feature", argv[-1]); } @@ -134,10 +134,10 @@ struct cmd_results *cmd_ipc_event_cmd(int argc, char **argv) { } if (enabled) { - config->ipc_policy |= type; - sway_log(L_DEBUG, "Enabled IPC %s event", argv[-1]); + //config->ipc_policy |= type; + sway_log(L_DEBUG, "Enabled IPC %s event %d", argv[-1], (int)type); } else { - config->ipc_policy &= ~type; + //config->ipc_policy &= ~type; sway_log(L_DEBUG, "Disabled IPC %s event", argv[-1]); } diff --git a/sway/commands/permit.c b/sway/commands/permit.c index 1b2a30bf..6eb71816 100644 --- a/sway/commands/permit.c +++ b/sway/commands/permit.c @@ -19,7 +19,6 @@ static enum secure_feature get_features(int argc, char **argv, { "fullscreen", FEATURE_FULLSCREEN }, { "keyboard", FEATURE_KEYBOARD }, { "mouse", FEATURE_MOUSE }, - { "ipc", FEATURE_IPC }, }; for (int i = 1; i < argc; ++i) { diff --git a/sway/config.c b/sway/config.c index 9e758c90..4a3c953d 100644 --- a/sway/config.c +++ b/sway/config.c @@ -379,7 +379,7 @@ static void config_defaults(struct sway_config *config) { // Security if (!(config->command_policies = create_list())) goto cleanup; if (!(config->feature_policies = create_list())) goto cleanup; - config->ipc_policy = UINT32_MAX; + if (!(config->ipc_policies = create_list())) goto cleanup; return; cleanup: diff --git a/sway/ipc-server.c b/sway/ipc-server.c index be6e411a..dfe88ed6 100644 --- a/sway/ipc-server.c +++ b/sway/ipc-server.c @@ -125,6 +125,7 @@ struct sockaddr_un *ipc_user_sockaddr(void) { return ipc_sockaddr; } +/* static pid_t get_client_pid(int client_fd) { // FreeBSD supports getting uid/gid, but not pid #ifdef __linux__ @@ -140,6 +141,7 @@ static pid_t get_client_pid(int client_fd) { return -1; #endif } +*/ int ipc_handle_connection(int fd, uint32_t mask, void *data) { (void) fd; (void) data; @@ -159,17 +161,6 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) { return 0; } - pid_t pid = get_client_pid(client_fd); - if (!(get_feature_policy(pid) & FEATURE_IPC)) { - sway_log(L_INFO, "Permission to connect to IPC socket denied to %d", pid); - const char *error = "{\"success\": false, \"message\": \"Permission denied\"}"; - if (write(client_fd, &error, sizeof(error)) < (int)sizeof(error)) { - sway_log(L_DEBUG, "Failed to write entire error"); - } - close(client_fd); - return 0; - } - struct ipc_client* client = malloc(sizeof(struct ipc_client)); if (!client) { sway_log(L_ERROR, "Unable to allocate ipc client"); @@ -351,9 +342,6 @@ void ipc_client_handle_command(struct ipc_client *client) { switch (client->current_command) { case IPC_COMMAND: { - if (!(config->ipc_policy & IPC_FEATURE_COMMAND)) { - goto exit_denied; - } struct cmd_results *results = handle_command(buf, CONTEXT_IPC); const char *json = cmd_results_to_json(results); char reply[256]; @@ -403,9 +391,6 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_WORKSPACES: { - if (!(config->ipc_policy & IPC_FEATURE_GET_WORKSPACES)) { - goto exit_denied; - } json_object *workspaces = json_object_new_array(); container_map(&root_container, ipc_get_workspaces_callback, workspaces); const char *json_string = json_object_to_json_string(workspaces); @@ -416,9 +401,6 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_INPUTS: { - if (!(config->ipc_policy & IPC_FEATURE_GET_INPUTS)) { - goto exit_denied; - } json_object *inputs = json_object_new_array(); if (input_devices) { for(int i=0; ilength; i++) { @@ -442,9 +424,6 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_OUTPUTS: { - if (!(config->ipc_policy & IPC_FEATURE_GET_OUTPUTS)) { - goto exit_denied; - } json_object *outputs = json_object_new_array(); container_map(&root_container, ipc_get_outputs_callback, outputs); const char *json_string = json_object_to_json_string(outputs); @@ -455,9 +434,6 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_TREE: { - if (!(config->ipc_policy & IPC_FEATURE_GET_TREE)) { - goto exit_denied; - } json_object *tree = ipc_json_describe_container_recursive(&root_container); const char *json_string = json_object_to_json_string(tree); ipc_send_reply(client, json_string, (uint32_t) strlen(json_string)); @@ -522,9 +498,6 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_BAR_CONFIG: { - if (!(config->ipc_policy & IPC_FEATURE_GET_BAR_CONFIG)) { - goto exit_denied; - } if (!buf[0]) { // Send list of configured bar IDs json_object *bars = json_object_new_array(); @@ -565,7 +538,7 @@ void ipc_client_handle_command(struct ipc_client *client) { goto exit_cleanup; } -exit_denied: +//exit_denied: ipc_send_reply(client, error_denied, (uint32_t)strlen(error_denied)); exit_cleanup: @@ -632,9 +605,6 @@ void ipc_send_event(const char *json_string, enum ipc_command_type event) { } void ipc_event_workspace(swayc_t *old, swayc_t *new, const char *change) { - if (!(config->ipc_policy & IPC_FEATURE_EVENT_WORKSPACE)) { - return; - } sway_log(L_DEBUG, "Sending workspace::%s event", change); json_object *obj = json_object_new_object(); json_object_object_add(obj, "change", json_object_new_string(change)); @@ -659,9 +629,6 @@ void ipc_event_workspace(swayc_t *old, swayc_t *new, const char *change) { } void ipc_event_window(swayc_t *window, const char *change) { - if (!(config->ipc_policy & IPC_FEATURE_EVENT_WINDOW)) { - return; - } sway_log(L_DEBUG, "Sending window::%s event", change); json_object *obj = json_object_new_object(); json_object_object_add(obj, "change", json_object_new_string(change)); @@ -687,9 +654,6 @@ void ipc_event_barconfig_update(struct bar_config *bar) { } void ipc_event_mode(const char *mode) { - if (!(config->ipc_policy & IPC_FEATURE_EVENT_MODE)) { - return; - } sway_log(L_DEBUG, "Sending mode::%s event", mode); json_object *obj = json_object_new_object(); json_object_object_add(obj, "change", json_object_new_string(mode)); @@ -715,9 +679,6 @@ void ipc_event_modifier(uint32_t modifier, const char *state) { } static void ipc_event_binding(json_object *sb_obj) { - if (!(config->ipc_policy & IPC_FEATURE_EVENT_BINDING)) { - return; - } sway_log(L_DEBUG, "Sending binding::run event"); json_object *obj = json_object_new_object(); json_object_object_add(obj, "change", json_object_new_string("run")); diff --git a/sway/security.c b/sway/security.c index 41a3b94b..9dfc7d2d 100644 --- a/sway/security.c +++ b/sway/security.c @@ -27,6 +27,29 @@ struct feature_policy *alloc_feature_policy(const char *program) { return policy; } +struct ipc_policy *alloc_ipc_policy(const char *program) { + uint32_t default_policy = 0; + for (int i = 0; i < config->ipc_policies->length; ++i) { + struct ipc_policy *policy = config->ipc_policies->items[i]; + if (strcmp(policy->program, "*") == 0) { + default_policy = policy->features; + break; + } + } + + struct ipc_policy *policy = malloc(sizeof(struct ipc_policy)); + if (!policy) { + return NULL; + } + policy->program = strdup(program); + if (!policy->program) { + free(policy); + return NULL; + } + policy->features = default_policy; + return policy; +} + struct command_policy *alloc_command_policy(const char *command) { struct command_policy *policy = malloc(sizeof(struct command_policy)); if (!policy) { @@ -41,7 +64,7 @@ struct command_policy *alloc_command_policy(const char *command) { return policy; } -enum secure_feature get_feature_policy(pid_t pid) { +static const char *get_pid_exe(pid_t pid) { #ifdef __FreeBSD__ const char *fmt = "/proc/%d/file"; #else @@ -52,9 +75,8 @@ enum secure_feature get_feature_policy(pid_t pid) { if (path) { snprintf(path, pathlen + 1, fmt, pid); } - static char link[2048]; - uint32_t default_policy = 0; + static char link[2048]; ssize_t len = !path ? -1 : readlink(path, link, sizeof(link)); if (len < 0) { @@ -67,6 +89,13 @@ enum secure_feature get_feature_policy(pid_t pid) { } free(path); + return link; +} + +uint32_t get_feature_policy(pid_t pid) { + uint32_t default_policy = 0; + const char *link = get_pid_exe(pid); + for (int i = 0; i < config->feature_policies->length; ++i) { struct feature_policy *policy = config->feature_policies->items[i]; if (strcmp(policy->program, "*") == 0) { @@ -80,7 +109,24 @@ enum secure_feature get_feature_policy(pid_t pid) { return default_policy; } -enum command_context get_command_policy(const char *cmd) { +uint32_t get_ipc_policy(pid_t pid) { + uint32_t default_policy = 0; + const char *link = get_pid_exe(pid); + + for (int i = 0; i < config->ipc_policies->length; ++i) { + struct ipc_policy *policy = config->ipc_policies->items[i]; + if (strcmp(policy->program, "*") == 0) { + default_policy = policy->features; + } + if (strcmp(policy->program, link) == 0) { + return policy->features; + } + } + + return default_policy; +} + +uint32_t get_command_policy(const char *cmd) { uint32_t default_policy = 0; for (int i = 0; i < config->command_policies->length; ++i) { From 1980a0835804b205da1fa00187640ae8a0c4f9be Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 06:30:25 -0500 Subject: [PATCH 3/6] Enforce new IPC policies --- sway/commands/ipc.c | 23 +++++++++++++-------- sway/ipc-server.c | 50 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/sway/commands/ipc.c b/sway/commands/ipc.c index 44d7a010..6b29706e 100644 --- a/sway/commands/ipc.c +++ b/sway/commands/ipc.c @@ -1,18 +1,23 @@ #include #include +#include "sway/security.h" #include "sway/commands.h" #include "sway/config.h" #include "ipc.h" #include "log.h" #include "util.h" +static struct ipc_policy *current_policy = NULL; + struct cmd_results *cmd_ipc(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "ipc", EXPECTED_EQUAL_TO, 1))) { + if ((error = checkarg(argc, "ipc", EXPECTED_EQUAL_TO, 2))) { return error; } - if (config->reading && strcmp("{", argv[0]) != 0) { + const char *program = argv[0]; + + if (config->reading && strcmp("{", argv[1]) != 0) { return cmd_results_new(CMD_INVALID, "ipc", "Expected '{' at start of IPC config definition."); } @@ -26,6 +31,8 @@ struct cmd_results *cmd_ipc(int argc, char **argv) { "This command is only permitted to run from " SYSCONFDIR "/sway/security"); } + current_policy = alloc_ipc_policy(program); + return cmd_results_new(CMD_BLOCK_IPC, NULL, NULL); } @@ -86,10 +93,10 @@ struct cmd_results *cmd_ipc_cmd(int argc, char **argv) { } if (enabled) { - //config->ipc_policy |= type; - sway_log(L_DEBUG, "Enabled IPC %s feature %d", argv[-1], (int)type); + current_policy->features |= type; + sway_log(L_DEBUG, "Enabled IPC %s feature", argv[-1]); } else { - //config->ipc_policy &= ~type; + current_policy->features &= ~type; sway_log(L_DEBUG, "Disabled IPC %s feature", argv[-1]); } @@ -134,10 +141,10 @@ struct cmd_results *cmd_ipc_event_cmd(int argc, char **argv) { } if (enabled) { - //config->ipc_policy |= type; - sway_log(L_DEBUG, "Enabled IPC %s event %d", argv[-1], (int)type); + current_policy->features |= type; + sway_log(L_DEBUG, "Enabled IPC %s event", argv[-1]); } else { - //config->ipc_policy &= ~type; + current_policy->features &= ~type; sway_log(L_DEBUG, "Disabled IPC %s event", argv[-1]); } diff --git a/sway/ipc-server.c b/sway/ipc-server.c index dfe88ed6..20a19b44 100644 --- a/sway/ipc-server.c +++ b/sway/ipc-server.c @@ -35,6 +35,7 @@ struct ipc_client { struct wlc_event_source *event_source; int fd; uint32_t payload_length; + uint32_t security_policy; enum ipc_command_type current_command; enum ipc_command_type subscribed_events; }; @@ -125,7 +126,6 @@ struct sockaddr_un *ipc_user_sockaddr(void) { return ipc_sockaddr; } -/* static pid_t get_client_pid(int client_fd) { // FreeBSD supports getting uid/gid, but not pid #ifdef __linux__ @@ -141,7 +141,6 @@ static pid_t get_client_pid(int client_fd) { return -1; #endif } -*/ int ipc_handle_connection(int fd, uint32_t mask, void *data) { (void) fd; (void) data; @@ -172,6 +171,9 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) { client->subscribed_events = 0; 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); + list_add(ipc_client_list, client); return 0; @@ -342,6 +344,9 @@ void ipc_client_handle_command(struct ipc_client *client) { switch (client->current_command) { case IPC_COMMAND: { + if (!(client->security_policy & IPC_FEATURE_COMMAND)) { + goto exit_denied; + } struct cmd_results *results = handle_command(buf, CONTEXT_IPC); const char *json = cmd_results_to_json(results); char reply[256]; @@ -353,6 +358,7 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_SUBSCRIBE: { + // TODO: Check if they're permitted to use these events struct json_object *request = json_tokener_parse(buf); if (request == NULL) { ipc_send_reply(client, "{\"success\": false}", 18); @@ -391,6 +397,9 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_WORKSPACES: { + if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + goto exit_denied; + } json_object *workspaces = json_object_new_array(); container_map(&root_container, ipc_get_workspaces_callback, workspaces); const char *json_string = json_object_to_json_string(workspaces); @@ -401,6 +410,9 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_INPUTS: { + if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + goto exit_denied; + } json_object *inputs = json_object_new_array(); if (input_devices) { for(int i=0; ilength; i++) { @@ -424,6 +436,9 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_OUTPUTS: { + if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + goto exit_denied; + } json_object *outputs = json_object_new_array(); container_map(&root_container, ipc_get_outputs_callback, outputs); const char *json_string = json_object_to_json_string(outputs); @@ -434,6 +449,9 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_TREE: { + if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + goto exit_denied; + } json_object *tree = ipc_json_describe_container_recursive(&root_container); const char *json_string = json_object_to_json_string(tree); ipc_send_reply(client, json_string, (uint32_t) strlen(json_string)); @@ -498,6 +516,9 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_BAR_CONFIG: { + if (!(client->security_policy & IPC_FEATURE_GET_BAR_CONFIG)) { + goto exit_denied; + } if (!buf[0]) { // Send list of configured bar IDs json_object *bars = json_object_new_array(); @@ -538,7 +559,7 @@ void ipc_client_handle_command(struct ipc_client *client) { goto exit_cleanup; } -//exit_denied: +exit_denied: ipc_send_reply(client, error_denied, (uint32_t)strlen(error_denied)); exit_cleanup: @@ -589,10 +610,33 @@ void ipc_get_outputs_callback(swayc_t *container, void *data) { } void ipc_send_event(const char *json_string, enum ipc_command_type event) { + static struct { + enum ipc_command_type event; + enum ipc_feature feature; + } security_mappings[] = { + { IPC_EVENT_WORKSPACE, IPC_FEATURE_EVENT_WORKSPACE }, + { IPC_EVENT_OUTPUT, IPC_FEATURE_EVENT_OUTPUT }, + { IPC_EVENT_MODE, IPC_FEATURE_EVENT_MODE }, + { IPC_EVENT_WINDOW, IPC_FEATURE_EVENT_WINDOW }, + { IPC_EVENT_BINDING, IPC_FEATURE_EVENT_BINDING }, + { IPC_EVENT_INPUT, IPC_FEATURE_EVENT_INPUT } + }; + + uint32_t security_mask = 0; + for (size_t i = 0; i < sizeof(security_mappings) / sizeof(security_mappings[0]); ++i) { + if (security_mappings[i].event == event) { + security_mask = security_mappings[i].feature; + break; + } + } + int i; struct ipc_client *client; for (i = 0; i < ipc_client_list->length; i++) { client = ipc_client_list->items[i]; + if (!(client->security_policy & security_mask)) { + continue; + } if ((client->subscribed_events & event_mask(event)) == 0) { continue; } From eabfb6c5598d5b655b40d8677d97b58cce757ef5 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 06:48:33 -0500 Subject: [PATCH 4/6] Add * policies and fix bug --- include/sway/config.h | 7 ++++++- sway/commands.c | 2 ++ sway/commands/ipc.c | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/sway/config.h b/include/sway/config.h index c3a916b1..ba49b9a0 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -224,7 +224,12 @@ enum ipc_feature { IPC_FEATURE_EVENT_MODE = 1024, IPC_FEATURE_EVENT_WINDOW = 2048, IPC_FEATURE_EVENT_BINDING = 4096, - IPC_FEATURE_EVENT_INPUT = 8192 + IPC_FEATURE_EVENT_INPUT = 8192, + + IPC_FEATURE_ALL_COMMANDS = 1 | 2 | 4 | 8 | 16 | 32 | 64 | 128, + IPC_FEATURE_ALL_EVENTS = 256 | 512 | 1024 | 2048 | 4096 | 8192, + + IPC_FEATURE_ALL = IPC_FEATURE_ALL_COMMANDS | IPC_FEATURE_ALL_EVENTS, }; struct ipc_policy { diff --git a/sway/commands.c b/sway/commands.c index c15cb00a..068e8866 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -297,6 +297,7 @@ static struct cmd_handler bar_colors_handlers[] = { }; static struct cmd_handler ipc_handlers[] = { + { "*", cmd_ipc_cmd }, { "bar-config", cmd_ipc_cmd }, { "command", cmd_ipc_cmd }, { "events", cmd_ipc_events }, @@ -308,6 +309,7 @@ static struct cmd_handler ipc_handlers[] = { }; static struct cmd_handler ipc_event_handlers[] = { + { "*", cmd_ipc_event_cmd }, { "binding", cmd_ipc_event_cmd }, { "input", cmd_ipc_event_cmd }, { "mode", cmd_ipc_event_cmd }, diff --git a/sway/commands/ipc.c b/sway/commands/ipc.c index 6b29706e..d49aab64 100644 --- a/sway/commands/ipc.c +++ b/sway/commands/ipc.c @@ -32,6 +32,7 @@ struct cmd_results *cmd_ipc(int argc, char **argv) { } current_policy = alloc_ipc_policy(program); + list_add(config->ipc_policies, current_policy); return cmd_results_new(CMD_BLOCK_IPC, NULL, NULL); } @@ -74,6 +75,7 @@ struct cmd_results *cmd_ipc_cmd(int argc, char **argv) { char *name; enum ipc_feature type; } types[] = { + { "*", IPC_FEATURE_ALL_COMMANDS }, { "command", IPC_FEATURE_COMMAND }, { "workspaces", IPC_FEATURE_GET_WORKSPACES }, { "outputs", IPC_FEATURE_GET_OUTPUTS }, @@ -123,6 +125,7 @@ struct cmd_results *cmd_ipc_event_cmd(int argc, char **argv) { char *name; enum ipc_feature type; } types[] = { + { "*", IPC_FEATURE_ALL_EVENTS }, { "workspace", IPC_FEATURE_EVENT_WORKSPACE }, { "output", IPC_FEATURE_EVENT_OUTPUT }, { "mode", IPC_FEATURE_EVENT_MODE }, From 126ce571dab09d84d8ee1b760981dbba7cbc1000 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 07:42:08 -0500 Subject: [PATCH 5/6] Read configs from /etc/sway/security.d/* --- include/sway/config.h | 2 ++ security.d/00-defaults.in | 5 ++++ sway/commands/commands.c | 8 +++--- sway/commands/ipc.c | 8 +++--- sway/commands/permit.c | 20 +++------------ sway/config.c | 52 ++++++++++++++++++++++++++++++++++++++- sway/ipc-server.c | 15 +++++------ sway/main.c | 7 ------ sway/sway-security.7.txt | 2 +- 9 files changed, 77 insertions(+), 42 deletions(-) diff --git a/include/sway/config.h b/include/sway/config.h index ba49b9a0..d77fbd51 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -340,6 +340,8 @@ void free_config(struct sway_config *config); */ char *do_var_replacement(char *str); +struct cmd_results *check_security_config(); + int input_identifier_cmp(const void *item, const void *data); void merge_input_config(struct input_config *dst, struct input_config *src); void apply_input_config(struct input_config *ic, struct libinput_device *dev); diff --git a/security.d/00-defaults.in b/security.d/00-defaults.in index 99859edd..f319e446 100644 --- a/security.d/00-defaults.in +++ b/security.d/00-defaults.in @@ -29,6 +29,11 @@ ipc __PREFIX__/bin/swaybar { outputs enabled workspaces enabled command enabled + + events { + workspace enabled + mode enabled + } } ipc __PREFIX__/bin/swaygrab { diff --git a/sway/commands/commands.c b/sway/commands/commands.c index 8c7ed487..0c64970c 100644 --- a/sway/commands/commands.c +++ b/sway/commands/commands.c @@ -10,6 +10,9 @@ struct cmd_results *cmd_commands(int argc, char **argv) { if ((error = checkarg(argc, "commands", EXPECTED_EQUAL_TO, 1))) { return error; } + if ((error = check_security_config())) { + return error; + } if (strcmp(argv[0], "{") != 0) { return cmd_results_new(CMD_FAILURE, "commands", "Expected block declaration"); @@ -19,10 +22,5 @@ struct cmd_results *cmd_commands(int argc, char **argv) { return cmd_results_new(CMD_FAILURE, "commands", "Can only be used in config file."); } - if (!current_config_path || strcmp(SYSCONFDIR "/sway/security", current_config_path) != 0) { - return cmd_results_new(CMD_INVALID, "permit", - "This command is only permitted to run from " SYSCONFDIR "/sway/security"); - } - return cmd_results_new(CMD_BLOCK_COMMANDS, NULL, NULL); } diff --git a/sway/commands/ipc.c b/sway/commands/ipc.c index d49aab64..8a7b849f 100644 --- a/sway/commands/ipc.c +++ b/sway/commands/ipc.c @@ -14,6 +14,9 @@ struct cmd_results *cmd_ipc(int argc, char **argv) { if ((error = checkarg(argc, "ipc", EXPECTED_EQUAL_TO, 2))) { return error; } + if ((error = check_security_config())) { + return error; + } const char *program = argv[0]; @@ -26,11 +29,6 @@ struct cmd_results *cmd_ipc(int argc, char **argv) { return cmd_results_new(CMD_FAILURE, "ipc", "Can only be used in config file."); } - if (!current_config_path || strcmp(SYSCONFDIR "/sway/security", current_config_path) != 0) { - return cmd_results_new(CMD_INVALID, "permit", - "This command is only permitted to run from " SYSCONFDIR "/sway/security"); - } - current_policy = alloc_ipc_policy(program); list_add(config->ipc_policies, current_policy); diff --git a/sway/commands/permit.c b/sway/commands/permit.c index 6eb71816..e2bec2e2 100644 --- a/sway/commands/permit.c +++ b/sway/commands/permit.c @@ -62,19 +62,13 @@ struct cmd_results *cmd_permit(int argc, char **argv) { if ((error = checkarg(argc, "permit", EXPECTED_MORE_THAN, 1))) { return error; } - - if (!current_config_path || strcmp(SYSCONFDIR "/sway/security", current_config_path) != 0) { - return cmd_results_new(CMD_INVALID, "permit", - "This command is only permitted to run from " SYSCONFDIR "/sway/security"); + if ((error = check_security_config())) { + return error; } struct feature_policy *policy = get_policy(argv[0]); policy->features |= get_features(argc, argv, &error); - if (error) { - return error; - } - sway_log(L_DEBUG, "Permissions granted to %s for features %d", policy->program, policy->features); @@ -86,19 +80,13 @@ struct cmd_results *cmd_reject(int argc, char **argv) { if ((error = checkarg(argc, "reject", EXPECTED_MORE_THAN, 1))) { return error; } - - if (!current_config_path || strcmp(SYSCONFDIR "/sway/security", current_config_path) != 0) { - return cmd_results_new(CMD_INVALID, "permit", - "This command is only permitted to run from " SYSCONFDIR "/sway/security"); + if ((error = check_security_config())) { + return error; } struct feature_policy *policy = get_policy(argv[0]); policy->features &= ~get_features(argc, argv, &error); - if (error) { - return error; - } - sway_log(L_DEBUG, "Permissions granted to %s for features %d", policy->program, policy->features); diff --git a/sway/config.c b/sway/config.c index 4a3c953d..88e6fad1 100644 --- a/sway/config.c +++ b/sway/config.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "wayland-desktop-shell-server-protocol.h" #include "sway/commands.h" #include "sway/config.h" @@ -485,6 +486,10 @@ static bool load_config(const char *path, struct sway_config *config) { return true; } +static int qstrcmp(const void* a, const void* b) { + return strcmp(*((char**) a), *((char**) b)); +} + bool load_main_config(const char *file, bool is_active) { input_init(); @@ -512,7 +517,43 @@ bool load_main_config(const char *file, bool is_active) { list_add(config->config_chain, path); config->reading = true; - bool success = load_config(SYSCONFDIR "/sway/security", config); + + // Read security configs + bool success = true; + DIR *dir = opendir(SYSCONFDIR "/sway/security.d"); + if (!dir) { + sway_log(L_ERROR, "%s does not exist, sway will have no security configuration" + " and will probably be broken", SYSCONFDIR "/sway/security.d"); + } else { + list_t *secconfigs = create_list(); + char *base = SYSCONFDIR "/sway/security.d/"; + struct dirent *ent = readdir(dir); + while (ent != NULL) { + if (ent->d_type == DT_REG) { + char *_path = malloc(strlen(ent->d_name) + strlen(base) + 1); + strcpy(_path, base); + strcat(_path, ent->d_name); + list_add(secconfigs, _path); + } + ent = readdir(dir); + } + closedir(dir); + + list_qsort(secconfigs, qstrcmp); + for (int i = 0; i < secconfigs->length; ++i) { + char *_path = secconfigs->items[i]; + struct stat s; + if (stat(_path, &s) || s.st_uid != 0 || s.st_gid != 0 || (s.st_mode & 0777) != 0644) { + sway_log(L_ERROR, "Refusing to load %s - it must be owned by root and mode 644", _path); + success = false; + } else { + success = success && load_config(_path, config); + } + } + + free_flat_list(secconfigs); + } + success = success && load_config(path, config); if (is_active) { @@ -620,6 +661,15 @@ bool load_include_configs(const char *path, struct sway_config *config) { return true; } +struct cmd_results *check_security_config() { + if (!current_config_path || strncmp(SYSCONFDIR "/sway/security.d/", current_config_path, + strlen(SYSCONFDIR "/sway/security.d/")) != 0) { + return cmd_results_new(CMD_INVALID, "permit", + "This command is only permitted to run from " SYSCONFDIR "/sway/security.d/*"); + } + return NULL; +} + bool read_config(FILE *file, struct sway_config *config) { bool success = true; enum cmd_status block = CMD_BLOCK_END; diff --git a/sway/ipc-server.c b/sway/ipc-server.c index 20a19b44..eddae461 100644 --- a/sway/ipc-server.c +++ b/sway/ipc-server.c @@ -241,8 +241,7 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) { return 0; } -void ipc_client_disconnect(struct ipc_client *client) -{ +void ipc_client_disconnect(struct ipc_client *client) { if (!sway_assert(client != NULL, "client != NULL")) { return; } @@ -326,8 +325,7 @@ void ipc_client_handle_command(struct ipc_client *client) { ipc_client_disconnect(client); return; } - if (client->payload_length > 0) - { + if (client->payload_length > 0) { ssize_t received = recv(client->fd, buf, client->payload_length, 0); if (received == -1) { @@ -397,7 +395,7 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_WORKSPACES: { - if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + if (!(client->security_policy & IPC_FEATURE_GET_WORKSPACES)) { goto exit_denied; } json_object *workspaces = json_object_new_array(); @@ -410,7 +408,7 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_INPUTS: { - if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + if (!(client->security_policy & IPC_FEATURE_GET_INPUTS)) { goto exit_denied; } json_object *inputs = json_object_new_array(); @@ -436,7 +434,7 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_GET_OUTPUTS: { - if (!(client->security_policy & IPC_FEATURE_GET_TREE)) { + if (!(client->security_policy & IPC_FEATURE_GET_OUTPUTS)) { goto exit_denied; } json_object *outputs = json_object_new_array(); @@ -561,6 +559,7 @@ void ipc_client_handle_command(struct ipc_client *client) { exit_denied: ipc_send_reply(client, error_denied, (uint32_t)strlen(error_denied)); + sway_log(L_DEBUG, "Denied IPC client access to %i", client->current_command); exit_cleanup: client->payload_length = 0; @@ -588,6 +587,8 @@ bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t pay return false; } + sway_log(L_DEBUG, "Send IPC reply: %s", payload); + return true; } diff --git a/sway/main.c b/sway/main.c index 1c4c56c0..0151e078 100644 --- a/sway/main.c +++ b/sway/main.c @@ -175,13 +175,6 @@ static void security_sanity_check() { cap_free(cap); } #endif - if (!stat(SYSCONFDIR "/sway", &s)) { - if (s.st_uid != 0 || s.st_gid != 0 - || (s.st_mode & S_IWGRP) || (s.st_mode & S_IWOTH)) { - sway_log(L_ERROR, - "!! DANGER !! " SYSCONFDIR "/sway is not secure! It should be owned by root and set to 0755 at the minimum"); - } - } } int main(int argc, char **argv) { diff --git a/sway/sway-security.7.txt b/sway/sway-security.7.txt index 98e3f5ac..fb47ffcf 100644 --- a/sway/sway-security.7.txt +++ b/sway/sway-security.7.txt @@ -21,7 +21,7 @@ you must make a few changes external to sway first. Configuration of security features is limited to files in the security directory (this is likely /etc/sway/security.d/*, but depends on your installation prefix). -Files in this directory must be owned by root:root and chmod 600. The default +Files in this directory must be owned by root:root and chmod 644. The default security configuration is installed to /etc/sway/security.d/00-defaults, and should not be modified - it will be updated with the latest recommended security defaults between releases. To override the defaults, you should add more files to From 276630eb9632fe2323d02c5d4113062830c49082 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 23:24:13 -0500 Subject: [PATCH 6/6] Update 00-defaults.in --- security.d/00-defaults.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security.d/00-defaults.in b/security.d/00-defaults.in index f319e446..34831c65 100644 --- a/security.d/00-defaults.in +++ b/security.d/00-defaults.in @@ -5,8 +5,8 @@ # You MUST read this man page if you intend to attempt to secure your sway # installation. # -# This file should live at __SYSCONFDIR__/sway/security and will be -# automatically read by sway. +# DO NOT CHANGE THIS FILE. Override these defaults by writing new files in +# __SYSCONFDIR__/sway/security.d/* # Configures enabled compositor features for specific programs permit * fullscreen keyboard mouse