[openssl] master update

Richard Levitte levitte at openssl.org
Sun Jan 19 01:48:03 UTC 2020


The branch master has been updated
       via  bddbfae1cd667f0c1458deff98cbc03171e90d5a (commit)
       via  9767a3dca781563a3dcc20094610d8ed0cb6061e (commit)
       via  0a054d2a0b1ccab07587185245455093454fe353 (commit)
      from  ed5cb1776b4759b745984c0c67c2d6453345c0a1 (commit)


- Log -----------------------------------------------------------------
commit bddbfae1cd667f0c1458deff98cbc03171e90d5a
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Jan 10 14:16:30 2020 +0000

    libssl: Eliminate as much use of EVP_PKEY_size() as possible
    
    Some uses were going against documented recommendations.
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/10798)

commit 9767a3dca781563a3dcc20094610d8ed0cb6061e
Author: Richard Levitte <levitte at openssl.org>
Date:   Thu Jan 9 21:38:47 2020 +0100

    libcrypto: Eliminate as much use of EVP_PKEY_size() as possible
    
    Some uses were going against documented recommendations.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/10798)

commit 0a054d2a0b1ccab07587185245455093454fe353
Author: Richard Levitte <levitte at openssl.org>
Date:   Thu Jan 9 21:37:32 2020 +0100

    APPS & TEST: Eliminate as much use of EVP_PKEY_size() as possible
    
    Some uses were going against documented recommendations.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/10798)

-----------------------------------------------------------------------

Summary of changes:
 apps/dgst.c              | 13 +++++++++----
 crypto/asn1/a_sign.c     |  7 ++++++-
 crypto/rsa/rsa_ameth.c   |  3 ++-
 crypto/rsa/rsa_pmeth.c   |  4 ++--
 ssl/statem/statem_clnt.c | 17 -----------------
 ssl/statem/statem_lib.c  | 48 +++++++++++++++++++++++++++++-------------------
 ssl/statem/statem_srvr.c | 39 +++++++++++----------------------------
 test/evp_extra_test.c    |  8 +-------
 8 files changed, 60 insertions(+), 79 deletions(-)

diff --git a/apps/dgst.c b/apps/dgst.c
index 21f0f01787..7a81cb28dc 100644
--- a/apps/dgst.c
+++ b/apps/dgst.c
@@ -541,11 +541,16 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout,
     }
     if (key != NULL) {
         EVP_MD_CTX *ctx;
-        int pkey_len;
+        size_t tmplen;
+
         BIO_get_md_ctx(bp, &ctx);
-        pkey_len = EVP_PKEY_size(key);
-        if (pkey_len > BUFSIZE) {
-            len = pkey_len;
+        if (!EVP_DigestSignFinal(ctx, NULL, &tmplen)) {
+            BIO_printf(bio_err, "Error Signing Data\n");
+            ERR_print_errors(bio_err);
+            goto end;
+        }
+        if (tmplen > BUFSIZE) {
+            len = tmplen;
             sigbuf = app_malloc(len, "Signature buffer");
             buf = sigbuf;
         }
diff --git a/crypto/asn1/a_sign.c b/crypto/asn1/a_sign.c
index fdf25b204b..564a500cf4 100644
--- a/crypto/asn1/a_sign.c
+++ b/crypto/asn1/a_sign.c
@@ -216,7 +216,12 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it,
         goto err;
     }
     inl = buf_len;
-    outll = outl = EVP_PKEY_size(pkey);
+    if (!EVP_DigestSign(ctx, NULL, &outll, buf_in, inl)) {
+        outl = 0;
+        ASN1err(ASN1_F_ASN1_ITEM_SIGN_CTX, ERR_R_EVP_LIB);
+        goto err;
+    }
+    outl = outll;
     buf_out = OPENSSL_malloc(outll);
     if (buf_in == NULL || buf_out == NULL) {
         outl = 0;
diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c
index ade3fe2578..3246f33688 100644
--- a/crypto/rsa/rsa_ameth.c
+++ b/crypto/rsa/rsa_ameth.c
@@ -589,6 +589,7 @@ static RSA_PSS_PARAMS *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
 {
     const EVP_MD *sigmd, *mgf1md;
     EVP_PKEY *pk = EVP_PKEY_CTX_get0_pkey(pkctx);
+    RSA *rsa = EVP_PKEY_get0_RSA(pk);
     int saltlen;
 
     if (EVP_PKEY_CTX_get_signature_md(pkctx, &sigmd) <= 0)
@@ -600,7 +601,7 @@ static RSA_PSS_PARAMS *rsa_ctx_to_pss(EVP_PKEY_CTX *pkctx)
     if (saltlen == -1) {
         saltlen = EVP_MD_size(sigmd);
     } else if (saltlen == -2 || saltlen == -3) {
-        saltlen = EVP_PKEY_size(pk) - EVP_MD_size(sigmd) - 2;
+        saltlen = RSA_size(rsa) - EVP_MD_size(sigmd) - 2;
         if ((EVP_PKEY_bits(pk) & 0x7) == 1)
             saltlen--;
         if (saltlen < 0)
diff --git a/crypto/rsa/rsa_pmeth.c b/crypto/rsa/rsa_pmeth.c
index 390188d13a..34cbba658e 100644
--- a/crypto/rsa/rsa_pmeth.c
+++ b/crypto/rsa/rsa_pmeth.c
@@ -104,7 +104,7 @@ static int setup_tbuf(RSA_PKEY_CTX *ctx, EVP_PKEY_CTX *pk)
 {
     if (ctx->tbuf != NULL)
         return 1;
-    if ((ctx->tbuf = OPENSSL_malloc(EVP_PKEY_size(pk->pkey))) == NULL) {
+    if ((ctx->tbuf = OPENSSL_malloc(RSA_size(pk->pkey->pkey.rsa))) == NULL) {
         RSAerr(RSA_F_SETUP_TBUF, ERR_R_MALLOC_FAILURE);
         return 0;
     }
@@ -147,7 +147,7 @@ static int pkey_rsa_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
                 return ret;
             ret = sltmp;
         } else if (rctx->pad_mode == RSA_X931_PADDING) {
-            if ((size_t)EVP_PKEY_size(ctx->pkey) < tbslen + 1) {
+            if ((size_t)RSA_size(rsa) < tbslen + 1) {
                 RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_KEY_SIZE_TOO_SMALL);
                 return -1;
             }
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 13610ba1b7..a13d2708b1 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2301,7 +2301,6 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
     /* if it was signed, check the signature */
     if (pkey != NULL) {
         PACKET params;
-        int maxsig;
         const EVP_MD *md = NULL;
         unsigned char *tbs;
         size_t tbslen;
@@ -2352,22 +2351,6 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt)
                      SSL_R_LENGTH_MISMATCH);
             goto err;
         }
-        maxsig = EVP_PKEY_size(pkey);
-        if (maxsig < 0) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_KEY_EXCHANGE,
-                     ERR_R_INTERNAL_ERROR);
-            goto err;
-        }
-
-        /*
-         * Check signature length
-         */
-        if (PACKET_remaining(&signature) > (size_t)maxsig) {
-            /* wrong packet length */
-            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_KEY_EXCHANGE,
-                   SSL_R_WRONG_SIGNATURE_LENGTH);
-            goto err;
-        }
 
         md_ctx = EVP_MD_CTX_new();
         if (md_ctx == NULL) {
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 960b345268..c478bb47aa 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -271,13 +271,6 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt)
                  ERR_R_INTERNAL_ERROR);
         goto err;
     }
