Turn on TLSv1.3 downgrade protection by default

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6741)

diff --git a/Configure b/Configure
index 059271275d74e85bdf4e12ff7aa4cda540e8a2bd..c9f6ea760b4ca6f23205e4c5f402fddfcc0e0f4e 100755 (executable)
--- a/Configure
+++ b/Configure
@@ -405,7 +405,6 @@ my @disablables = (
     "tests",
     "threads",
     "tls",
     "tests",
     "threads",
     "tls",

-    "tls13downgrade",

     "ts",
     "ubsan",
     "ui-console",
     "ts",
     "ubsan",
     "ui-console",


@@ -449,7 +448,6 @@ our %disabled = ( # "what"         => "comment"
                  "ssl3"                => "default",
                  "ssl3-method"         => "default",
                   "ubsan"              => "default",
                  "ssl3"                => "default",
                  "ssl3-method"         => "default",
                   "ubsan"              => "default",

-                 "tls13downgrade"      => "default",

                  "unit-test"           => "default",
                  "weak-ssl-ciphers"    => "default",
                  "zlib"                => "default",
                  "unit-test"           => "default",
                  "weak-ssl-ciphers"    => "default",
                  "zlib"                => "default",

diff --git a/INSTALL b/INSTALL
index 34023dcd75846919214019602abc5e4097264d5d..ff0aa6d12792482856f8f0f48189bb1e849c82f2 100644 (file)
--- a/INSTALL
+++ b/INSTALL
@@ -476,16 +476,6 @@
                    require additional system-dependent options! See "Note on
                    multi-threading" below.
 
                    require additional system-dependent options! See "Note on
                    multi-threading" below.
 

-  enable-tls13downgrade
-                   TODO(TLS1.3): Make this enabled by default and remove the
-                   option when TLSv1.3 is out of draft
-                   TLSv1.3 offers a downgrade protection mechanism. This is
-                   implemented but disabled by default. It should not typically
-                   be enabled except for testing purposes. Otherwise this could
-                   cause problems if a pre-RFC version of OpenSSL talks to an
-                   RFC implementation (it will erroneously be detected as a
-                   downgrade).
-

   no-ts
                    Don't build Time Stamping Authority support.
 
   no-ts
                    Don't build Time Stamping Authority support.
 

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index c170eed5e15eef6db9225ad4950e03c987baf82f..5ecbc3c5543acafe9fbf6cbad6012d8e82310a25 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -4568,7 +4568,7 @@ int ssl_fill_hello_random(SSL *s, int server, unsigned char *result, size_t len,
     } else {
         ret = RAND_bytes(result, len);
     }
     } else {
         ret = RAND_bytes(result, len);
     }

-#ifndef OPENSSL_NO_TLS13DOWNGRADE
+

     if (ret > 0) {
         if (!ossl_assert(sizeof(tls11downgrade) < len)
                 || !ossl_assert(sizeof(tls12downgrade) < len))
     if (ret > 0) {
         if (!ossl_assert(sizeof(tls11downgrade) < len)
                 || !ossl_assert(sizeof(tls12downgrade) < len))


@@ -4580,7 +4580,7 @@ int ssl_fill_hello_random(SSL *s, int server, unsigned char *result, size_t len,
             memcpy(result + len - sizeof(tls11downgrade), tls11downgrade,
                    sizeof(tls11downgrade));
     }
             memcpy(result + len - sizeof(tls11downgrade), tls11downgrade,
                    sizeof(tls11downgrade));
     }

-#endif
+

     return ret;
 }
 
     return ret;
 }
 

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index d04f8773debc188e0d8c1235148d75a4638f4dcd..38121b7fd2552d02f084b394694556fcb97ccbd7 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1914,7 +1914,6 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
         if (s->version != vent->version)
             continue;
 
         if (s->version != vent->version)
             continue;
 

-#ifndef OPENSSL_NO_TLS13DOWNGRADE

         /* Check for downgrades */
         if (s->version == TLS1_2_VERSION && highver > s->version) {
             if (memcmp(tls12downgrade,
         /* Check for downgrades */
         if (s->version == TLS1_2_VERSION && highver > s->version) {
             if (memcmp(tls12downgrade,


@@ -1941,7 +1940,6 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
                 return 0;
             }
         }
                 return 0;
             }
         }

-#endif

 
         s->method = method;
         return 1;
 
         s->method = method;
         return 1;

diff --git a/test/recipes/70-test_tls13downgrade.t b/test/recipes/70-test_tls13downgrade.t
index cc5fb16d2b91198dd3a1f8e49d7e446c3f0d90d7..f7c881234517d742bbc981978756e24ba65498fa 100644 (file)
--- a/test/recipes/70-test_tls13downgrade.t
+++ b/test/recipes/70-test_tls13downgrade.t
@@ -26,10 +26,6 @@ plan skip_all => "$test_name needs the sock feature enabled"
 plan skip_all => "$test_name needs TLS1.3 and TLS1.2 enabled"
     if disabled("tls1_3") || disabled("tls1_2");
 
 plan skip_all => "$test_name needs TLS1.3 and TLS1.2 enabled"
     if disabled("tls1_3") || disabled("tls1_2");
 

-# TODO(TLS1.3): Enable this when TLSv1.3 comes out of draft
-plan skip_all => "$test_name not run in pre TLSv1.3 RFC implementation"
-    if disabled("tls13downgrade");
-

 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 
 my $proxy = TLSProxy::Proxy->new(
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 
 my $proxy = TLSProxy::Proxy->new(