From: Matt Caswell <matt@openssl.org>
Date: Fri, 20 Mar 2020 12:37:20 +0000 (+0000)
Subject: Use a fetched cipher for the TLSv1.3 early secret
X-Git-Tag: openssl-3.0.0-alpha1~205
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=9727f4e7fd02e55b637058249cd8e1bc80501c7f;p=oweals%2Fopenssl.git

Use a fetched cipher for the TLSv1.3 early secret

We should use an explicitly fetched cipher to ensure that we are using
the correct libctx and property query.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11402)
---

diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index 64c791636a..23d156a702 100644
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -439,6 +439,32 @@ static int load_builtin_compressions(void)
 }
 #endif
 
+int ssl_cipher_get_evp_cipher(SSL_CTX *ctx, const SSL_CIPHER *sslc,
+                              const EVP_CIPHER **enc)
+{
+    int i = ssl_cipher_info_lookup(ssl_cipher_table_cipher, sslc->algorithm_enc);
+
+    if (i == -1) {
+        *enc = NULL;
+    } else {
+        if (i == SSL_ENC_NULL_IDX) {
+            /*
+             * We assume we don't care about this coming from an ENGINE so
+             * just do a normal EVP_CIPHER_fetch instead of
+             * ssl_evp_cipher_fetch()
+             */
+            *enc = EVP_CIPHER_fetch(ctx->libctx, "NULL", ctx->propq);
+            if (*enc == NULL)
+                return 0;
+        } else {
+            if (!ssl_evp_cipher_up_ref(ctx->ssl_cipher_methods[i]))
+                return 0;
+            *enc = ctx->ssl_cipher_methods[i];
+        }
+    }
+    return 1;
+}
+
 int ssl_cipher_get_evp(SSL_CTX *ctx, const SSL_SESSION *s,
                        const EVP_CIPHER **enc, const EVP_MD **md,
                        int *mac_pkey_type, size_t *mac_secret_size,
@@ -474,24 +500,8 @@ int ssl_cipher_get_evp(SSL_CTX *ctx, const SSL_SESSION *s,
     if ((enc == NULL) || (md == NULL))
         return 0;
 
-    i = ssl_cipher_info_lookup(ssl_cipher_table_cipher, c->algorithm_enc);
-
-    if (i == -1) {
-        *enc = NULL;
-    } else {
-        if (i == SSL_ENC_NULL_IDX) {
-            /*
-             * We assume we don't care about this coming from an ENGINE so
-             * just do a normal EVP_CIPHER_fetch instead of
-             * ssl_evp_cipher_fetch()
-             */
-            *enc = EVP_CIPHER_fetch(ctx->libctx, "NULL", ctx->propq);
-        } else {
-            if (!ssl_evp_cipher_up_ref(ctx->ssl_cipher_methods[i]))
-                return 0;
-            *enc = ctx->ssl_cipher_methods[i];
-        }
-    }
+    if (!ssl_cipher_get_evp_cipher(ctx, c, enc))
+        return 0;
 
     i = ssl_cipher_info_lookup(ssl_cipher_table_mac, c->algorithm_mac);
     if (i == -1) {
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index d9092161ff..c48bcb9a9a 100644
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2363,6 +2363,8 @@ __owur int bytes_to_cipher_list(SSL *s, PACKET *cipher_suites,
                                 STACK_OF(SSL_CIPHER) **scsvs, int sslv2format,
                                 int fatal);
 void ssl_update_cache(SSL *s, int mode);
+__owur int ssl_cipher_get_evp_cipher(SSL_CTX *ctx, const SSL_CIPHER *sslc,
+                                     const EVP_CIPHER **enc);
 __owur int ssl_cipher_get_evp(SSL_CTX *ctxc, const SSL_SESSION *s,
                               const EVP_CIPHER **enc, const EVP_MD **md,
                               int *mac_pkey_type, size_t *mac_secret_size,
diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index 9ca63b7550..fba12fe5e4 100644
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -599,7 +599,18 @@ int tls13_change_cipher_state(SSL *s, int which)
                          SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_MALLOC_FAILURE);
                 goto err;
             }
-            cipher = EVP_get_cipherbynid(SSL_CIPHER_get_cipher_nid(sslcipher));
+
+            /*
+             * This ups the ref count on cipher so we better make sure we free
+             * it again
+             */
+            if (!ssl_cipher_get_evp_cipher(s->ctx, sslcipher, &cipher)) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS13_CHANGE_CIPHER_STATE,
+                         SSL_R_ALGORITHM_FETCH_FAILED);
+                goto err;
+            }
+
             md = ssl_md(s->ctx, sslcipher->algorithm2);
             if (md == NULL || !EVP_DigestInit_ex(mdctx, md, NULL)
                     || !EVP_DigestUpdate(mdctx, hdata, handlen)
@@ -755,6 +766,10 @@ int tls13_change_cipher_state(SSL *s, int which)
         s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
     ret = 1;
  err:
+    if ((which & SSL3_CC_EARLY) != 0) {
+        /* We up-refed this so now we need to down ref */
+        ssl_evp_cipher_free(cipher);
+    }
     OPENSSL_cleanse(secret, sizeof(secret));
     return ret;
 }
diff --git a/test/tls13secretstest.c b/test/tls13secretstest.c
index 5d614768f8..c6f65eaded 100644
--- a/test/tls13secretstest.c
+++ b/test/tls13secretstest.c
@@ -165,6 +165,12 @@ void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl)
 {
 }
 
+int ssl_cipher_get_evp_cipher(SSL_CTX *ctx, const SSL_CIPHER *sslc,
+                                     const EVP_CIPHER **enc)
+{
+    return 0;
+}
+
 int ssl_cipher_get_evp(SSL_CTX *ctx, const SSL_SESSION *s,
                        const EVP_CIPHER **enc, const EVP_MD **md,
                        int *mac_pkey_type, size_t *mac_secret_size,