Commit 27e462f1 authored by Matt Caswell's avatar Matt Caswell
Browse files

Only allow supported_versions in a TLSv1.3 ServerHello 



As per the latest text in TLSv1.3 draft-26

Reviewed-by: default avatarBen Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)
parent 3295d242

ssl/statem/extensions.c

+2 −3
Original line number Diff line number Diff line
@@ -307,9 +307,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { 
    },

    {

        TLSEXT_TYPE_supported_versions,

        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO

        | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST

        | SSL_EXT_TLS_IMPLEMENTATION_ONLY,

        SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO

        | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST | SSL_EXT_TLS_IMPLEMENTATION_ONLY,

        NULL,

        /* Processed inline as part of version selection */

        NULL, tls_parse_stoc_supported_versions,

ssl/statem/extensions_clnt.c

+12 −12
Original line number Diff line number Diff line
@@ -1780,20 +1780,20 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context, 
    if (version == TLS1_3_VERSION_DRAFT)

        version = TLS1_3_VERSION;



    /* We ignore this extension for HRRs except to sanity check it */

    if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) {

    /*

         * The only protocol version we support which has an HRR message is

         * TLSv1.3, therefore we shouldn't be getting an HRR for anything else.

     * The only protocol version we support which is valid in this extension in

     * a ServerHello is TLSv1.3 therefore we shouldn't be getting anything else.

     */

    if (version != TLS1_3_VERSION) {

            SSLfatal(s, SSL_AD_PROTOCOL_VERSION,

        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,

                 SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,

                     SSL_R_BAD_HRR_VERSION);

                 SSL_R_BAD_PROTOCOL_VERSION_NUMBER);

        return 0;

    }



    /* We ignore this extension for HRRs except to sanity check it */

    if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST)

        return 1;

    }



    /* We just set it here. We validate it in ssl_choose_client_version */

    s->version = version;

ssl/statem/extensions_srvr.c

+6 −2
Original line number Diff line number Diff line
@@ -1572,8 +1572,12 @@ EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt, 
                                                 unsigned int context, X509 *x,

                                                 size_t chainidx)

{

    if (!SSL_IS_TLS13(s))

        return EXT_RETURN_NOT_SENT;

    if (!ossl_assert(SSL_IS_TLS13(s))) {

        SSLfatal(s, SSL_AD_INTERNAL_ERROR,

                 SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS,

                 ERR_R_INTERNAL_ERROR);

        return EXT_RETURN_FAIL;

    }



    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)

            || !WPACKET_start_sub_packet_u16(pkt)