Fixed ctr_crypt zero-length bug, reported by Tim Kosse.

4 files changed, 48 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index c1373f1e..93390291 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2012-12-04  Niels Möller  <nisse@lysator.liu.se>
+
+	* ctr.c (ctr_crypt): Fix bug reported by Tim Kosse. Don't
+	increment the counter when length is zero (was broken for the
+	in-place case).
+
+	* testsuite/ctr-test.c (test_main): Added test with zero-length
+	data.
+	* testsuite/testutils.c (test_cipher_ctr): Check the ctr value
+	after encrypt and decrypt.
+
 2012-12-03  Niels Möller  <nisse@lysator.liu.se>
 
 	* sha3-permute.c (sha3_permute): Optimized, to reduce number of
diff --git a/ctr.c b/ctr.c
index 49cfa2a9..6b970305 100644
--- a/ctr.c
+++ b/ctr.c
@@ -82,16 +82,7 @@ ctr_crypt(void *ctx, nettle_crypt_func *f,
     }
   else
     {
-      if (length <= block_size)
-	{
-	  TMP_DECL(buffer, uint8_t, NETTLE_MAX_CIPHER_BLOCK_SIZE);
-	  TMP_ALLOC(buffer, block_size);
-
-	  f(ctx, block_size, buffer, ctr);
-	  INCREMENT(block_size, ctr);
-	  memxor3(dst, src, buffer, length);
-	}
-      else
+      if (length > block_size)
 	{
 	  TMP_DECL(buffer, uint8_t, NBLOCKS * NETTLE_MAX_CIPHER_BLOCK_SIZE);
 	  unsigned chunk = NBLOCKS * block_size;
@@ -124,5 +115,14 @@ ctr_crypt(void *ctx, nettle_crypt_func *f,
 	      memxor3(dst, src, buffer, length);
 	    }
 	}
+      else if (length > 0)
+      	{
+	  TMP_DECL(buffer, uint8_t, NETTLE_MAX_CIPHER_BLOCK_SIZE);
+	  TMP_ALLOC(buffer, block_size);
+
+	  f(ctx, block_size, buffer, ctr);
+	  INCREMENT(block_size, ctr);
+	  memxor3(dst, src, buffer, length);
+	}
     }
 }
diff --git a/testsuite/ctr-test.c b/testsuite/ctr-test.c
index b47b8aef..be312529 100644
--- a/testsuite/ctr-test.c
+++ b/testsuite/ctr-test.c
@@ -12,6 +12,13 @@ test_main(void)
    * F.5  CTR Example Vectors
    */
 
+  /* Zero-length data. Exposes bug reported by Tim Kosse, where
+     ctr_crypt increment the ctr when it shouldn't. */
+  test_cipher_ctr(&nettle_aes128,
+		  SHEX("2b7e151628aed2a6abf7158809cf4f3c"),
+		  SHEX(""), SHEX(""),
+		  SHEX("f0f1f2f3f4f5f6f7f8f9fafbfcfdfeff"));
+  
   /* F.5.1  CTR-AES128.Encrypt */
   test_cipher_ctr(&nettle_aes128,
 		  SHEX("2b7e151628aed2a6abf7158809cf4f3c"),
diff --git a/testsuite/testutils.c b/testsuite/testutils.c
index 78cd0d45..028a4bd0 100644
--- a/testsuite/testutils.c
+++ b/testsuite/testutils.c
@@ -5,6 +5,7 @@
 #include "cbc.h"
 #include "ctr.h"
 #include "knuth-lfib.h"
+#include "macros.h"
 #include "nettle-internal.h"
 
 #include <ctype.h>
@@ -308,13 +309,26 @@ test_cipher_ctr(const struct nettle_cipher *cipher,
   void *ctx = xalloc(cipher->context_size);
   uint8_t *data;
   uint8_t *ctr = xalloc(cipher->block_size);
+  uint8_t *octr = xalloc(cipher->block_size);
   unsigned length;
+  unsigned low, nblocks;
 
   ASSERT (cleartext->length == ciphertext->length);
   length = cleartext->length;
 
   ASSERT (ictr->length == cipher->block_size);
-  
+
+  /* Compute expected counter value after the operation. */
+  nblocks = (length + cipher->block_size - 1) / cipher->block_size;
+  ASSERT (nblocks < 0x100);
+
+  memcpy (octr, ictr->data, cipher->block_size - 1);
+  low = ictr->data[cipher->block_size - 1] + nblocks;
+  octr[cipher->block_size - 1] = low;
+
+  if (low >= 0x100)
+    INCREMENT (cipher->block_size - 1, octr);
+
   data = xalloc(length);  
 
   cipher->set_encrypt_key(ctx, key->length, key->data);
@@ -336,6 +350,8 @@ test_cipher_ctr(const struct nettle_cipher *cipher,
       FAIL();
     }
 
+  ASSERT (MEMEQ (cipher->block_size, ctr, octr));
+
   memcpy(ctr, ictr->data, cipher->block_size);
 
   ctr_crypt(ctx, cipher->encrypt,
@@ -354,8 +370,11 @@ test_cipher_ctr(const struct nettle_cipher *cipher,
       FAIL();
     }
 
+  ASSERT (MEMEQ (cipher->block_size, ctr, octr));
+
   free(ctx);
   free(data);
+  free(octr);
   free(ctr);
 }