Bug fix for multi-stanza key use and replay attack detection
authorMichael Rash <mbr@cipherdyne.org>
Sun, 8 Jul 2012 01:31:30 +0000 (21:31 -0400)
committerMichael Rash <mbr@cipherdyne.org>
Sun, 8 Jul 2012 01:31:30 +0000 (21:31 -0400)
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
client/config_init.c
lib/fko.h
lib/fko_context.h
lib/fko_digest.c
lib/fko_funcs.c
server/incoming_spa.c
server/replay_cache.c
server/replay_cache.h

diff --git a/CREDITS b/CREDITS
index f912289..1d3c17a 100644 (file)
--- 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.
index f0cd41b..19ec13e 100644 (file)
@@ -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)
index 9dadcc3..1f5af83 100644 (file)
--- 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);
 
index 8a98b55..a57a2dc 100644 (file)
@@ -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;
index 6eaf126..f2e2577 100644 (file)
@@ -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***/
index 4a74438..5ea4f64 100644 (file)
@@ -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);
 
index 27e17f0..024efe3 100644 (file)
@@ -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);
index 77566af..fdc99e5 100644 (file)
 #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 */
 }
index 04ee11a..8d785b7 100644 (file)
@@ -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 */