From 016098a2543126f2fa01b3f4057646f0ad2842c5 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 29 Jul 2012 23:31:15 -0400 Subject: [PATCH] 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 | 6 ------ lib/fko.h | 8 ++++++++ server/incoming_spa.c | 14 +++++++++++++ test/test-fwknop.pl | 48 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/lib/cipher_funcs.h b/lib/cipher_funcs.h index 07edace9..be9d6e0d 100644 --- a/lib/cipher_funcs.h +++ b/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). diff --git a/lib/fko.h b/lib/fko.h index 64f4b12f..3acf2671 100644 --- a/lib/fko.h +++ b/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. diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 9ab8dd5d..7f272484 100644 --- a/server/incoming_spa.c +++ b/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. diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 5634d24d..5ddc9505 100755 --- a/test/test-fwknop.pl +++ b/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], - $MATCH_ALL, $server_test_file)) { - $rv = 0; + if ($test_hr->{'replay_positive_output_matches'}) { + $rv = 0 unless &file_find_regex( + $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;