[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Fri May 19 07:51:15 UTC 2017


The branch master has been updated
       via  fb34a0f4e033246ef5f957bc57d2ebc904a519fc (commit)
      from  d8028b202bfe337200a0cc89b80983ea1838cb30 (commit)


- Log -----------------------------------------------------------------
commit fb34a0f4e033246ef5f957bc57d2ebc904a519fc
Author: Matt Caswell <matt at openssl.org>
Date:   Tue May 16 17:28:23 2017 +0100

    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 <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3480)

-----------------------------------------------------------------------

Summary of changes:
 ssl/record/ssl3_record.c     |  6 +++---
 ssl/ssl_lib.c                |  6 +++---
 ssl/ssl_rsa.c                |  2 +-
 ssl/ssl_sess.c               |  2 +-
 ssl/statem/extensions.c      |  2 +-
 ssl/statem/extensions_clnt.c | 32 ++++++++++++++++++++++----------
 ssl/statem/extensions_srvr.c | 16 ++++++++--------
 ssl/statem/statem_clnt.c     | 16 +++++++---------
 ssl/statem/statem_dtls.c     |  6 +++---
 ssl/statem/statem_lib.c      |  8 +++-----
 ssl/statem/statem_srvr.c     | 35 +++++++++++++++++++----------------
 ssl/t1_lib.c                 |  6 +++---
 12 files changed, 74 insertions(+), 63 deletions(-)

diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index bafc976..85d726f 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 a12800d..b81b9ea 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 6f1c380..1ee8056 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 7a3d858..5bef168 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 68d8cea..d77d493 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 bbe94d0..c5f8d3d 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 9cde695..95bacdf 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 e6a0b35..d4382e8 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, &params,
                                    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 b2ba357..5cab355 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 4a399ca..e6f6293 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 5c22ba7..02c6e56 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 0e1a97e..232bb41 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;
                 }
             }


More information about the openssl-commits mailing list