Use a fetched MD if appropriate in ssl3_cbc_digest_record
authorMatt Caswell <matt@openssl.org>
Sat, 18 Apr 2020 10:54:23 +0000 (11:54 +0100)
committerMatt Caswell <matt@openssl.org>
Mon, 20 Apr 2020 10:29:31 +0000 (11:29 +0100)
HMACs used via the legacy EVP_DigestSign interface are strange in
that they use legacy codepath's which eventually (under the covers)
transform the operation into a new style EVP_MAC. This can mean the
digest in use can be a legacy one, so we need to be careful with any
digest we extract from the ctx.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11511)

ssl/record/ssl3_record.c
ssl/s3_cbc.c
ssl/ssl_local.h

index a5ef78d..23bbc95 100644 (file)
@@ -1270,7 +1270,7 @@ int n_ssl3_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
         header[j++] = (unsigned char)(rec->length & 0xff);
 
         /* Final param == is SSLv3 */
-        if (ssl3_cbc_digest_record(hash,
+        if (ssl3_cbc_digest_record(ssl, hash,
                                    md, &md_size,
                                    header, rec->input,
                                    rec->length + md_size, rec->orig_len,
@@ -1374,7 +1374,7 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
          * are hashing because that gives an attacker a timing-oracle.
          */
         /* Final param == not SSLv3 */
-        if (ssl3_cbc_digest_record(mac_ctx,
+        if (ssl3_cbc_digest_record(ssl, mac_ctx,
                                    md, &md_size,
                                    header, rec->input,
                                    rec->length + md_size, rec->orig_len,
index 888ff3c..bbc24cc 100644 (file)
@@ -131,7 +131,8 @@ char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx)
  * padding too. )
  * Returns 1 on success or 0 on error
  */
-int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
+int ssl3_cbc_digest_record(SSL *s,
+                           const EVP_MD_CTX *ctx,
                            unsigned char *md_out,
                            size_t *md_out_size,
                            const unsigned char header[13],
@@ -166,7 +167,8 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
      */
     size_t md_length_size = 8;
     char length_is_big_endian = 1;
-    int ret;
+    int ret = 0;
+    const EVP_MD *md = NULL;
 
     /*
      * This is a, hopefully redundant, check that allows us to forget about
@@ -461,7 +463,11 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
     md_ctx = EVP_MD_CTX_new();
     if (md_ctx == NULL)
         goto err;
-    if (EVP_DigestInit_ex(md_ctx, EVP_MD_CTX_md(ctx), NULL /* engine */ ) <= 0)
+    md = ssl_evp_md_fetch(s->ctx->libctx, EVP_MD_type(EVP_MD_CTX_md(ctx)),
+                          s->ctx->propq);
+    if (md == NULL)
+        goto err;
+    if (EVP_DigestInit_ex(md_ctx, md, NULL /* engine */ ) <= 0)
         goto err;
     if (is_sslv3) {
         /* We repurpose |hmac_pad| to contain the SSLv3 pad2 block. */
@@ -484,10 +490,10 @@ int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
     ret = EVP_DigestFinal(md_ctx, md_out, &md_out_size_u);
     if (ret && md_out_size)
         *md_out_size = md_out_size_u;
-    EVP_MD_CTX_free(md_ctx);
 
-    return 1;
+    ret = 1;
  err:
     EVP_MD_CTX_free(md_ctx);
-    return 0;
+    ssl_evp_md_free(md);
+    return ret;
 }
index d8b25bb..22cae48 100644 (file)
@@ -2713,7 +2713,8 @@ __owur int ssl_log_secret(SSL *ssl, const char *label,
 
 /* s3_cbc.c */
 __owur char ssl3_cbc_record_digest_supported(const EVP_MD_CTX *ctx);
-__owur int ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
+__owur int ssl3_cbc_digest_record(SSL *s,
+                                  const EVP_MD_CTX *ctx,
                                   unsigned char *md_out,
                                   size_t *md_out_size,
                                   const unsigned char header[13],