-    siglen = EVP_PKEY_size(pkey);
-    sig = OPENSSL_malloc(siglen);
-    if (sig == NULL) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
-                 ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
 
     if (EVP_DigestSignInit(mctx, &pctx, md, NULL, pkey) <= 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
@@ -295,6 +288,10 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt)
         }
     }
     if (s->version == SSL3_VERSION) {
+        /*
+         * Here we use EVP_DigestSignUpdate followed by EVP_DigestSignFinal
+         * in order to add the EVP_CTRL_SSL3_MASTER_SECRET call between them.
+         */
         if (EVP_DigestSignUpdate(mctx, hdata, hdatalen) <= 0
             /*
              * TODO(3.0) Replace this when EVP_MD_CTX_ctrl() is deprecated
@@ -303,16 +300,36 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt)
             || EVP_MD_CTX_ctrl(mctx, EVP_CTRL_SSL3_MASTER_SECRET,
                                (int)s->session->master_key_length,
                                s->session->master_key) <= 0
-            || EVP_DigestSignFinal(mctx, sig, &siglen) <= 0) {
+            || EVP_DigestSignFinal(mctx, NULL, &siglen) <= 0) {
 
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
                      ERR_R_EVP_LIB);
             goto err;
         }
-    } else if (EVP_DigestSign(mctx, sig, &siglen, hdata, hdatalen) <= 0) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
-                 ERR_R_EVP_LIB);
-        goto err;
+        sig = OPENSSL_malloc(siglen);
+        if (sig == NULL
+                || EVP_DigestSignFinal(mctx, sig, &siglen) <= 0) {
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
+                     ERR_R_EVP_LIB);
+            goto err;
+        }
+    } else {
+        /*
+         * Here we *must* use EVP_DigestSign() because Ed25519/Ed448 does not
+         * support streaming via EVP_DigestSignUpdate/EVP_DigestSignFinal
+         */
+        if (EVP_DigestSign(mctx, NULL, &siglen, hdata, hdatalen) <= 0) {
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
+                     ERR_R_EVP_LIB);
+            goto err;
+        }
+        sig = OPENSSL_malloc(siglen);
+        if (sig == NULL
+                || EVP_DigestSign(mctx, sig, &siglen, hdata, hdatalen) <= 0) {
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY,
+                     ERR_R_EVP_LIB);
+            goto err;
+        }
     }
 
 #ifndef OPENSSL_NO_GOST
