From 9727f4e7fd02e55b637058249cd8e1bc80501c7f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 20 Mar 2020 12:37:20 +0000 Subject: 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 (Merged from https://github.com/openssl/openssl/pull/11402) --- ssl/ssl_ciph.c | 46 ++++++++++++++++++++++++++++------------------ ssl/ssl_local.h | 2 ++ ssl/tls13_enc.c | 17 ++++++++++++++++- 3 files changed, 46 insertions(+), 19 deletions(-) (limited to 'ssl') 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; } -- cgit v1.2.3