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

Address some supported_versions review comments 



Added some TODOs, refactored a couple of things and added a SSL_IS_TLS13()
macro.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent 60e3b3c5

ssl/ssl_locl.h

+4 −0
Original line number Diff line number Diff line
@@ -349,6 +349,10 @@ 


/* Check if an SSL structure is using DTLS */

# define SSL_IS_DTLS(s)  (s->method->ssl3_enc->enc_flags & SSL_ENC_FLAG_DTLS)



/* Check if we are using TLSv1.3 */

# define SSL_IS_TLS13(s) (!SSL_IS_DTLS(s) && (s)->version >= TLS1_3_VERSION)



/* See if we need explicit IV */

# define SSL_USE_EXPLICIT_IV(s)  \

                (s->method->ssl3_enc->enc_flags & SSL_ENC_FLAG_EXPLICIT_IV)

ssl/statem/statem_clnt.c

+3 −4
Original line number Diff line number Diff line
@@ -703,6 +703,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) 
    SSL_COMP *comp;

#endif

    SSL_SESSION *sess = s->session;

    int client_version;



    if (!WPACKET_set_max_size(pkt, SSL3_RT_MAX_PLAIN_LENGTH)) {

        /* Should not happen */
@@ -783,10 +784,8 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) 
     * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the

     * supported_versions extension for the real supported versions.

     */

    if (!WPACKET_put_bytes_u16(pkt,

                               (!SSL_IS_DTLS(s)

                                   && s->client_version >= TLS1_3_VERSION)

                               ? TLS1_2_VERSION : s->client_version)

    client_version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->client_version;

    if (!WPACKET_put_bytes_u16(pkt, client_version)

            || !WPACKET_memcpy(pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) {

        SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);

        return 0;

ssl/statem/statem_lib.c

+5 −0
Original line number Diff line number Diff line
@@ -1044,6 +1044,11 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello) 
            /* TODO(TLS1.3): Remove this before release */

            if (candidate_vers == TLS1_3_VERSION_DRAFT)

                candidate_vers = TLS1_3_VERSION;

            /*

             * TODO(TLS1.3): There is some discussion on the TLS list about

             * wheter to ignore versions <TLS1.2 in supported_versions. At the

             * moment we honour them if present. To be reviewed later

             */

            if ((int)candidate_vers > s->client_version)

                s->client_version = candidate_vers;

            if (version_cmp(s, candidate_vers, best_vers) <= 0)

ssl/statem/statem_srvr.c

+3 −2
Original line number Diff line number Diff line
@@ -1546,10 +1546,11 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) 
{

    int compm, al = SSL_AD_INTERNAL_ERROR;

    size_t sl, len;

    int version;



    /* TODO(TLS1.3): Remove the DRAFT conditional before release */

    if (!WPACKET_put_bytes_u16(pkt, (s->version == TLS1_3_VERSION)

                                    ? TLS1_3_VERSION_DRAFT : s->version)

    version = SSL_IS_TLS13(s) ? TLS1_3_VERSION_DRAFT : s->version;

    if (!WPACKET_put_bytes_u16(pkt, version)

               /*

                * Random stuff. Filling of the server_random takes place in

                * tls_process_client_hello()

ssl/t1_lib.c

+6 −1
Original line number Diff line number Diff line
@@ -1371,7 +1371,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) 
        return 0;

    }



    if (!SSL_IS_DTLS(s) && s->version >= TLS1_3_VERSION) {

    if (SSL_IS_TLS13(s)) {

        int min_version, max_version, reason, currv;

        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions)

                || !WPACKET_start_sub_packet_u16(pkt)
@@ -1384,6 +1384,11 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al) 
            SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, reason);

            return 0;

        }

        /*

         * TODO(TLS1.3): There is some discussion on the TLS list as to wheter

         * we should include versions <TLS1.2. For the moment we do. To be

         * reviewed later.

         */

        for (currv = max_version; currv >= min_version; currv--) {

            /* TODO(TLS1.3): Remove this first if clause prior to release!! */

            if (currv == TLS1_3_VERSION) {