@@ -434,13 +451,6 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
         goto err;
     }
 
-    j = EVP_PKEY_size(pkey);
-    if (((int)len > j) || ((int)PACKET_remaining(pkt) > j)
-        || (PACKET_remaining(pkt) == 0)) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_CERT_VERIFY,
-                 SSL_R_WRONG_SIGNATURE_SIZE);
-        goto err;
-    }
     if (!PACKET_get_bytes(pkt, &data, len)) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_CERT_VERIFY,
                  SSL_R_LENGTH_MISMATCH);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 4e4005f61d..c744bf64eb 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2762,8 +2762,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
         EVP_PKEY *pkey = s->s3.tmp.cert->privatekey;
         const EVP_MD *md;
         unsigned char *sigbytes1, *sigbytes2, *tbs;
-        size_t siglen, tbslen;
-        int rv;
+        size_t siglen = 0, tbslen;
 
         if (pkey == NULL || !tls1_lookup_md(lu, &md)) {
             /* Should never happen */
@@ -2786,15 +2785,8 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
                      ERR_R_INTERNAL_ERROR);
             goto err;
         }
-        /*
-         * Create the signature. We don't know the actual length of the sig
-         * until after we've created it, so we reserve enough bytes for it
-         * up front, and then properly allocate them in the WPACKET
-         * afterwards.
-         */
-        siglen = EVP_PKEY_size(pkey);
-        if (!WPACKET_sub_reserve_bytes_u16(pkt, siglen, &sigbytes1)
-            || EVP_DigestSignInit(md_ctx, &pctx, md, NULL, pkey) <= 0) {
+
+        if (EVP_DigestSignInit(md_ctx, &pctx, md, NULL, pkey) <= 0) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                      ERR_R_INTERNAL_ERROR);
@@ -2816,15 +2808,19 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
             /* SSLfatal() already called */
             goto err;
         }
-        rv = EVP_DigestSign(md_ctx, sigbytes1, &siglen, tbs, tbslen);
-        OPENSSL_free(tbs);
-        if (rv <= 0 || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2)
-            || sigbytes1 != sigbytes2) {
+
+        if (EVP_DigestSign(md_ctx, NULL, &siglen, tbs, tbslen) <=0
+                || !WPACKET_sub_reserve_bytes_u16(pkt, siglen, &sigbytes1)
+                || EVP_DigestSign(md_ctx, sigbytes1, &siglen, tbs, tbslen) <= 0
+                || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2)
+                || sigbytes1 != sigbytes2) {
+            OPENSSL_free(tbs);
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                      ERR_R_INTERNAL_ERROR);
             goto err;
         }
+        OPENSSL_free(tbs);
     }
 
     EVP_MD_CTX_free(md_ctx);
@@ -3009,19 +3005,6 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt)
         }
     }
 
-    /*
-     * We want to be sure that the plaintext buffer size makes it safe to
-     * iterate over the entire size of a premaster secret
-     * (SSL_MAX_MASTER_KEY_LENGTH). Reject overly short RSA keys because
-     * their ciphertext cannot accommodate a premaster secret anyway.
-     */
-    if (EVP_PKEY_size(rsa) < RSA_PKCS1_PADDING_SIZE
-                             + SSL_MAX_MASTER_KEY_LENGTH) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CKE_RSA,
-                 RSA_R_KEY_SIZE_TOO_SMALL);
-        return 0;
-    }
-
     outlen = SSL_MAX_MASTER_KEY_LENGTH;
     rsa_decrypt = OPENSSL_malloc(outlen);
     if (rsa_decrypt == NULL) {
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index e7e73cd150..5f2bcc1a51 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -582,10 +582,7 @@ static int test_EVP_DigestSignInit(int tst)
 
     /* Determine the size of the signature. */
     if (!TEST_true(EVP_DigestSignFinal(md_ctx, NULL, &sig_len))
-            || !TEST_size_t_le(sig_len, (size_t)EVP_PKEY_size(pkey)))
-        goto out;
-
-    if (!TEST_ptr(sig = OPENSSL_malloc(sig_len))
+            || !TEST_ptr(sig = OPENSSL_malloc(sig_len))
             || !TEST_true(EVP_DigestSignFinal(md_ctx, sig, &sig_len)))
         goto out;
 
@@ -919,9 +916,6 @@ static int test_EVP_SM2(void)
     if (!TEST_true(EVP_DigestSignFinal(md_ctx, NULL, &sig_len)))
         goto done;
 
-    if (!TEST_size_t_eq(sig_len, (size_t)EVP_PKEY_size(pkey)))
-        goto done;
-
     if (!TEST_ptr(sig = OPENSSL_malloc(sig_len)))
         goto done;
 


More information about the openssl-commits mailing list