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

Validate legacy_version 



The spec says that a client MUST set legacy_version to TLSv1.2, and
requires servers to verify that it isn't SSLv3.

Fixes #6600

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6747)
parent d6ce9da4

crypto/err/openssl.txt

+1 −0
Original line number Diff line number Diff line
@@ -2581,6 +2581,7 @@ SSL_R_BAD_HELLO_REQUEST:105:bad hello request 
SSL_R_BAD_HRR_VERSION:263:bad hrr version

SSL_R_BAD_KEY_SHARE:108:bad key share

SSL_R_BAD_KEY_UPDATE:122:bad key update

SSL_R_BAD_LEGACY_VERSION:292:bad legacy version

SSL_R_BAD_LENGTH:271:bad length

SSL_R_BAD_PACKET:240:bad packet

SSL_R_BAD_PACKET_LENGTH:115:bad packet length

include/openssl/sslerr.h

+1 −0
Original line number Diff line number Diff line
@@ -471,6 +471,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_R_BAD_HRR_VERSION                            263

# define SSL_R_BAD_KEY_SHARE                              108

# define SSL_R_BAD_KEY_UPDATE                             122

# define SSL_R_BAD_LEGACY_VERSION                         292

# define SSL_R_BAD_LENGTH                                 271

# define SSL_R_BAD_PACKET                                 240

# define SSL_R_BAD_PACKET_LENGTH                          115

ssl/ssl_err.c

+1 −0
Original line number Diff line number Diff line
@@ -757,6 +757,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { 
    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_HRR_VERSION), "bad hrr version"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_KEY_SHARE), "bad key share"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_KEY_UPDATE), "bad key update"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_LEGACY_VERSION), "bad legacy version"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_LENGTH), "bad length"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_PACKET), "bad packet"},

    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_PACKET_LENGTH), "bad packet length"},

ssl/statem/statem_lib.c

+12 −0
Original line number Diff line number Diff line
@@ -1753,6 +1753,18 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd) 
            return SSL_R_LENGTH_MISMATCH;

        }



        /*

         * The TLSv1.3 spec says the client MUST set this to TLS1_2_VERSION.

         * The spec only requires servers to check that it isn't SSLv3:

         * "Any endpoint receiving a Hello message with

         * ClientHello.legacy_version or ServerHello.legacy_version set to

         * 0x0300 MUST abort the handshake with a "protocol_version" alert."

         * We are slightly stricter and require that it isn't SSLv3 or lower.

         * We tolerate TLSv1 and TLSv1.1.

         */

        if (client_version <= SSL3_VERSION)

            return SSL_R_BAD_LEGACY_VERSION;



        while (PACKET_get_net_2(&versionslist, &candidate_vers)) {

            /* TODO(TLS1.3): Remove this before release */

            if (candidate_vers == TLS1_3_VERSION_DRAFT

test/recipes/70-test_sslversions.t

+13 −5
Original line number Diff line number Diff line
@@ -18,7 +18,8 @@ use constant { 
    NO_EXTENSION => 3,

    EMPTY_EXTENSION => 4,

    TLS1_1_AND_1_0_ONLY => 5,

    WITH_TLS1_4 => 6

    WITH_TLS1_4 => 6,

    BAD_LEGACY_VERSION => 7

};



my $testtype;
@@ -55,7 +56,7 @@ my $proxy = TLSProxy::Proxy->new( 
$testtype = EMPTY_EXTENSION;

$proxy->filter(\&modify_supported_versions_filter);

$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";

plan tests => 7;

plan tests => 8;

ok(TLSProxy::Message->fail(), "Empty supported versions");



#Test 2: supported_versions extension with no recognised versions should not
@@ -111,6 +112,12 @@ ok(TLSProxy::Message->success() 
   && TLSProxy::Proxy->is_tls13(),

   "TLS1.4 in supported versions extension");



#Test 8: Set the legacy version to SSLv3 with supported versions. Should fail

$proxy->clear();

$testtype = BAD_LEGACY_VERSION;

$proxy->start();

ok(TLSProxy::Message->fail(), "Legacy version is SSLv3 with supported versions");



sub modify_supported_versions_filter

{

    my $proxy = shift;
@@ -165,14 +172,15 @@ sub modify_supported_versions_filter 
            } elsif ($testtype == EMPTY_EXTENSION) {

                $message->set_extension(

                    TLSProxy::Message::EXT_SUPPORTED_VERSIONS, "");

            } else {

            } elsif ($testtype == NO_EXTENSION) {

                $message->delete_extension(

                    TLSProxy::Message::EXT_SUPPORTED_VERSIONS);

            } else {

                # BAD_LEGACY_VERSION

                $message->client_version(TLSProxy::Record::VERS_SSL_3_0);

            }



            $message->repack();

        }

    }

}