From bcb0fcfc1adc78cc39ebf9d5b89965bda4522016 Mon Sep 17 00:00:00 2001
From: Michael Rash <mbr@cipherdyne.org>
Date: Wed, 8 Feb 2012 14:16:42 -0500
Subject: [PATCH] Re-worked encryption/decryption handling

For SPA packets encrypted with Rjindael, fwknop has always used CBC mode
even though ECB mode is mentioned in a couple of places.  This change makes
more transparent use of block_encrypt() and block_decrypt() to ensure that
the appropriate mode is used.  The default is CBC mode, but others can be
selected as well (-M <mode> for the fwknop client, and ENCRYPTION_MODE in
access.conf for the fwknopd server).
---
 lib/cipher_funcs.c   | 66 ++++++++------------------------------------
 lib/fko.h            |  2 +-
 lib/fko_encryption.c | 36 ++++++++++++------------
 3 files changed, 32 insertions(+), 72 deletions(-)

diff --git a/lib/cipher_funcs.c b/lib/cipher_funcs.c
index 6621fcb5..560471e5 100644
--- a/lib/cipher_funcs.c
+++ b/lib/cipher_funcs.c
@@ -207,50 +207,28 @@ rij_encrypt(unsigned char *in, size_t in_len,
     const char *pass, unsigned char *out, int encryption_mode)
 {
     RIJNDAEL_context    ctx;
-    unsigned char       plaintext[RIJNDAEL_BLOCKSIZE];
-    unsigned char       mixtext[RIJNDAEL_BLOCKSIZE];
-    unsigned char       ciphertext[RIJNDAEL_BLOCKSIZE];
     int                 i, pad_val;
-
     unsigned char      *ondx = out;
 
     rijndael_init(&ctx, pass, NULL, encryption_mode);
 
-    /* Prepend the salt...
+    /* Prepend the salt to the ciphertext...
     */
     memcpy(ondx, "Salted__", 8);
     ondx+=8;
     memcpy(ondx, ctx.salt, 8);
     ondx+=8;
 
-    /* Now iterate of the input data and encrypt in 16-byte chunks.
+    /* Add padding to the original plaintext to ensure that it is a
+     * multiple of the Rijndael block size
     */
-    while(in_len)
-    {
-        for(i=0; i<sizeof(plaintext); i++)
-        {
-            if(in_len < 1)
-                break;
+    pad_val = RIJNDAEL_BLOCKSIZE - (in_len % RIJNDAEL_BLOCKSIZE);
+    for (i = in_len; i < in_len+pad_val; i++)
+        in[i] = pad_val;
 
-            plaintext[i] = *in++;
-            in_len--;
-        }
+    block_encrypt(&ctx, in, in_len+pad_val, ondx, ctx.iv);
 
-        pad_val = sizeof(plaintext) - i;
-
-        for(; i < sizeof(plaintext); i++)
-            plaintext[i] = pad_val;
-
-        for(i=0; i<RIJNDAEL_BLOCKSIZE; i++)
-            mixtext[i] = plaintext[i] ^ ctx.iv[i];
-
-        block_encrypt(&ctx, mixtext, RIJNDAEL_BLOCKSIZE, ciphertext, ctx.iv);
-
-        memcpy(ctx.iv, ciphertext, RIJNDAEL_BLOCKSIZE);
-
-        for(i=0; i<sizeof(ciphertext); i++)
-            *ondx++ = ciphertext[i];
-    }
+    ondx += in_len+pad_val;
 
     return(ondx - out);
 }
