Fix up a few places in the state machine that got missed with SSLfatal()

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4778)
This commit is contained in:
Matt Caswell 2017-11-23 11:41:40 +00:00
parent d273b60b41
commit 635c8f7715
7 changed files with 52 additions and 39 deletions

View file

@ -2371,6 +2371,7 @@ SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST:353:bad srtp protection profile list
SSL_R_BAD_SSL_FILETYPE:124:bad ssl filetype SSL_R_BAD_SSL_FILETYPE:124:bad ssl filetype
SSL_R_BAD_VALUE:384:bad value SSL_R_BAD_VALUE:384:bad value
SSL_R_BAD_WRITE_RETRY:127:bad write retry SSL_R_BAD_WRITE_RETRY:127:bad write retry
SSL_R_BINDER_DOES_NOT_VERIFY:253:binder does not verify
SSL_R_BIO_NOT_SET:128:bio not set SSL_R_BIO_NOT_SET:128:bio not set
SSL_R_BLOCK_CIPHER_PAD_IS_WRONG:129:block cipher pad is wrong SSL_R_BLOCK_CIPHER_PAD_IS_WRONG:129:block cipher pad is wrong
SSL_R_BN_LIB:130:bn lib SSL_R_BN_LIB:130:bn lib

View file

@ -455,6 +455,7 @@ int ERR_load_SSL_strings(void);
# define SSL_R_BAD_SSL_FILETYPE 124 # define SSL_R_BAD_SSL_FILETYPE 124
# define SSL_R_BAD_VALUE 384 # define SSL_R_BAD_VALUE 384
# define SSL_R_BAD_WRITE_RETRY 127 # define SSL_R_BAD_WRITE_RETRY 127
# define SSL_R_BINDER_DOES_NOT_VERIFY 253
# define SSL_R_BIO_NOT_SET 128 # define SSL_R_BIO_NOT_SET 128
# define SSL_R_BLOCK_CIPHER_PAD_IS_WRONG 129 # define SSL_R_BLOCK_CIPHER_PAD_IS_WRONG 129
# define SSL_R_BN_LIB 130 # define SSL_R_BN_LIB 130

View file

@ -714,6 +714,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_SSL_FILETYPE), "bad ssl filetype"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_SSL_FILETYPE), "bad ssl filetype"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_VALUE), "bad value"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_VALUE), "bad value"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_WRITE_RETRY), "bad write retry"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_WRITE_RETRY), "bad write retry"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BINDER_DOES_NOT_VERIFY),
"binder does not verify"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BIO_NOT_SET), "bio not set"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BIO_NOT_SET), "bio not set"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BLOCK_CIPHER_PAD_IS_WRONG), {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BLOCK_CIPHER_PAD_IS_WRONG),
"block cipher pad is wrong"}, "block cipher pad is wrong"},

View file

