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

Use the same min-max version range on the client consistently 



We need to ensure that the min-max version range we use when constructing
the ClientHello is the same range we use when we validate the version
selected by the ServerHello. Otherwise this may appear as a fallback or
downgrade.

Fixes #6964

Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7013)
parent bc420ebe

ssl/ssl_locl.h

+1 −1
Original line number Diff line number Diff line
@@ -2384,7 +2384,7 @@ __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, 
__owur int ssl_choose_client_version(SSL *s, int version,

                                     RAW_EXTENSION *extensions);

__owur int ssl_get_min_max_version(const SSL *s, int *min_version,

                                   int *max_version);

                                   int *max_version, int *real_max);



__owur long tls1_default_timeout(void);

__owur int dtls1_do_write(SSL *s, int type);

ssl/statem/extensions.c

+1 −1
Original line number Diff line number Diff line
@@ -810,7 +810,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, 
    }



    if ((context & SSL_EXT_CLIENT_HELLO) != 0) {

        reason = ssl_get_min_max_version(s, &min_version, &max_version);

        reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);

        if (reason != 0) {

            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_EXTENSIONS,

                     reason);

ssl/statem/extensions_clnt.c

+1 −1
Original line number Diff line number Diff line
@@ -507,7 +507,7 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt, 
{

    int currv, min_version, max_version, reason;



    reason = ssl_get_min_max_version(s, &min_version, &max_version);

    reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);

    if (reason != 0) {

        SSLfatal(s, SSL_AD_INTERNAL_ERROR,

                 SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);

ssl/statem/statem_lib.c

+75 −59
Original line number Diff line number Diff line
@@ -105,7 +105,7 @@ int tls_setup_handshake(SSL *s) 
         * enabled. For clients we do this check during construction of the

         * ClientHello.

         */

        if (ssl_get_min_max_version(s, &ver_min, &ver_max) != 0) {

        if (ssl_get_min_max_version(s, &ver_min, &ver_max, NULL) != 0) {

            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_SETUP_HANDSHAKE,

                     ERR_R_INTERNAL_ERROR);

            return 0;
@@ -1835,8 +1835,7 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
{

    const version_info *vent;

    const version_info *table;

    int highver = 0;

    int origv;

    int ret, ver_min, ver_max, real_max, origv;



    origv = s->version;

    s->version = version;
@@ -1883,39 +1882,32 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
        break;

    }



    for (vent = table; vent->version != 0; ++vent) {

        const SSL_METHOD *method;

        int err;



        if (vent->cmeth == NULL)

            continue;



        if (highver != 0 && s->version != vent->version)

            continue;



        if (highver == 0 && (s->mode & SSL_MODE_SEND_FALLBACK_SCSV) != 0)

            highver = vent->version;



        method = vent->cmeth();

        err = ssl_method_error(s, method);

        if (err != 0) {

            if (s->version == vent->version) {

    ret = ssl_get_min_max_version(s, &ver_min, &ver_max, &real_max);

    if (ret != 0) {

        s->version = origv;

        SSLfatal(s, SSL_AD_PROTOCOL_VERSION,

                         SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);

                 SSL_F_SSL_CHOOSE_CLIENT_VERSION, ret);

        return 0;

    }



            continue;

    if (SSL_IS_DTLS(s) ? DTLS_VERSION_LT(s->version, ver_min)

                       : s->version < ver_min) {

        s->version = origv;

        SSLfatal(s, SSL_AD_PROTOCOL_VERSION,

                 SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_UNSUPPORTED_PROTOCOL);

        return 0;

    } else if (SSL_IS_DTLS(s) ? DTLS_VERSION_GT(s->version, ver_max)

                              : s->version > ver_max) {

        s->version = origv;

        SSLfatal(s, SSL_AD_PROTOCOL_VERSION,

                 SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_UNSUPPORTED_PROTOCOL);

        return 0;

    }

        if (highver == 0)

            highver = vent->version;



        if (s->version != vent->version)

            continue;

    if ((s->mode & SSL_MODE_SEND_FALLBACK_SCSV) == 0)

        real_max = ver_max;



    /* Check for downgrades */

        if (s->version == TLS1_2_VERSION && highver > s->version) {

    if (s->version == TLS1_2_VERSION && real_max > s->version) {

        if (memcmp(tls12downgrade,

                   s->s3->server_random + SSL3_RANDOM_SIZE

                                        - sizeof(tls12downgrade),
@@ -1928,7 +1920,7 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
        }

    } else if (!SSL_IS_DTLS(s)

               && s->version < TLS1_2_VERSION

                   && highver > s->version) {

               && real_max > s->version) {

        if (memcmp(tls11downgrade,

                   s->s3->server_random + SSL3_RANDOM_SIZE

                                        - sizeof(tls11downgrade),
@@ -1941,7 +1933,11 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
        }

    }



        s->method = method;

    for (vent = table; vent->version != 0; ++vent) {

        if (vent->cmeth == NULL || s->version != vent->version)

            continue;



        s->method = vent->cmeth();

        return 1;

    }
@@ -1956,6 +1952,9 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
 * @s: The SSL connection

 * @min_version: The minimum supported version

 * @max_version: The maximum supported version

 * @real_max:    The highest version below the lowest compile time version hole

 *               where that hole lies above at least one run-time enabled

 *               protocol.

 *

 * Work out what version we should be using for the initial ClientHello if the

 * version is initially (D)TLS_ANY_VERSION.  We apply any explicit SSL_OP_NO_xxx
@@ -1970,9 +1969,10 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) 
 * Returns 0 on success or an SSL error reason number on failure.  On failure

 * min_version and max_version will also be set to 0.

 */

int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)

int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version,

                            int *real_max)

{

    int version;

    int version, tmp_real_max;

    int hole;

    const SSL_METHOD *single = NULL;

    const SSL_METHOD *method;
@@ -1989,6 +1989,12 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version) 
         * ssl_method_error(s, s->method)

         */

        *min_version = *max_version = s->version;

        /*

         * Providing a real_max only makes sense where we're using a version

         * flexible method.

         */

        if (!ossl_assert(real_max == NULL))

            return ERR_R_INTERNAL_ERROR;

        return 0;

    case TLS_ANY_VERSION:

        table = tls_version_table;
@@ -2021,6 +2027,9 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version) 
     */

    *min_version = version = 0;

    hole = 1;

    if (real_max != NULL)

        *real_max = 0;

    tmp_real_max = 0;

    for (vent = table; vent->version != 0; ++vent) {

        /*

         * A table entry with a NULL client method is still a hole in the
@@ -2028,15 +2037,22 @@ int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version) 
         */

        if (vent->cmeth == NULL) {

            hole = 1;

            tmp_real_max = 0;

            continue;

        }

        method = vent->cmeth();



        if (hole == 1 && tmp_real_max == 0)

            tmp_real_max = vent->version;



        if (ssl_method_error(s, method) != 0) {

            hole = 1;

        } else if (!hole) {

            single = NULL;

            *min_version = method->version;

        } else {

            if (real_max != NULL && tmp_real_max != 0)

                *real_max = tmp_real_max;

            version = (single = method)->version;

            *min_version = version;

            hole = 0;
@@ -2071,7 +2087,7 @@ int ssl_set_client_hello_version(SSL *s) 
    if (!SSL_IS_FIRST_HANDSHAKE(s))

        return 0;



    ret = ssl_get_min_max_version(s, &ver_min, &ver_max);

    ret = ssl_get_min_max_version(s, &ver_min, &ver_max, NULL);



    if (ret != 0)

        return ret;

ssl/t1_lib.c

+1 −1
Original line number Diff line number Diff line
@@ -1103,7 +1103,7 @@ int ssl_set_client_disabled(SSL *s) 
    s->s3->tmp.mask_k = 0;

    ssl_set_sig_mask(&s->s3->tmp.mask_a, s, SSL_SECOP_SIGALG_MASK);

    if (ssl_get_min_max_version(s, &s->s3->tmp.min_ver,

                                &s->s3->tmp.max_ver) != 0)

                                &s->s3->tmp.max_ver, NULL) != 0)

        return 0;

#ifndef OPENSSL_NO_PSK

    /* with PSK there must be client callback set */