From fbbcae3a0db81336f45b45e3c4698a79f113c393 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 22 Jan 2013 22:20:54 -0500 Subject: [PATCH] [libfko] Don't trundate > 16 byte Rijndael keys Significant bug fix to honor the full encryption key length for user-supplied Rijndael keys > 16 bytes long. Previous to this bug fix, only the first 16 bytes of a key were actually used in the encryption/ decryption process even if the supplied key was longer. The result was a weakening of expected security for users that had keys > 16 bytes, although this is probably not too common. Note that "passphrase" is perhaps technically a better word for "user-supplied key" in this context since Rijndael in CBC mode derives a real encryption/decryption 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. --- CREDITS | 4 +++ ChangeLog | 11 ++++++ lib/cipher_funcs.c | 42 ++++++++++++---------- lib/rijndael.h | 10 +++--- test/test-fwknop.pl | 85 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 24 deletions(-) diff --git a/CREDITS b/CREDITS index d711813f..ffb5e279 100644 --- a/CREDITS +++ b/CREDITS @@ -95,3 +95,7 @@ Vlad Glagolev Sean Greven - Created a port of fwknop for FreeBSD: http://portsmon.freebsd.org/portoverview.py?category=security&portname=fwknop + +Michael T. Dean + - Reported the Rijndael key truncation issue for user-supplied keys + (passphrases) greater than 16 bytes long. diff --git a/ChangeLog b/ChangeLog index 26365dd8..78e25d97 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,17 @@ fwknop-2.5 (//2013): - Major release of new functionality - HMAC SHA-256 support in the encrypt-then-authenticate model. + - [libfko] Significant bug fix to honor the full encryption key length for + user-supplied Rijndael keys > 16 bytes long. Previous to this bug fix, + only the first 16 bytes of a key were actually used in the encryption/ + decryption process even if the supplied key was longer. The result was + a weakening of expected security for users that had keys > 16 bytes, + although this is probably not too common. Note that "passphrase" is + perhaps technically a better word for "user-supplied key" in this + context since Rijndael in CBC mode derives a real encryption/decryption + 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. - (Vlad Glagolev) Submitted an OpenBSD port for fwknop-2.0.4, and this has been checked in under the extras/openbsd/fwknop-2.0.4 directory. diff --git a/lib/cipher_funcs.c b/lib/cipher_funcs.c index 1fd1163c..34d4fc35 100644 --- a/lib/cipher_funcs.c +++ b/lib/cipher_funcs.c @@ -118,11 +118,12 @@ static void rij_salt_and_iv(RIJNDAEL_context *ctx, const char *key, const int key_len, const unsigned char *data) { - char pw_buf[RIJNDAEL_MIN_KEYSIZE]; + char pw_buf[RIJNDAEL_MAX_KEYSIZE]; unsigned char tmp_buf[64]; /* How big does this need to be? */ - unsigned char kiv_buf[48]; /* Key and IV buffer */ - unsigned char md5_buf[16]; /* Buffer for computed md5 hash */ + unsigned char kiv_buf[RIJNDAEL_MAX_KEYSIZE+RIJNDAEL_BLOCKSIZE]; /* Key and IV buffer */ + unsigned char md5_buf[MD5_DIGEST_LEN]; /* Buffer for computed md5 hash */ + int final_key_len = RIJNDAEL_MIN_KEYSIZE; size_t kiv_len = 0; /* First make pw 32 bytes (pad with "0" (ascii 0x30)) or truncate. @@ -135,7 +136,10 @@ rij_salt_and_iv(RIJNDAEL_context *ctx, const char *key, memset(pw_buf+key_len, '0', RIJNDAEL_MIN_KEYSIZE - key_len); } else - memcpy(pw_buf, key, RIJNDAEL_MIN_KEYSIZE); + { + memcpy(pw_buf, key, key_len); + final_key_len = key_len; + } /* If we are decrypting, data will contain the salt. Otherwise, * for encryption, we generate a random salt. @@ -144,38 +148,38 @@ rij_salt_and_iv(RIJNDAEL_context *ctx, const char *key, { /* Pull the salt from the data */ - memcpy(ctx->salt, (data+8), 8); + memcpy(ctx->salt, (data+SALT_LEN), SALT_LEN); } else { /* Generate a random 8-byte salt. */ - get_random_data(ctx->salt, 8); + get_random_data(ctx->salt, SALT_LEN); } /* Now generate the key and initialization vector. * (again it is the perl Crypt::CBC way, with a touch of * fwknop). */ - memcpy(tmp_buf+RIJNDAEL_MIN_KEYSIZE, pw_buf, RIJNDAEL_MIN_KEYSIZE); - memcpy(tmp_buf+32, ctx->salt, 8); + memcpy(tmp_buf+MD5_DIGEST_LEN, pw_buf, final_key_len); + memcpy(tmp_buf+MD5_DIGEST_LEN+final_key_len, ctx->salt, SALT_LEN); while(kiv_len < sizeof(kiv_buf)) { if(kiv_len == 0) - md5(md5_buf, tmp_buf+16, 24); + md5(md5_buf, tmp_buf+MD5_DIGEST_LEN, final_key_len+SALT_LEN); else - md5(md5_buf, tmp_buf, 40); + md5(md5_buf, tmp_buf, MD5_DIGEST_LEN+final_key_len+SALT_LEN); - memcpy(tmp_buf, md5_buf, 16); + memcpy(tmp_buf, md5_buf, MD5_DIGEST_LEN); - memcpy(kiv_buf + kiv_len, md5_buf, 16); + memcpy(kiv_buf + kiv_len, md5_buf, MD5_DIGEST_LEN); - kiv_len += 16; + kiv_len += MD5_DIGEST_LEN; } - memcpy(ctx->key, kiv_buf, 32); - memcpy(ctx->iv, kiv_buf+32, 16); + memcpy(ctx->key, kiv_buf, RIJNDAEL_MAX_KEYSIZE); + memcpy(ctx->iv, kiv_buf+RIJNDAEL_MAX_KEYSIZE, RIJNDAEL_BLOCKSIZE); } /* Initialization entry point. @@ -216,10 +220,10 @@ rij_encrypt(unsigned char *in, size_t in_len, /* Prepend the salt to the ciphertext... */ - memcpy(ondx, "Salted__", 8); - ondx+=8; - memcpy(ondx, ctx.salt, 8); - ondx+=8; + memcpy(ondx, "Salted__", SALT_LEN); + ondx+=SALT_LEN; + memcpy(ondx, ctx.salt, SALT_LEN); + ondx+=SALT_LEN; /* Add padding to the original plaintext to ensure that it is a * multiple of the Rijndael block size diff --git a/lib/rijndael.h b/lib/rijndael.h index 02e350a3..691ce820 100644 --- a/lib/rijndael.h +++ b/lib/rijndael.h @@ -44,6 +44,9 @@ */ #define RIJNDAEL_BLOCKSIZE 16 #define RIJNDAEL_KEYSIZE 32 +#define RIJNDAEL_MIN_KEYSIZE 16 +#define RIJNDAEL_MAX_KEYSIZE 32 +#define SALT_LEN 8 #define MODE_ECB 1 /* Are we ciphering in ECB mode? */ #define MODE_CBC 2 /* Are we ciphering in CBC mode? */ @@ -54,9 +57,6 @@ /* Allow keys of size 128 <= bits <= 256 */ -#define RIJNDAEL_MIN_KEYSIZE 16 -#define RIJNDAEL_MAX_KEYSIZE 32 - typedef struct { uint32_t keys[60]; /* maximum size of key schedule */ uint32_t ikeys[60]; /* inverse key schedule */ @@ -64,8 +64,8 @@ typedef struct { int mode; /* encryption mode */ /* Added by DSS */ uint8_t key[RIJNDAEL_MAX_KEYSIZE]; - uint8_t iv[16]; - uint8_t salt[8]; + uint8_t iv[RIJNDAEL_BLOCKSIZE]; + uint8_t salt[SALT_LEN]; } RIJNDAEL_context; /* This basically performs Rijndael's key scheduling algorithm, as it's the diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index ecbd8ec2..03f42550 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -2407,6 +2407,14 @@ my @tests = ( 'function' => \&perl_fko_module_complete_cycle, 'fatal' => $NO }, + { + 'category' => 'perl FKO module', + 'subcategory' => 'encrypt/decrypt', + 'detail' => 'truncated keys', + 'err_msg' => 'allowed truncated keys to decrypt SPA data', + 'function' => \&perl_fko_module_rijndael_truncated_keys, + 'fatal' => $NO + }, { 'category' => 'perl FKO module', 'subcategory' => 'encrypt/decrypt', @@ -4190,6 +4198,83 @@ sub fuzzing_cmd_messages() { return \@msgs; } +sub perl_fko_module_rijndael_truncated_keys() { + my $test_hr = shift; + + my $rv = 1; + + for my $msg (@{valid_access_messages()}[0]) { + for my $user (@{valid_usernames()}[0]) { + for my $digest_type (@{valid_spa_digest_types()}[0]) { + + my $key = '1'; + for (my $i=2; $i <= 32; $i++) { + + $key .= $i % 10; + + if (length($key) < 16 and $key =~ /0$/) { + ### word around the trailing zero problem for now + $key =~ s/0$/X/; + } + + &write_test_file("[+] key: $key (" . length($key) . " bytes)\n", + $curr_test_file); + for (my $j=1; $j < length($key); $j++) { + + &write_test_file(" msg: $msg, user: $user, " . + "digest type: $digest_type\n", + $curr_test_file); + + $fko_obj = FKO->new(); + unless ($fko_obj) { + &write_test_file("[-] error FKO->new(): " . FKO::error_str() . "\n", + $curr_test_file); + return 0; + } + + $fko_obj->spa_message($msg); + $fko_obj->username($user); + $fko_obj->spa_message_type(FKO->FKO_ACCESS_MSG); + $fko_obj->digest_type($digest_type); + $fko_obj->spa_data_final($key, length($key), '', 0); + + my $encrypted_msg = $fko_obj->spa_data(); + + $fko_obj->destroy(); + + ### now get new object for decryption + $fko_obj = FKO->new(); + unless ($fko_obj) { + &write_test_file("[-] error FKO->new(): " . FKO::error_str() . "\n", + $curr_test_file); + return 0; + } + $fko_obj->spa_data($encrypted_msg); + my $truncated_key = $key; + $truncated_key =~ s/^(.{$j}).*/$1/; + if ($fko_obj->decrypt_spa_data($truncated_key, + length($truncated_key)) == FKO->FKO_SUCCESS) { + &write_test_file("[-] $msg decrypt success with truncated key " . + "($key -> $truncated_key)\n", + $curr_test_file); + $rv = 0; + } else { + &write_test_file("[+] $msg decrypt rejected truncated " . + "key ($key -> $truncated_key)\n", + $curr_test_file); + } + + $fko_obj->destroy(); + } + &write_test_file("\n", $curr_test_file); + } + } + } + } + + return $rv; +} + sub perl_fko_module_complete_cycle() { my $test_hr = shift;