Permalink
Browse files

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.
  • Loading branch information...
1 parent fcf40b5 commit ba3b7d1d11d681549f1bb27e0af1307499fad0d5 @mrash committed Jul 8, 2012
Showing with 227 additions and 53 deletions.
  1. +6 −0 CREDITS
  2. +1 −1 client/config_init.c
  3. +4 −0 lib/fko.h
  4. +6 −0 lib/fko_context.h
  5. +84 −19 lib/fko_digest.c
  6. +3 −0 lib/fko_funcs.c
  7. +36 −14 server/incoming_spa.c
  8. +84 −16 server/replay_cache.c
  9. +3 −3 server/replay_cache.h
View
@@ -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.
View
@@ -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)
View
@@ -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);
View
@@ -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;
View
@@ -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,30 +82,35 @@ 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);
if(md == NULL)
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***/
View
@@ -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);
View
@@ -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);
Oops, something went wrong.

0 comments on commit ba3b7d1

Please sign in to comment.