From ba90d7c56e9cfb0e0db3c241e65b6a0aed12a684 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@chromium.org>
Date: Wed, 28 Jan 2015 13:56:01 -0500
Subject: [PATCH] Test HMAC_CTX initial state and remove now unneccessary code.

The special-case in HMAC is no longer needed. Test that HMAC_CTX is initialized
with the zero key.

Change-Id: I4ee2b495047760765c7d7fdfb4ccb510723aa263
Reviewed-on: https://boringssl-review.googlesource.com/3121
Reviewed-by: Adam Langley <agl@google.com>
---
 crypto/hmac/hmac.c      |  7 ----
 crypto/hmac/hmac_test.c | 92 ++++++++++++++++++++++++++++++++---------
 2 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/crypto/hmac/hmac.c b/crypto/hmac/hmac.c
index de00aae79..c5b4bd19f 100644
--- a/crypto/hmac/hmac.c
+++ b/crypto/hmac/hmac.c
@@ -74,13 +74,6 @@ uint8_t *HMAC(const EVP_MD *evp_md, const void *key, size_t key_len,
     out = static_out_buffer;
   }
 
-  /* If key_len is 0, the value of key doesn't matter. However, if we pass
-   * key == NULL into HMAC_Init, it interprets it to mean "use the previous
-   * value" instead of using a key of length 0. */
-  if (key == NULL && key_len == 0) {
-    key = static_out_buffer;
-  }
-
   HMAC_CTX_init(&ctx);
   if (!HMAC_Init(&ctx, key, key_len, evp_md) ||
       !HMAC_Update(&ctx, data, data_len) ||
diff --git a/crypto/hmac/hmac_test.c b/crypto/hmac/hmac_test.c
index def773b1a..0ebfb98e7 100644
--- a/crypto/hmac/hmac_test.c
+++ b/crypto/hmac/hmac_test.c
@@ -54,6 +54,7 @@
  * copied and put under another distribution licence
  * [including the GNU Public Licence.] */
 
+#include <assert.h>
 #include <stdio.h>
 
 #include <openssl/crypto.h>
@@ -62,10 +63,10 @@
 
 
 struct test_st {
-  unsigned char key[16];
-  unsigned key_len;
-  unsigned char data[64];
-  unsigned data_len;
+  uint8_t key[16];
+  size_t key_len;
+  uint8_t data[64];
+  size_t data_len;
   const char *hex_digest;
 };
 
@@ -130,34 +131,87 @@ int main(int argc, char *argv[]) {
   for (i = 0; i < NUM_TESTS; i++) {
     const struct test_st *test = &kTests[i];
 
+    /* Test using the one-shot API. */
     if (NULL == HMAC(EVP_md5(), test->key, test->key_len, test->data,
                      test->data_len, out, &out_len)) {
-      printf("%u: HMAC failed.\n", i);
+      fprintf(stderr, "%u: HMAC failed.\n", i);
       err++;
       continue;
     }
-
     p = to_hex(out, out_len);
-
     if (strcmp(p, test->hex_digest) != 0) {
-      printf("%u: got %s instead of %s\n", i, p, test->hex_digest);
+      fprintf(stderr, "%u: got %s instead of %s\n", i, p, test->hex_digest);
+      err++;
+    }
+
+    /* Test using HMAC_CTX. */
+    HMAC_CTX ctx;
+    HMAC_CTX_init(&ctx);
+    if (!HMAC_Init_ex(&ctx, test->key, test->key_len, EVP_md5(), NULL) ||
+        !HMAC_Update(&ctx, test->data, test->data_len) ||
+        !HMAC_Final(&ctx, out, &out_len)) {
+      fprintf(stderr, "%u: HMAC failed.\n", i);
+      err++;
+      HMAC_CTX_cleanup(&ctx);
+      continue;
+    }
+    p = to_hex(out, out_len);
+    if (strcmp(p, test->hex_digest) != 0) {
+      fprintf(stderr, "%u: got %s instead of %s\n", i, p, test->hex_digest);
+      err++;
+    }
+
+    /* Test that an HMAC_CTX may be reset with the same key. */
+    if (!HMAC_Init_ex(&ctx, NULL, 0, EVP_md5(), NULL) ||
+        !HMAC_Update(&ctx, test->data, test->data_len) ||
+        !HMAC_Final(&ctx, out, &out_len)) {
+      fprintf(stderr, "%u: HMAC failed.\n", i);
+      err++;
+      HMAC_CTX_cleanup(&ctx);
+      continue;
+    }
+    p = to_hex(out, out_len);
+    if (strcmp(p, test->hex_digest) != 0) {
+      fprintf(stderr, "%u: got %s instead of %s\n", i, p, test->hex_digest);
+      err++;
+    }
+
+    HMAC_CTX_cleanup(&ctx);
+  }
+
+  /* Test that HMAC() uses the empty key when called with key = NULL. */
+  const struct test_st *test = &kTests[0];
+  assert(test->key_len == 0);
+  if (NULL == HMAC(EVP_md5(), NULL, 0, test->data, test->data_len, out,
+                   &out_len)) {
+    fprintf(stderr, "HMAC failed.\n");
+    err++;
+  } else {
+    p = to_hex(out, out_len);
+    if (strcmp(p, test->hex_digest) != 0) {
+      fprintf(stderr, "got %s instead of %s\n", p, test->hex_digest);
       err++;
     }
   }
 
-  /* Test that HMAC() functions corretly when called with key=NULL */
-  const struct test_st *test = &kTests[0];
-  if (NULL == HMAC(EVP_md5(), NULL, test->key_len, test->data,
-                   test->data_len, out, &out_len)) {
-    printf("HMAC failed.\n");
-    err++;
-  }
-
-  p = to_hex(out, out_len);
-  if (strcmp(p, test->hex_digest) != 0) {
-    printf("got %s instead of %s\n", p, test->hex_digest);
+  /* Test that HMAC_Init, etc., uses the empty key when called initially with
+   * key = NULL. */
+  assert(test->key_len == 0);
+  HMAC_CTX ctx;
+  HMAC_CTX_init(&ctx);
+  if (!HMAC_Init_ex(&ctx, NULL, 0, EVP_md5(), NULL) ||
+      !HMAC_Update(&ctx, test->data, test->data_len) ||
+      !HMAC_Final(&ctx, out, &out_len)) {
+    fprintf(stderr, "HMAC failed.\n");
     err++;
+  } else {
+    p = to_hex(out, out_len);
+    if (strcmp(p, test->hex_digest) != 0) {
+      fprintf(stderr, "got %s instead of %s\n", p, test->hex_digest);
+      err++;
+    }
   }
+  HMAC_CTX_cleanup(&ctx);
 
   if (err) {
     return 1;