From e1e588acae078f05159683876540235d8a70a648 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 8 Jul 2016 15:41:36 +0100 Subject: [PATCH] Tidy up tls_process_key_exchange() After the refactor of tls_process_key_exchange(), this commit tidies up some loose ends. Reviewed-by: Richard Levitte --- ssl/statem/statem_clnt.c | 73 ++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index d5c6e87158..1b184ac282 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1574,29 +1574,17 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) { - EVP_MD_CTX *md_ctx; int al = -1; - long alg_k, alg_a; + long alg_k; EVP_PKEY *pkey = NULL; PACKET save_param_start, signature; - md_ctx = EVP_MD_CTX_new(); - if (md_ctx == NULL) { - al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); - goto f_err; - } - alg_k = s->s3->tmp.new_cipher->algorithm_mkey; save_param_start = *pkt; -#if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) EVP_PKEY_free(s->s3->peer_tmp); s->s3->peer_tmp = NULL; -#endif - - alg_a = s->s3->tmp.new_cipher->algorithm_auth; if (alg_k & SSL_PSK) { if (!tls_process_ske_psk_preamble(s, pkt, &al)) @@ -1617,7 +1605,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) } else if (alg_k) { al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_UNEXPECTED_MESSAGE); - goto f_err; + goto err; } /* if it was signed, check the signature */ @@ -1625,6 +1613,8 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) PACKET params; int maxsig; const EVP_MD *md = NULL; + EVP_MD_CTX *md_ctx; + /* * |pkt| now points to the beginning of the signature, so the difference * equals the length of the parameters. @@ -1634,21 +1624,24 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) PACKET_remaining(pkt))) { al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); - goto f_err; + goto err; } if (SSL_USE_SIGALGS(s)) { const unsigned char *sigalgs; int rv; if (!PACKET_get_bytes(pkt, &sigalgs, 2)) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT); - goto f_err; + goto err; } rv = tls12_check_peer_sigalg(&md, s, sigalgs, pkey); - if (rv == -1) + if (rv == -1) { + al = SSL_AD_INTERNAL_ERROR; + goto err; + } else if (rv == 0) { + al = SSL_AD_DECODE_ERROR; goto err; - else if (rv == 0) { - goto f_err; } #ifdef SSL_DEBUG fprintf(stderr, "USING TLSv1.2 HASH %s\n", EVP_MD_name(md)); @@ -1661,13 +1654,15 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) if (!PACKET_get_length_prefixed_2(pkt, &signature) || PACKET_remaining(pkt) != 0) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_LENGTH_MISMATCH); - goto f_err; + goto err; } maxsig = EVP_PKEY_size(pkey); if (maxsig < 0) { + al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); - goto f_err; + goto err; } /* @@ -1675,9 +1670,18 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) */ if (PACKET_remaining(&signature) > (size_t)maxsig) { /* wrong packet length */ + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_WRONG_SIGNATURE_LENGTH); - goto f_err; + goto err; } + + md_ctx = EVP_MD_CTX_new(); + if (md_ctx == NULL) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE); + goto err; + } + if (EVP_VerifyInit_ex(md_ctx, md, NULL) <= 0 || EVP_VerifyUpdate(md_ctx, &(s->s3->client_random[0]), SSL3_RANDOM_SIZE) <= 0 @@ -1685,41 +1689,46 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) SSL3_RANDOM_SIZE) <= 0 || EVP_VerifyUpdate(md_ctx, PACKET_data(¶ms), PACKET_remaining(¶ms)) <= 0) { + EVP_MD_CTX_free(md_ctx); al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); - goto f_err; + goto err; } if (EVP_VerifyFinal(md_ctx, PACKET_data(&signature), PACKET_remaining(&signature), pkey) <= 0) { /* bad signature */ + EVP_MD_CTX_free(md_ctx); al = SSL_AD_DECRYPT_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_BAD_SIGNATURE); - goto f_err; + goto err; } + EVP_MD_CTX_free(md_ctx); } else { /* aNULL, aSRP or PSK do not need public keys */ - if (!(alg_a & (SSL_aNULL | SSL_aSRP)) && !(alg_k & SSL_PSK)) { + if (!(s->s3->tmp.new_cipher->algorithm_auth & (SSL_aNULL | SSL_aSRP)) + && !(alg_k & SSL_PSK)) { /* Might be wrong key type, check it */ - if (ssl3_check_cert_and_algorithm(s)) + if (ssl3_check_cert_and_algorithm(s)) { /* Otherwise this shouldn't happen */ + al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); + } else { + al = SSL_AD_DECODE_ERROR; + } goto err; } /* still data left over */ if (PACKET_remaining(pkt) != 0) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_EXTRA_DATA_IN_MESSAGE); - goto f_err; + goto err; } } - EVP_MD_CTX_free(md_ctx); + return MSG_PROCESS_CONTINUE_READING; - f_err: - if (al == -1) - al = SSL_AD_DECODE_ERROR; err: if (al != -1) ssl3_send_alert(s, SSL3_AL_FATAL, al); - EVP_MD_CTX_free(md_ctx); ossl_statem_set_error(s); return MSG_PROCESS_ERROR; } -- 2.34.1