From 4c7679fabd225e27145e5770d6e36931926560f0 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 2 Jul 2015 23:44:16 -0400 Subject: [PATCH] [server] start on sudo usage for command exec (issue #159) --- doc/fwknopd.man.asciidoc | 27 +++++++++++++++++++++++---- server/access.c | 8 ++++++++ server/cmd_opts.h | 3 +++ server/config_init.c | 20 +++++++++++++++++++- server/fwknopd_common.h | 7 +++++++ server/utils.c | 37 ++++++++++++++++++++++++++++++------- server/utils.h | 4 ++++ 7 files changed, 94 insertions(+), 12 deletions(-) diff --git a/doc/fwknopd.man.asciidoc b/doc/fwknopd.man.asciidoc index e36bed60..2924217a 100644 --- a/doc/fwknopd.man.asciidoc +++ b/doc/fwknopd.man.asciidoc @@ -526,11 +526,30 @@ directive starts a new stanza. the *fwknopd* server as the user specified by the ``CMD_EXEC_USER'' or as the user that started *fwknopd* if that is not set. +*CMD_EXEC_ALL_SUDO* '':: + *sudo* provides a powerful means of restricting the sets of commands that + users can execute via the ``sudoers'' file. By enabling this feature (and + in ``ENABLE_CMD_EXEC'' mode), all incoming commands from valid SPA packets + will be prefixed by ``/path/to/sudo -u '' where the path to sudo is + set by the ``SUDO_EXE'' variable, and ``'' is set by the + ``CMD_EXEC_USER'' variable (default is ``root'' if not set). + *CMD_EXEC_USER* '':: - This specifies the user that will execute commands contained within a SPA - packet. If not specified, fwknopd will execute it as the user it is - running as (most likely root). Setting this to a non-root user is highly - recommended. + This specifies the user (via setuid) that will execute commands contained + within a SPA packet. If not specified, fwknopd will execute it as the user + it is running as (most likely root). Setting this to a non-root user such + as ``nobody'' is highly recommended if elevated permissions are not + needed. + +*CMD_EXEC_GROUP* '':: + This specifies the group (via setgid) that will execute commands contained + within a SPA packet. If not specified, fwknopd will execute it as the user + it is running as (most likely root). Setting this to a non-root user such + as ``nobody'' is highly recommended if elevated permissions are not + needed. + +*SUDO_EXE* '':: + Define the path to the sudo binary. Default is ``/usr/bin/sudo''. *REQUIRE_USERNAME* '':: Require a specific username from the client system as encoded in the SPA diff --git a/server/access.c b/server/access.c index 5686a8da..4717e225 100644 --- a/server/access.c +++ b/server/access.c @@ -1480,6 +1480,10 @@ parse_access_file(fko_srv_options_t *opts) { add_acc_bool(&(curr_acc->enable_cmd_exec), val); } + else if(CONF_VAR_IS(var, "ENABLE_CMD_SUDO_EXEC")) + { + add_acc_bool(&(curr_acc->enable_cmd_sudo_exec), val); + } else if(CONF_VAR_IS(var, "CMD_EXEC_USER")) { if(add_acc_string(&(curr_acc->cmd_exec_user), val) != SUCCESS) @@ -1956,7 +1960,9 @@ dump_access_list(const fko_srv_options_t *opts) " HMAC_DIGEST_TYPE: %d\n" " FW_ACCESS_TIMEOUT: %i\n" " ENABLE_CMD_EXEC: %s\n" + " ENABLE_CMD_SUDO_EXEC: %s\n" " CMD_EXEC_USER: %s\n" + " CMD_EXEC_GROUP: %s\n" " REQUIRE_USERNAME: %s\n" " REQUIRE_SOURCE_ADDRESS: %s\n" " FORCE_NAT (ip): %s\n" @@ -1989,7 +1995,9 @@ dump_access_list(const fko_srv_options_t *opts) acc->hmac_type, acc->fw_access_timeout, acc->enable_cmd_exec ? "Yes" : "No", + acc->enable_cmd_sudo_exec ? "Yes" : "No", (acc->cmd_exec_user == NULL) ? "" : acc->cmd_exec_user, + (acc->cmd_exec_group == NULL) ? "" : acc->cmd_exec_group, (acc->require_username == NULL) ? "" : acc->require_username, acc->require_source_address ? "Yes" : "No", acc->force_nat ? acc->force_nat_ip : "", diff --git a/server/cmd_opts.h b/server/cmd_opts.h index d7a645cb..bda46183 100644 --- a/server/cmd_opts.h +++ b/server/cmd_opts.h @@ -126,6 +126,7 @@ static char *config_map[NUMBER_OF_CONFIG_ENTRIES] = { #endif "GPG_HOME_DIR", "GPG_EXE", + "SUDO_EXE", "FIREWALL_EXE", "VERBOSE", #if AFL_FUZZING @@ -148,6 +149,7 @@ enum { AFL_PKT_FILE, GPG_HOME_DIR, GPG_EXE_PATH, + SUDO_EXE_PATH, FIREWD_DISABLE_CHECK_SUPPORT, IPT_DISABLE_CHECK_SUPPORT, PCAP_FILE, @@ -205,6 +207,7 @@ static struct option cmd_opts[] = {"run-dir", 1, NULL, 'r'}, {"restart", 0, NULL, 'R'}, {"status", 0, NULL, 'S'}, + {"sudo-exe", 1, NULL, SUDO_EXE_PATH }, {"test", 0, NULL, 't'}, {"udp-server", 0, NULL, 'U'}, {"verbose", 0, NULL, 'v'}, diff --git a/server/config_init.c b/server/config_init.c index 2c05efcf..ca9111f9 100644 --- a/server/config_init.c +++ b/server/config_init.c @@ -864,6 +864,11 @@ validate_options(fko_srv_options_t *opts) if(opts->config[CONF_GPG_EXE] == NULL) set_config_entry(opts, CONF_GPG_EXE, DEF_GPG_EXE); + /* sudo executable + */ + if(opts->config[CONF_SUDO_EXE] == NULL) + set_config_entry(opts, CONF_SUDO_EXE, DEF_SUDO_EXE); + /* Enable SPA over HTTP. */ if(opts->config[CONF_ENABLE_SPA_OVER_HTTP] == NULL) @@ -1233,6 +1238,19 @@ config_init(fko_srv_options_t *opts, int argc, char **argv) case FW_FLUSH: opts->fw_flush = 1; break; + case GPG_EXE_PATH: + if (is_valid_exe(optarg)) + { + set_config_entry(opts, CONF_GPG_EXE, optarg); + } + else + { + log_msg(LOG_ERR, + "[*] gpg path '%s' could not stat()/does not exist?", + optarg); + clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE); + } + break; case GPG_HOME_DIR: if (is_valid_dir(optarg)) { @@ -1241,7 +1259,7 @@ config_init(fko_srv_options_t *opts, int argc, char **argv) else { log_msg(LOG_ERR, - "[*] Directory '%s' could not stat()/does not exist?", + "[*] gpg home directory '%s' could not stat()/does not exist?", optarg); clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE); } diff --git a/server/fwknopd_common.h b/server/fwknopd_common.h index 8c33eaa0..4f3534cd 100644 --- a/server/fwknopd_common.h +++ b/server/fwknopd_common.h @@ -98,6 +98,11 @@ #else #define DEF_GPG_EXE "/usr/bin/gpg" #endif +#ifdef SUDO_EXE + #define DEF_SUDO_EXE SUDO_EXE +#else + #define DEF_SUDO_EXE "/usr/bin/sudo" +#endif #define DEF_ENABLE_SPA_OVER_HTTP "N" #define DEF_ENABLE_TCP_SERVER "N" #define DEF_TCPSERV_PORT "62201" @@ -305,6 +310,7 @@ enum { #endif CONF_GPG_HOME_DIR, CONF_GPG_EXE, + CONF_SUDO_EXE, CONF_FIREWALL_EXE, CONF_VERBOSE, #if AFL_FUZZING @@ -366,6 +372,7 @@ typedef struct acc_stanza unsigned char use_rijndael; int fw_access_timeout; unsigned char enable_cmd_exec; + unsigned char enable_cmd_sudo_exec; char *cmd_exec_user; char *cmd_exec_group; uid_t cmd_exec_uid; diff --git a/server/utils.c b/server/utils.c index ec91396d..37fce304 100644 --- a/server/utils.c +++ b/server/utils.c @@ -76,31 +76,54 @@ hex_dump(const unsigned char *data, const int size) return; } -/* Basic directory checks (stat() and whether the path is actually - * a directory). +/* Basic directory/binary checks (stat() and whether the path is actually + * a directory or an executable). */ -int -is_valid_dir(const char *path) +static int +is_valid_path(const char *path, const int file_type) { #if HAVE_STAT struct stat st; - /* If we are unable to stat the given dir, then return with error. + /* If we are unable to stat the given path, then return with error. */ if(stat(path, &st) != 0) { - log_msg(LOG_ERR, "[-] unable to stat() directory: %s: %s", + log_msg(LOG_ERR, "[-] unable to stat() path: %s: %s", path, strerror(errno)); return(0); } - if(!S_ISDIR(st.st_mode)) + if(file_type == IS_DIR) + { + if(!S_ISDIR(st.st_mode)) + return(0); + } + else if(file_type == IS_EXE) + { + if(!S_ISREG(st.st_mode) || ! (st.st_mode & S_IXUSR)) + return(0); + } + else return(0); + #endif /* HAVE_STAT */ return(1); } +int +is_valid_dir(const char *path) +{ + return is_valid_path(path, IS_DIR); +} + +int +is_valid_exe(const char *path) +{ + return is_valid_path(path, IS_EXE); +} + int verify_file_perms_ownership(const char *file) { diff --git a/server/utils.h b/server/utils.h index 3d37a45f..58fc9599 100644 --- a/server/utils.h +++ b/server/utils.h @@ -54,11 +54,15 @@ x == '#' || x == '\n' || x == '\r' || x == ';' || x == '\0' \ ) +#define IS_DIR 1 +#define IS_EXE 2 + /* Prototypes */ void hex_dump(const unsigned char *data, const int size); char* dump_ctx(fko_ctx_t ctx); int is_valid_dir(const char *path); +int is_valid_exe(const char *path); int verify_file_perms_ownership(const char *file); int strtoargv(const char * const args_str, char **argv_new, int *argc_new, const fko_srv_options_t * const opts);