From 4b464e7b46682f568a5df550426b0cf4b22e2485 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 12 Jun 2015 13:08:04 +0100 Subject: [PATCH] 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 --- crypto/hmac/hmac.c | 20 ++++++++------------ include/openssl/hmac.h | 1 - test/hmactest.c | 7 ++++++- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crypto/hmac/hmac.c b/crypto/hmac/hmac.c index d50fabb7c1..7699b0b1fe 100644 --- 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 61946fc456..81aa49da49 100644 --- 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 13344d69e5..a9b829d078 100644 --- 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; -- 2.34.1