From e7eb02f82df2949c1a9092745b771fa8ffaf6723 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 27 Jan 2013 13:18:29 -0500 Subject: [PATCH] Maintain backwards compatibility with old "zero padding" code [libfko] Added the ability to maintain backwards compatibility with the now deprecated "zero padding" strategy in AES mode that was a hold over from the old perl fwknop implementation. This enables the backwards compatiblity tests to continue to pass in the test suite. --- ChangeLog | 3 ++ lib/cipher_funcs.c | 20 +++++++------ lib/fko.h | 1 + server/access.c | 2 ++ test/conf/android_legacy_iv_access.conf | 4 +++ test/conf/legacy_iv_access.conf | 4 +++ test/test-fwknop.pl | 38 ++++++++++++++----------- 7 files changed, 46 insertions(+), 26 deletions(-) create mode 100644 test/conf/android_legacy_iv_access.conf create mode 100644 test/conf/legacy_iv_access.conf diff --git a/ChangeLog b/ChangeLog index b1836a39..cc29b3ee 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,9 @@ fwknop-2.5 (//2013): key from the passphrase through a series of applications of md5 against the passphrase and a random salt. This issue was reported by Michael T. Dean. Closes issue #18 on github. + - [libfko] Added the ability to maintain backwards compatibility with the + now deprecated "zero padding" strategy in AES mode that was a hold over + from the old perl fwknop implementation. - [test suite] Added --enable-openssl-checks to send all SPA packets encrypted via libfko through the OpenSSL library to ensure that the libfko usage of AES is always compatible with OpenSSL. This ensures diff --git a/lib/cipher_funcs.c b/lib/cipher_funcs.c index f3a1ee33..09fad5b5 100644 --- a/lib/cipher_funcs.c +++ b/lib/cipher_funcs.c @@ -116,7 +116,7 @@ get_random_data(unsigned char *data, const size_t len) */ static void rij_salt_and_iv(RIJNDAEL_context *ctx, const char *key, - const int key_len, const unsigned char *data, const int legacy_enc_mode) + const int key_len, const unsigned char *data, const int mode_flag) { char pw_buf[RIJNDAEL_MAX_KEYSIZE]; unsigned char tmp_buf[MD5_DIGEST_LEN+RIJNDAEL_MAX_KEYSIZE+RIJNDAEL_BLOCKSIZE]; @@ -131,16 +131,15 @@ rij_salt_and_iv(RIJNDAEL_context *ctx, const char *key, memset(kiv_buf, 0x00, RIJNDAEL_MAX_KEYSIZE+RIJNDAEL_BLOCKSIZE); memset(md5_buf, 0x00, MD5_DIGEST_LEN); - if(legacy_enc_mode == 1) + if(mode_flag == FKO_ENC_MODE_CBC_LEGACY_IV) { - /* First make pw 32 bytes (pad with "0" (ascii 0x30)) or truncate. - * Note: pw_buf was initialized with '0' chars (again, not the value - * 0, but the digit '0' character). + /* Pad the pw with '0' chars up to the minimum Rijndael key size. * * This maintains compatibility with the old perl code if absolutely * necessary in some scenarios, but is not recommended to use since it - * breaks compatibility with how OpenSSL implements AES. This code - * will be removed altogether in a future version of fwknop. + * breaks compatibility with how OpenSSL implements AES and introduces + * other problems. This code will be removed altogether in a future + * version of fwknop. */ if(key_len < RIJNDAEL_MIN_KEYSIZE) { @@ -211,11 +210,14 @@ rijndael_init(RIJNDAEL_context *ctx, const char *key, /* The default (set in fko.h) is CBC mode */ - ctx->mode = encryption_mode; + if(encryption_mode == FKO_ENC_MODE_CBC_LEGACY_IV) + ctx->mode = FKO_ENC_MODE_CBC; + else + ctx->mode = encryption_mode; /* Generate the salt and initialization vector. */ - rij_salt_and_iv(ctx, key, key_len, data, 0); + rij_salt_and_iv(ctx, key, key_len, data, encryption_mode); /* Intialize our Rijndael context. */ diff --git a/lib/fko.h b/lib/fko.h index 3e9c7197..3fa1b372 100644 --- a/lib/fko.h +++ b/lib/fko.h @@ -118,6 +118,7 @@ typedef enum { FKO_ENC_MODE_OFB = MODE_OFB, FKO_ENC_MODE_CTR = MODE_CTR, FKO_ENC_MODE_ASYMMETRIC, /* placeholder when GPG is used */ + FKO_ENC_MODE_CBC_LEGACY_IV, /* for the old zero-padding strategy */ FKO_LAST_ENC_MODE /* Always leave this as the last one */ } fko_encryption_mode_t; diff --git a/server/access.c b/server/access.c index 82af9d5b..34effc80 100644 --- a/server/access.c +++ b/server/access.c @@ -176,6 +176,8 @@ enc_mode_strtoint(const char *enc_mode_str) return(FKO_ENC_MODE_OFB); else if(strcasecmp(enc_mode_str, "ctr") == 0) return(FKO_ENC_MODE_CTR); + else if(strcasecmp(enc_mode_str, "legacy") == 0) + return(FKO_ENC_MODE_CBC_LEGACY_IV); else return(-1); } diff --git a/test/conf/android_legacy_iv_access.conf b/test/conf/android_legacy_iv_access.conf new file mode 100644 index 00000000..2ffb8242 --- /dev/null +++ b/test/conf/android_legacy_iv_access.conf @@ -0,0 +1,4 @@ +SOURCE: ANY; +KEY: asdfasdf; +FW_ACCESS_TIMEOUT: 3; +ENCRYPTION_MODE: legacy; diff --git a/test/conf/legacy_iv_access.conf b/test/conf/legacy_iv_access.conf new file mode 100644 index 00000000..bbe1f55b --- /dev/null +++ b/test/conf/legacy_iv_access.conf @@ -0,0 +1,4 @@ +SOURCE: ANY; +KEY: fwknoptest; +FW_ACCESS_TIMEOUT: 3; +ENCRYPTION_MODE: legacy; diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 996a5813..8ae830f7 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -40,8 +40,10 @@ my %cf = ( 'local_nat' => "$conf_dir/local_nat_fwknopd.conf", 'ipfw_active_expire' => "$conf_dir/ipfw_active_expire_equal_fwknopd.conf", 'android_access' => "$conf_dir/android_access.conf", + 'android_legacy_iv_access' => "$conf_dir/android_legacy_iv_access.conf", 'dual_key_access' => "$conf_dir/dual_key_usage_access.conf", 'gpg_access' => "$conf_dir/gpg_access.conf", + 'legacy_iv_access' => "$conf_dir/legacy_iv_access.conf", 'gpg_no_pw_access' => "$conf_dir/gpg_no_pw_access.conf", 'tcp_server' => "$conf_dir/tcp_server_fwknopd.conf", 'tcp_pcap_filter' => "$conf_dir/tcp_pcap_filter_fwknopd.conf", @@ -1635,7 +1637,8 @@ my @tests = ( 'function' => \&process_pcap_file_directly, 'cmdline' => '', 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd $default_server_conf_args " . + "$fwknopdCmd -c $cf{'def'} -a $cf{'legacy_iv_access'} " . + "-d $default_digest_file -p $default_pid_file " . "--pcap-file $replay_pcap_file --foreground --verbose --verbose " . "--verbose", 'server_positive_output_matches' => [qr/Replay\sdetected/i, @@ -1835,7 +1838,7 @@ my @tests = ( '55krFt+1B2TtNSAH005kyDEZEOIGoY9Q/iU', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1852,7 +1855,7 @@ my @tests = ( 'Hhro2tH34nqfTRIpevfLTMx7r+N8ZQ4V8', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1869,7 +1872,7 @@ my @tests = ( 'Vw2/Va/aUjvEvNPtwuipQS6DLTzOw/qy+/g', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1885,7 +1888,7 @@ my @tests = ( 'M+GledHfz2d49aYThoQ2Cr8Iw1ycViawY', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1901,7 +1904,7 @@ my @tests = ( 'XZMcWgMsIzhpprJ7JX41DrWd0OtBnE3rVwsN0', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1919,7 +1922,7 @@ my @tests = ( 'DJR92YhjYtL4Q', 'server_positive_output_matches' => [qr/with expire time/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'android_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'android_legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1949,7 +1952,7 @@ my @tests = ( 'ZUYoCaJ7X78ULyJTi5eT7nug', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -1977,7 +1980,7 @@ my @tests = ( 'oGnOXu3/DeWHJAwtSeh7EAr4', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2007,7 +2010,7 @@ my @tests = ( '8lGrxcPSfbCOW61k0MP+q1EhLZkc1qAm5g2+2cLNZcoBNEdh3yj8OTPZJyBVw', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2030,7 +2033,7 @@ my @tests = ( 'iGXWxSL4u+AWSSePK3qiiYoRQVw', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2053,7 +2056,7 @@ my @tests = ( 'ekw+EUscVvUkrsRcVtSvOm+fCNo', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2086,7 +2089,7 @@ my @tests = ( 'rPAEnw', 'server_positive_output_matches' => [qr/No\sstanza\sencryption\smode\smatch/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2115,7 +2118,7 @@ my @tests = ( 'MKY', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2139,7 +2142,7 @@ my @tests = ( 'hW04WpY8mdUNu9i+PrfPr7/KxqU', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -2162,7 +2165,7 @@ my @tests = ( 'yEkDgP5+f49xrRA', 'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/], 'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopdCmd -c $cf{'disable_aging_nat'} -a $cf{'def_access'} " . + "$fwknopdCmd -c $cf{'disable_aging_nat'} -a $cf{'legacy_iv_access'} " . "-d $default_digest_file -p $default_pid_file $intf_str", 'fatal' => $NO }, @@ -3328,7 +3331,8 @@ sub client_send_spa_packet() { $rv = 0 unless &file_find_regex([qr/final\spacked/i], $MATCH_ALL, $curr_test_file); - if ($enable_openssl_compatibility_tests) { + if ($enable_openssl_compatibility_tests + and $test_hr->{'detail'} !~ /iptables.*not\sduplicated/) { ### extract the SPA packet from the cmd tmp file before ### openssl command execution overwrites it