From 7dbecdde95d1f309d8fdd02fe480dc3fbef7c7c1 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Sun, 19 Feb 2017 02:36:36 -0500 Subject: Revise IPC security configuration --- sway/sway-security.7.txt | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) (limited to 'sway/sway-security.7.txt') 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 ------- -- cgit v1.2.3 From 126ce571dab09d84d8ee1b760981dbba7cbc1000 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Mon, 20 Feb 2017 07:42:08 -0500 Subject: 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(-) (limited to 'sway/sway-security.7.txt') 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 -- cgit v1.2.3