Fix ABI break with HMAC

Recent HMAC changes broke ABI compatibility due to a new field in HMAC_CTX.
This backs that change out, and does it a different way.

Thanks to Timo Teras for the concept.

Reviewed-by: Richard Levitte <levitte@openssl.org>

diff --git a/crypto/hmac/hmac.c b/crypto/hmac/hmac.c
index d50fabb7c11faa4c96e42b28ffb2db13ea2c086f..7699b0b1fe1e0fb6f218d08341996f4a178ff8c6 100644 (file)
--- a/crypto/hmac/hmac.c
+++ b/crypto/hmac/hmac.c
@@ -68,6 +68,10 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
     int i, j, reset = 0;
     unsigned char pad[HMAC_MAX_MD_CBLOCK];
 
+    /* If we are changing MD then we must have a key */
+    if (md != NULL && md != ctx->md && (key == NULL || len < 0))
+        return 0;
+
     if (md != NULL) {
         reset = 1;
         ctx->md = md;
@@ -77,9 +81,6 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
         return 0;
     }
 
-    if (!ctx->key_init && key == NULL)
-        return 0;
-
     if (key != NULL) {
         reset = 1;
         j = M_EVP_MD_block_size(md);
@@ -101,7 +102,6 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
         if (ctx->key_length != HMAC_MAX_MD_CBLOCK)
             memset(&ctx->key[ctx->key_length], 0,
                    HMAC_MAX_MD_CBLOCK - ctx->key_length);
-        ctx->key_init = 1;
     }
 
     if (reset) {
@@ -137,7 +137,7 @@ int HMAC_Init(HMAC_CTX *ctx, const void *key, int len, const EVP_MD *md)
 
 int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data, size_t len)
 {
-    if (!ctx->key_init)
+    if (!ctx->md)
         return 0;
     return EVP_DigestUpdate(&ctx->md_ctx, data, len);
 }
@@ -147,7 +147,7 @@ int HMAC_Final(HMAC_CTX *ctx, unsigned char *md, unsigned int *len)
     unsigned int i;
     unsigned char buf[EVP_MAX_MD_SIZE];
 
-    if (!ctx->key_init)
+    if (!ctx->md)
         goto err;
 
     if (!EVP_DigestFinal_ex(&ctx->md_ctx, buf, &i))
@@ -168,7 +168,6 @@ void HMAC_CTX_init(HMAC_CTX *ctx)
     EVP_MD_CTX_init(&ctx->i_ctx);
     EVP_MD_CTX_init(&ctx->o_ctx);
     EVP_MD_CTX_init(&ctx->md_ctx);
-    ctx->key_init = 0;
     ctx->md = NULL;
 }
 
@@ -181,11 +180,8 @@ int HMAC_CTX_copy(HMAC_CTX *dctx, HMAC_CTX *sctx)
         goto err;
     if (!EVP_MD_CTX_copy_ex(&dctx->md_ctx, &sctx->md_ctx))
         goto err;
-    dctx->key_init = sctx->key_init;
-    if (sctx->key_init) {
-        memcpy(dctx->key, sctx->key, HMAC_MAX_MD_CBLOCK);
-        dctx->key_length = sctx->key_length;
-    }
+    memcpy(dctx->key, sctx->key, HMAC_MAX_MD_CBLOCK);
+    dctx->key_length = sctx->key_length;
     dctx->md = sctx->md;
     return 1;
  err:

diff --git a/include/openssl/hmac.h b/include/openssl/hmac.h
index 61946fc4567bac34664b6c078b1668952bed57ef..81aa49da49969d42aee5dd1fad8d0ad1e1b5f03d 100644 (file)
--- a/include/openssl/hmac.h
+++ b/include/openssl/hmac.h
@@ -75,7 +75,6 @@ typedef struct hmac_ctx_st {
     EVP_MD_CTX o_ctx;
     unsigned int key_length;
     unsigned char key[HMAC_MAX_MD_CBLOCK];
-    int key_init;
 } HMAC_CTX;
 
 # define HMAC_size(e)    (EVP_MD_size((e)->md))

diff --git a/test/hmactest.c b/test/hmactest.c
index 13344d69e52d351fd0410a8be91bd5870324a622..a9b829d078aacec20059eec9748bb8fd9bd7289b 100644 (file)
--- a/test/hmactest.c
+++ b/test/hmactest.c
@@ -226,7 +226,12 @@ test5:
         err++;
         goto test6;
     }
-    if (!HMAC_Init_ex(&ctx, NULL, 0, EVP_sha256(), NULL)) {
+    if (HMAC_Init_ex(&ctx, NULL, 0, EVP_sha256(), NULL)) {
+        printf("Should disallow changing MD without a new key (test 5)\n");
+        err++;
+        goto test6;
+    }
+    if (!HMAC_Init_ex(&ctx, test[4].key, test[4].key_len, EVP_sha256(), NULL)) {
         printf("Failed to reinitialise HMAC (test 5)\n");
         err++;
         goto test6;