@@ -262,41 +240,21 @@ rij_decrypt(unsigned char *in, size_t in_len,
     const char *pass, unsigned char *out, int encryption_mode)
 {
     RIJNDAEL_context    ctx;
-    unsigned char       plaintext[RIJNDAEL_BLOCKSIZE];
-    unsigned char       mixtext[RIJNDAEL_BLOCKSIZE];
-    unsigned char       ciphertext[RIJNDAEL_BLOCKSIZE];
     int                 i, pad_val, pad_err = 0;
     unsigned char      *pad_s;
     unsigned char      *ondx = out;
 
     rijndael_init(&ctx, pass, in, encryption_mode);
 
-    /* Remove the salt from the input.
+    /* Remove the first block since it contains the salt (it was consumed
+     * by the rijndael_init() function above).
     */
     in_len -= 16;
     memmove(in, in+16, in_len);
 
-    while(in_len)
-    {
-        for(i=0; i<sizeof(ciphertext); i++)
-        {
-            if(in_len < 1)
-                break;
+    block_decrypt(&ctx, in, in_len, out, ctx.iv);
 
-            ciphertext[i] = *in++;
-            in_len--;
-        }
-
-        block_decrypt(&ctx, ciphertext, RIJNDAEL_BLOCKSIZE, mixtext, ctx.iv);
-
-        for(i=0; i<sizeof(ciphertext); i++)
-            plaintext[i] = mixtext[i] ^ ctx.iv[i];
-
-        memcpy(ctx.iv, ciphertext, RIJNDAEL_BLOCKSIZE);
-
-        for(i=0; i<sizeof(plaintext); i++)
-            *ondx++ = plaintext[i];
-    }
+    ondx += in_len;
 
     /* Find and remove padding.
     */
diff --git a/lib/fko.h b/lib/fko.h
index d6bdbfd4..e694a718 100644
--- a/lib/fko.h
+++ b/lib/fko.h
@@ -176,7 +176,7 @@ typedef enum {
 #define FKO_DEFAULT_MSG_TYPE    FKO_ACCESS_MSG
 #define FKO_DEFAULT_DIGEST      FKO_DIGEST_SHA256
 #define FKO_DEFAULT_ENCRYPTION  FKO_ENCRYPTION_RIJNDAEL
-#define FKO_DEFAULT_ENC_MODE    MODE_ECB
+#define FKO_DEFAULT_ENC_MODE    MODE_CBC
 
 /* The context holds the global state and config options, as
  * well as some intermediate results during processing. This
diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c
index 6eca9294..5f1788a0 100644
--- a/lib/fko_encryption.c
+++ b/lib/fko_encryption.c
@@ -45,47 +45,49 @@
 static int
 _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key)
 {
-    char           *plain;
-    char           *b64cipher;
-    unsigned char  *cipher;
+    char           *plaintext;
+    char           *b64ciphertext;
+    unsigned char  *ciphertext;
     int             cipher_len;
 
     /* Make a bucket big enough to hold the enc msg + digest (plaintext)
      * and populate it appropriately.
     */
-    plain = malloc(strlen(ctx->encoded_msg) + strlen(ctx->digest) + 2);
-    if(plain == NULL)
+    plaintext = calloc(1, strlen(ctx->encoded_msg)
+                    + strlen(ctx->digest) + RIJNDAEL_BLOCKSIZE + 2);
+
+    if(plaintext == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
-    sprintf(plain, "%s:%s", ctx->encoded_msg, ctx->digest);
+    sprintf(plaintext, "%s:%s", ctx->encoded_msg, ctx->digest);
 
     /* Make a bucket for the encrypted version and populate it.
     */
-    cipher = malloc(strlen(plain) + 32); /* Plus padding for salt and Block */
-    if(cipher == NULL)
+    ciphertext = calloc(1, strlen(plaintext) + 32); /* Plus padding for salt and Block */
+    if(ciphertext == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
     cipher_len = rij_encrypt(
-        (unsigned char*)plain, strlen(plain), (char*)enc_key, cipher,
+        (unsigned char*)plaintext, strlen(plaintext), (char*)enc_key, ciphertext,
         ctx->encryption_mode
     );
 
     /* Now make a bucket for the base64-encoded version and populate it.
     */
-    b64cipher = malloc(((cipher_len / 3) * 4) + 8);
-    if(b64cipher == NULL)
+    b64ciphertext = malloc(((cipher_len / 3) * 4) + 8);
+    if(b64ciphertext == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
-    b64_encode(cipher, b64cipher, cipher_len);
-    strip_b64_eq(b64cipher);
+    b64_encode(ciphertext, b64ciphertext, cipher_len);
+    strip_b64_eq(b64ciphertext);
 
-    ctx->encrypted_msg = strdup(b64cipher);
+    ctx->encrypted_msg = strdup(b64ciphertext);
 
     /* Clean-up
     */
-    free(plain);
-    free(cipher);
-    free(b64cipher);
+    free(plaintext);
+    free(ciphertext);
+    free(b64ciphertext);
 
     if(ctx->encrypted_msg == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);