From ba3b7d1d11d681549f1bb27e0af1307499fad0d5 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sat, 7 Jul 2012 21:31:30 -0400 Subject: [PATCH 1/5] Bug fix for multi-stanza key use and replay attack detection This commit fixes a bug where the same encryption key used for two stanzas in the access.conf file would result in access requests that matched the second stanza to always be treated as a replay attack. This has been fixed for the fwknop-2.0.1 release, and was reported by Andy Rowland. Now the fwknopd server computes the SHA256 digest of raw incoming payload data before decryption, and compares this against all previous hashes. Previous to this commit, fwknopd would add a new hash to the replay digest list right after the first access.conf stanza match, so when SPA packet data matched the second access.conf stanza a matching replay digest would already be there. --- CREDITS | 6 ++ client/config_init.c | 2 +- lib/fko.h | 4 + lib/fko_context.h | 6 ++ lib/fko_digest.c | 185 ++++++++++++++++++++++++++++-------------- lib/fko_funcs.c | 3 + server/incoming_spa.c | 50 ++++++++---- server/replay_cache.c | 100 +++++++++++++++++++---- server/replay_cache.h | 6 +- 9 files changed, 268 insertions(+), 94 deletions(-) diff --git a/CREDITS b/CREDITS index f912289e..1d3c17a1 100644 --- a/CREDITS +++ b/CREDITS @@ -18,3 +18,9 @@ Max Kastanas Ted Wynnychenko - Helped test fwknop PF support on OpenBSD. + +Andy Rowland + - Reported a bug where the same encryption key used for two stanzas in the + access.conf file would result in access requests that matched the second + stanza to always be treated as a replay attack. This has been fixed for + the fwknop-2.0.1 release. diff --git a/client/config_init.c b/client/config_init.c index f0cd41b7..19ec13ec 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -33,7 +33,7 @@ #include "cmd_opts.h" #include "utils.h" -/* Convert a digest_type string to its intger value. +/* Convert a digest_type string to its integer value. */ static int digest_strtoint(const char *dt_str) diff --git a/lib/fko.h b/lib/fko.h index 9dadcc3c..1f5af83e 100644 --- a/lib/fko.h +++ b/lib/fko.h @@ -206,6 +206,8 @@ DLL_API int fko_set_spa_server_auth(fko_ctx_t ctx, const char *server_auth); DLL_API int fko_set_spa_client_timeout(fko_ctx_t ctx, const int timeout); DLL_API int fko_set_spa_digest_type(fko_ctx_t ctx, const short digest_type); DLL_API int fko_set_spa_digest(fko_ctx_t ctx); +DLL_API int fko_set_raw_spa_digest_type(fko_ctx_t ctx, const short raw_digest_type); +DLL_API int fko_set_raw_spa_digest(fko_ctx_t ctx); DLL_API int fko_set_spa_encryption_type(fko_ctx_t ctx, const short encrypt_type); DLL_API int fko_set_spa_data(fko_ctx_t ctx, const char *enc_msg); @@ -233,7 +235,9 @@ DLL_API int fko_get_spa_nat_access(fko_ctx_t ctx, char **nat_access); DLL_API int fko_get_spa_server_auth(fko_ctx_t ctx, char **server_auth); DLL_API int fko_get_spa_client_timeout(fko_ctx_t ctx, int *client_timeout); DLL_API int fko_get_spa_digest_type(fko_ctx_t ctx, short *spa_digest_type); +DLL_API int fko_get_raw_spa_digest_type(fko_ctx_t ctx, short *raw_spa_digest_type); DLL_API int fko_get_spa_digest(fko_ctx_t ctx, char **spa_digest); +DLL_API int fko_get_raw_spa_digest(fko_ctx_t ctx, char **raw_spa_digest); DLL_API int fko_get_spa_encryption_type(fko_ctx_t ctx, short *spa_enc_type); DLL_API int fko_get_spa_data(fko_ctx_t ctx, char **spa_data); diff --git a/lib/fko_context.h b/lib/fko_context.h index 8a98b557..a57a2dce 100644 --- a/lib/fko_context.h +++ b/lib/fko_context.h @@ -68,6 +68,12 @@ struct fko_context { char *version; char *digest; + /* Digest of raw encrypted/base64 data - this is used + * for replay attack detection + */ + char *raw_digest; + short raw_digest_type; + /* Computed processed data (encodings, etc.) */ char *encoded_msg; char *encrypted_msg; diff --git a/lib/fko_digest.c b/lib/fko_digest.c index 6eaf1265..f2e2577b 100644 --- a/lib/fko_digest.c +++ b/lib/fko_digest.c @@ -36,8 +36,9 @@ /* Set the SPA digest type. */ -int -fko_set_spa_digest_type(fko_ctx_t ctx, const short digest_type) +static int +set_spa_digest_type(fko_ctx_t ctx, + short *digest_type_field, const short digest_type) { /* Must be initialized */ @@ -47,13 +48,25 @@ fko_set_spa_digest_type(fko_ctx_t ctx, const short digest_type) if(digest_type < 1 || digest_type >= FKO_LAST_DIGEST_TYPE) return(FKO_ERROR_INVALID_DATA); - ctx->digest_type = digest_type; + *digest_type_field = digest_type; ctx->state |= FKO_DIGEST_TYPE_MODIFIED; return(FKO_SUCCESS); } +int +fko_set_spa_digest_type(fko_ctx_t ctx, const short digest_type) +{ + return set_spa_digest_type(ctx, &ctx->digest_type, digest_type); +} + +int +fko_set_raw_spa_digest_type(fko_ctx_t ctx, const short raw_digest_type) +{ + return set_spa_digest_type(ctx, &ctx->raw_digest_type, raw_digest_type); +} + /* Return the SPA digest type. */ int @@ -69,11 +82,91 @@ fko_get_spa_digest_type(fko_ctx_t ctx, short *digest_type) return(FKO_SUCCESS); } +/* Return the SPA digest type. +*/ int -fko_set_spa_digest(fko_ctx_t ctx) +fko_get_raw_spa_digest_type(fko_ctx_t ctx, short *raw_digest_type) +{ + /* Must be initialized + */ + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); + + *raw_digest_type = ctx->raw_digest_type; + + return(FKO_SUCCESS); +} + +static int +set_digest(char *data, char **digest, short digest_type) { char *md = NULL; + switch(digest_type) + { + case FKO_DIGEST_MD5: + md = malloc(MD_HEX_SIZE(MD5_DIGEST_LENGTH)+1); + if(md == NULL) + return(FKO_ERROR_MEMORY_ALLOCATION); + + md5_base64(md, + (unsigned char*)data, strlen(data)); + break; + + case FKO_DIGEST_SHA1: + md = malloc(MD_HEX_SIZE(SHA1_DIGEST_LENGTH)+1); + if(md == NULL) + return(FKO_ERROR_MEMORY_ALLOCATION); + + sha1_base64(md, + (unsigned char*)data, strlen(data)); + break; + + case FKO_DIGEST_SHA256: + md = malloc(MD_HEX_SIZE(SHA256_DIGEST_LENGTH)+1); + if(md == NULL) + return(FKO_ERROR_MEMORY_ALLOCATION); + + sha256_base64(md, + (unsigned char*)data, strlen(data)); + break; + + case FKO_DIGEST_SHA384: + md = malloc(MD_HEX_SIZE(SHA384_DIGEST_LENGTH)+1); + if(md == NULL) + return(FKO_ERROR_MEMORY_ALLOCATION); + + sha384_base64(md, + (unsigned char*)data, strlen(data)); + break; + + case FKO_DIGEST_SHA512: + md = malloc(MD_HEX_SIZE(SHA512_DIGEST_LENGTH)+1); + if(md == NULL) + return(FKO_ERROR_MEMORY_ALLOCATION); + + sha512_base64(md, + (unsigned char*)data, strlen(data)); + break; + + default: + return(FKO_ERROR_INVALID_DIGEST_TYPE); + } + + /* Just in case this is a subsquent call to this function. We + * do not want to be leaking memory. + */ + if(*digest != NULL) + free(*digest); + + *digest = md; + + return(FKO_SUCCESS); +} + +int +fko_set_spa_digest(fko_ctx_t ctx) +{ /* Must be initialized */ if(!CTX_INITIALIZED(ctx)) @@ -84,66 +177,25 @@ fko_set_spa_digest(fko_ctx_t ctx) if(ctx->encoded_msg == NULL) return(FKO_ERROR_MISSING_ENCODED_DATA); - switch(ctx->digest_type) - { - case FKO_DIGEST_MD5: - md = malloc(MD_HEX_SIZE(MD5_DIGEST_LENGTH)+1); - if(md == NULL) - return(FKO_ERROR_MEMORY_ALLOCATION); + return set_digest(ctx->encoded_msg, + &ctx->digest, ctx->digest_type); +} - md5_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); - break; - - case FKO_DIGEST_SHA1: - md = malloc(MD_HEX_SIZE(SHA1_DIGEST_LENGTH)+1); - if(md == NULL) - return(FKO_ERROR_MEMORY_ALLOCATION); - - sha1_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); - break; - - case FKO_DIGEST_SHA256: - md = malloc(MD_HEX_SIZE(SHA256_DIGEST_LENGTH)+1); - if(md == NULL) - return(FKO_ERROR_MEMORY_ALLOCATION); - - sha256_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); - break; - - case FKO_DIGEST_SHA384: - md = malloc(MD_HEX_SIZE(SHA384_DIGEST_LENGTH)+1); - if(md == NULL) - return(FKO_ERROR_MEMORY_ALLOCATION); - - sha384_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); - break; - - case FKO_DIGEST_SHA512: - md = malloc(MD_HEX_SIZE(SHA512_DIGEST_LENGTH)+1); - if(md == NULL) - return(FKO_ERROR_MEMORY_ALLOCATION); - - sha512_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); - break; - - default: - return(FKO_ERROR_INVALID_DIGEST_TYPE); - } - - /* Just in case this is a subsquent call to this function. We - * do not want to be leaking memory. +int +fko_set_raw_spa_digest(fko_ctx_t ctx) +{ + /* Must be initialized */ - if(ctx->digest != NULL) - free(ctx->digest); + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); - ctx->digest = md; + /* Must have encoded message data to start with. + */ + if(ctx->encrypted_msg == NULL) + return(FKO_ERROR_MISSING_ENCODED_DATA); - return(FKO_SUCCESS); + return set_digest(ctx->encrypted_msg, + &ctx->raw_digest, ctx->raw_digest_type); } int @@ -159,4 +211,17 @@ fko_get_spa_digest(fko_ctx_t ctx, char **md) return(FKO_SUCCESS); } +int +fko_get_raw_spa_digest(fko_ctx_t ctx, char **md) +{ + /* Must be initialized + */ + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); + + *md = ctx->raw_digest; + + return(FKO_SUCCESS); +} + /***EOF***/ diff --git a/lib/fko_funcs.c b/lib/fko_funcs.c index 4a74438d..5ea4f644 100644 --- a/lib/fko_funcs.c +++ b/lib/fko_funcs.c @@ -238,6 +238,9 @@ fko_destroy(fko_ctx_t ctx) if(ctx->digest != NULL) free(ctx->digest); + if(ctx->raw_digest != NULL) + free(ctx->raw_digest); + if(ctx->encoded_msg != NULL) free(ctx->encoded_msg); diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 27e17f09..024efe33 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -152,6 +152,22 @@ get_spa_data_fields(fko_ctx_t ctx, spa_data_t *spdat) return(res); } +/* Check for access.conf stanza SOURCE match based on SPA packet + * source IP +*/ +static int +is_src_match(acc_stanza_t *acc, const uint32_t ip) +{ + while (acc) + { + if(compare_addr_list(acc->source_list, ip)) + return 1; + + acc = acc->next; + } + return 0; +} + /* Process the SPA packet data */ void @@ -192,6 +208,26 @@ incoming_spa(fko_srv_options_t *opts) return; } + if (is_src_match(opts->acc_stanzas, ntohl(spa_pkt->packet_src_ip))) + { + if(strncasecmp(opts->config[CONF_ENABLE_DIGEST_PERSISTENCE], "Y", 1) == 0) + /* Check for a replay attack + */ + if (is_replay(opts, spa_pkt->packet_data) != SPA_MSG_SUCCESS) + return; + } + else + { + log_msg(LOG_WARNING, + "No access data found for source IP: %s", spadat.pkt_source_ip + ); + return; + } + + /* Now that we know there is a matching access.conf stanza and the + * incoming SPA packet is not a replay, see if we should grant any + * access + */ while(acc) { stanza_num++; @@ -387,20 +423,6 @@ incoming_spa(fko_srv_options_t *opts) } } - /* Check for replays if so configured. - */ - if(strncasecmp(opts->config[CONF_ENABLE_DIGEST_PERSISTENCE], "Y", 1) == 0) - { - res = replay_check(opts, ctx); - if(res != 0) /* non-zero means we have seen this packet before. */ - { - if(ctx != NULL) - fko_destroy(ctx); - acc = acc->next; - continue; - } - } - /* Populate our spa data struct for future reference. */ res = get_spa_data_fields(ctx, &spadat); diff --git a/server/replay_cache.c b/server/replay_cache.c index 77566af8..fdc99e53 100644 --- a/server/replay_cache.c +++ b/server/replay_cache.c @@ -77,6 +77,61 @@ #define DATE_LEN 18 #define MAX_DIGEST_SIZE 64 +static int +get_raw_digest(char **digest, char *pkt_data) +{ + fko_ctx_t ctx = NULL; + char *tmp_digest = NULL; + int res = FKO_SUCCESS; + + /* initialize an FKO context with no decryption key just so + * we can get the outer message digest + */ + res = fko_new_with_data(&ctx, (char *)pkt_data, NULL); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error initializing FKO context from SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_FKO_CTX_ERROR); + } + + res = fko_set_raw_spa_digest_type(ctx, FKO_DEFAULT_DIGEST); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error setting digest type for SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + res = fko_set_raw_spa_digest(ctx); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error setting digest for SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + res = fko_get_raw_spa_digest(ctx, &tmp_digest); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + *digest = strdup(tmp_digest); + + if (digest == NULL) + return SPA_MSG_ERROR; + + fko_destroy(ctx); + return res; +} + /* Rotate the digest file by simply renaming it. */ static void @@ -424,23 +479,23 @@ replay_db_cache_init(fko_srv_options_t *opts) * 0 for no match, and -1 on error. */ int -replay_check(fko_srv_options_t *opts, fko_ctx_t ctx) +is_replay(fko_srv_options_t *opts, unsigned char *pkt_data) { #ifdef NO_DIGEST_CACHE return(-1); #else #if USE_FILE_CACHE - return replay_check_file_cache(opts, ctx); + return is_replay_file_cache(opts, pkt_data); #else - return replay_check_dbm_cache(opts, ctx); + return is_replay_dbm_cache(opts, pkt_data); #endif #endif /* NO_DIGEST_CACHE */ } #if USE_FILE_CACHE int -replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) +is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) { char *digest = NULL; char src_ip[INET_ADDRSTRLEN+1] = {0}; @@ -450,15 +505,18 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) struct digest_cache_list *digest_list_ptr = NULL, *digest_elm = NULL; - res = fko_get_spa_digest(ctx, &digest); + res = get_raw_digest(&digest, (char *)pkt_data); + if(res != FKO_SUCCESS) { - log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", - fko_errstr(res)); - - return(SPA_MSG_DIGEST_ERROR); + if (digest != NULL) + free(digest); + return res; } + if (digest == NULL) + return SPA_MSG_ERROR; + digest_len = strlen(digest); /* Check the cache for the SPA packet digest @@ -471,6 +529,7 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) replay_warning(opts, &(digest_list_ptr->cache_info)); + free(digest); return(SPA_MSG_REPLAY); } } @@ -484,6 +543,7 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) log_msg(LOG_WARNING, "Error calloc() returned NULL for digest cache element", fko_errstr(SPA_MSG_ERROR)); + free(digest); return(SPA_MSG_ERROR); } if ((digest_elm->cache_info.digest = calloc(1, digest_len+1)) == NULL) @@ -491,6 +551,7 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) log_msg(LOG_WARNING, "Error calloc() returned NULL for digest cache string", fko_errstr(SPA_MSG_ERROR)); free(digest_elm); + free(digest); return(SPA_MSG_ERROR); } @@ -513,6 +574,7 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) { log_msg(LOG_WARNING, "Could not open digest cache: %s", opts->config[CONF_DIGEST_FILE]); + free(digest); return(SPA_MSG_DIGEST_CACHE_ERROR); } @@ -531,13 +593,14 @@ replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx) fclose(digest_file_ptr); + free(digest); return(SPA_MSG_SUCCESS); } #endif /* USE_FILE_CACHE */ #if !USE_FILE_CACHE int -replay_check_dbm_cache(fko_srv_options_t *opts, fko_ctx_t ctx) +is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) { #ifdef NO_DIGEST_CACHE return 0; @@ -550,20 +613,23 @@ replay_check_dbm_cache(fko_srv_options_t *opts, fko_ctx_t ctx) #endif datum db_key, db_ent; - char *digest; + char *digest = NULL; int digest_len, res; digest_cache_info_t dc_info; - res = fko_get_spa_digest(ctx, &digest); + res = get_raw_digest(&digest, (char *)pkt_data); + if(res != FKO_SUCCESS) { - log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", - fko_errstr(res)); - - return(SPA_MSG_DIGEST_ERROR); + if (digest != NULL) + free(digest); + return res; } + if (digest == NULL) + return SPA_MSG_ERROR; + digest_len = strlen(digest); db_key.dptr = digest; @@ -586,6 +652,7 @@ replay_check_dbm_cache(fko_srv_options_t *opts, fko_ctx_t ctx) MY_DBM_STRERROR(errno) ); + free(digest); return(SPA_MSG_DIGEST_CACHE_ERROR); } @@ -639,6 +706,7 @@ replay_check_dbm_cache(fko_srv_options_t *opts, fko_ctx_t ctx) MY_DBM_CLOSE(rpdb); + free(digest); return(res); #endif /* NO_DIGEST_CACHE */ } diff --git a/server/replay_cache.h b/server/replay_cache.h index 04ee11a2..8d785b7a 100644 --- a/server/replay_cache.h +++ b/server/replay_cache.h @@ -59,14 +59,14 @@ struct digest_cache_list { /* Prototypes */ int replay_cache_init(fko_srv_options_t *opts); -int replay_check(fko_srv_options_t *opts, fko_ctx_t ctx); +int is_replay(fko_srv_options_t *opts, unsigned char *pkt_data); #ifdef USE_FILE_CACHE int replay_file_cache_init(fko_srv_options_t *opts); -int replay_check_file_cache(fko_srv_options_t *opts, fko_ctx_t ctx); +int is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data); void free_replay_list(fko_srv_options_t *opts); #else int replay_db_cache_init(fko_srv_options_t *opts); -int replay_check_dbm_cache(fko_srv_options_t *opts, fko_ctx_t ctx); +int is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data); #endif #endif /* REPLAY_CACHE_H */ From 6b3e5ef3c235e4c4721ca0d6b5f9861489cc3e5c Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 8 Jul 2012 08:35:50 -0400 Subject: [PATCH 2/5] Added a test for a dual-usage key in access.conf --- test/conf/dual_key_usage_access.conf | 9 +++++++++ test/test-fwknop.pl | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 test/conf/dual_key_usage_access.conf diff --git a/test/conf/dual_key_usage_access.conf b/test/conf/dual_key_usage_access.conf new file mode 100644 index 00000000..0cc0d8ec --- /dev/null +++ b/test/conf/dual_key_usage_access.conf @@ -0,0 +1,9 @@ +SOURCE: ANY; +KEY: fwknoptest; +OPEN_PORTS: tcp/22; +FW_ACCESS_TIMEOUT: 2; + +SOURCE: ANY; +KEY: fwknoptest; +OPEN_PORTS: tcp/80; +FW_ACCESS_TIMEOUT: 3; diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index ab3c4c93..a34d5b35 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -27,6 +27,7 @@ my $future_expired_access_conf = "$conf_dir/future_expired_stanza_access.conf"; my $expired_epoch_access_conf = "$conf_dir/expired_epoch_stanza_access.conf"; my $invalid_expire_access_conf = "$conf_dir/invalid_expire_access.conf"; my $force_nat_access_conf = "$conf_dir/force_nat_access.conf"; +my $dual_key_usage_access_conf = "$conf_dir/dual_key_usage_access.conf"; my $gpg_access_conf = "$conf_dir/gpg_access.conf"; my $default_digest_file = "$run_dir/digest.cache"; my $default_pid_file = "$run_dir/fwknopd.pid"; @@ -590,6 +591,25 @@ my @tests = ( 'fw_rule_removed' => $NEW_RULE_REMOVED, 'fatal' => $NO }, + { + 'category' => 'Rijndael SPA', + 'subcategory' => 'client+server', + 'detail' => 'dual usage access key (tcp/80 http)', + 'err_msg' => 'could not complete SPA cycle', + 'function' => \&spa_cycle, + 'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . + "$fwknopCmd -A tcp/80 -a $fake_ip -D $loopback_ip --get-key " . + "$local_key_file --verbose --verbose", + 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . + "$fwknopdCmd -c $default_conf -a $dual_key_usage_access_conf " . + "-d $default_digest_file -p $default_pid_file $intf_str", + ### check for the first stanza that does not allow tcp/80 - the + ### second stanza allows this + 'server_positive_output_matches' => [qr/stanza #1\)\sOne\sor\smore\srequested\sprotocol\/ports\swas\sdenied/], + 'fw_rule_created' => $NEW_RULE_REQUIRED, + 'fw_rule_removed' => $NEW_RULE_REMOVED, + 'fatal' => $NO + }, { 'category' => 'Rijndael SPA', 'subcategory' => 'client+server', From be4193d734850fe60f14a26b547525ea0b9ce1e9 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 8 Jul 2012 08:36:30 -0400 Subject: [PATCH 3/5] Only cache replay digests for SPA packets that decrypt This change ensures that we only cache replay digests for those SPA packets that actually decrypt. Not doing this would have allowed an attacker to potentially fill up digest cache space with digests for garbage packets. --- server/incoming_spa.c | 104 ++++++++++++++++++--- server/replay_cache.c | 206 ++++++++++++++++++++---------------------- server/replay_cache.h | 9 +- 3 files changed, 197 insertions(+), 122 deletions(-) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 024efe33..2c33192a 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -110,6 +110,64 @@ preprocess_spa_data(fko_srv_options_t *opts, const char *src_ip) return(FKO_SUCCESS); } +/* For replay attack detection +*/ +static int +get_raw_digest(char **digest, char *pkt_data) +{ + fko_ctx_t ctx = NULL; + char *tmp_digest = NULL; + int res = FKO_SUCCESS; + + /* initialize an FKO context with no decryption key just so + * we can get the outer message digest + */ + res = fko_new_with_data(&ctx, (char *)pkt_data, NULL); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error initializing FKO context from SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_FKO_CTX_ERROR); + } + + res = fko_set_raw_spa_digest_type(ctx, FKO_DEFAULT_DIGEST); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error setting digest type for SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + res = fko_set_raw_spa_digest(ctx); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error setting digest for SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + res = fko_get_raw_spa_digest(ctx, &tmp_digest); + if(res != FKO_SUCCESS) + { + log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", + fko_errstr(res)); + fko_destroy(ctx); + return(SPA_MSG_DIGEST_ERROR); + } + + *digest = strdup(tmp_digest); + + if (digest == NULL) + return SPA_MSG_ERROR; + + fko_destroy(ctx); + return res; +} + + /* Popluate a spa_data struct from an initialized (and populated) FKO context. */ static int @@ -178,9 +236,10 @@ incoming_spa(fko_srv_options_t *opts) */ fko_ctx_t ctx = NULL; - char *spa_ip_demark, *gpg_id; + char *spa_ip_demark, *gpg_id, *raw_digest = NULL; time_t now_ts; - int res, status, ts_diff, enc_type, found_acc_sip=0, stanza_num=0; + int res, status, ts_diff, enc_type, stanza_num=0; + int added_replay_digest = 0; spa_pkt_info_t *spa_pkt = &(opts->spa_pkt); @@ -213,7 +272,17 @@ incoming_spa(fko_srv_options_t *opts) if(strncasecmp(opts->config[CONF_ENABLE_DIGEST_PERSISTENCE], "Y", 1) == 0) /* Check for a replay attack */ - if (is_replay(opts, spa_pkt->packet_data) != SPA_MSG_SUCCESS) + res = get_raw_digest(&raw_digest, (char *)spa_pkt->packet_data); + if(res != FKO_SUCCESS) + { + if (raw_digest != NULL) + free(raw_digest); + return; + } + if (raw_digest == NULL) + return; + + if (is_replay(opts, raw_digest) != SPA_MSG_SUCCESS) return; } else @@ -240,8 +309,6 @@ incoming_spa(fko_srv_options_t *opts) continue; } - found_acc_sip = 1; - log_msg(LOG_INFO, "(stanza #%d) SPA Packet from IP: %s received with access source match", stanza_num, spadat.pkt_source_ip); @@ -366,7 +433,7 @@ incoming_spa(fko_srv_options_t *opts) continue; } - /* Do we have a valid FKO context? + /* Do we have a valid FKO context? Did the SPA decrypt properly? */ if(res != FKO_SUCCESS) { @@ -383,6 +450,23 @@ incoming_spa(fko_srv_options_t *opts) continue; } + /* Add this SPA packet into the replay detection cache + */ + if (! added_replay_digest) + { + res = add_replay(opts, raw_digest); + if (res != SPA_MSG_SUCCESS) + { + log_msg(LOG_WARNING, "(stanza #%d) Could not add digest to replay cache", + stanza_num); + if(ctx != NULL) + fko_destroy(ctx); + acc = acc->next; + continue; + } + added_replay_digest = 1; + } + /* At this point, we assume the SPA data is valid. Now we need to see * if it meets our access criteria. */ @@ -656,12 +740,8 @@ incoming_spa(fko_srv_options_t *opts) break; } - if(! found_acc_sip) - { - log_msg(LOG_WARNING, - "No access data found for source IP: %s", spadat.pkt_source_ip - ); - } + if (raw_digest != NULL) + free(raw_digest); return; } diff --git a/server/replay_cache.c b/server/replay_cache.c index fdc99e53..2b164e94 100644 --- a/server/replay_cache.c +++ b/server/replay_cache.c @@ -77,61 +77,6 @@ #define DATE_LEN 18 #define MAX_DIGEST_SIZE 64 -static int -get_raw_digest(char **digest, char *pkt_data) -{ - fko_ctx_t ctx = NULL; - char *tmp_digest = NULL; - int res = FKO_SUCCESS; - - /* initialize an FKO context with no decryption key just so - * we can get the outer message digest - */ - res = fko_new_with_data(&ctx, (char *)pkt_data, NULL); - if(res != FKO_SUCCESS) - { - log_msg(LOG_WARNING, "Error initializing FKO context from SPA data: %s", - fko_errstr(res)); - fko_destroy(ctx); - return(SPA_MSG_FKO_CTX_ERROR); - } - - res = fko_set_raw_spa_digest_type(ctx, FKO_DEFAULT_DIGEST); - if(res != FKO_SUCCESS) - { - log_msg(LOG_WARNING, "Error setting digest type for SPA data: %s", - fko_errstr(res)); - fko_destroy(ctx); - return(SPA_MSG_DIGEST_ERROR); - } - - res = fko_set_raw_spa_digest(ctx); - if(res != FKO_SUCCESS) - { - log_msg(LOG_WARNING, "Error setting digest for SPA data: %s", - fko_errstr(res)); - fko_destroy(ctx); - return(SPA_MSG_DIGEST_ERROR); - } - - res = fko_get_raw_spa_digest(ctx, &tmp_digest); - if(res != FKO_SUCCESS) - { - log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", - fko_errstr(res)); - fko_destroy(ctx); - return(SPA_MSG_DIGEST_ERROR); - } - - *digest = strdup(tmp_digest); - - if (digest == NULL) - return SPA_MSG_ERROR; - - fko_destroy(ctx); - return res; -} - /* Rotate the digest file by simply renaming it. */ static void @@ -475,47 +420,45 @@ replay_db_cache_init(fko_srv_options_t *opts) #endif /* USE_FILE_CACHE */ /* Take an fko context, pull the digest and use it as the key to check the - * replay db (digest cache). Returns 1 if there was a match (a replay), - * 0 for no match, and -1 on error. + * replay db (digest cache). */ int -is_replay(fko_srv_options_t *opts, unsigned char *pkt_data) +is_replay(fko_srv_options_t *opts, char *digest) { #ifdef NO_DIGEST_CACHE return(-1); #else #if USE_FILE_CACHE - return is_replay_file_cache(opts, pkt_data); + return is_replay_file_cache(opts, digest); #else - return is_replay_dbm_cache(opts, pkt_data); + return is_replay_dbm_cache(opts, digest); +#endif +#endif /* NO_DIGEST_CACHE */ +} + +int +add_replay(fko_srv_options_t *opts, char *digest) +{ +#ifdef NO_DIGEST_CACHE + return(-1); +#else + +#if USE_FILE_CACHE + return add_replay_file_cache(opts, digest); +#else + return add_replay_dbm_cache(opts, digest); #endif #endif /* NO_DIGEST_CACHE */ } #if USE_FILE_CACHE int -is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) +is_replay_file_cache(fko_srv_options_t *opts, char *digest) { - char *digest = NULL; - char src_ip[INET_ADDRSTRLEN+1] = {0}; - char dst_ip[INET_ADDRSTRLEN+1] = {0}; - int res = 0, digest_len = 0; - FILE *digest_file_ptr = NULL; + int digest_len = 0; - struct digest_cache_list *digest_list_ptr = NULL, *digest_elm = NULL; - - res = get_raw_digest(&digest, (char *)pkt_data); - - if(res != FKO_SUCCESS) - { - if (digest != NULL) - free(digest); - return res; - } - - if (digest == NULL) - return SPA_MSG_ERROR; + struct digest_cache_list *digest_list_ptr = NULL; digest_len = strlen(digest); @@ -529,21 +472,29 @@ is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) replay_warning(opts, &(digest_list_ptr->cache_info)); - free(digest); return(SPA_MSG_REPLAY); } } + return(SPA_MSG_SUCCESS); +} + +int +add_replay_file_cache(fko_srv_options_t *opts, char *digest) +{ + FILE *digest_file_ptr = NULL; + int digest_len = 0; + char src_ip[INET_ADDRSTRLEN+1] = {0}; + char dst_ip[INET_ADDRSTRLEN+1] = {0}; + + struct digest_cache_list *digest_elm = NULL; + + digest_len = strlen(digest); - /* If we make it here, then this is a new SPA packet that needs to be - * added to the cache. We've already decrypted the data, so we know that - * the contents are valid. - */ if ((digest_elm = calloc(1, sizeof(struct digest_cache_list))) == NULL) { log_msg(LOG_WARNING, "Error calloc() returned NULL for digest cache element", fko_errstr(SPA_MSG_ERROR)); - free(digest); return(SPA_MSG_ERROR); } if ((digest_elm->cache_info.digest = calloc(1, digest_len+1)) == NULL) @@ -551,7 +502,6 @@ is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) log_msg(LOG_WARNING, "Error calloc() returned NULL for digest cache string", fko_errstr(SPA_MSG_ERROR)); free(digest_elm); - free(digest); return(SPA_MSG_ERROR); } @@ -574,7 +524,6 @@ is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) { log_msg(LOG_WARNING, "Could not open digest cache: %s", opts->config[CONF_DIGEST_FILE]); - free(digest); return(SPA_MSG_DIGEST_CACHE_ERROR); } @@ -593,14 +542,13 @@ is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data) fclose(digest_file_ptr); - free(digest); return(SPA_MSG_SUCCESS); } #endif /* USE_FILE_CACHE */ #if !USE_FILE_CACHE int -is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) +is_replay_dbm_cache(fko_srv_options_t *opts, char *digest) { #ifdef NO_DIGEST_CACHE return 0; @@ -614,22 +562,10 @@ is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) datum db_key, db_ent; char *digest = NULL; - int digest_len, res; + int digest_len, res = SPA_MSG_SUCCESS; digest_cache_info_t dc_info; - res = get_raw_digest(&digest, (char *)pkt_data); - - if(res != FKO_SUCCESS) - { - if (digest != NULL) - free(digest); - return res; - } - - if (digest == NULL) - return SPA_MSG_ERROR; - digest_len = strlen(digest); db_key.dptr = digest; @@ -652,7 +588,6 @@ is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) MY_DBM_STRERROR(errno) ); - free(digest); return(SPA_MSG_DIGEST_CACHE_ERROR); } @@ -676,9 +611,65 @@ is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) #ifdef HAVE_LIBGDBM free(db_ent.dptr); #endif - res = SPA_MSG_REPLAY; - } else { + } + + MY_DBM_CLOSE(rpdb); + + return(res); +#endif /* NO_DIGEST_CACHE */ +} + +int +add_replay_dbm_cache(fko_srv_options_t *opts, char *digest) +{ +#ifdef NO_DIGEST_CACHE + return 0; +#else + +#ifdef HAVE_LIBGDBM + GDBM_FILE rpdb; +#elif HAVE_LIBNDBM + DBM *rpdb; +#endif + datum db_key, db_ent; + + char *digest = NULL; + int digest_len, res = SPA_MSG_SUCCESS; + + digest_cache_info_t dc_info; + + digest_len = strlen(digest); + + db_key.dptr = digest; + db_key.dsize = digest_len; + + /* Check the db for the key + */ +#ifdef HAVE_LIBGDBM + rpdb = gdbm_open( + opts->config[CONF_DIGEST_DB_FILE], 512, GDBM_WRCREAT, S_IRUSR|S_IWUSR, 0 + ); +#elif HAVE_LIBNDBM + rpdb = dbm_open(opts->config[CONF_DIGEST_DB_FILE], O_RDWR, 0); +#endif + + if(!rpdb) + { + log_msg(LOG_WARNING, "Error opening digest_cache: '%s': %s", + opts->config[CONF_DIGEST_DB_FILE], + MY_DBM_STRERROR(errno) + ); + + return(SPA_MSG_DIGEST_CACHE_ERROR); + } + + db_ent = MY_DBM_FETCH(rpdb, db_key); + + /* If the datum is null, we have a new entry. + */ + if(db_ent.dptr == NULL) + { /* This is a new SPA packet that needs to be added to the cache. */ dc_info.src_ip = opts->spa_pkt.packet_src_ip; @@ -703,13 +694,14 @@ is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data) res = SPA_MSG_SUCCESS; } + else + res = SPA_MSG_DIGEST_CACHE_ERROR; MY_DBM_CLOSE(rpdb); - free(digest); return(res); #endif /* NO_DIGEST_CACHE */ -} + #endif /* USE_FILE_CACHE */ #if USE_FILE_CACHE diff --git a/server/replay_cache.h b/server/replay_cache.h index 8d785b7a..d523fa10 100644 --- a/server/replay_cache.h +++ b/server/replay_cache.h @@ -59,14 +59,17 @@ struct digest_cache_list { /* Prototypes */ int replay_cache_init(fko_srv_options_t *opts); -int is_replay(fko_srv_options_t *opts, unsigned char *pkt_data); +int is_replay(fko_srv_options_t *opts, char *digest); +int add_replay(fko_srv_options_t *opts, char *digest); #ifdef USE_FILE_CACHE int replay_file_cache_init(fko_srv_options_t *opts); -int is_replay_file_cache(fko_srv_options_t *opts, unsigned char *pkt_data); +int is_replay_file_cache(fko_srv_options_t *opts, char *digest); +int add_replay_file_cache(fko_srv_options_t *opts, char *digest); void free_replay_list(fko_srv_options_t *opts); #else int replay_db_cache_init(fko_srv_options_t *opts); -int is_replay_dbm_cache(fko_srv_options_t *opts, unsigned char *pkt_data); +int is_replay_dbm_cache(fko_srv_options_t *opts, char *digest); +int add_replay_dbm_cache(fko_srv_options_t *opts, char *digest); #endif #endif /* REPLAY_CACHE_H */ From 9497044f24831cb39f61768d9eb900eeeb6976dd Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 8 Jul 2012 15:30:35 -0400 Subject: [PATCH 4/5] added new test in --enable-valgrind mode to collect suspect functions --- test/test-fwknop.pl | 127 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 109 insertions(+), 18 deletions(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index a34d5b35..76ac5854 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -71,7 +71,10 @@ my $test_include = ''; my @tests_to_include = (); my $test_exclude = ''; my @tests_to_exclude = (); +my %valgrind_suspect_functions = (); my $list_mode = 0; +my $diff_dir1 = ''; +my $diff_dir2 = ''; my $loopback_intf = ''; my $anonymize_results = 0; my $current_test_file = "$output_dir/init"; @@ -115,7 +118,10 @@ exit 1 unless GetOptions( 'List-mode' => \$list_mode, 'enable-valgrind' => \$use_valgrind, 'valgrind-path=s' => \$valgrindCmd, + 'output-dir=s' => \$output_dir, 'diff' => \$diff_mode, + 'diff-dir1=s' => \$diff_dir1, + 'diff-dir2=s' => \$diff_dir2, 'help' => \$help ); @@ -1239,7 +1245,6 @@ my @tests = ( 'fwknopd_cmdline' => $default_server_gpg_args, 'fatal' => $NO }, - { 'category' => 'GnuPG (GPG) SPA', 'subcategory' => 'server', @@ -1250,6 +1255,18 @@ my @tests = ( }, ); +if ($use_valgrind) { + push @tests, + { + 'category' => 'valgrind output', + 'subcategory' => 'suspect functions', + 'detail' => '', + 'err_msg' => 'could not parse suspect functions', + 'function' => \&parse_valgrind_suspect_functions, + 'fatal' => $NO + }; +} + my %test_keys = ( 'category' => $REQUIRED, 'subcategory' => $OPTIONAL, @@ -1345,7 +1362,8 @@ sub process_include_exclude() { if (@tests_to_include) { my $found = 0; for my $test (@tests_to_include) { - if ($msg =~ /$test/) { + if ($msg =~ /$test/ or ($use_valgrind + and $msg =~ /valgrind\soutput/)) { $found = 1; last; } @@ -1366,17 +1384,21 @@ sub process_include_exclude() { } sub diff_test_results() { + + $diff_dir1 = "${output_dir}.last" unless $diff_dir2; + $diff_dir2 = $output_dir unless $diff_dir1; + die "[*] Need results from a previous run before running --diff" - unless -d "${output_dir}.last"; - die "[*] Current results set does not exist." unless -d $output_dir; + unless -d $diff_dir2; + die "[*] Current results set does not exist." unless -d $diff_dir1; my %current_tests = (); my %previous_tests = (); ### Only diff results for matching tests (parse the logfile to see which ### test numbers match across the two test cycles). - &build_results_hash(\%current_tests, $output_dir); - &build_results_hash(\%previous_tests, "${output_dir}.last"); + &build_results_hash(\%current_tests, $diff_dir1); + &build_results_hash(\%previous_tests, $diff_dir2); for my $test_msg (sort {$current_tests{$a}{'num'} <=> $current_tests{$b}{'num'}} keys %current_tests) { @@ -1407,25 +1429,25 @@ sub diff_results() { ### remove CMD timestamps my $cmd_search_re = qr/^\S+\s.*?\s\d{4}\sCMD\:/; - for my $file ("${output_dir}.last/${previous_num}.test", - "${output_dir}.last/${previous_num}_fwknopd.test", - "${output_dir}/${current_num}.test", - "${output_dir}/${current_num}_fwknopd.test", + for my $file ("$diff_dir1/${previous_num}.test", + "$diff_dir1/${previous_num}_fwknopd.test", + "$diff_dir2/${current_num}.test", + "$diff_dir2/${current_num}_fwknopd.test", ) { system qq{perl -p -i -e 's|$valgrind_search_re||' $file} if -e $file; system qq{perl -p -i -e 's|$cmd_search_re|CMD:|' $file} if -e $file; } - if (-e "${output_dir}.last/${previous_num}.test" - and -e "${output_dir}/${current_num}.test") { - system "diff -u ${output_dir}.last/${previous_num}.test " . - "${output_dir}/${current_num}.test"; + if (-e "$diff_dir1/${previous_num}.test" + and -e "$diff_dir2/${current_num}.test") { + system "diff -u $diff_dir1/${previous_num}.test " . + "$diff_dir2/${current_num}.test"; } - if (-e "${output_dir}.last/${previous_num}_fwknopd.test" - and -e "${output_dir}/${current_num}_fwknopd.test") { - system "diff -u ${output_dir}.last/${previous_num}_fwknopd.test " . - "${output_dir}/${current_num}_fwknopd.test"; + if (-e "$diff_dir1/${previous_num}_fwknopd.test" + and -e "$diff_dir2/${current_num}_fwknopd.test") { + system "diff -u $diff_dir1/${previous_num}_fwknopd.test " . + "$diff_dir2/${current_num}_fwknopd.test"; } return; @@ -2592,6 +2614,34 @@ sub identify_loopback_intf() { return; } +sub parse_valgrind_suspect_functions() { + for my $file (glob("$output_dir/*.test")) { + my $type = 'server'; + $type = 'client' if $file =~ /\d\.test/; + open F, "< $file" or die $!; + while () { + ### ==30969== by 0x4E3983A: fko_set_username (fko_user.c:65) + if (/^==.*\sby\s\S+\:\s(.*)/) { + $valgrind_suspect_functions{$type}{$1}++; + } + } + close F; + } + + open F, ">> $current_test_file" or die $!; + for my $type ('client', 'server') { + print F "[+] fwknop $type functions:\n"; + next unless defined $valgrind_suspect_functions{$type}; + for my $fcn (sort {$valgrind_suspect_functions{$type}{$b} + <=> $valgrind_suspect_functions{$type}{$a}} keys %{$valgrind_suspect_functions{$type}}) { + printf F " %5d : %s\n", $valgrind_suspect_functions{$type}{$fcn}, $fcn; + } + print F "\n"; + } + close F; + return 1; +} + sub is_fw_rule_active() { my $test_hr = shift; @@ -2707,3 +2757,44 @@ sub logr() { close F; return; } + +sub usage() { + print <<_HELP_; + +[+] $0 + + -A --Anonymize-results - Prepare anonymized results at: + $tarfile + --diff - Compare the results of one test run to + another. By default this compares output + in ${output_dir}.last to $output_dir + --diff-dir1= - Left hand side of diff directory path, + default is: ${output_dir}.last + --diff-dir2= - Right hand side of diff directory path, + default is: $output_dir + --include= - Specify a regex to be used over test + names that must match. + --exclude= - Specify a regex to be used over test + names that must not match. + --enable-recompile - Recompile fwknop sources and look for + compilation warnings. + --enable-valgrind - Run every test underneath valgrind. + --List - List test names. + --loopback-intf= - Specify loopback interface name (default + depends on the OS where the test suite + is executed). + --output-dir= - Path to output directory, default is: + $output_dir + --fwknop-path= - Path to fwknop binary, default is: + $fwknopCmd + --fwknopd-path= - Path to fwknopd binary, default is: + $fwknopdCmd + --libfko-path= - Path to libfko, default is: + $libfko_bin + --valgrind-path= - Path to valgrind, default is: + $valgrindCmd + -h --help - Display usage on STDOUT and exit. + +_HELP_ + exit 0; +} From bc2e41fd472b7709897b89264853e2941de74652 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 8 Jul 2012 21:21:36 -0400 Subject: [PATCH 5/5] added unique function names to --enable-valgrind suspect functions test --- test/test-fwknop.pl | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 76ac5854..30d1af8d 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -71,7 +71,8 @@ my $test_include = ''; my @tests_to_include = (); my $test_exclude = ''; my @tests_to_exclude = (); -my %valgrind_suspect_functions = (); +my %valgrind_flagged_fcns = (); +my %valgrind_flagged_fcns_unique = (); my $list_mode = 0; my $diff_dir1 = ''; my $diff_dir2 = ''; @@ -1259,10 +1260,10 @@ if ($use_valgrind) { push @tests, { 'category' => 'valgrind output', - 'subcategory' => 'suspect functions', + 'subcategory' => 'flagged functions', 'detail' => '', - 'err_msg' => 'could not parse suspect functions', - 'function' => \&parse_valgrind_suspect_functions, + 'err_msg' => 'could not parse flagged functions', + 'function' => \&parse_valgrind_flagged_functions, 'fatal' => $NO }; } @@ -2614,15 +2615,16 @@ sub identify_loopback_intf() { return; } -sub parse_valgrind_suspect_functions() { +sub parse_valgrind_flagged_functions() { for my $file (glob("$output_dir/*.test")) { my $type = 'server'; $type = 'client' if $file =~ /\d\.test/; open F, "< $file" or die $!; while () { ### ==30969== by 0x4E3983A: fko_set_username (fko_user.c:65) - if (/^==.*\sby\s\S+\:\s(.*)/) { - $valgrind_suspect_functions{$type}{$1}++; + if (/^==.*\sby\s\S+\:\s(\S+)\s(.*)/) { + $valgrind_flagged_fcns{$type}{"$1 $2"}++; + $valgrind_flagged_fcns_unique{$type}{$1}++; } } close F; @@ -2630,13 +2632,20 @@ sub parse_valgrind_suspect_functions() { open F, ">> $current_test_file" or die $!; for my $type ('client', 'server') { - print F "[+] fwknop $type functions:\n"; - next unless defined $valgrind_suspect_functions{$type}; - for my $fcn (sort {$valgrind_suspect_functions{$type}{$b} - <=> $valgrind_suspect_functions{$type}{$a}} keys %{$valgrind_suspect_functions{$type}}) { - printf F " %5d : %s\n", $valgrind_suspect_functions{$type}{$fcn}, $fcn; + print F "\n[+] fwknop $type functions (unique view):\n"; + next unless defined $valgrind_flagged_fcns_unique{$type}; + for my $fcn (sort {$valgrind_flagged_fcns_unique{$type}{$b} + <=> $valgrind_flagged_fcns_unique{$type}{$a}} + keys %{$valgrind_flagged_fcns_unique{$type}}) { + printf F " %5d : %s\n", $valgrind_flagged_fcns_unique{$type}{$fcn}, $fcn; } - print F "\n"; + print F "\n[+] fwknop $type functions (with call line numbers):\n"; + for my $fcn (sort {$valgrind_flagged_fcns{$type}{$b} + <=> $valgrind_flagged_fcns{$type}{$a}} keys %{$valgrind_flagged_fcns{$type}}) { + printf F " %5d : %s\n", $valgrind_flagged_fcns{$type}{$fcn}, $fcn; + } + next unless defined $valgrind_flagged_fcns{$type}; + } close F; return 1;