Replay attack bug fix (encryption prefixes)

Ensure that an attacker cannot force a replay attack by intercepting an
SPA packet and the replaying it with the base64 version of "Salted__"
(for Rindael) or the "hQ" prefix (for GnuPG).  This is an important fix.
The following comment was added into the fwknopd code:

/* Ignore any SPA packets that contain the Rijndael or GnuPG prefixes
 * since an attacker might have tacked them on to a previously seen
 * SPA packet in an attempt to get past the replay check.  And, we're
 * no worse off since a legitimate SPA packet that happens to include
 * a prefix after the outer one is stripped off won't decrypt properly
 * anyway because libfko would not add a new one.
*/

Conflicts:

	lib/cipher_funcs.h

lib/cipher_funcs.h

@@ -35,12 +35,6 @@
 #include "rijndael.h"
 #include "gpgme_funcs.h"
 
-/* Define the consistent prefixes or salt on some ecryption schemes.
- * We identify them here so we can remove and reinsert when needed.
-*/
-#define B64_RIJNDAEL_SALT "U2FsdGVkX1"
-#define B64_GPG_PREFIX "hQ"
-
 /* Provide the predicted encrypted data size for given input data based
  * on a 16-byte block size (for Rijndael implementation,this also accounts
  * for the 16-byte salt as well).

lib/fko.h

@@ -162,6 +162,14 @@ typedef enum {
 #define FKO_DEFAULT_DIGEST      FKO_DIGEST_SHA256
 #define FKO_DEFAULT_ENCRYPTION  FKO_ENCRYPTION_RIJNDAEL
 
+/* Define the consistent prefixes or salt on some encryption schemes.
+*/
+#define B64_RIJNDAEL_SALT "U2FsdGVkX1"
+#define B64_RIJNDAEL_SALT_STR_LEN 10
+
+#define B64_GPG_PREFIX "hQ"
+#define B64_GPG_PREFIX_STR_LEN 2
+
 /* The context holds the global state and config options, as
  * well as some intermediate results during processing. This
  * is an opaque pointer.

server/incoming_spa.c

@@ -62,6 +62,20 @@ preprocess_spa_data(fko_srv_options_t *opts, const char *src_ip)
     */
     spa_pkt->packet_data_len = 0;
 
+    /* Ignore any SPA packets that contain the Rijndael or GnuPG prefixes
+     * since an attacker might have tacked them on to a previously seen
+     * SPA packet in an attempt to get past the replay check.  And, we're
+     * no worse off since a legitimate SPA packet that happens to include
+     * a prefix after the outer one is stripped off won't decrypt properly
+     * anyway because libfko would not add a new one.
+    */
+    if(strncmp(ndx, B64_RIJNDAEL_SALT, B64_RIJNDAEL_SALT_STR_LEN) == 0)
+        return(SPA_MSG_BAD_DATA);
+
+    if(pkt_data_len > MIN_GNUPG_MSG_SIZE
+            && strncmp(ndx, B64_GPG_PREFIX, B64_GPG_PREFIX_STR_LEN) == 0)
+        return(SPA_MSG_BAD_DATA);
+
     /* Detect and parse out SPA data from an HTTP request. If the SPA data
      * starts with "GET /" and the user agent starts with "Fwknop", then
      * assume it is a SPA over HTTP request.

test/test-fwknop.pl

@@ -1092,6 +1092,20 @@ my @tests = (
         'cmdline'  => $default_client_args,
         'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
             "$fwknopdCmd $default_server_conf_args $intf_str",
+        'replay_positive_output_matches' => [qr/Replay\sdetected\sfrom\ssource\sIP/],
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'Rijndael SPA',
+        'subcategory' => 'client+server',
+        'detail'   => 'replay detection (Rijndael prefix)',
+        'err_msg'  => 'could not detect replay attack',
+        'function' => \&replay_detection,
+        'pkt_prefix' => 'U2FsdGVkX1',
+        'cmdline'  => $default_client_args,
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd $default_server_conf_args $intf_str",
+        'replay_positive_output_matches' => [qr/Data\sis\snot\sa\svalid\sSPA\smessage\sformat/],
         'fatal'    => $NO
     },
     {
@@ -1236,6 +1250,20 @@ my @tests = (
         'function' => \&replay_detection,
         'cmdline'  => $default_client_gpg_args,
         'fwknopd_cmdline'  => $default_server_gpg_args,
+        'replay_positive_output_matches' => [qr/Replay\sdetected\sfrom\ssource\sIP/],
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GnuPG (GPG) SPA',
+        'subcategory' => 'client+server',
+        'detail'   => 'replay detection (GnuPG prefix)',
+        'err_msg'  => 'could not detect replay attack',
+        'function' => \&replay_detection,
+        'pkt_prefix' => 'hQ',
+        'cmdline'  => $default_client_gpg_args,
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd $default_server_conf_args $intf_str",
+        'replay_positive_output_matches' => [qr/Data\sis\snot\sa\svalid\sSPA\smessage\sformat/],
         'fatal'    => $NO
     },
 
@@ -1324,10 +1352,13 @@ my %test_keys = (
     'fw_rule_created' => $OPTIONAL,
     'fw_rule_removed' => $OPTIONAL,
     'server_conf'     => $OPTIONAL,
+    'pkt_prefix'      => $OPTIONAL,
     'positive_output_matches' => $OPTIONAL,
     'negative_output_matches' => $OPTIONAL,
     'server_positive_output_matches' => $OPTIONAL,
     'server_negative_output_matches' => $OPTIONAL,
+    'replay_positive_output_matches' => $OPTIONAL,
+    'replay_negative_output_matches' => $OPTIONAL,
 );
 
 if ($diff_mode) {
@@ -1681,6 +1712,10 @@ sub replay_detection() {
         return 0;
     }
 
+    if ($test_hr->{'pkt_prefix'}) {
+        $spa_pkt = $test_hr->{'pkt_prefix'} . $spa_pkt;
+    }
+
     my @packets = (
         {
             'proto'  => 'udp',
@@ -1695,9 +1730,16 @@ sub replay_detection() {
 
     $rv = 0 unless $server_was_stopped;
 
-    unless (&file_find_regex([qr/Replay\sdetected\sfrom\ssource\sIP/i],
+    if ($test_hr->{'replay_positive_output_matches'}) {
-            $MATCH_ALL, $server_test_file)) {
+        $rv = 0 unless &file_find_regex(
-        $rv = 0;
+            $test_hr->{'replay_positive_output_matches'},
+            $MATCH_ALL, $server_test_file);
+    }
+
+    if ($test_hr->{'replay_negative_output_matches'}) {
+        $rv = 0 if &file_find_regex(
+            $test_hr->{'replay_negative_output_matches'},
+            $MATCH_ANY, $server_test_file);
     }
 
     return $rv;