@ -1296,7 +1296,8 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
} }
if (sess->master_key_length != hashsize) { if (sess->master_key_length != hashsize) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, SSL_R_BAD_PSK); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
SSL_R_BAD_PSK);
goto err; goto err;
} }
@ -1308,7 +1309,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
(const unsigned char *)nonce_label, (const unsigned char *)nonce_label,
sizeof(nonce_label) - 1, sess->ext.tick_nonce, sizeof(nonce_label) - 1, sess->ext.tick_nonce,
sess->ext.tick_nonce_len, psk, hashsize)) { sess->ext.tick_nonce_len, psk, hashsize)) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); /* SSLfatal() already called */
goto err; goto err;
} }
} }
@ -1326,7 +1327,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
else else
early_secret = (unsigned char *)sess->early_secret; early_secret = (unsigned char *)sess->early_secret;
if (!tls13_generate_secret(s, md, NULL, psk, hashsize, early_secret)) { if (!tls13_generate_secret(s, md, NULL, psk, hashsize, early_secret)) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); /* SSLfatal() already called */
goto err; goto err;
} }
@ -1338,25 +1339,27 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
if (mctx == NULL if (mctx == NULL
|| EVP_DigestInit_ex(mctx, md, NULL) <= 0 || EVP_DigestInit_ex(mctx, md, NULL) <= 0
|| EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) { || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
/* Generate the binder key */ /* Generate the binder key */
if (!tls13_hkdf_expand(s, md, early_secret, (unsigned char *)label, if (!tls13_hkdf_expand(s, md, early_secret, (unsigned char *)label,
labelsize, hash, hashsize, binderkey, hashsize)) { labelsize, hash, hashsize, binderkey, hashsize)) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); /* SSLfatal() already called */
goto err; goto err;
} }
/* Generate the finished key */ /* Generate the finished key */
if (!tls13_derive_finishedkey(s, md, binderkey, finishedkey, hashsize)) { if (!tls13_derive_finishedkey(s, md, binderkey, finishedkey, hashsize)) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); /* SSLfatal() already called */
goto err; goto err;
} }
if (EVP_DigestInit_ex(mctx, md, NULL) <= 0) { if (EVP_DigestInit_ex(mctx, md, NULL) <= 0) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
@ -1371,7 +1374,8 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata); hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
if (hdatalen <= 0) { if (hdatalen <= 0) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, SSL_R_BAD_HANDSHAKE_LENGTH); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
SSL_R_BAD_HANDSHAKE_LENGTH);
goto err; goto err;
} }
@ -1388,27 +1392,31 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
|| !PACKET_get_length_prefixed_3(&hashprefix, &msg) || !PACKET_get_length_prefixed_3(&hashprefix, &msg)
|| !PACKET_forward(&hashprefix, 1) || !PACKET_forward(&hashprefix, 1)
|| !PACKET_get_length_prefixed_3(&hashprefix, &msg)) { || !PACKET_get_length_prefixed_3(&hashprefix, &msg)) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
hdatalen -= PACKET_remaining(&hashprefix); hdatalen -= PACKET_remaining(&hashprefix);
} }
if (EVP_DigestUpdate(mctx, hdata, hdatalen) <= 0) { if (EVP_DigestUpdate(mctx, hdata, hdatalen) <= 0) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
} }
if (EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0 if (EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
|| EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) { || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
mackey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, finishedkey, hashsize); mackey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, finishedkey, hashsize);
if (mackey == NULL) { if (mackey == NULL) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
@ -1420,7 +1428,8 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
|| EVP_DigestSignUpdate(mctx, hash, hashsize) <= 0 || EVP_DigestSignUpdate(mctx, hash, hashsize) <= 0
|| EVP_DigestSignFinal(mctx, binderout, &bindersize) <= 0 || EVP_DigestSignFinal(mctx, binderout, &bindersize) <= 0
|| bindersize != hashsize) { || bindersize != hashsize) {
SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
@ -1429,6 +1438,9 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
} else { } else {
/* HMAC keys can't do EVP_DigestVerify* - use CRYPTO_memcmp instead */ /* HMAC keys can't do EVP_DigestVerify* - use CRYPTO_memcmp instead */
ret = (CRYPTO_memcmp(binderin, binderout, hashsize) == 0); ret = (CRYPTO_memcmp(binderin, binderout, hashsize) == 0);
if (!ret)
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PSK_DO_BINDER,
SSL_R_BINDER_DOES_NOT_VERIFY);
} }
err: err:

View file

