[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

@@ -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.

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.
 

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   kiv_buf[RIJNDAEL_MAX_KEYSIZE+RIJNDAEL_BLOCKSIZE]; /* Key and IV buffer */
-    unsigned char   md5_buf[16];    /* Buffer for computed md5 hash */
+    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+MD5_DIGEST_LEN, pw_buf, final_key_len);
-    memcpy(tmp_buf+32, ctx->salt, 8);
+    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->key, kiv_buf, RIJNDAEL_MAX_KEYSIZE);
-    memcpy(ctx->iv,  kiv_buf+32, 16);
+    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);
+    memcpy(ondx, "Salted__", SALT_LEN);
-    ondx+=8;
+    ondx+=SALT_LEN;
-    memcpy(ondx, ctx.salt, 8);
+    memcpy(ondx, ctx.salt, SALT_LEN);
-    ondx+=8;
+    ondx+=SALT_LEN;
 
     /* Add padding to the original plaintext to ensure that it is a
      * multiple of the Rijndael block size

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 iv[RIJNDAEL_BLOCKSIZE];
-  uint8_t salt[8];
+  uint8_t salt[SALT_LEN];
 } RIJNDAEL_context;
 
 /* This basically performs Rijndael's key scheduling algorithm, as it's the

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;