From b7785a930460014bb0c3b9b2ee4e160040cbe67a Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 7 Oct 2014 21:01:17 -0400 Subject: [PATCH] [server] extend run_extcmd() to allow the caller to specify whether to collect stderr --- server/extcmd.c | 49 ++++++++++++---------------- server/extcmd.h | 13 +++++--- server/fw_util_iptables.c | 68 +++++++++++++++++++++++++-------------- server/fw_util_pf.c | 4 +-- server/incoming_spa.c | 6 ++-- 5 files changed, 79 insertions(+), 61 deletions(-) diff --git a/server/extcmd.c b/server/extcmd.c index e3961c08..0f6330a1 100644 --- a/server/extcmd.c +++ b/server/extcmd.c @@ -81,8 +81,9 @@ alarm_handler(int sig) * to implement a reliable timeout mechanism. */ static int -_run_extcmd(uid_t uid, gid_t gid, const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, const char *substr_search, int *pid_status, +_run_extcmd(uid_t uid, gid_t gid, const char *cmd, char *so_buf, + const size_t so_buf_sz, const int want_stderr, const int timeout, + const char *substr_search, int *pid_status, const fko_srv_options_t * const opts) { char so_read_buf[IO_READ_BUF_LEN] = {0}; @@ -134,26 +135,21 @@ _run_extcmd(uid_t uid, gid_t gid, const char *cmd, char *so_buf, const size_t so { close(pipe_fd[0]); dup2(pipe_fd[1], STDOUT_FILENO); - dup2(pipe_fd[1], STDERR_FILENO); + if(want_stderr) + dup2(pipe_fd[1], STDERR_FILENO); + else + close(STDERR_FILENO); } /* Take care of gid/uid settings before running the command. */ if(gid > 0) - { if(setgid(gid) < 0) - { exit(EXTCMD_SETGID_ERROR); - } - } if(uid > 0) - { if(setuid(uid) < 0) - { exit(EXTCMD_SETUID_ERROR); - } - } /* don't use env */ @@ -243,20 +239,13 @@ _run_extcmd(uid_t uid, gid_t gid, const char *cmd, char *so_buf, const size_t so /* Take care of gid/uid settings before running the command. */ if(gid > 0) - { if(setgid(gid) < 0) - { exit(EXTCMD_SETGID_ERROR); - } - } if(uid > 0) - { if(setuid(uid) < 0) - { exit(EXTCMD_SETUID_ERROR); - } - } + *pid_status = system(cmd); exit(*pid_status); } @@ -554,29 +543,31 @@ _run_extcmd(uid_t uid, gid_t gid, const char *cmd, char *so_buf, const size_t so */ int run_extcmd(const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, int *pid_status, const fko_srv_options_t * const opts) + const int want_stderr, const int timeout, int *pid_status, + const fko_srv_options_t * const opts) { - return _run_extcmd(0, 0, cmd, so_buf, so_buf_sz, timeout, - NULL, pid_status, opts); + return _run_extcmd(0, 0, cmd, so_buf, so_buf_sz, want_stderr, + timeout, NULL, pid_status, opts); } /* _run_extcmd() wrapper, run an external command as the specified user. */ int run_extcmd_as(uid_t uid, gid_t gid, const char *cmd,char *so_buf, - const size_t so_buf_sz, const int timeout, int *pid_status, - const fko_srv_options_t * const opts) + const size_t so_buf_sz, const int want_stderr, const int timeout, + int *pid_status, const fko_srv_options_t * const opts) { return _run_extcmd(uid, gid, cmd, so_buf, so_buf_sz, - timeout, NULL, pid_status, opts); + want_stderr, timeout, NULL, pid_status, opts); } /* _run_extcmd() wrapper, search command output for a substring. */ int -search_extcmd(const char *cmd, const int timeout, const char *substr_search, - int *pid_status, const fko_srv_options_t * const opts) +search_extcmd(const char *cmd, const int want_stderr, const int timeout, + const char *substr_search, int *pid_status, + const fko_srv_options_t * const opts) { - return _run_extcmd(0, 0, cmd, NULL, 0, timeout, substr_search, - pid_status, opts); + return _run_extcmd(0, 0, cmd, NULL, 0, want_stderr, timeout, + substr_search, pid_status, opts); } diff --git a/server/extcmd.h b/server/extcmd.h index 1487c413..54e59143 100644 --- a/server/extcmd.h +++ b/server/extcmd.h @@ -33,6 +33,9 @@ #define IO_READ_BUF_LEN 256 #define EXTCMD_DEF_TIMEOUT 15 +#define NO_TIMEOUT 0 +#define WANT_STDERR 1 +#define NO_STDERR 0 /* The various return status states in which an external command result * may end up in. @@ -76,11 +79,13 @@ enum { /* Function prototypes */ int run_extcmd(const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, int *pid_status, const fko_srv_options_t * const opts); + const int want_stderr, const int timeout, int *pid_status, + const fko_srv_options_t * const opts); int run_extcmd_as(uid_t uid, gid_t gid, const char *cmd, char *so_buf, - const size_t so_buf_sz, const int timeout, int *pid_status, - const fko_srv_options_t * const opts); -int search_extcmd(const char *cmd, const int timeout, const char *substr_search, + const size_t so_buf_sz, const int want_stderr, const int timeout, + int *pid_status, const fko_srv_options_t * const opts); +int search_extcmd(const char *cmd, const int want_stderr, + const int timeout, const char *substr_search, int *pid_status, const fko_srv_options_t * const opts); #endif /* EXTCMD_H */ diff --git a/server/fw_util_iptables.c b/server/fw_util_iptables.c index 84ebd4b3..1c6db3f8 100644 --- a/server/fw_util_iptables.c +++ b/server/fw_util_iptables.c @@ -102,18 +102,23 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, /* search for each of the substrings, and require the returned * rule number to be the same across all searches to return true */ - rtmp = search_extcmd(cmd_buf, 0, exp_ts_search, &pid_status, opts); + rtmp = search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, exp_ts_search, &pid_status, opts); if(rtmp > 0) { rule_num = rtmp; - rtmp = search_extcmd(cmd_buf, 0, proto_search, &pid_status, opts); + rtmp = search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, proto_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, ip_search, &pid_status, opts); + rtmp = search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, ip_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, target_search, &pid_status, opts); + rtmp = search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, target_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, port_search, &pid_status, opts); + rtmp = search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, port_search, &pid_status, opts); if(rtmp == rule_num) rule_exists = 1; } @@ -142,7 +147,8 @@ rule_exists_chk_support(const fko_srv_options_t * const opts, snprintf(cmd_buf, CMD_BUFSIZE-1, "%s " IPT_CHK_RULE_ARGS, opts->fw_config->fw_command, chain, rule); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "rule_exists_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -206,7 +212,8 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "ipt_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -223,7 +230,8 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "ipt_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -250,7 +258,8 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->from_chain, 1 ); - run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); return; } @@ -276,7 +285,8 @@ comment_match_exists(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "comment_match_exists() CMD: '%s' (res: %d, err: %s)", @@ -291,7 +301,7 @@ comment_match_exists(const fko_srv_options_t * const opts) ); res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, - 0, &pid_status, opts); + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(cmd_out); if(!EXTCMD_IS_SUCCESS(res)) @@ -315,7 +325,8 @@ comment_match_exists(const fko_srv_options_t * const opts) in_chain->from_chain, 1 ); - run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); } return res; @@ -336,7 +347,8 @@ add_jump_rule(const fko_srv_options_t * const opts, const int chain_num) fwc.chain[chain_num].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); log_msg(LOG_DEBUG, "add_jump_rule() CMD: '%s' (res: %d, err: %s)", cmd_buf, res, err_buf); @@ -364,7 +376,8 @@ chain_exists(const fko_srv_options_t * const opts, const int chain_num) fwc.chain[chain_num].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "chain_exists() CMD: '%s' (res: %d, err: %s)", @@ -420,7 +433,8 @@ jump_rule_exists_no_chk_support(const fko_srv_options_t * const opts, const int snprintf(chain_search, CMD_BUFSIZE-1, " %s ", fwc.chain[chain_num].to_chain); - if(search_extcmd(cmd_buf, 0, chain_search, &pid_status, opts) > 0) + if(search_extcmd(cmd_buf, WANT_STDERR, + NO_TIMEOUT, chain_search, &pid_status, opts) > 0) exists = 1; if(exists) @@ -474,7 +488,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) ch[i].table ); - res = run_extcmd(cmd_buf, NULL, 0, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, NULL, 0, 0, 0, &pid_status, opts); log_msg(LOG_DEBUG, "fw_dump_rules() CMD: '%s' (res: %d)", cmd_buf, res); @@ -511,7 +525,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) fprintf(stdout, "\n"); fflush(stdout); - res = run_extcmd(cmd_buf, NULL, 0, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, NULL, 0, 0, 0, &pid_status, opts); log_msg(LOG_DEBUG, "fw_dump_rules() CMD: '%s' (res: %d)", cmd_buf, res); @@ -555,7 +569,8 @@ delete_all_chains(const fko_srv_options_t * const opts) fwc.chain[i].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -579,7 +594,8 @@ delete_all_chains(const fko_srv_options_t * const opts) fwc.chain[i].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, WANT_STDERR, + NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -598,7 +614,8 @@ delete_all_chains(const fko_srv_options_t * const opts) fwc.chain[i].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, WANT_STDERR, + NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -626,7 +643,8 @@ create_chain(const fko_srv_options_t * const opts, const int chain_num) fwc.chain[chain_num].to_chain ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, WANT_STDERR, + NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "create_chain() CMD: '%s' (res: %d, err: %s)", @@ -902,7 +920,8 @@ create_rule(const fko_srv_options_t * const opts, snprintf(cmd_buf, CMD_BUFSIZE-1, "%s -A %s %s", opts->fw_config->fw_command, fw_chain, fw_rule); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, WANT_STDERR, + NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "create_rule() CMD: '%s' (res: %d, err: %s)", @@ -1372,7 +1391,7 @@ check_firewall_rules(const fko_srv_options_t * const opts) ); res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, - 0, &pid_status, opts); + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(cmd_out); log_msg(LOG_DEBUG, "check_firewall_rules() CMD: '%s' (res: %d, cmd_out: %s)", @@ -1480,7 +1499,8 @@ check_firewall_rules(const fko_srv_options_t * const opts) rule_num - rn_offset ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "check_firewall_rules() CMD: '%s' (res: %d, err: %s)", diff --git a/server/fw_util_pf.c b/server/fw_util_pf.c index a851ea6f..bca01310 100644 --- a/server/fw_util_pf.c +++ b/server/fw_util_pf.c @@ -57,7 +57,7 @@ zero_cmd_buffers(void) int fw_dump_rules(const fko_srv_options_t * const opts) { - int res, got_err = 0; + int res, got_err = 0, pid_status = 0; fprintf(stdout, "Listing fwknopd pf rules...\n"); fflush(stdout); @@ -73,7 +73,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) fprintf(stdout, "\nActive Rules in PF anchor '%s':\n", opts->fw_config->anchor); fflush(stdout); - res = system(cmd_buf); + res = run_extcmd(cmd_buf, NULL, 0, 0, &pid_status, opts); /* Expect full success on this */ if(! EXTCMD_IS_SUCCESS(res)) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 1be68a29..544f102e 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -886,10 +886,12 @@ incoming_spa(fko_srv_options_t *opts) acc->cmd_exec_uid, acc->cmd_exec_gid); res = run_extcmd_as(acc->cmd_exec_uid, acc->cmd_exec_gid, - spadat.spa_message_remain, NULL, 0, 0, &pid_status, opts); + spadat.spa_message_remain, NULL, 0, + WANT_STDERR, NO_TIMEOUT, &pid_status, opts); } else /* Just run it as we are (root that is). */ - res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5, &pid_status, opts); + res = run_extcmd(spadat.spa_message_remain, NULL, 0, + WANT_STDERR, 5, &pid_status, opts); /* should only call WEXITSTATUS() if WIFEXITED() is true */