Adapt HMAC to the EVP_MD_CTX changes
authorRichard Levitte <levitte@openssl.org>
Fri, 27 Nov 2015 13:10:15 +0000 (14:10 +0100)
committerRichard Levitte <levitte@openssl.org>
Mon, 7 Dec 2015 16:36:57 +0000 (17:36 +0100)
This change required some special treatment, as HMAC is intertwined
with EVP_MD.  For now, all local HMAC_CTX variables MUST be
initialised with HMAC_CTX_EMPTY, or whatever happens to be on the
stack will be mistaken for actual pointers to EVP_MD_CTX.  This will
change as soon as HMAC_CTX becomes opaque.

Also, since HMAC_CTX_init() can fail now, its return type changes from
void to int, and it will return 0 on failure, 1 on success.

Reviewed-by: Rich Salz <rsalz@openssl.org>
apps/speed.c
crypto/engine/eng_openssl.c
crypto/evp/p5_crpt2.c
crypto/hmac/hm_pmeth.c
crypto/hmac/hmac.c
crypto/pkcs12/p12_mutl.c
include/openssl/hmac.h
ssl/statem/statem_srvr.c
ssl/t1_lib.c
test/hmactest.c

index 68530b1..c90729c 100644 (file)
@@ -1298,7 +1298,7 @@ int speed_main(int argc, char **argv)
 
 #if !defined(OPENSSL_NO_MD5)
     if (doit[D_HMAC]) {
-        HMAC_CTX hctx;
+        HMAC_CTX hctx = HMAC_CTX_EMPTY;
 
         HMAC_CTX_init(&hctx);
         HMAC_Init_ex(&hctx, (unsigned char *)"This is a key...",
index ba9adf0..8927ee1 100644 (file)
@@ -448,7 +448,8 @@ static int ossl_hmac_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src)
     sctx = EVP_PKEY_CTX_get_data(src);
     dctx = EVP_PKEY_CTX_get_data(dst);
     dctx->md = sctx->md;
-    HMAC_CTX_init(&dctx->ctx);
+    /* Because HMAC_CTX_copy does HMAC_CTX_init */
+    HMAC_CTX_cleanup(&dctx->ctx);
     if (!HMAC_CTX_copy(&dctx->ctx, &sctx->ctx))
         return 0;
     if (sctx->ktmp.data) {
index 4986a21..dcc0463 100644 (file)
@@ -85,7 +85,7 @@ int PKCS5_PBKDF2_HMAC(const char *pass, int passlen,
     unsigned char digtmp[EVP_MAX_MD_SIZE], *p, itmp[4];
     int cplen, j, k, tkeylen, mdlen;
     unsigned long i = 1;
-    HMAC_CTX hctx_tpl, hctx;
+    HMAC_CTX hctx_tpl = HMAC_CTX_EMPTY, hctx = HMAC_CTX_EMPTY;
 
     mdlen = EVP_MD_size(digest);
     if (mdlen < 0)
index e06a1db..e603764 100644 (file)
@@ -96,7 +96,6 @@ static int pkey_hmac_copy(EVP_PKEY_CTX *dst, EVP_PKEY_CTX *src)
     sctx = src->data;
     dctx = dst->data;
     dctx->md = sctx->md;
-    HMAC_CTX_init(&dctx->ctx);
     if (!HMAC_CTX_copy(&dctx->ctx, &sctx->ctx))
         return 0;
     if (sctx->ktmp.data) {
@@ -111,9 +110,12 @@ static void pkey_hmac_cleanup(EVP_PKEY_CTX *ctx)
 {
     HMAC_PKEY_CTX *hctx = ctx->data;
 
-    HMAC_CTX_cleanup(&hctx->ctx);
-    OPENSSL_clear_free(hctx->ktmp.data, hctx->ktmp.length);
-    OPENSSL_free(hctx);
+    if (hctx != NULL) {
+        HMAC_CTX_cleanup(&hctx->ctx);
+        OPENSSL_clear_free(hctx->ktmp.data, hctx->ktmp.length);
+        OPENSSL_free(hctx);
+        ctx->data = NULL;
+    }
 }
 
 static int pkey_hmac_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
@@ -132,7 +134,7 @@ static int pkey_hmac_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 
 static int int_update(EVP_MD_CTX *ctx, const void *data, size_t count)
 {
-    HMAC_PKEY_CTX *hctx = ctx->pctx->data;
+    HMAC_PKEY_CTX *hctx = EVP_MD_CTX_pkey_ctx(ctx)->data;
     if (!HMAC_Update(&hctx->ctx, data, count))
         return 0;
     return 1;
@@ -141,9 +143,10 @@ static int int_update(EVP_MD_CTX *ctx, const void *data, size_t count)
 static int hmac_signctx_init(EVP_PKEY_CTX *ctx, EVP_MD_CTX *mctx)
 {
     HMAC_PKEY_CTX *hctx = ctx->data;
-    HMAC_CTX_set_flags(&hctx->ctx, mctx->flags & ~EVP_MD_CTX_FLAG_NO_INIT);
+    HMAC_CTX_set_flags(&hctx->ctx,
+                       EVP_MD_CTX_test_flags(mctx, ~EVP_MD_CTX_FLAG_NO_INIT));
     EVP_MD_CTX_set_flags(mctx, EVP_MD_CTX_FLAG_NO_INIT);
-    mctx->update = int_update;
+    EVP_MD_CTX_set_update_fn(mctx, int_update);
     return 1;
 }
 
index 7699b0b..e0bfbb1 100644 (file)
@@ -83,14 +83,14 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
 
     if (key != NULL) {
         reset = 1;
-        j = M_EVP_MD_block_size(md);
+        j = EVP_MD_block_size(md);
         OPENSSL_assert(j <= (int)sizeof(ctx->key));
         if (j < len) {
-            if (!EVP_DigestInit_ex(&ctx->md_ctx, md, impl))
+            if (!EVP_DigestInit_ex(ctx->md_ctx, md, impl))
                 goto err;
-            if (!EVP_DigestUpdate(&ctx->md_ctx, key, len))
+            if (!EVP_DigestUpdate(ctx->md_ctx, key, len))
                 goto err;
-            if (!EVP_DigestFinal_ex(&(ctx->md_ctx), ctx->key,
+            if (!EVP_DigestFinal_ex(ctx->md_ctx, ctx->key,
                                     &ctx->key_length))
                 goto err;
         } else {
@@ -107,19 +107,19 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
     if (reset) {
         for (i = 0; i < HMAC_MAX_MD_CBLOCK; i++)
             pad[i] = 0x36 ^ ctx->key[i];
-        if (!EVP_DigestInit_ex(&ctx->i_ctx, md, impl))
+        if (!EVP_DigestInit_ex(ctx->i_ctx, md, impl))
             goto err;
-        if (!EVP_DigestUpdate(&ctx->i_ctx, pad, M_EVP_MD_block_size(md)))
+        if (!EVP_DigestUpdate(ctx->i_ctx, pad, EVP_MD_block_size(md)))
             goto err;
 
         for (i = 0; i < HMAC_MAX_MD_CBLOCK; i++)
             pad[i] = 0x5c ^ ctx->key[i];
-        if (!EVP_DigestInit_ex(&ctx->o_ctx, md, impl))
+        if (!EVP_DigestInit_ex(ctx->o_ctx, md, impl))
             goto err;
-        if (!EVP_DigestUpdate(&ctx->o_ctx, pad, M_EVP_MD_block_size(md)))
+        if (!EVP_DigestUpdate(ctx->o_ctx, pad, EVP_MD_block_size(md)))
             goto err;
     }
-    if (!EVP_MD_CTX_copy_ex(&ctx->md_ctx, &ctx->i_ctx))
+    if (!EVP_MD_CTX_copy_ex(ctx->md_ctx, ctx->i_ctx))
         goto err;
     return 1;
  err:
@@ -139,7 +139,7 @@ int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data, size_t len)
 {
     if (!ctx->md)
         return 0;
-    return EVP_DigestUpdate(&ctx->md_ctx, data, len);
+    return EVP_DigestUpdate(ctx->md_ctx, data, len);
 }
 
 int HMAC_Final(HMAC_CTX *ctx, unsigned char *md, unsigned int *len)
@@ -150,49 +150,70 @@ int HMAC_Final(HMAC_CTX *ctx, unsigned char *md, unsigned int *len)
     if (!ctx->md)
         goto err;
 
-    if (!EVP_DigestFinal_ex(&ctx->md_ctx, buf, &i))
+    if (!EVP_DigestFinal_ex(ctx->md_ctx, buf, &i))
         goto err;
-    if (!EVP_MD_CTX_copy_ex(&ctx->md_ctx, &ctx->o_ctx))
+    if (!EVP_MD_CTX_copy_ex(ctx->md_ctx, ctx->o_ctx))
         goto err;
-    if (!EVP_DigestUpdate(&ctx->md_ctx, buf, i))
+    if (!EVP_DigestUpdate(ctx->md_ctx, buf, i))
         goto err;
-    if (!EVP_DigestFinal_ex(&ctx->md_ctx, md, len))
+    if (!EVP_DigestFinal_ex(ctx->md_ctx, md, len))
         goto err;
     return 1;
  err:
     return 0;
 }
 
-void HMAC_CTX_init(HMAC_CTX *ctx)
+int 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);
+    if (ctx->i_ctx == NULL)
+        ctx->i_ctx = EVP_MD_CTX_create();
+    else
+        EVP_MD_CTX_init(ctx->i_ctx);
+    if (ctx->i_ctx == NULL)
+        goto err;
+    if (ctx->o_ctx == NULL)
+        ctx->o_ctx = EVP_MD_CTX_create();
+    else
+        EVP_MD_CTX_init(ctx->o_ctx);
+    if (ctx->o_ctx == NULL)
+        goto err;
+    if (ctx->md_ctx == NULL)
+        ctx->md_ctx = EVP_MD_CTX_create();
+    else
+        EVP_MD_CTX_init(ctx->md_ctx);
+    if (ctx->md_ctx == NULL)
+        goto err;
     ctx->md = NULL;
+    return 1;
+ err:
+    HMAC_CTX_cleanup(ctx);
+    return 0;
 }
 
 int HMAC_CTX_copy(HMAC_CTX *dctx, HMAC_CTX *sctx)
 {
-    HMAC_CTX_init(dctx);
-    if (!EVP_MD_CTX_copy_ex(&dctx->i_ctx, &sctx->i_ctx))
+    if (!HMAC_CTX_init(dctx))
+        goto err;
+    if (!EVP_MD_CTX_copy_ex(dctx->i_ctx, sctx->i_ctx))
         goto err;
-    if (!EVP_MD_CTX_copy_ex(&dctx->o_ctx, &sctx->o_ctx))
+    if (!EVP_MD_CTX_copy_ex(dctx->o_ctx, sctx->o_ctx))
         goto err;
-    if (!EVP_MD_CTX_copy_ex(&dctx->md_ctx, &sctx->md_ctx))
+    if (!EVP_MD_CTX_copy_ex(dctx->md_ctx, sctx->md_ctx))
         goto err;
     memcpy(dctx->key, sctx->key, HMAC_MAX_MD_CBLOCK);
     dctx->key_length = sctx->key_length;
     dctx->md = sctx->md;
     return 1;
  err:
+    HMAC_CTX_cleanup(dctx);
     return 0;
 }
 
 void HMAC_CTX_cleanup(HMAC_CTX *ctx)
 {
-    EVP_MD_CTX_cleanup(&ctx->i_ctx);
-    EVP_MD_CTX_cleanup(&ctx->o_ctx);
-    EVP_MD_CTX_cleanup(&ctx->md_ctx);
+    EVP_MD_CTX_destroy(ctx->i_ctx);
+    EVP_MD_CTX_destroy(ctx->o_ctx);
+    EVP_MD_CTX_destroy(ctx->md_ctx);
     memset(ctx, 0, sizeof(*ctx));
 }
 
@@ -200,7 +221,7 @@ unsigned char *HMAC(const EVP_MD *evp_md, const void *key, int key_len,
                     const unsigned char *d, size_t n, unsigned char *md,
                     unsigned int *md_len)
 {
-    HMAC_CTX c;
+    HMAC_CTX c = HMAC_CTX_EMPTY;
     static unsigned char m[EVP_MAX_MD_SIZE];
 
     if (md == NULL)
@@ -221,7 +242,7 @@ unsigned char *HMAC(const EVP_MD *evp_md, const void *key, int key_len,
 
 void HMAC_CTX_set_flags(HMAC_CTX *ctx, unsigned long flags)
 {
-    M_EVP_MD_CTX_set_flags(&ctx->i_ctx, flags);
-    M_EVP_MD_CTX_set_flags(&ctx->o_ctx, flags);
-    M_EVP_MD_CTX_set_flags(&ctx->md_ctx, flags);
+    EVP_MD_CTX_set_flags(ctx->i_ctx, flags);
+    EVP_MD_CTX_set_flags(ctx->o_ctx, flags);
+    EVP_MD_CTX_set_flags(ctx->md_ctx, flags);
 }
index 4cf68e1..46a04b4 100644 (file)
@@ -91,7 +91,7 @@ int PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
                    unsigned char *mac, unsigned int *maclen)
 {
     const EVP_MD *md_type;
-    HMAC_CTX hmac;
+    HMAC_CTX hmac = HMAC_CTX_EMPTY;
     unsigned char key[EVP_MAX_MD_SIZE], *salt;
     int saltlen, iter;
     int md_size = 0;
index 011e2ae..7962087 100644 (file)
@@ -77,9 +77,10 @@ typedef struct hmac_ctx_st {
     unsigned char key[HMAC_MAX_MD_CBLOCK];
 } HMAC_CTX;
 
+# define HMAC_CTX_EMPTY      { NULL, NULL, NULL, NULL, 0, "" }
 # define HMAC_size(e)    (EVP_MD_size((e)->md))
 
-void HMAC_CTX_init(HMAC_CTX *ctx);
+int HMAC_CTX_init(HMAC_CTX *ctx);
 void HMAC_CTX_cleanup(HMAC_CTX *ctx);
 
 #ifdef OPENSSL_USE_DEPRECATED
index a39e288..09718c5 100644 (file)
@@ -3151,7 +3151,7 @@ int tls_construct_new_session_ticket(SSL *s)
 {
     unsigned char *senc = NULL;
     EVP_CIPHER_CTX ctx;
-    HMAC_CTX hctx;
+    HMAC_CTX hctx = HMAC_CTX_EMPTY;
     unsigned char *p, *macstart;
     const unsigned char *const_p;
     int len, slen_full, slen;
index 971aad3..622bdd9 100644 (file)
@@ -3041,7 +3041,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     const unsigned char *p;
     int slen, mlen, renew_ticket = 0;
     unsigned char tick_hmac[EVP_MAX_MD_SIZE];
-    HMAC_CTX hctx;
+    HMAC_CTX hctx = HMAC_CTX_EMPTY;
     EVP_CIPHER_CTX ctx;
     SSL_CTX *tctx = s->initial_ctx;
     /* Need at least keyname + iv + some encrypted data */
index f8d5350..20c7a8f 100644 (file)
@@ -134,7 +134,7 @@ int main(int argc, char *argv[])
     char *p;
 # endif
     int err = 0;
-    HMAC_CTX ctx, ctx2;
+    HMAC_CTX ctx = HMAC_CTX_EMPTY, ctx2 = HMAC_CTX_EMPTY;
     unsigned char buf[EVP_MAX_MD_SIZE];
     unsigned int len;