From 4ff518d54a3b64457defe41328a65664b0c63fe0 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sat, 6 Jul 2013 14:52:46 -0400 Subject: [PATCH] [server] zero out access stanza key information before exit (in support of #93) --- server/access.c | 28 ++++++++++++++++++++++++++++ server/fw_util.h | 2 +- server/fw_util_ipf.c | 3 ++- server/fw_util_ipfw.c | 10 ++++++---- server/fw_util_iptables.c | 13 +++++-------- server/fw_util_pf.c | 4 ++-- server/fwknopd.c | 3 ++- 7 files changed, 46 insertions(+), 17 deletions(-) diff --git a/server/access.c b/server/access.c index aade2d2b..78be5163 100644 --- a/server/access.c +++ b/server/access.c @@ -631,6 +631,22 @@ free_acc_string_list(acc_string_list_t *stl) } } +/* zero out key information in a way that isn't optimized out by the compiler +*/ +static void +zero_key(char *key, int len) +{ + int i; + + memset(key, 0x0, len); + + for(i=0; i < len; i++) + if(key[i] != 0x0) + log_msg(LOG_ERR, "[*] Could not zero out key data."); + + return; +} + /* Free any allocated content of an access stanza. * * NOTE: If a new access.conf parameter is created, and it is a string @@ -663,16 +679,28 @@ free_acc_stanza_data(acc_stanza_t *acc) free(acc->force_nat_ip); if(acc->key != NULL) + { + zero_key(acc->key, acc->key_len); free(acc->key); + } if(acc->key_base64 != NULL) + { + zero_key(acc->key, strlen(acc->key_base64)); free(acc->key_base64); + } if(acc->hmac_key != NULL) + { + zero_key(acc->hmac_key, acc->hmac_key_len); free(acc->hmac_key); + } if(acc->hmac_key_base64 != NULL) + { + zero_key(acc->hmac_key_base64, strlen(acc->hmac_key_base64)); free(acc->hmac_key_base64); + } if(acc->cmd_exec_user != NULL) free(acc->cmd_exec_user); diff --git a/server/fw_util.h b/server/fw_util.h index 8cbc4318..fcfe9b29 100644 --- a/server/fw_util.h +++ b/server/fw_util.h @@ -61,7 +61,7 @@ * fw_util_.c files. */ void fw_config_init(fko_srv_options_t * const opts); -void fw_initialize(const fko_srv_options_t * const opts); +int fw_initialize(const fko_srv_options_t * const opts); int fw_cleanup(const fko_srv_options_t * const opts); void check_firewall_rules(const fko_srv_options_t * const opts); int fw_dump_rules(const fko_srv_options_t * const opts); diff --git a/server/fw_util_ipf.c b/server/fw_util_ipf.c index 15087b99..8a7e4c07 100644 --- a/server/fw_util_ipf.c +++ b/server/fw_util_ipf.c @@ -92,8 +92,9 @@ fw_initialize(const fko_srv_options_t *opts) { log_msg(LOG_WARNING, "Warning: Errors detected during fw_initialize()."); - clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE); + return 0; } + return 1; } int diff --git a/server/fw_util_ipfw.c b/server/fw_util_ipfw.c index 78e6271a..bfd8a82b 100644 --- a/server/fw_util_ipfw.c +++ b/server/fw_util_ipfw.c @@ -255,7 +255,7 @@ fw_initialize(const fko_srv_options_t * const opts) if(res != 0) { log_msg(LOG_ERR, "[*] Fatal: Errors detected during ipfw rules initialization."); - exit(EXIT_FAILURE); + return 0; } /* Allocate our rule_map array for tracking active (and expired) rules. @@ -265,7 +265,7 @@ fw_initialize(const fko_srv_options_t * const opts) if(fwc.rule_map == NULL) { log_msg(LOG_ERR, "[*] Fatal: Memory allocation error in fw_initialize()."); - exit(EXIT_FAILURE); + return 0; } /* Create a check-state rule if necessary. @@ -340,7 +340,7 @@ fw_initialize(const fko_srv_options_t * const opts) if(!EXTCMD_IS_SUCCESS(res)) { log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, cmd_out); - return; + return 0; } log_msg(LOG_DEBUG, "RULES LIST: %s", cmd_out); @@ -352,7 +352,7 @@ fw_initialize(const fko_srv_options_t * const opts) /* Assume no disabled rules if we did not see the string. */ if(ndx == NULL) - return; + return 1; /* Otherwise we walk each line to pull the rule number and * set the appropriate rule map entries. @@ -385,6 +385,8 @@ fw_initialize(const fko_srv_options_t * const opts) */ ndx = strstr(ndx, "# DISABLED "); } + + return 1; } int diff --git a/server/fw_util_iptables.c b/server/fw_util_iptables.c index 51c82140..8da0fc9e 100644 --- a/server/fw_util_iptables.c +++ b/server/fw_util_iptables.c @@ -560,11 +560,9 @@ fw_config_init(fko_srv_options_t * const opts) return; } -void +int fw_initialize(const fko_srv_options_t * const opts) { - int res; - /* Flush the chains (just in case) so we can start fresh. */ if(strncasecmp(opts->config[CONF_FLUSH_IPT_AT_INIT], "Y", 1) == 0) @@ -572,13 +570,11 @@ fw_initialize(const fko_srv_options_t * const opts) /* Now create any configured chains. */ - res = create_fw_chains(opts); - - if(res != 0) + if(create_fw_chains(opts) != 0) { log_msg(LOG_WARNING, "Warning: Errors detected during fwknop custom chain creation."); - exit(EXIT_FAILURE); + return 0; } /* Make sure that the 'comment' match is available @@ -587,8 +583,9 @@ fw_initialize(const fko_srv_options_t * const opts) && (comment_match_exists(opts) != 1)) { log_msg(LOG_WARNING, "Warning: Could not use the 'comment' match."); - exit(EXIT_FAILURE); + return 0; } + return 1; } int diff --git a/server/fw_util_pf.c b/server/fw_util_pf.c index a0667780..2d0b01f4 100644 --- a/server/fw_util_pf.c +++ b/server/fw_util_pf.c @@ -170,14 +170,14 @@ fw_initialize(const fko_srv_options_t * const opts) { log_msg(LOG_WARNING, "Warning: the fwknop anchor is not active in the pf policy"); - exit(EXIT_FAILURE); + return 0; } /* Delete any existing rules in the fwknop anchor */ delete_all_anchor_rules(opts); - return; + return 1; } int diff --git a/server/fwknopd.c b/server/fwknopd.c index e4d66ccf..b3890fed 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -286,7 +286,8 @@ main(int argc, char **argv) /* Prepare the firewall - i.e. flush any old rules and (for iptables) * create fwknop chains. */ - fw_initialize(&opts); + if(fw_initialize(&opts) != 1) + clean_exit(&opts, FW_CLEANUP, EXIT_FAILURE); /* If the TCP server option was set, fire it up here. */