[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  d8434cf85691f32a17dcdfed6e81769a001074dd (commit)
      from  d6ce9da49b131cad85da8c94c617febf6c8d9073 (commit)


- Log -----------------------------------------------------------------
commit d8434cf85691f32a17dcdfed6e81769a001074dd
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Jul 19 16:51:58 2018 +0100

    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: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6747)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/openssl.txt             |  1 +
 include/openssl/sslerr.h           |  1 +
 ssl/ssl_err.c                      |  1 +
 ssl/statem/statem_lib.c            | 12 ++++++++++++
 test/recipes/70-test_sslversions.t | 18 +++++++++++++-----
 5 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 3e2bc69..a0dc3c5 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -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
diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index 9eba6d8..a5b2c55 100644
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -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
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 9ce643a..d3e8056 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -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"},
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 6262a06..ebb21de 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -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
diff --git a/test/recipes/70-test_sslversions.t b/test/recipes/70-test_sslversions.t
index 5c9ee6c..8ef85af 100644
--- a/test/recipes/70-test_sslversions.t
+++ b/test/recipes/70-test_sslversions.t
@@ -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();
         }
     }
 }
-
-