From ba3b7d1d11d681549f1bb27e0af1307499fad0d5 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sat, 7 Jul 2012 21:31:30 -0400 Subject: [PATCH] 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 | 103 ++++++++++++++++++++++++++++++++++++++++--------- lib/fko_funcs.c | 3 + server/incoming_spa.c | 50 +++++++++++++++++------- server/replay_cache.c | 100 ++++++++++++++++++++++++++++++++++++++++-------- server/replay_cache.h | 6 +- 9 files changed, 227 insertions(+), 53 deletions(-) diff --git a/CREDITS b/CREDITS index f912289..1d3c17a 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 f0cd41b..19ec13e 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 9dadcc3..1f5af83 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 8a98b55..a57a2dc 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 6eaf126..f2e2577 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,22 +82,27 @@ 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) { - char *md = NULL; - /* Must be initialized */ if(!CTX_INITIALIZED(ctx)) return(FKO_ERROR_CTX_NOT_INITIALIZED); - /* Must have encoded message data to start with. - */ - if(ctx->encoded_msg == NULL) - return(FKO_ERROR_MISSING_ENCODED_DATA); + *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(ctx->digest_type) + switch(digest_type) { case FKO_DIGEST_MD5: md = malloc(MD_HEX_SIZE(MD5_DIGEST_LENGTH)+1); @@ -92,7 +110,7 @@ fko_set_spa_digest(fko_ctx_t ctx) return(FKO_ERROR_MEMORY_ALLOCATION); md5_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); + (unsigned char*)data, strlen(data)); break; case FKO_DIGEST_SHA1: @@ -101,7 +119,7 @@ fko_set_spa_digest(fko_ctx_t ctx) return(FKO_ERROR_MEMORY_ALLOCATION); sha1_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); + (unsigned char*)data, strlen(data)); break; case FKO_DIGEST_SHA256: @@ -110,7 +128,7 @@ fko_set_spa_digest(fko_ctx_t ctx) return(FKO_ERROR_MEMORY_ALLOCATION); sha256_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); + (unsigned char*)data, strlen(data)); break; case FKO_DIGEST_SHA384: @@ -119,7 +137,7 @@ fko_set_spa_digest(fko_ctx_t ctx) return(FKO_ERROR_MEMORY_ALLOCATION); sha384_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); + (unsigned char*)data, strlen(data)); break; case FKO_DIGEST_SHA512: @@ -128,7 +146,7 @@ fko_set_spa_digest(fko_ctx_t ctx) return(FKO_ERROR_MEMORY_ALLOCATION); sha512_base64(md, - (unsigned char*)ctx->encoded_msg, strlen(ctx->encoded_msg)); + (unsigned char*)data, strlen(data)); break; default: @@ -138,15 +156,49 @@ fko_set_spa_digest(fko_ctx_t ctx) /* Just in case this is a subsquent call to this function. We * do not want to be leaking memory. */ - if(ctx->digest != NULL) - free(ctx->digest); + if(*digest != NULL) + free(*digest); - ctx->digest = md; + *digest = md; return(FKO_SUCCESS); } int +fko_set_spa_digest(fko_ctx_t ctx) +{ + /* Must be initialized + */ + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); + + /* Must have encoded message data to start with. + */ + if(ctx->encoded_msg == NULL) + return(FKO_ERROR_MISSING_ENCODED_DATA); + + return set_digest(ctx->encoded_msg, + &ctx->digest, ctx->digest_type); +} + +int +fko_set_raw_spa_digest(fko_ctx_t ctx) +{ + /* Must be initialized + */ + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); + + /* Must have encoded message data to start with. + */ + if(ctx->encrypted_msg == NULL) + return(FKO_ERROR_MISSING_ENCODED_DATA); + + return set_digest(ctx->encrypted_msg, + &ctx->raw_digest, ctx->raw_digest_type); +} + +int fko_get_spa_digest(fko_ctx_t ctx, char **md) { /* Must be initialized @@ -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 4a74438..5ea4f64 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 27e17f0..024efe3 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 77566af..fdc99e5 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 04ee11a..8d785b7 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 */ -- 1.7.5.4