Commit fb34a0f4 authored by Matt Caswell's avatar Matt Caswell
Browse files

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: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3480)
parent d8028b20

ssl/record/ssl3_record.c

+3 −3
Original line number Diff line number Diff line
@@ -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;

                }

ssl/ssl_lib.c

+3 −3
Original line number Diff line number Diff line
@@ -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;

    }

ssl/ssl_rsa.c

+1 −1
Original line number Diff line number Diff line
@@ -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)

ssl/ssl_sess.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

        }

ssl/statem/extensions.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

    }