From: Matt Caswell Date: Tue, 16 May 2017 16:28:23 +0000 (+0100) Subject: Try to be more consistent about the alerts we send X-Git-Tag: OpenSSL_1_1_1-pre1~1491 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=fb34a0f4e033246ef5f957bc57d2ebc904a519fc;p=oweals%2Fopenssl.git Try to be more consistent about the alerts we send We are quite inconsistent about which alerts get sent. Specifically, these alerts should be used (normally) in the following circumstances: SSL_AD_DECODE_ERROR = The peer sent a syntactically incorrect message SSL_AD_ILLEGAL_PARAMETER = The peer sent a message which was syntactically correct, but a parameter given is invalid for the context SSL_AD_HANDSHAKE_FAILURE = The peer's messages were syntactically and semantically correct, but the parameters provided were unacceptable to us (e.g. because we do not support the requested parameters) SSL_AD_INTERNAL_ERROR = We messed up (e.g. malloc failure) The standards themselves aren't always consistent but I think the above represents the best interpretation. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3480) --- diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c index bafc976cde..85d726fb20 100644 --- a/ssl/record/ssl3_record.c +++ b/ssl/record/ssl3_record.c @@ -209,7 +209,7 @@ int ssl3_get_record(SSL *s) sslv2pkt = pkt; if (!PACKET_get_net_2_len(&sslv2pkt, &sslv2len) || !PACKET_get_1(&sslv2pkt, &type)) { - al = SSL_AD_INTERNAL_ERROR; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_RECORD, ERR_R_INTERNAL_ERROR); goto f_err; } @@ -241,7 +241,7 @@ int ssl3_get_record(SSL *s) } if (thisrr->length < MIN_SSL2_RECORD_LEN) { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_LENGTH_TOO_SHORT); goto f_err; } @@ -255,7 +255,7 @@ int ssl3_get_record(SSL *s) if (!PACKET_get_1(&pkt, &type) || !PACKET_get_net_2(&pkt, &version) || !PACKET_get_net_2_len(&pkt, &thisrr->length)) { - al = SSL_AD_INTERNAL_ERROR; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_RECORD, ERR_R_INTERNAL_ERROR); goto f_err; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index a12800d6d3..b81b9ea477 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -4753,7 +4753,7 @@ int ssl_cache_cipherlist(SSL *s, PACKET *cipher_suites, int sslv2format, TLS_CIPHER_LEN)) || (leadbyte != 0 && !PACKET_forward(&sslv2ciphers, TLS_CIPHER_LEN))) { - *al = SSL_AD_INTERNAL_ERROR; + *al = SSL_AD_DECODE_ERROR; OPENSSL_free(s->s3->tmp.ciphers_raw); s->s3->tmp.ciphers_raw = NULL; s->s3->tmp.ciphers_rawlen = 0; @@ -4840,8 +4840,8 @@ int bytes_to_cipher_list(SSL *s, PACKET *cipher_suites, } } if (PACKET_remaining(cipher_suites) > 0) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, ERR_R_INTERNAL_ERROR); + *al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, SSL_R_BAD_LENGTH); goto err; } diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index 6f1c380b9b..1ee80568ff 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -775,7 +775,7 @@ static int serverinfoex_srv_add_cb(SSL *s, unsigned int ext_type, int retval = serverinfo_find_extension(serverinfo, serverinfo_length, ext_type, out, outlen); if (retval == -1) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return -1; /* Error */ } if (retval == 0) diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 7a3d858c0a..5bef168abd 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -603,7 +603,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al) /* If old session includes extms, but new does not: abort handshake */ if (!(s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS)) { SSLerr(SSL_F_SSL_GET_PREV_SESSION, SSL_R_INCONSISTENT_EXTMS); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); fatal = 1; goto err; } diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 68d8cea0bd..d77d4935e9 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1116,7 +1116,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) && (!s->hit || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0)) { /* Nothing left we can do - just fail */ - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_MISSING_EXTENSION; SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE); return 0; } diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index bbe94d0020..c5f8d3d1e5 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -930,7 +930,7 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context, if (!PACKET_get_1_len(pkt, &ilen)) { SSLerr(SSL_F_TLS_PARSE_STOC_RENEGOTIATE, SSL_R_RENEGOTIATION_ENCODING_ERR); - *al = SSL_AD_ILLEGAL_PARAMETER; + *al = SSL_AD_DECODE_ERROR; return 0; } @@ -938,7 +938,7 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context, if (PACKET_remaining(pkt) != ilen) { SSLerr(SSL_F_TLS_PARSE_STOC_RENEGOTIATE, SSL_R_RENEGOTIATION_ENCODING_ERR); - *al = SSL_AD_ILLEGAL_PARAMETER; + *al = SSL_AD_DECODE_ERROR; return 0; } @@ -946,7 +946,7 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context, if (ilen != expected_len) { SSLerr(SSL_F_TLS_PARSE_STOC_RENEGOTIATE, SSL_R_RENEGOTIATION_MISMATCH); - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_ILLEGAL_PARAMETER; return 0; } @@ -955,7 +955,7 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context, s->s3->previous_client_finished_len) != 0) { SSLerr(SSL_F_TLS_PARSE_STOC_RENEGOTIATE, SSL_R_RENEGOTIATION_MISMATCH); - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_ILLEGAL_PARAMETER; return 0; } @@ -975,8 +975,13 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context, int tls_parse_stoc_server_name(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { - if (s->ext.hostname == NULL || PACKET_remaining(pkt) > 0) { - *al = SSL_AD_UNRECOGNIZED_NAME; + if (s->ext.hostname == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + return 0; + } + + if (PACKET_remaining(pkt) > 0) { + *al = SSL_AD_DECODE_ERROR; return 0; } @@ -1042,10 +1047,14 @@ int tls_parse_stoc_session_ticket(SSL *s, PACKET *pkt, unsigned int context, return 0; } - if (!tls_use_ticket(s) || PACKET_remaining(pkt) > 0) { + if (!tls_use_ticket(s)) { *al = SSL_AD_UNSUPPORTED_EXTENSION; return 0; } + if (PACKET_remaining(pkt) > 0) { + *al = SSL_AD_DECODE_ERROR; + return 0; + } s->ext.ticket_expected = 1; @@ -1060,11 +1069,14 @@ int tls_parse_stoc_status_request(SSL *s, PACKET *pkt, unsigned int context, * MUST only be sent if we've requested a status * request message. In TLS <= 1.2 it must also be empty. */ - if (s->ext.status_type != TLSEXT_STATUSTYPE_ocsp - || (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) > 0)) { + if (s->ext.status_type != TLSEXT_STATUSTYPE_ocsp) { *al = SSL_AD_UNSUPPORTED_EXTENSION; return 0; } + if (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) > 0) { + *al = SSL_AD_DECODE_ERROR; + return 0; + } if (SSL_IS_TLS13(s)) { /* We only know how to handle this if it's for the first Certificate in @@ -1407,7 +1419,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } if (!EVP_PKEY_set1_tls_encodedpoint(skey, PACKET_data(&encoded_pt), PACKET_remaining(&encoded_pt))) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_ECPOINT); EVP_PKEY_free(skey); return 0; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 9cde6955d3..95bacdffc4 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -25,7 +25,7 @@ int tls_parse_ctos_renegotiate(SSL *s, PACKET *pkt, unsigned int context, || !PACKET_get_bytes(pkt, &data, ilen)) { SSLerr(SSL_F_TLS_PARSE_CTOS_RENEGOTIATE, SSL_R_RENEGOTIATION_ENCODING_ERR); - *al = SSL_AD_ILLEGAL_PARAMETER; + *al = SSL_AD_DECODE_ERROR; return 0; } @@ -154,7 +154,7 @@ int tls_parse_ctos_srp(SSL *s, PACKET *pkt, unsigned int context, X509 *x, * upon resumption. Instead, we MUST ignore the login. */ if (!PACKET_strndup(&srp_I, &s->srp_ctx.login)) { - *al = TLS1_AD_INTERNAL_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return 0; } @@ -178,7 +178,7 @@ int tls_parse_ctos_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, if (!PACKET_memdup(&ec_point_format_list, &s->session->ext.ecpointformats, &s->session->ext.ecpointformats_len)) { - *al = TLS1_AD_INTERNAL_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return 0; } } @@ -194,7 +194,7 @@ int tls_parse_ctos_session_ticket(SSL *s, PACKET *pkt, unsigned int context, !s->ext.session_ticket_cb(s, PACKET_data(pkt), PACKET_remaining(pkt), s->ext.session_ticket_cb_arg)) { - *al = TLS1_AD_INTERNAL_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return 0; } @@ -213,7 +213,7 @@ int tls_parse_ctos_sig_algs(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } if (!s->hit && !tls1_save_sigalgs(s, &supported_sig_algs)) { - *al = TLS1_AD_DECODE_ERROR; + *al = SSL_AD_DECODE_ERROR; return 0; } @@ -368,7 +368,7 @@ int tls_parse_ctos_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x, s->s3->alpn_proposed_len = 0; if (!PACKET_memdup(&save_protocol_list, &s->s3->alpn_proposed, &s->s3->alpn_proposed_len)) { - *al = TLS1_AD_INTERNAL_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return 0; } @@ -614,7 +614,7 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, if (!EVP_PKEY_set1_tls_encodedpoint(s->s3->peer_tmp, PACKET_data(&encoded_pt), PACKET_remaining(&encoded_pt))) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, SSL_R_BAD_ECPOINT); return 0; } @@ -646,7 +646,7 @@ int tls_parse_ctos_supported_groups(SSL *s, PACKET *pkt, unsigned int context, if (!PACKET_memdup(&supported_groups_list, &s->session->ext.supportedgroups, &s->session->ext.supportedgroups_len)) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_INTERNAL_ERROR; return 0; } diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index e6a0b35148..d4382e89cc 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1183,7 +1183,6 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) /* TLS extensions */ if (!tls_construct_extensions(s, pkt, SSL_EXT_CLIENT_HELLO, NULL, 0, &al)) { - ssl3_send_alert(s, SSL3_AL_FATAL, al); SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); return 0; } @@ -1928,7 +1927,6 @@ static int tls_process_ske_srp(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) } if (!srp_verify_server_param(s, al)) { - *al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_SKE_SRP, SSL_R_BAD_SRP_PARAMETERS); return 0; } @@ -1987,7 +1985,7 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) /* test non-zero pubkey */ if (BN_is_zero(bnpub_key)) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE); goto err; } @@ -2000,7 +1998,7 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) p = g = NULL; if (DH_check_params(dh, &check_bits) == 0 || check_bits != 0) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_SKE_DHE, SSL_R_BAD_DH_VALUE); goto err; } @@ -2075,7 +2073,7 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) * invalid curve. ECParameters is 3 bytes. */ if (!tls1_check_curve(s, ecparams, 3)) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_WRONG_CURVE); return 0; } @@ -2124,7 +2122,7 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) if (!EVP_PKEY_set1_tls_encodedpoint(s->s3->peer_tmp, PACKET_data(&encoded_pt), PACKET_remaining(&encoded_pt))) { - *al = SSL_AD_DECODE_ERROR; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_BAD_ECPOINT); return 0; } @@ -2201,7 +2199,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) if (!PACKET_get_sub_packet(&save_param_start, ¶ms, PACKET_remaining(&save_param_start) - PACKET_remaining(pkt))) { - al = SSL_AD_INTERNAL_ERROR; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); goto err; } @@ -2750,7 +2748,7 @@ static int tls_construct_cke_psk_preamble(SSL *s, WPACKET *pkt, int *al) identitylen = strlen(identity); if (identitylen > PSK_MAX_IDENTITY_LEN) { SSLerr(SSL_F_TLS_CONSTRUCT_CKE_PSK_PREAMBLE, ERR_R_INTERNAL_ERROR); - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_INTERNAL_ERROR; goto err; } @@ -3137,7 +3135,7 @@ int tls_construct_client_key_exchange(SSL *s, WPACKET *pkt) if (!tls_construct_cke_srp(s, pkt, &al)) goto err; } else if (!(alg_k & SSL_kPSK)) { - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); goto err; } diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c index b2ba35763a..5cab35544f 100644 --- a/ssl/statem/statem_dtls.c +++ b/ssl/statem/statem_dtls.c @@ -770,7 +770,7 @@ static int dtls_get_reassembled_message(SSL *s, int *errtype, size_t *len) * Fragments must not span records. */ if (frag_len > RECORD_LAYER_get_rrec_length(&s->rlayer)) { - al = SSL3_AD_ILLEGAL_PARAMETER; + al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_DTLS_GET_REASSEMBLED_MESSAGE, SSL_R_BAD_LENGTH); goto f_err; } @@ -845,8 +845,8 @@ static int dtls_get_reassembled_message(SSL *s, int *errtype, size_t *len) * to fail */ if (readbytes != frag_len) { - al = SSL3_AD_ILLEGAL_PARAMETER; - SSLerr(SSL_F_DTLS_GET_REASSEMBLED_MESSAGE, SSL3_AD_ILLEGAL_PARAMETER); + al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_DTLS_GET_REASSEMBLED_MESSAGE, SSL_R_BAD_LENGTH); goto f_err; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 4a399ca15c..e6f62937cb 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -333,10 +333,8 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) peer = s->session->peer; pkey = X509_get0_pubkey(peer); - if (pkey == NULL) { - al = SSL_AD_INTERNAL_ERROR; + if (pkey == NULL) goto f_err; - } type = X509_certificate_type(peer, pkey); @@ -670,14 +668,14 @@ MSG_PROCESS_RETURN tls_process_change_cipher_spec(SSL *s, PACKET *pkt) && remain != DTLS1_CCS_HEADER_LENGTH + 1) || (s->version != DTLS1_BAD_VER && remain != DTLS1_CCS_HEADER_LENGTH - 1)) { - al = SSL_AD_ILLEGAL_PARAMETER; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC, SSL_R_BAD_CHANGE_CIPHER_SPEC); goto f_err; } } else { if (remain != 0) { - al = SSL_AD_ILLEGAL_PARAMETER; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CHANGE_CIPHER_SPEC, SSL_R_BAD_CHANGE_CIPHER_SPEC); goto f_err; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 5c22ba7b1c..02c6e569d8 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1260,7 +1260,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) unsigned int mt; if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_UNEXPECTED_MESSAGE; SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE); goto f_err; } @@ -1318,7 +1318,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) } if (session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH) { - al = SSL_AD_DECODE_ERROR; + al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH); goto f_err; } @@ -1376,8 +1376,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) if (!PACKET_copy_all(&cookie, clienthello->dtls_cookie, DTLS1_COOKIE_LENGTH, &clienthello->dtls_cookie_len)) { - al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH); + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto f_err; } /* @@ -1419,8 +1419,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) if (!PACKET_copy_all(&compression, clienthello->compressions, MAX_COMPRESSIONS_SIZE, &clienthello->compressions_len)) { - al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH); + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); goto f_err; } @@ -2221,7 +2221,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) pkdhp = pkdh; } if (pkdhp == NULL) { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, SSL_R_MISSING_TMP_DH_KEY); goto f_err; @@ -2314,7 +2314,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) } else #endif { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE); goto f_err; @@ -2631,7 +2631,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al) rsa = EVP_PKEY_get0_RSA(s->cert->pkeys[SSL_PKEY_RSA].privatekey); if (rsa == NULL) { - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, SSL_R_MISSING_RSA_CERTIFICATE); return 0; } @@ -2794,20 +2794,20 @@ static int tls_process_cke_dhe(SSL *s, PACKET *pkt, int *al) int ret = 0; if (!PACKET_get_net_2(pkt, &i) || PACKET_remaining(pkt) != i) { - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG); goto err; } skey = s->s3->tmp.pkey; if (skey == NULL) { - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY); goto err; } if (PACKET_remaining(pkt) == 0L) { - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY); goto err; } @@ -2886,7 +2886,7 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt, int *al) goto err; } if (EVP_PKEY_set1_tls_encodedpoint(ckey, data, i) == 0) { - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_ILLEGAL_PARAMETER; SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EC_LIB); goto err; } @@ -2926,6 +2926,7 @@ static int tls_process_cke_srp(SSL *s, PACKET *pkt, int *al) return 0; } if ((s->srp_ctx.A = BN_bin2bn(data, i, NULL)) == NULL) { + *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_BN_LIB); return 0; } @@ -3070,7 +3071,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) if (alg_k & SSL_kPSK) { /* Identity extracted earlier: should be nothing left */ if (PACKET_remaining(pkt) != 0) { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_LENGTH_MISMATCH); goto err; @@ -3097,7 +3098,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt) if (!tls_process_cke_gost(s, pkt, &al)) goto err; } else { - al = SSL_AD_HANDSHAKE_FAILURE; + al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, SSL_R_UNKNOWN_CIPHER_TYPE); goto err; @@ -3584,6 +3585,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt) { PACKET next_proto, padding; size_t next_proto_len; + int al = SSL_AD_INTERNAL_ERROR; /*- * The payload looks like: @@ -3595,6 +3597,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt) if (!PACKET_get_length_prefixed_1(pkt, &next_proto) || !PACKET_get_length_prefixed_1(pkt, &padding) || PACKET_remaining(pkt) > 0) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_NEXT_PROTO, SSL_R_LENGTH_MISMATCH); goto err; } @@ -3608,6 +3611,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt) return MSG_PROCESS_CONTINUE_READING; err: + ssl3_send_alert(s, SSL3_AL_FATAL, al); ossl_statem_set_error(s); return MSG_PROCESS_ERROR; } @@ -3621,7 +3625,6 @@ static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt) NULL, 0, &al)) { ssl3_send_alert(s, SSL3_AL_FATAL, al); SSLerr(SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS, ERR_R_INTERNAL_ERROR); - ssl3_send_alert(s, SSL3_AL_FATAL, al); return 0; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 0e1a97e175..232bb41fe0 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1117,9 +1117,9 @@ int tls1_set_server_sigalgs(SSL *s) } if (s->cert->shared_sigalgs != NULL) return 1; - /* Fatal error is no shared signature algorithms */ + /* Fatal error if no shared signature algorithms */ SSLerr(SSL_F_TLS1_SET_SERVER_SIGALGS, SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS); - al = SSL_AD_ILLEGAL_PARAMETER; + al = SSL_AD_HANDSHAKE_FAILURE; err: ssl3_send_alert(s, SSL3_AL_FATAL, al); return 0; @@ -2408,7 +2408,7 @@ int tls_choose_sigalg(SSL *s, int *al) if (al == NULL) return 1; SSLerr(SSL_F_TLS_CHOOSE_SIGALG, SSL_R_WRONG_SIGNATURE_TYPE); - *al = SSL_AD_HANDSHAKE_FAILURE; + *al = SSL_AD_ILLEGAL_PARAMETER; return 0; } }