From f695571e10a3e63930424940acc8dafd13c6c35c Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Mon, 13 Feb 2017 18:07:00 +0000 Subject: [PATCH] Simplify tls_construct_server_key_exchange Use negotiated signature algorithm and certificate index in tls_construct_key_exchange instead of recalculating it. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2623) --- ssl/statem/statem_srvr.c | 130 +++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 72 deletions(-) diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 94a7caf31b..a251a6ff8c 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1996,8 +1996,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) size_t encodedlen = 0; int curve_id = 0; #endif - EVP_PKEY *pkey; - const EVP_MD *md = NULL; + const SIGALG_LOOKUP *lu = s->s3->tmp.sigalg; int al = SSL_AD_INTERNAL_ERROR, i; unsigned long type; const BIGNUM *r[4]; @@ -2154,16 +2153,12 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) goto f_err; } - if (!(s->s3->tmp.new_cipher->algorithm_auth & (SSL_aNULL | SSL_aSRP)) - && !(s->s3->tmp.new_cipher->algorithm_mkey & SSL_PSK)) { - if (s->s3->tmp.cert_idx == -1 || s->s3->tmp.sigalg == NULL) { - al = SSL_AD_DECODE_ERROR; - goto f_err; - } - pkey = s->cert->pkeys[s->s3->tmp.cert_idx].privatekey; - md = ssl_md(s->s3->tmp.sigalg->hash_idx); - } else { - pkey = NULL; + if (((s->s3->tmp.new_cipher->algorithm_auth & (SSL_aNULL | SSL_aSRP)) != 0) + || ((s->s3->tmp.new_cipher->algorithm_mkey & SSL_PSK)) != 0) { + lu = NULL; + } else if (lu == NULL) { + al = SSL_AD_DECODE_ERROR; + goto f_err; } #ifndef OPENSSL_NO_PSK @@ -2253,75 +2248,66 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) #endif /* not anonymous */ - if (pkey != NULL) { + if (lu != NULL) { + EVP_PKEY *pkey = s->cert->pkeys[s->s3->tmp.cert_idx].privatekey; + const EVP_MD *md = ssl_md(lu->hash_idx); + unsigned char *sigbytes1, *sigbytes2; + size_t siglen; + + if (pkey == NULL || md == NULL) { + /* Should never happen */ + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + goto f_err; + } /* * n is the length of the params, they start at &(d[4]) and p * points to the space at the end. */ - if (md) { - unsigned char *sigbytes1, *sigbytes2; - size_t siglen; - int ispss = 0; - /* Get length of the parameters we have written above */ - if (!WPACKET_get_length(pkt, ¶mlen)) { - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); - goto f_err; - } - /* send signature algorithm */ - if (SSL_USE_SIGALGS(s)) { - if (!tls12_get_sigandhash(s, pkt, pkey, md, &ispss)) { - /* Should never happen */ - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); - goto f_err; - } - } -#ifdef SSL_DEBUG - fprintf(stderr, "Using hash %s\n", EVP_MD_name(md)); -#endif - /* - * 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) { - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); - goto f_err; - } - if (ispss) { - if (EVP_PKEY_CTX_set_rsa_padding(pctx, - RSA_PKCS1_PSS_PADDING) <= 0 - || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_DIGEST) <= 0) { - SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - ERR_R_EVP_LIB); - goto f_err; - } - } - if (EVP_DigestSignUpdate(md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_DigestSignUpdate(md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_DigestSignUpdate(md_ctx, - s->init_buf->data + paramoffset, - paramlen) <= 0 - || EVP_DigestSignFinal(md_ctx, sigbytes1, &siglen) <= 0 - || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2) - || sigbytes1 != sigbytes2) { + /* Get length of the parameters we have written above */ + if (!WPACKET_get_length(pkt, ¶mlen)) { + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + goto f_err; + } + /* send signature algorithm */ + if (SSL_USE_SIGALGS(s) && !WPACKET_put_bytes_u16(pkt, lu->sigalg)) + return 0; + /* + * 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) { + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + goto f_err; + } + if (lu->sig == EVP_PKEY_RSA_PSS) { + if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0 + || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, RSA_PSS_SALTLEN_DIGEST) <= 0) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - ERR_R_INTERNAL_ERROR); + ERR_R_EVP_LIB); goto f_err; } - } else { - /* Is this error check actually needed? */ - al = SSL_AD_HANDSHAKE_FAILURE; + } + if (EVP_DigestSignUpdate(md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestSignUpdate(md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestSignUpdate(md_ctx, + s->init_buf->data + paramoffset, + paramlen) <= 0 + || EVP_DigestSignFinal(md_ctx, sigbytes1, &siglen) <= 0 + || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2) + || sigbytes1 != sigbytes2) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, - SSL_R_UNKNOWN_PKEY_TYPE); + ERR_R_INTERNAL_ERROR); goto f_err; } } -- 2.25.1