Skip to content

Commit

Permalink
Fix ABI break with HMAC
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mattcaswell committed Jun 12, 2015
1 parent 063dccd commit 4b464e7
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
20 changes: 8 additions & 12 deletions crypto/hmac/hmac.c
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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))
Expand All @@ -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;
}

Expand All @@ -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:
Expand Down
1 change: 0 additions & 1 deletion include/openssl/hmac.h
Expand Up @@ -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))
Expand Down
7 changes: 6 additions & 1 deletion test/hmactest.c
Expand Up @@ -226,7 +226,12 @@ int main(int argc, char *argv[])
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;
Expand Down

0 comments on commit 4b464e7

Please sign in to comment.