@ -196,15 +196,17 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) { if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
if (!WPACKET_put_bytes_u16(pkt, ctmp)) { if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, SSLfatal(s, SSL_AD_INTERNAL_ERROR,
ERR_R_INTERNAL_ERROR); SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL; return EXT_RETURN_FAIL;
} }
} }
} }
if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, SSLfatal(s, SSL_AD_INTERNAL_ERROR,
ERR_R_INTERNAL_ERROR); SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
ERR_R_INTERNAL_ERROR);
return EXT_RETURN_FAIL; return EXT_RETURN_FAIL;
} }
@ -934,7 +936,6 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
size_t reshashsize = 0, pskhashsize = 0, binderoffset, msglen; size_t reshashsize = 0, pskhashsize = 0, binderoffset, msglen;
unsigned char *resbinder = NULL, *pskbinder = NULL, *msgstart = NULL; unsigned char *resbinder = NULL, *pskbinder = NULL, *msgstart = NULL;
const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL; const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL;
EXT_RETURN ret = EXT_RETURN_FAIL;
int dores = 0; int dores = 0;
s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY; s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY;
@ -961,7 +962,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
if (s->session->cipher == NULL) { if (s->session->cipher == NULL) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; return EXT_RETURN_FAIL;
} }
mdres = ssl_md(s->session->cipher->algorithm2); mdres = ssl_md(s->session->cipher->algorithm2);
if (mdres == NULL) { if (mdres == NULL) {
@ -1033,7 +1034,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
*/ */
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
SSL_R_BAD_PSK); SSL_R_BAD_PSK);
goto err; return EXT_RETURN_FAIL;
} }
if (s->hello_retry_request && mdpsk != handmd) { if (s->hello_retry_request && mdpsk != handmd) {
@ -1043,7 +1044,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
*/ */
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
SSL_R_BAD_PSK); SSL_R_BAD_PSK);
goto err; return EXT_RETURN_FAIL;
} }
pskhashsize = EVP_MD_size(mdpsk); pskhashsize = EVP_MD_size(mdpsk);
@ -1055,7 +1056,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|| !WPACKET_start_sub_packet_u16(pkt)) { || !WPACKET_start_sub_packet_u16(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; return EXT_RETURN_FAIL;
} }
if (dores) { if (dores) {
@ -1064,7 +1065,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|| !WPACKET_put_bytes_u32(pkt, agems)) { || !WPACKET_put_bytes_u32(pkt, agems)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; return EXT_RETURN_FAIL;
} }
} }
@ -1074,7 +1075,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|| !WPACKET_put_bytes_u32(pkt, 0)) { || !WPACKET_put_bytes_u32(pkt, 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; return EXT_RETURN_FAIL;
} }
} }
@ -1095,7 +1096,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
|| !WPACKET_fill_lengths(pkt)) { || !WPACKET_fill_lengths(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK,
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; return EXT_RETURN_FAIL;
} }
msgstart = WPACKET_get_curr(pkt) - msglen; msgstart = WPACKET_get_curr(pkt) - msglen;
@ -1103,17 +1104,15 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
if (dores if (dores
&& tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL, && tls_psk_do_binder(s, mdres, msgstart, binderoffset, NULL,
resbinder, s->session, 1, 0) != 1) { resbinder, s->session, 1, 0) != 1) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, /* SSLfatal() already called */
ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL;
goto err;
} }
if (s->psksession != NULL if (s->psksession != NULL
&& tls_psk_do_binder(s, mdpsk, msgstart, binderoffset, NULL, && tls_psk_do_binder(s, mdpsk, msgstart, binderoffset, NULL,
pskbinder, s->psksession, 1, 1) != 1) { pskbinder, s->psksession, 1, 1) != 1) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_PSK, /* SSLfatal() already called */
ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL;
goto err;
} }
if (dores) if (dores)
@ -1121,9 +1120,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
if (s->psksession != NULL) if (s->psksession != NULL)
s->psksession->ext.tick_identity = (dores ? 1 : 0); s->psksession->ext.tick_identity = (dores ? 1 : 0);
ret = EXT_RETURN_SENT; return EXT_RETURN_SENT;
err:
return ret;
#else #else
return EXT_RETURN_NOT_SENT; return EXT_RETURN_NOT_SENT;
#endif #endif

View file

@ -858,8 +858,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
(const unsigned char *)s->init_buf->data, (const unsigned char *)s->init_buf->data,
binderoffset, PACKET_data(&binder), NULL, binderoffset, PACKET_data(&binder), NULL,
sess, 0, ext) != 1) { sess, 0, ext) != 1) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PARSE_CTOS_PSK, /* SSLfatal() already called */
SSL_R_BAD_EXTENSION);
goto err; goto err;
} }

View file

@ -1152,7 +1152,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
return 0; return 0;
} }
/* ssl_cipher_list_to_bytes() raises SSLerr if appropriate */
if (!ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), pkt)) { if (!ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), pkt)) {
/* SSLfatal() already called */ /* SSLfatal() already called */
return 0; return 0;
@ -3643,8 +3643,9 @@ int tls_construct_end_of_early_data(SSL *s, WPACKET *pkt)
{ {
if (s->early_data_state != SSL_EARLY_DATA_WRITE_RETRY if (s->early_data_state != SSL_EARLY_DATA_WRITE_RETRY
&& s->early_data_state != SSL_EARLY_DATA_FINISHED_WRITING) { && s->early_data_state != SSL_EARLY_DATA_FINISHED_WRITING) {
SSLerr(SSL_F_TLS_CONSTRUCT_END_OF_EARLY_DATA, SSLfatal(s, SSL_AD_INTERNAL_ERROR,
ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); SSL_F_TLS_CONSTRUCT_END_OF_EARLY_DATA,
ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
return 0; return 0;
} }