From 652b8cb80eda66c30b89b5111b700f72e8e17e7b Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 5 Oct 2014 20:21:05 -0400 Subject: [PATCH] [server] have run_extcmd() collect process exit status for calling function (in addition to return value) --- server/extcmd.c | 39 ++++++++++++-------- server/extcmd.h | 7 ++-- server/fw_util_iptables.c | 75 ++++++++++++++++++++------------------- server/incoming_spa.c | 28 +++++++-------- 4 files changed, 81 insertions(+), 68 deletions(-) diff --git a/server/extcmd.c b/server/extcmd.c index e02738dd..0b2512ab 100644 --- a/server/extcmd.c +++ b/server/extcmd.c @@ -82,7 +82,8 @@ alarm_handler(int sig) */ static int _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, const char *substr_search, const fko_srv_options_t * const opts) + 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}; char *argv_new[MAX_CMDLINE_ARGS]; /* for execvpe() */ @@ -90,12 +91,13 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s int pipe_fd[2]; pid_t pid=0; FILE *output; - int retval = 0, status; + int retval = 0; int line_ctr = 0, found_str = 0; + *pid_status = 0; memset(argv_new, 0x0, sizeof(argv_new)); - if(opts->verbose > 2) + if(opts->verbose > 1) log_msg(LOG_INFO, "run_extcmd(): running CMD: %s", cmd); if(strtoargv(cmd, argv_new, &argc_new, opts) != 1) @@ -128,7 +130,6 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s /* If user is not null, then we setuid to that user before running the * command. */ -#if 0 if(user_uid > 0) { if(setuid(user_uid) < 0) @@ -136,7 +137,6 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s exit(EXTCMD_SETUID_ERROR); } } -#endif /* don't use env */ @@ -192,16 +192,19 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s log_msg(LOG_ERR, "run_extcmd(): could not fdopen() pipe output file descriptor."); free_argv(argv_new, &argc_new); - return -1; + return EXTCMD_OPEN_ERROR; } } free_argv(argv_new, &argc_new); - waitpid(pid, &status, 0); + waitpid(pid, pid_status, 0); if(substr_search != NULL) { + /* The semantics of the return value changes in search mode to the line + * number where the substring match was found, or zero if it wasn't found + */ if(found_str) retval = line_ctr; else @@ -209,7 +212,7 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s } else { - if(WIFEXITED(status)) + if(WIFEXITED(*pid_status)) { /* Even if the child exited with an error condition, if we make it here * then the child exited normally as far as the OS is concerned (i.e. didn't @@ -221,6 +224,11 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s retval = EXTCMD_EXECUTION_ERROR; } + if(opts->verbose > 1) + log_msg(LOG_INFO, + "run_extcmd(): returning %d, pid_status: %d", + retval, WIFEXITED(*pid_status) ? WEXITSTATUS(*pid_status) : *pid_status); + return(retval); } @@ -439,25 +447,28 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s */ int run_extcmd(const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, const fko_srv_options_t * const opts) + const int timeout, int *pid_status, const fko_srv_options_t * const opts) { - return _run_extcmd(0, cmd, so_buf, so_buf_sz, timeout, NULL, opts); + return _run_extcmd(0, cmd, so_buf, so_buf_sz, timeout, + NULL, pid_status, opts); } /* _run_extcmd() wrapper, run an external command as the specified user. */ int run_extcmd_as(uid_t user_uid, const char *cmd,char *so_buf, const size_t so_buf_sz, - const int timeout, const fko_srv_options_t * const opts) + const int timeout, int *pid_status, const fko_srv_options_t * const opts) { - return _run_extcmd(user_uid, cmd, so_buf, so_buf_sz, timeout, NULL, opts); + return _run_extcmd(user_uid, cmd, so_buf, so_buf_sz, 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, - const fko_srv_options_t * const opts) + int *pid_status, const fko_srv_options_t * const opts) { - return _run_extcmd(0, cmd, NULL, 0, timeout, substr_search, opts); + return _run_extcmd(0, cmd, NULL, 0, timeout, substr_search, + pid_status, opts); } diff --git a/server/extcmd.h b/server/extcmd.h index c2ea974e..1351a341 100644 --- a/server/extcmd.h +++ b/server/extcmd.h @@ -74,11 +74,12 @@ enum { /* Function prototypes */ int run_extcmd(const char *cmd, char *so_buf, const size_t so_buf_sz, - const int timeout, const fko_srv_options_t * const opts); + const int timeout, int *pid_status, const fko_srv_options_t * const opts); int run_extcmd_as(uid_t uid, const char *cmd, char *so_buf, - const size_t so_buf_sz, const int timeout, const fko_srv_options_t * const opts); + 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 fko_srv_options_t * const opts); + 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 d0ffd2f6..84ebd4b3 100644 --- a/server/fw_util_iptables.c +++ b/server/fw_util_iptables.c @@ -71,7 +71,7 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, const char * const ip, const unsigned int port, const unsigned int exp_ts) { - int rule_exists=0, rule_num=0, rtmp=0; + int rule_exists=0, rule_num=0, rtmp=0, pid_status=0; char cmd_buf[CMD_BUFSIZE] = {0}; char target_search[CMD_BUFSIZE] = {0}; char proto_search[CMD_BUFSIZE] = {0}; @@ -102,18 +102,18 @@ 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, opts); + rtmp = search_extcmd(cmd_buf, 0, exp_ts_search, &pid_status, opts); if(rtmp > 0) { rule_num = rtmp; - rtmp = search_extcmd(cmd_buf, 0, proto_search, opts); + rtmp = search_extcmd(cmd_buf, 0, proto_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, ip_search, opts); + rtmp = search_extcmd(cmd_buf, 0, ip_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, target_search, opts); + rtmp = search_extcmd(cmd_buf, 0, target_search, &pid_status, opts); if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, 0, port_search, opts); + rtmp = search_extcmd(cmd_buf, 0, port_search, &pid_status, opts); if(rtmp == rule_num) rule_exists = 1; } @@ -135,14 +135,14 @@ rule_exists_chk_support(const fko_srv_options_t * const opts, const char * const chain, const char * const rule) { int rule_exists = 0; - int res = 0; + int res = 0, pid_status=0; zero_cmd_buffers(); 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "rule_exists_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -189,7 +189,7 @@ rule_exists(const fko_srv_options_t * const opts, static void ipt_chk_support(const fko_srv_options_t * const opts) { - int res = 1; + int res = 1, pid_status = 0; struct fw_chain *in_chain = &(opts->fw_config->chain[IPT_INPUT_ACCESS]); zero_cmd_buffers(); @@ -206,7 +206,7 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "ipt_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -223,7 +223,7 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "ipt_chk_support() CMD: '%s' (res: %d, err: %s)", @@ -250,7 +250,7 @@ ipt_chk_support(const fko_srv_options_t * const opts) in_chain->from_chain, 1 ); - run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); return; } @@ -258,7 +258,7 @@ ipt_chk_support(const fko_srv_options_t * const opts) static int comment_match_exists(const fko_srv_options_t * const opts) { - int res = 1; + int res = 1, pid_status = 0; char *ndx = NULL; struct fw_chain *in_chain = &(opts->fw_config->chain[IPT_INPUT_ACCESS]); @@ -276,7 +276,7 @@ comment_match_exists(const fko_srv_options_t * const opts) in_chain->target ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "comment_match_exists() CMD: '%s' (res: %d, err: %s)", @@ -290,7 +290,8 @@ comment_match_exists(const fko_srv_options_t * const opts) in_chain->from_chain ); - res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, + 0, &pid_status, opts); chop_newline(cmd_out); if(!EXTCMD_IS_SUCCESS(res)) @@ -314,7 +315,7 @@ comment_match_exists(const fko_srv_options_t * const opts) in_chain->from_chain, 1 ); - run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); } return res; @@ -323,7 +324,7 @@ comment_match_exists(const fko_srv_options_t * const opts) static int add_jump_rule(const fko_srv_options_t * const opts, const int chain_num) { - int res = 0; + int res = 0, pid_status = 0; zero_cmd_buffers(); @@ -335,7 +336,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); log_msg(LOG_DEBUG, "add_jump_rule() CMD: '%s' (res: %d, err: %s)", cmd_buf, res, err_buf); @@ -353,7 +354,7 @@ add_jump_rule(const fko_srv_options_t * const opts, const int chain_num) static int chain_exists(const fko_srv_options_t * const opts, const int chain_num) { - int res = 0; + int res = 0, pid_status = 0; zero_cmd_buffers(); @@ -363,7 +364,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "chain_exists() CMD: '%s' (res: %d, err: %s)", @@ -404,7 +405,7 @@ jump_rule_exists_chk_support(const fko_srv_options_t * const opts, const int cha static int jump_rule_exists_no_chk_support(const fko_srv_options_t * const opts, const int chain_num) { - int exists = 0; + int exists = 0, pid_status = 0; char cmd_buf[CMD_BUFSIZE] = {0}; char chain_search[CMD_BUFSIZE] = {0}; @@ -419,7 +420,7 @@ 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, opts) > 0) + if(search_extcmd(cmd_buf, 0, chain_search, &pid_status, opts) > 0) exists = 1; if(exists) @@ -449,8 +450,7 @@ jump_rule_exists(const fko_srv_options_t * const opts, const int chain_num) int fw_dump_rules(const fko_srv_options_t * const opts) { - int i; - int res, got_err = 0; + int i, res, got_err = 0, pid_status; struct fw_chain *ch = opts->fw_config->chain; @@ -474,7 +474,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) ch[i].table ); - res = run_extcmd(cmd_buf, NULL, 0, 0, opts); + res = run_extcmd(cmd_buf, NULL, 0, 0, &pid_status, opts); log_msg(LOG_DEBUG, "fw_dump_rules() CMD: '%s' (res: %d)", cmd_buf, res); @@ -511,7 +511,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) fprintf(stdout, "\n"); fflush(stdout); - res = run_extcmd(cmd_buf, NULL, 0, 0, opts); + res = run_extcmd(cmd_buf, NULL, 0, 0, &pid_status, opts); log_msg(LOG_DEBUG, "fw_dump_rules() CMD: '%s' (res: %d)", cmd_buf, res); @@ -533,7 +533,7 @@ fw_dump_rules(const fko_srv_options_t * const opts) static void delete_all_chains(const fko_srv_options_t * const opts) { - int i, res, cmd_ctr = 0; + int i, res, cmd_ctr = 0, pid_status=0; for(i=0; i<(NUM_FWKNOP_ACCESS_TYPES); i++) { @@ -555,7 +555,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -579,7 +579,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -598,7 +598,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "delete_all_chains() CMD: '%s' (res: %d, err: %s)", @@ -614,7 +614,7 @@ delete_all_chains(const fko_srv_options_t * const opts) static int create_chain(const fko_srv_options_t * const opts, const int chain_num) { - int res = 0; + int res = 0, pid_status = 0; zero_cmd_buffers(); @@ -626,7 +626,7 @@ 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "create_chain() CMD: '%s' (res: %d, err: %s)", @@ -896,13 +896,13 @@ static int create_rule(const fko_srv_options_t * const opts, const char * const fw_chain, const char * const fw_rule) { - int res = 0; + int res = 0, pid_status = 0; zero_cmd_buffers(); 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, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "create_rule() CMD: '%s' (res: %d, err: %s)", @@ -1341,7 +1341,7 @@ check_firewall_rules(const fko_srv_options_t * const opts) char rule_num_str[6] = {0}; char *ndx, *rn_start, *rn_end, *tmp_mark; - int i, res, rn_offset, rule_num, is_err; + int i, res, rn_offset, rule_num, is_err, pid_status=0; time_t now, rule_exp, min_exp = 0; struct fw_chain *ch = opts->fw_config->chain; @@ -1371,7 +1371,8 @@ check_firewall_rules(const fko_srv_options_t * const opts) ch[i].to_chain ); - res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, + 0, &pid_status, opts); chop_newline(cmd_out); log_msg(LOG_DEBUG, "check_firewall_rules() CMD: '%s' (res: %d, cmd_out: %s)", @@ -1479,7 +1480,7 @@ check_firewall_rules(const fko_srv_options_t * const opts) rule_num - rn_offset ); - res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, opts); + res = run_extcmd(cmd_buf, err_buf, CMD_BUFSIZE, 0, &pid_status, opts); chop_newline(err_buf); log_msg(LOG_DEBUG, "check_firewall_rules() CMD: '%s' (res: %d, err: %s)", diff --git a/server/incoming_spa.c b/server/incoming_spa.c index cfb7a10d..b1f0e387 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -299,7 +299,7 @@ incoming_spa(fko_srv_options_t *opts) char *spa_ip_demark, *gpg_id, *gpg_fpr, *raw_digest = NULL; time_t now_ts; - int res, status, ts_diff, enc_type, stanza_num=0; + int res, ts_diff, enc_type, stanza_num=0, pid_status=0; int added_replay_digest = 0, pkt_data_len=0; int is_err, cmd_exec_success = 0, attempted_decrypt = 0; int conf_pkt_age = 0; @@ -883,24 +883,24 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num, acc->cmd_exec_user, acc->cmd_exec_uid); res = run_extcmd_as(acc->cmd_exec_uid, spadat.spa_message_remain, - NULL, 0, 0, opts); + NULL, 0, 0, &pid_status, opts); } else /* Just run it as we are (root that is). */ - res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5, opts); + res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5, &pid_status, opts); - /* --DSS XXX: I have found that the status (and res for that - * matter) have been unreliable indicators of the - * actual exit status of some commands. Not sure - * why yet. For now, we will take what we get. + /* should only call WEXITSTATUS() if WIFEXITED() is true */ - status = WEXITSTATUS(res); + log_msg(LOG_INFO, + "[%s] (stanza #%d) CMD_EXEC: command returned %i, pid_status: %d", + spadat.pkt_source_ip, stanza_num, res, + WIFEXITED(pid_status) ? WEXITSTATUS(pid_status) : pid_status); - if(opts->verbose > 1) - log_msg(LOG_WARNING, - "[%s] (stanza #%d) CMD_EXEC: command returned %i", - spadat.pkt_source_ip, stanza_num, status); - - if(status != 0) + if(WIFEXITED(pid_status)) + { + if(WEXITSTATUS(pid_status) != 0) + res = SPA_MSG_COMMAND_ERROR; + } + else res = SPA_MSG_COMMAND_ERROR; /* we processed the command on a matching access stanza, so we