From: Matt Caswell Date: Thu, 23 Nov 2017 10:37:51 +0000 (+0000) Subject: Convert more functions in ssl/statem/statem.c to use SSLfatal() X-Git-Tag: OpenSSL_1_1_1-pre1~389 X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=d4d2f3a4c14113c20eaa9350cbbf32cfb3e4f10c Convert more functions in ssl/statem/statem.c to use SSLfatal() Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/4778) --- diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index d6644835bb..9a95662b11 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1195,6 +1195,7 @@ SSL_F_STATE_MACHINE:353:state_machine SSL_F_TLS12_CHECK_PEER_SIGALG:333:tls12_check_peer_sigalg SSL_F_TLS12_COPY_SIGALGS:533:tls12_copy_sigalgs SSL_F_TLS13_CHANGE_CIPHER_STATE:440:tls13_change_cipher_state +SSL_F_TLS13_FINAL_FINISH_MAC:605:tls13_final_finish_mac SSL_F_TLS13_GENERATE_SECRET:591:tls13_generate_secret SSL_F_TLS13_HKDF_EXPAND:561:tls13_hkdf_expand SSL_F_TLS13_SETUP_KEY_BLOCK:441:tls13_setup_key_block diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index e4dfc0354a..be7e0c6f6b 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -247,6 +247,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS12_CHECK_PEER_SIGALG 333 # define SSL_F_TLS12_COPY_SIGALGS 533 # define SSL_F_TLS13_CHANGE_CIPHER_STATE 440 +# define SSL_F_TLS13_FINAL_FINISH_MAC 605 # define SSL_F_TLS13_GENERATE_SECRET 591 # define SSL_F_TLS13_HKDF_EXPAND 561 # define SSL_F_TLS13_SETUP_KEY_BLOCK 441 diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index c160533ac4..351b10f357 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -30,7 +30,8 @@ static int ssl3_generate_key_block(SSL *s, unsigned char *km, int num) m5 = EVP_MD_CTX_new(); s1 = EVP_MD_CTX_new(); if (m5 == NULL || s1 == NULL) { - SSLerr(SSL_F_SSL3_GENERATE_KEY_BLOCK, ERR_R_MALLOC_FAILURE); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GENERATE_KEY_BLOCK, + ERR_R_MALLOC_FAILURE); goto err; } EVP_MD_CTX_set_flags(m5, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); @@ -38,7 +39,8 @@ static int ssl3_generate_key_block(SSL *s, unsigned char *km, int num) k++; if (k > sizeof(buf)) { /* bug: 'buf' is too small for this ciphersuite */ - SSLerr(SSL_F_SSL3_GENERATE_KEY_BLOCK, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GENERATE_KEY_BLOCK, + ERR_R_INTERNAL_ERROR); goto err; } @@ -55,15 +57,24 @@ static int ssl3_generate_key_block(SSL *s, unsigned char *km, int num) || !EVP_DigestInit_ex(m5, EVP_md5(), NULL) || !EVP_DigestUpdate(m5, s->session->master_key, s->session->master_key_length) - || !EVP_DigestUpdate(m5, smd, SHA_DIGEST_LENGTH)) + || !EVP_DigestUpdate(m5, smd, SHA_DIGEST_LENGTH)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GENERATE_KEY_BLOCK, + ERR_R_INTERNAL_ERROR); goto err; + } if ((int)(i + MD5_DIGEST_LENGTH) > num) { - if (!EVP_DigestFinal_ex(m5, smd, NULL)) + if (!EVP_DigestFinal_ex(m5, smd, NULL)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_SSL3_GENERATE_KEY_BLOCK, ERR_R_INTERNAL_ERROR); goto err; + } memcpy(km, smd, (num - i)); } else { - if (!EVP_DigestFinal_ex(m5, km, NULL)) + if (!EVP_DigestFinal_ex(m5, km, NULL)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_SSL3_GENERATE_KEY_BLOCK, ERR_R_INTERNAL_ERROR); goto err; + } } km += MD5_DIGEST_LENGTH; @@ -279,6 +290,7 @@ int ssl3_setup_key_block(SSL *s) s->s3->tmp.key_block_length = num; s->s3->tmp.key_block = p; + /* Calls SSLfatal() as required */ ret = ssl3_generate_key_block(s, p, num); if (!(s->options & SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS)) { @@ -408,26 +420,33 @@ size_t ssl3_final_finish_mac(SSL *s, const char *sender, size_t len, int ret; EVP_MD_CTX *ctx = NULL; - if (!ssl3_digest_cached_records(s, 0)) + if (!ssl3_digest_cached_records(s, 0)) { + /* SSLfatal() already called */ return 0; + } if (EVP_MD_CTX_type(s->s3->handshake_dgst) != NID_md5_sha1) { - SSLerr(SSL_F_SSL3_FINAL_FINISH_MAC, SSL_R_NO_REQUIRED_DIGEST); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_FINAL_FINISH_MAC, + SSL_R_NO_REQUIRED_DIGEST); return 0; } ctx = EVP_MD_CTX_new(); if (ctx == NULL) { - SSLerr(SSL_F_SSL3_FINAL_FINISH_MAC, ERR_R_MALLOC_FAILURE); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_FINAL_FINISH_MAC, + ERR_R_MALLOC_FAILURE); return 0; } if (!EVP_MD_CTX_copy_ex(ctx, s->s3->handshake_dgst)) { - SSLerr(SSL_F_SSL3_FINAL_FINISH_MAC, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_FINAL_FINISH_MAC, + ERR_R_INTERNAL_ERROR); return 0; } ret = EVP_MD_CTX_size(ctx); if (ret < 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_FINAL_FINISH_MAC, + ERR_R_INTERNAL_ERROR); EVP_MD_CTX_reset(ctx); return 0; } @@ -437,7 +456,8 @@ size_t ssl3_final_finish_mac(SSL *s, const char *sender, size_t len, (int)s->session->master_key_length, s->session->master_key) <= 0 || EVP_DigestFinal_ex(ctx, p, NULL) <= 0) { - SSLerr(SSL_F_SSL3_FINAL_FINISH_MAC, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_FINAL_FINISH_MAC, + ERR_R_INTERNAL_ERROR); ret = 0; } diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 62e671a059..1bfa56328e 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -358,6 +358,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS12_COPY_SIGALGS, 0), "tls12_copy_sigalgs"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS13_CHANGE_CIPHER_STATE, 0), "tls13_change_cipher_state"}, + {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS13_FINAL_FINISH_MAC, 0), + "tls13_final_finish_mac"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS13_GENERATE_SECRET, 0), "tls13_generate_secret"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS13_HKDF_EXPAND, 0), "tls13_hkdf_expand"}, diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index 97fd797f7e..db2de6e3bf 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -324,18 +324,24 @@ static int state_machine(SSL *s, int server) if (SSL_IS_DTLS(s)) { if ((s->version & 0xff00) != (DTLS1_VERSION & 0xff00) && (server || (s->version & 0xff00) != (DTLS1_BAD_VER & 0xff00))) { - SSLerr(SSL_F_STATE_MACHINE, ERR_R_INTERNAL_ERROR); + /* We've failed to even initialise so no alert sent */ + SSLfatal(s, SSL_AD_NO_ALERT, SSL_F_STATE_MACHINE, + ERR_R_INTERNAL_ERROR); goto end; } } else { if ((s->version >> 8) != SSL3_VERSION_MAJOR) { - SSLerr(SSL_F_STATE_MACHINE, ERR_R_INTERNAL_ERROR); + /* We've failed to even initialise so no alert sent */ + SSLfatal(s, SSL_AD_NO_ALERT, SSL_F_STATE_MACHINE, + ERR_R_INTERNAL_ERROR); goto end; } } if (!ssl_security(s, SSL_SECOP_VERSION, 0, s->version, NULL)) { - SSLerr(SSL_F_STATE_MACHINE, SSL_R_VERSION_TOO_LOW); + /* We've failed to even initialise so no alert sent */ + SSLfatal(s, SSL_AD_NO_ALERT, SSL_F_STATE_MACHINE, + ERR_R_INTERNAL_ERROR); goto end; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 2a63fbece6..65c3aa3374 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -230,19 +230,22 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) const SIGALG_LOOKUP *lu = s->s3->tmp.sigalg; if (lu == NULL || s->s3->tmp.cert == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_INTERNAL_ERROR); goto err; } pkey = s->s3->tmp.cert->privatekey; if (pkey == NULL || !tls1_lookup_md(lu, &md)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_INTERNAL_ERROR); goto err; } mctx = EVP_MD_CTX_new(); if (mctx == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_MALLOC_FAILURE); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_MALLOC_FAILURE); goto err; } @@ -253,18 +256,21 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) } if (SSL_USE_SIGALGS(s) && !WPACKET_put_bytes_u16(pkt, lu->sigalg)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_INTERNAL_ERROR); goto err; } siglen = EVP_PKEY_size(pkey); sig = OPENSSL_malloc(siglen); if (sig == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_MALLOC_FAILURE); + 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) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_EVP_LIB); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_EVP_LIB); goto err; } @@ -272,7 +278,8 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) 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_CERT_VERIFY, ERR_R_EVP_LIB); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_EVP_LIB); goto err; } } @@ -283,11 +290,13 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) s->session->master_key) || EVP_DigestSignFinal(mctx, sig, &siglen) <= 0) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_EVP_LIB); + 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) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_EVP_LIB); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_EVP_LIB); goto err; } @@ -303,13 +312,16 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) #endif if (!WPACKET_sub_memcpy_u16(pkt, sig, siglen)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CERT_VERIFY, + ERR_R_INTERNAL_ERROR); goto err; } /* Digest cached records and discard handshake buffer */ - if (!ssl3_digest_cached_records(s, 0)) + if (!ssl3_digest_cached_records(s, 0)) { + /* SSLfatal() already called */ goto err; + } OPENSSL_free(sig); EVP_MD_CTX_free(mctx); @@ -317,7 +329,6 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) err: OPENSSL_free(sig); EVP_MD_CTX_free(mctx); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return 0; } @@ -511,13 +522,8 @@ int tls_construct_finished(SSL *s, WPACKET *pkt) && !s->server && s->s3->tmp.cert_req == 0 && (!s->method->ssl3_enc->change_cipher_state(s, - SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_WRITE))) { - SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, SSL_R_CANNOT_CHANGE_CIPHER); - /* - * This is a fatal error, which leaves - * enc_write_ctx in an inconsistent state - * and thus ssl3_send_alert may crash. - */ + SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_WRITE))) {; + /* SSLfatal() already called */ return 0; } @@ -533,15 +539,16 @@ int tls_construct_finished(SSL *s, WPACKET *pkt) sender, slen, s->s3->tmp.finish_md); if (finish_md_len == 0) { - SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR); - goto err; + /* SSLfatal() already called */ + return 0; } s->s3->tmp.finish_md_len = finish_md_len; if (!WPACKET_memcpy(pkt, s->s3->tmp.finish_md, finish_md_len)) { - SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR); - goto err; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_FINISHED, + ERR_R_INTERNAL_ERROR); + return 0; } /* @@ -551,16 +558,17 @@ int tls_construct_finished(SSL *s, WPACKET *pkt) if (!SSL_IS_TLS13(s) && !ssl_log_secret(s, MASTER_SECRET_LABEL, s->session->master_key, s->session->master_key_length)) { - SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR); - goto err; + /* SSLfatal() already called */ + return 0; } /* * Copy the finished so we can use it for renegotiation checks */ if (!ossl_assert(finish_md_len <= EVP_MAX_MD_SIZE)) { - SSLerr(SSL_F_TLS_CONSTRUCT_FINISHED, ERR_R_INTERNAL_ERROR); - goto err; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_FINISHED, + ERR_R_INTERNAL_ERROR); + return 0; } if (!s->server) { memcpy(s->s3->previous_client_finished, s->s3->tmp.finish_md, @@ -573,24 +581,18 @@ int tls_construct_finished(SSL *s, WPACKET *pkt) } return 1; - err: - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return 0; } int tls_construct_key_update(SSL *s, WPACKET *pkt) { if (!WPACKET_put_bytes_u8(pkt, s->key_update)) { - SSLerr(SSL_F_TLS_CONSTRUCT_KEY_UPDATE, ERR_R_INTERNAL_ERROR); - goto err; + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_KEY_UPDATE, + ERR_R_INTERNAL_ERROR); + return 0; } s->key_update = SSL_KEY_UPDATE_NONE; return 1; - - err: - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return 0; } MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt) @@ -836,8 +838,8 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt) int tls_construct_change_cipher_spec(SSL *s, WPACKET *pkt) { if (!WPACKET_put_bytes_u8(pkt, SSL3_MT_CCS)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CHANGE_CIPHER_SPEC, ERR_R_INTERNAL_ERROR); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS_CONSTRUCT_CHANGE_CIPHER_SPEC, ERR_R_INTERNAL_ERROR); return 0; } @@ -1088,7 +1090,7 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs) int tls_get_message_header(SSL *s, int *mt) { /* s->init_num < SSL3_HM_HEADER_LENGTH */ - int skip_message, i, recvd_type, al; + int skip_message, i, recvd_type; unsigned char *p; size_t l, readbytes; @@ -1110,10 +1112,10 @@ int tls_get_message_header(SSL *s, int *mt) * in the middle of a handshake message. */ if (s->init_num != 0 || readbytes != 1 || p[0] != SSL3_MT_CCS) { - al = SSL_AD_UNEXPECTED_MESSAGE; - SSLerr(SSL_F_TLS_GET_MESSAGE_HEADER, - SSL_R_BAD_CHANGE_CIPHER_SPEC); - goto f_err; + SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, + SSL_F_TLS_GET_MESSAGE_HEADER, + SSL_R_BAD_CHANGE_CIPHER_SPEC); + return 0; } s->s3->tmp.message_type = *mt = SSL3_MT_CHANGE_CIPHER_SPEC; s->init_num = readbytes - 1; @@ -1121,9 +1123,10 @@ int tls_get_message_header(SSL *s, int *mt) s->s3->tmp.message_size = readbytes; return 1; } else if (recvd_type != SSL3_RT_HANDSHAKE) { - al = SSL_AD_UNEXPECTED_MESSAGE; - SSLerr(SSL_F_TLS_GET_MESSAGE_HEADER, SSL_R_CCS_RECEIVED_EARLY); - goto f_err; + SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, + SSL_F_TLS_GET_MESSAGE_HEADER, + SSL_R_CCS_RECEIVED_EARLY); + return 0; } s->init_num += readbytes; } @@ -1171,9 +1174,9 @@ int tls_get_message_header(SSL *s, int *mt) n2l3(p, l); /* BUF_MEM_grow takes an 'int' parameter */ if (l > (INT_MAX - SSL3_HM_HEADER_LENGTH)) { - al = SSL_AD_ILLEGAL_PARAMETER; - SSLerr(SSL_F_TLS_GET_MESSAGE_HEADER, SSL_R_EXCESSIVE_MESSAGE_SIZE); - goto f_err; + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_GET_MESSAGE_HEADER, + SSL_R_EXCESSIVE_MESSAGE_SIZE); + return 0; } s->s3->tmp.message_size = l; @@ -1182,9 +1185,6 @@ int tls_get_message_header(SSL *s, int *mt) } return 1; - f_err: - ssl3_send_alert(s, SSL3_AL_FATAL, al); - return 0; } int tls_get_message_body(SSL *s, size_t *len) @@ -1226,8 +1226,7 @@ int tls_get_message_body(SSL *s, size_t *len) if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) { if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, s->init_num)) { - SSLerr(SSL_F_TLS_GET_MESSAGE_BODY, ERR_R_EVP_LIB); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + /* SSLfatal() already called */ *len = 0; return 0; } @@ -1242,8 +1241,7 @@ int tls_get_message_body(SSL *s, size_t *len) if (s->s3->tmp.message_type != SSL3_MT_HELLO_RETRY_REQUEST && !ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, s->init_num + SSL3_HM_HEADER_LENGTH)) { - SSLerr(SSL_F_TLS_GET_MESSAGE_BODY, ERR_R_EVP_LIB); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + /* SSLfatal() already called */ *len = 0; return 0; } diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 24978353ff..465d483407 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -23,37 +23,39 @@ static int tls1_PRF(SSL *s, const void *seed4, size_t seed4_len, const void *seed5, size_t seed5_len, const unsigned char *sec, size_t slen, - unsigned char *out, size_t olen) + unsigned char *out, size_t olen, int fatal) { const EVP_MD *md = ssl_prf_md(s); EVP_PKEY_CTX *pctx = NULL; - int ret = 0; if (md == NULL) { /* Should never happen */ - SSLerr(SSL_F_TLS1_PRF, ERR_R_INTERNAL_ERROR); + if (fatal) + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_PRF, + ERR_R_INTERNAL_ERROR); + else + SSLerr(SSL_F_TLS1_PRF, ERR_R_INTERNAL_ERROR); return 0; } pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL); if (pctx == NULL || EVP_PKEY_derive_init(pctx) <= 0 || EVP_PKEY_CTX_set_tls1_prf_md(pctx, md) <= 0 - || EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, sec, (int)slen) <= 0) - goto err; - - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed1, (int)seed1_len) <= 0) - goto err; - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed2, (int)seed2_len) <= 0) - goto err; - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed3, (int)seed3_len) <= 0) - goto err; - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed4, (int)seed4_len) <= 0) - goto err; - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed5, (int)seed5_len) <= 0) + || EVP_PKEY_CTX_set1_tls1_prf_secret(pctx, sec, (int)slen) <= 0 + || EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed1, (int)seed1_len) <= 0 + || EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed2, (int)seed2_len) <= 0 + || EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed3, (int)seed3_len) <= 0 + || EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed4, (int)seed4_len) <= 0 + || EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, seed5, (int)seed5_len) <= 0 + || EVP_PKEY_derive(pctx, out, &olen) <= 0) { + if (fatal) + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_PRF, + ERR_R_INTERNAL_ERROR); + else + SSLerr(SSL_F_TLS1_PRF, ERR_R_INTERNAL_ERROR); goto err; + } - if (EVP_PKEY_derive(pctx, out, &olen) <= 0) - goto err; ret = 1; err: @@ -64,12 +66,14 @@ static int tls1_PRF(SSL *s, static int tls1_generate_key_block(SSL *s, unsigned char *km, size_t num) { int ret; + + /* Calls SSLfatal() as required */ ret = tls1_PRF(s, TLS_MD_KEY_EXPANSION_CONST, TLS_MD_KEY_EXPANSION_CONST_SIZE, s->s3->server_random, SSL3_RANDOM_SIZE, s->s3->client_random, SSL3_RANDOM_SIZE, NULL, 0, NULL, 0, s->session->master_key, - s->session->master_key_length, km, num); + s->session->master_key_length, km, num, 1); return ret; } @@ -402,8 +406,10 @@ int tls1_setup_key_block(SSL *s) ((z + 1) % 16) ? ' ' : '\n'); } #endif - if (!tls1_generate_key_block(s, p, num)) + if (!tls1_generate_key_block(s, p, num)) { + /* SSLfatal() already called */ goto err; + } #ifdef SSL_DEBUG printf("\nkey block\n"); { @@ -443,16 +449,22 @@ size_t tls1_final_finish_mac(SSL *s, const char *str, size_t slen, size_t hashlen; unsigned char hash[EVP_MAX_MD_SIZE]; - if (!ssl3_digest_cached_records(s, 0)) + if (!ssl3_digest_cached_records(s, 0)) { + /* SSLfatal() already called */ return 0; + } - if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) + if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) { + /* SSLfatal() already called */ return 0; + } if (!tls1_PRF(s, str, slen, hash, hashlen, NULL, 0, NULL, 0, NULL, 0, s->session->master_key, s->session->master_key_length, - out, TLS1_FINISH_MAC_LENGTH)) + out, TLS1_FINISH_MAC_LENGTH, 1)) { + /* SSLfatal() already called */ return 0; + } OPENSSL_cleanse(hash, hashlen); return TLS1_FINISH_MAC_LENGTH; } @@ -477,24 +489,30 @@ int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, fprintf(stderr, "Handshake hashes:\n"); BIO_dump_fp(stderr, (char *)hash, hashlen); #endif - tls1_PRF(s, - TLS_MD_EXTENDED_MASTER_SECRET_CONST, - TLS_MD_EXTENDED_MASTER_SECRET_CONST_SIZE, - hash, hashlen, - NULL, 0, - NULL, 0, - NULL, 0, p, len, out, - SSL3_MASTER_SECRET_SIZE); + if (!tls1_PRF(s, + TLS_MD_EXTENDED_MASTER_SECRET_CONST, + TLS_MD_EXTENDED_MASTER_SECRET_CONST_SIZE, + hash, hashlen, + NULL, 0, + NULL, 0, + NULL, 0, p, len, out, + SSL3_MASTER_SECRET_SIZE, 1)) { + /* SSLfatal() already called */ + return 0; + } OPENSSL_cleanse(hash, hashlen); } else { - tls1_PRF(s, - TLS_MD_MASTER_SECRET_CONST, - TLS_MD_MASTER_SECRET_CONST_SIZE, - s->s3->client_random, SSL3_RANDOM_SIZE, - NULL, 0, - s->s3->server_random, SSL3_RANDOM_SIZE, - NULL, 0, p, len, out, - SSL3_MASTER_SECRET_SIZE); + if (!tls1_PRF(s, + TLS_MD_MASTER_SECRET_CONST, + TLS_MD_MASTER_SECRET_CONST_SIZE, + s->s3->client_random, SSL3_RANDOM_SIZE, + NULL, 0, + s->s3->server_random, SSL3_RANDOM_SIZE, + NULL, 0, p, len, out, + SSL3_MASTER_SECRET_SIZE, 1)) { + /* SSLfatal() already called */ + return 0; + } } #ifdef SSL_DEBUG fprintf(stderr, "Premaster Secret:\n"); @@ -580,7 +598,7 @@ int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen, NULL, 0, NULL, 0, s->session->master_key, s->session->master_key_length, - out, olen); + out, olen, 0); goto ret; err1: diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index aa933b2a5d..fe817f8f65 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -242,8 +242,10 @@ size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen, EVP_PKEY *key = NULL; EVP_MD_CTX *ctx = EVP_MD_CTX_new(); - if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) + if (!ssl_handshake_hash(s, hash, sizeof(hash), &hashlen)) { + /* SSLfatal() already called */ goto err; + } if (str == s->method->ssl3_enc->server_finished_label) key = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, @@ -256,8 +258,11 @@ size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen, || ctx == NULL || EVP_DigestSignInit(ctx, NULL, md, NULL, key) <= 0 || EVP_DigestSignUpdate(ctx, hash, hashlen) <= 0 - || EVP_DigestSignFinal(ctx, out, &hashlen) <= 0) + || EVP_DigestSignFinal(ctx, out, &hashlen) <= 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_FINAL_FINISH_MAC, + ERR_R_INTERNAL_ERROR); goto err; + } ret = hashlen; err: