Bug 1483129 - TLS 1.3 RFC version, r=ekr
authorMartin Thomson <martin.thomson@gmail.com>
Mon, 09 Jul 2018 12:24:13 +1000 (2018-07-09)
changeset 14455 ee357b00f2e6c44589dcd406097357888d59ef06
parent 14454 9e9dcafdc5351d7db0e82b5fe8b393ad2cf2f8aa
child 14456 91b6fb509cb01bbf4466f647cccfa2a2e060c504
push id3169
push usermartin.thomson@gmail.com
push dateSun, 26 Aug 2018 23:51:29 +0000 (2018-08-26)
reviewersekr
bugs1483129
Bug 1483129 - TLS 1.3 RFC version, r=ekr This retains the ability to negotiate draft versions of DTLS 1.3, but uses the final RFC version for TLS 1.3. This also refactors the handling of the downgrade sentinel. As we've discovered - to our dismay - some MitM boxes forward handshake messages when they shouldn't. This could result in triggering the downgrade sentinel. I've done two things here: - The server always sets the sentinel. It reduces the assumed version if it only supports a draft version though on the basis that the client might expect the full version. - The client has a new option SSL_ENABLE_HELLO_DOWNGRADE_CHECK which is disabled by default. The client will reject a handshake that appears to be a downgrade only when this is explicitly enabled. The client will allow an apparent downgrade to TLS 1.2 if it is running a draft version of TLS 1.3. The allowance for a draft version is now only effective for DTLS 1.3. Tests for version downgrade have been updated and enabled. These were rotten in a few ways, but nothing dramatic.
gtests/ssl_gtest/ssl_agent_unittest.cc
gtests/ssl_gtest/ssl_extension_unittest.cc
gtests/ssl_gtest/ssl_hrr_unittest.cc
gtests/ssl_gtest/ssl_version_unittest.cc
gtests/ssl_gtest/tls_agent.cc
gtests/ssl_gtest/tls_agent.h
lib/ssl/ssl.h
lib/ssl/ssl3con.c
lib/ssl/ssl3prot.h
lib/ssl/sslimpl.h
lib/ssl/sslsock.c
lib/ssl/tls13con.c
lib/ssl/tls13con.h
lib/ssl/tls13exthandle.c
--- a/gtests/ssl_gtest/ssl_agent_unittest.cc
+++ b/gtests/ssl_gtest/ssl_agent_unittest.cc
@@ -29,17 +29,17 @@ const static uint8_t kCannedTls13ClientH
     0x0a, 0x00, 0x12, 0x00, 0x10, 0x00, 0x17, 0x00, 0x18, 0x00, 0x19, 0x01,
     0x00, 0x01, 0x01, 0x01, 0x02, 0x01, 0x03, 0x01, 0x04, 0x00, 0x33, 0x00,
     0x47, 0x00, 0x45, 0x00, 0x17, 0x00, 0x41, 0x04, 0x86, 0x4a, 0xb9, 0xdc,
     0x6a, 0x38, 0xa7, 0xce, 0xe7, 0xc2, 0x4f, 0xa6, 0x28, 0xb9, 0xdc, 0x65,
     0xbf, 0x73, 0x47, 0x3c, 0x9c, 0x65, 0x8c, 0x47, 0x6d, 0x57, 0x22, 0x8a,
     0xc2, 0xb3, 0xc6, 0x80, 0x72, 0x86, 0x08, 0x86, 0x8f, 0x52, 0xc5, 0xcb,
     0xbf, 0x2a, 0xb5, 0x59, 0x64, 0xcc, 0x0c, 0x49, 0x95, 0x36, 0xe4, 0xd9,
     0x2f, 0xd4, 0x24, 0x66, 0x71, 0x6f, 0x5d, 0x70, 0xe2, 0xa0, 0xea, 0x26,
-    0x00, 0x2b, 0x00, 0x03, 0x02, 0x7f, kD13, 0x00, 0x0d, 0x00, 0x20, 0x00,
+    0x00, 0x2b, 0x00, 0x03, 0x02, 0x03, 0x04, 0x00, 0x0d, 0x00, 0x20, 0x00,
     0x1e, 0x04, 0x03, 0x05, 0x03, 0x06, 0x03, 0x02, 0x03, 0x08, 0x04, 0x08,
     0x05, 0x08, 0x06, 0x04, 0x01, 0x05, 0x01, 0x06, 0x01, 0x02, 0x01, 0x04,
     0x02, 0x05, 0x02, 0x06, 0x02, 0x02, 0x02};
 static const size_t kFirstFragmentSize = 20;
 static const char *k0RttData = "ABCDEF";
 
 TEST_P(TlsAgentTest, EarlyFinished) {
   DataBuffer buffer;
--- a/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -4,16 +4,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ssl.h"
 #include "ssl3prot.h"
 #include "sslerr.h"
 #include "sslproto.h"
 
+// This is only to get DTLS_1_3_DRAFT_VERSION
+#include "ssl3prot.h"
+
 #include <memory>
 
 #include "tls_connect.h"
 #include "tls_filter.h"
 #include "tls_parser.h"
 
 namespace nss_test {
 
@@ -926,33 +929,42 @@ TEST_P(TlsExtensionTest13, RemoveTls13Fr
   ConnectWithReplacementVersionList(SSL_LIBRARY_VERSION_TLS_1_2);
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
   server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 }
 
 // 3. Server supports 1.2 and 1.3, client supports 1.2 and 1.3
 // but advertises 1.2 (because we changed things).
 TEST_P(TlsExtensionTest13, RemoveTls13FromVersionListBothV12) {
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_3);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_3);
-#ifndef TLS_1_3_DRAFT_VERSION
-  ExpectAlert(server_, kTlsAlertIllegalParameter);
-#else
-  ExpectAlert(server_, kTlsAlertDecryptError);
+// The downgrade check is disabled in DTLS 1.3, so all that happens when we
+// tamper with the supported versions is that the Finished check fails.
+#ifdef DTLS_1_3_DRAFT_VERSION
+  if (variant_ == ssl_variant_datagram) {
+    ExpectAlert(server_, kTlsAlertDecryptError);
+  } else
 #endif
+  {
+    ExpectAlert(client_, kTlsAlertIllegalParameter);
+  }
   ConnectWithReplacementVersionList(SSL_LIBRARY_VERSION_TLS_1_2);
-#ifndef TLS_1_3_DRAFT_VERSION
-  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
-  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
-#else
-  client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
-  server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
+#ifdef DTLS_1_3_DRAFT_VERSION
+  if (variant_ == ssl_variant_datagram) {
+    client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
+    server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
+  } else
 #endif
+  {
+    client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+    server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  }
 }
 
 TEST_P(TlsExtensionTest13, HrrThenRemoveSignatureAlgorithms) {
   ExpectAlert(server_, kTlsAlertMissingExtension);
   HrrThenRemoveExtensionsTest(ssl_signature_algorithms_xtn,
                               SSL_ERROR_MISSING_EXTENSION_ALERT,
                               SSL_ERROR_MISSING_SIGNATURE_ALGORITHMS_EXTENSION);
 }
--- a/gtests/ssl_gtest/ssl_hrr_unittest.cc
+++ b/gtests/ssl_gtest/ssl_hrr_unittest.cc
@@ -4,17 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "secerr.h"
 #include "ssl.h"
 #include "sslerr.h"
 #include "sslproto.h"
 
-// This is internal, just to get TLS_1_3_DRAFT_VERSION.
+// This is internal, just to get DTLS_1_3_DRAFT_VERSION.
 #include "ssl3prot.h"
 
 #include "gtest_utils.h"
 #include "scoped_ptrs.h"
 #include "tls_connect.h"
 #include "tls_filter.h"
 #include "tls_parser.h"
 
@@ -1002,17 +1002,20 @@ class HelloRetryRequestAgentTest : publi
     i = hrr_data.Write(i, static_cast<uint32_t>(0), 1);  // session_id
     i = hrr_data.Write(i, TLS_AES_128_GCM_SHA256, 2);
     i = hrr_data.Write(i, ssl_compression_null, 1);
     // Add extensions.  First a length, which includes the supported version.
     i = hrr_data.Write(i, static_cast<uint32_t>(len) + 6, 2);
     // Now the supported version.
     i = hrr_data.Write(i, ssl_tls13_supported_versions_xtn, 2);
     i = hrr_data.Write(i, 2, 2);
-    i = hrr_data.Write(i, 0x7f00 | TLS_1_3_DRAFT_VERSION, 2);
+    i = hrr_data.Write(i, (variant_ == ssl_variant_datagram)
+                              ? (0x7f00 | DTLS_1_3_DRAFT_VERSION)
+                              : SSL_LIBRARY_VERSION_TLS_1_3,
+                       2);
     if (len) {
       hrr_data.Write(i, body, len);
     }
     DataBuffer hrr;
     MakeHandshakeMessage(kTlsHandshakeServerHello, hrr_data.data(),
                          hrr_data.len(), &hrr, seq_num);
     MakeRecord(ssl_ct_handshake, SSL_LIBRARY_VERSION_TLS_1_3, hrr.data(),
                hrr.len(), hrr_record, seq_num);
--- a/gtests/ssl_gtest/ssl_version_unittest.cc
+++ b/gtests/ssl_gtest/ssl_version_unittest.cc
@@ -43,100 +43,117 @@ TEST_P(TlsConnectGeneric, ServerNegotiat
 
   uint16_t minver, maxver;
   client_->GetVersionRange(&minver, &maxver);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, maxver);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_2);
   Connect();
 }
-#ifndef TLS_1_3_DRAFT_VERSION
 
 // Test the ServerRandom version hack from
 // [draft-ietf-tls-tls13-11 Section 6.3.1.1].
 // The first three tests test for active tampering. The next
 // two validate that we can also detect fallback using the
 // SSL_SetDowngradeCheckVersion() API.
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls11) {
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
   MakeTlsFilter<TlsClientHelloVersionSetter>(client_,
                                              SSL_LIBRARY_VERSION_TLS_1_1);
-  ConnectExpectFail();
-  ASSERT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
-/* Attempt to negotiate the bogus DTLS 1.1 version. */
+// Attempt to negotiate the bogus DTLS 1.1 version.
 TEST_F(DtlsConnectTest, TestDtlsVersion11) {
   MakeTlsFilter<TlsClientHelloVersionSetter>(client_, ((~0x0101) & 0xffff));
-  ConnectExpectFail();
+  ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
   // It's kind of surprising that SSL_ERROR_NO_CYPHER_OVERLAP is
   // what is returned here, but this is deliberate in ssl3_HandleAlert().
-  EXPECT_EQ(SSL_ERROR_NO_CYPHER_OVERLAP, client_->error_code());
-  EXPECT_EQ(SSL_ERROR_UNSUPPORTED_VERSION, server_->error_code());
+  client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
+  server_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
 }
 
-// Disabled as long as we have draft version.
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls12) {
-  EnsureTlsSetup();
-  MakeTlsFilter<TlsClientHelloVersionSetter>(client_,
-                                             SSL_LIBRARY_VERSION_TLS_1_2);
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
+  MakeTlsFilter<TlsExtensionDropper>(client_, ssl_tls13_supported_versions_xtn);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_3);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_3);
-  ConnectExpectFail();
-  ASSERT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+}
+
+// Disabling downgrade checks will be caught when the Finished MAC check fails.
+TEST_F(TlsConnectTest, TestDisableDowngradeDetection) {
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_FALSE);
+  MakeTlsFilter<TlsExtensionDropper>(client_, ssl_tls13_supported_versions_xtn);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  ConnectExpectAlert(server_, kTlsAlertDecryptError);
+  client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
+  server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
 }
 
 // TLS 1.1 clients do not check the random values, so we should
 // instead get a handshake failure alert from the server.
 TEST_F(TlsConnectTest, TestDowngradeDetectionToTls10) {
+  // Setting the option here has no effect.
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
   MakeTlsFilter<TlsClientHelloVersionSetter>(client_,
                                              SSL_LIBRARY_VERSION_TLS_1_0);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
                            SSL_LIBRARY_VERSION_TLS_1_1);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
                            SSL_LIBRARY_VERSION_TLS_1_2);
-  ConnectExpectFail();
-  ASSERT_EQ(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE, server_->error_code());
-  ASSERT_EQ(SSL_ERROR_DECRYPT_ERROR_ALERT, client_->error_code());
+  ConnectExpectAlert(server_, kTlsAlertDecryptError);
+  server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
+  client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
 }
 
 TEST_F(TlsConnectTest, TestFallbackFromTls12) {
-  EnsureTlsSetup();
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
   client_->SetDowngradeCheckVersion(SSL_LIBRARY_VERSION_TLS_1_2);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
                            SSL_LIBRARY_VERSION_TLS_1_1);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
                            SSL_LIBRARY_VERSION_TLS_1_2);
-  ConnectExpectFail();
-  ASSERT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
 TEST_F(TlsConnectTest, TestFallbackFromTls13) {
-  EnsureTlsSetup();
+  client_->SetOption(SSL_ENABLE_HELLO_DOWNGRADE_CHECK, PR_TRUE);
   client_->SetDowngradeCheckVersion(SSL_LIBRARY_VERSION_TLS_1_3);
   client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
                            SSL_LIBRARY_VERSION_TLS_1_2);
   server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
                            SSL_LIBRARY_VERSION_TLS_1_3);
-  ConnectExpectFail();
-  ASSERT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code());
+  ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
+  client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
+  server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
-#endif
 
 TEST_P(TlsConnectGeneric, TestFallbackSCSVVersionMatch) {
   client_->SetOption(SSL_ENABLE_FALLBACK_SCSV, PR_TRUE);
   Connect();
 }
 
 TEST_P(TlsConnectGenericPre13, TestFallbackSCSVVersionMismatch) {
   client_->SetOption(SSL_ENABLE_FALLBACK_SCSV, PR_TRUE);
   server_->SetVersionRange(version_, version_ + 1);
   ConnectExpectAlert(server_, kTlsAlertInappropriateFallback);
   client_->CheckErrorCode(SSL_ERROR_INAPPROPRIATE_FALLBACK_ALERT);
+  server_->CheckErrorCode(SSL_ERROR_INAPPROPRIATE_FALLBACK_ALERT);
 }
 
 // The TLS v1.3 spec section C.4 states that 'Implementations MUST NOT send or
 // accept any records with a version less than { 3, 0 }'. Thus we will not
 // allow version ranges including both SSL v3 and TLS v1.3.
 TEST_F(TlsConnectTest, DisallowSSLv3HelloWithTLSv13Enabled) {
   SECStatus rv;
   SSLVersionRange vrange = {SSL_LIBRARY_VERSION_3_0,
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -10,16 +10,19 @@
 #include "pk11func.h"
 #include "ssl.h"
 #include "sslerr.h"
 #include "sslexp.h"
 #include "sslproto.h"
 #include "tls_filter.h"
 #include "tls_parser.h"
 
+// This is an internal header, used to get DTLS_1_3_DRAFT_VERSION.
+#include "ssl3prot.h"
+
 extern "C" {
 // This is not something that should make you happy.
 #include "libssl_internals.h"
 }
 
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
@@ -48,17 +51,17 @@ const std::string TlsAgent::kServerDsa =
 static const uint8_t kCannedTls13ServerHello[] = {
     0x03, 0x03, 0x9c, 0xbc, 0x14, 0x9b, 0x0e, 0x2e, 0xfa, 0x0d, 0xf3,
     0xf0, 0x5c, 0x70, 0x7a, 0xe0, 0xd1, 0x9b, 0x3e, 0x5a, 0x44, 0x6b,
     0xdf, 0xe5, 0xc2, 0x28, 0x64, 0xf7, 0x00, 0xc1, 0x9c, 0x08, 0x76,
     0x08, 0x00, 0x13, 0x01, 0x00, 0x00, 0x2e, 0x00, 0x33, 0x00, 0x24,
     0x00, 0x1d, 0x00, 0x20, 0xc2, 0xcf, 0x23, 0x17, 0x64, 0x23, 0x03,
     0xf0, 0xfb, 0x45, 0x98, 0x26, 0xd1, 0x65, 0x24, 0xa1, 0x6c, 0xa9,
     0x80, 0x8f, 0x2c, 0xac, 0x0a, 0xea, 0x53, 0x3a, 0xcb, 0xe3, 0x08,
-    0x84, 0xae, 0x19, 0x00, 0x2b, 0x00, 0x02, 0x7f, kD13};
+    0x84, 0xae, 0x19, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04};
 
 TlsAgent::TlsAgent(const std::string& nm, Role rl, SSLProtocolVariant var)
     : name_(nm),
       variant_(var),
       role_(rl),
       server_key_bits_(0),
       adapter_(new DummyPrSocket(role_str(), var)),
       ssl_fd_(nullptr),
@@ -1169,13 +1172,18 @@ void TlsAgentTestBase::MakeTrivialHandsh
     index = out->Write(index, 1, 1);
   }
 }
 
 DataBuffer TlsAgentTestBase::MakeCannedTls13ServerHello() {
   DataBuffer sh(kCannedTls13ServerHello, sizeof(kCannedTls13ServerHello));
   if (variant_ == ssl_variant_datagram) {
     sh.Write(0, SSL_LIBRARY_VERSION_DTLS_1_2_WIRE, 2);
+    // The version should be at the end.
+    uint32_t v;
+    EXPECT_TRUE(sh.Read(sh.len() - 2, 2, &v));
+    EXPECT_EQ(static_cast<uint32_t>(SSL_LIBRARY_VERSION_TLS_1_3), v);
+    sh.Write(sh.len() - 2, 0x7f00 | DTLS_1_3_DRAFT_VERSION, 2);
   }
   return sh;
 }
 
 }  // namespace nss_test
--- a/gtests/ssl_gtest/tls_agent.h
+++ b/gtests/ssl_gtest/tls_agent.h
@@ -5,19 +5,16 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef tls_agent_h_
 #define tls_agent_h_
 
 #include "prio.h"
 #include "ssl.h"
 
-// This is an internal header, used to get TLS_1_3_DRAFT_VERSION.
-#include "ssl3prot.h"
-
 #include <functional>
 #include <iostream>
 
 #include "test_io.h"
 
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "scoped_ptrs.h"
@@ -55,18 +52,16 @@ typedef std::function<SECStatus(TlsAgent
     AuthCertificateCallbackFunction;
 
 typedef std::function<void(TlsAgent* agent)> HandshakeCallbackFunction;
 
 typedef std::function<int32_t(TlsAgent* agent, const SECItem* srvNameArr,
                               PRUint32 srvNameArrSize)>
     SniCallbackFunction;
 
-static const uint8_t kD13 = TLS_1_3_DRAFT_VERSION;
-
 class TlsAgent : public PollTarget {
  public:
   enum Role { CLIENT, SERVER };
   enum State { STATE_INIT, STATE_CONNECTING, STATE_CONNECTED, STATE_ERROR };
 
   static const std::string kClient;     // the client key is sign only
   static const std::string kRsa2048;    // bigger sign and encrypt for either
   static const std::string kRsa8192;    // biggest sign and encrypt for either
--- a/lib/ssl/ssl.h
+++ b/lib/ssl/ssl.h
@@ -277,16 +277,25 @@ SSL_IMPORT PRFileDesc *DTLS_ImportFD(PRF
  * are dropped if there is reordering.
  *
  * This applies to TLS 1.3 only.  This is not a parameter that is negotiated
  * during the TLS handshake. Unlike other socket options, this option can be
  * changed after a handshake is complete.
  */
 #define SSL_ENABLE_DTLS_SHORT_HEADER 36
 
+/*
+ * Enables the processing of the downgrade sentinel that can be added to the
+ * ServerHello.random by a server that supports Section 4.1.3 of TLS 1.3
+ * [RFC8446].  This sentinel will always be generated by a server that
+ * negotiates a version lower than its maximum, this only controls whether a
+ * client will treat receipt of a value that indicates a downgrade as an error.
+ */
+#define SSL_ENABLE_HELLO_DOWNGRADE_CHECK 37
+
 #ifdef SSL_DEPRECATED_FUNCTION
 /* Old deprecated function names */
 SSL_IMPORT SECStatus SSL_Enable(PRFileDesc *fd, int option, PRIntn on);
 SSL_IMPORT SECStatus SSL_EnableDefault(int option, PRIntn on);
 #endif
 
 /* Set (and get) options for sockets and defaults for newly created sockets.
  *
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6409,19 +6409,16 @@ ssl3_HandleServerHello(sslSocket *ss, PR
     int errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO;
     PRUint32 compression;
     SECStatus rv;
     SECItem sidBytes = { siBuffer, NULL, 0 };
     PRBool isHelloRetry;
     SSL3AlertDescription desc = illegal_parameter;
     const PRUint8 *savedMsg = b;
     const PRUint32 savedLength = length;
-#ifndef TLS_1_3_DRAFT_VERSION
-    SSL3ProtocolVersion downgradeCheckVersion;
-#endif
 
     SSL_TRC(3, ("%d: SSL3[%d]: handle server_hello handshake",
                 SSL_GETPID(), ss->fd));
     PORT_Assert(ss->opt.noLocks || ssl_HaveRecvBufLock(ss));
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
 
     if (ss->ssl3.hs.ws != wait_server_hello) {
         errCode = SSL_ERROR_RX_UNEXPECTED_SERVER_HELLO;
@@ -6548,49 +6545,53 @@ ssl3_HandleServerHello(sslSocket *ss, PR
      * us to be getting this version number, but it's what we have.
      * (1294697). */
     if (ss->firstHsDone && (ss->version != ss->ssl3.crSpec->version)) {
         desc = protocol_version;
         errCode = SSL_ERROR_UNSUPPORTED_VERSION;
         goto alert_loser;
     }
 
-#ifndef TLS_1_3_DRAFT_VERSION
-    /* Check the ServerHello.random per
-     * [draft-ietf-tls-tls13-11 Section 6.3.1.1].
-     *
-     * TLS 1.3 clients receiving a TLS 1.2 or below ServerHello MUST check
-     * that the top eight octets are not equal to either of these values.
-     * TLS 1.2 clients SHOULD also perform this check if the ServerHello
-     * indicates TLS 1.1 or below.  If a match is found the client MUST
-     * abort the handshake with a fatal "illegal_parameter" alert.
-     *
-     * Disable this test during the TLS 1.3 draft version period.
-     */
-    downgradeCheckVersion = ss->ssl3.downgradeCheckVersion ? ss->ssl3.downgradeCheckVersion
-                                                           : ss->vrange.max;
-
-    if (downgradeCheckVersion >= SSL_LIBRARY_VERSION_TLS_1_2 &&
-        downgradeCheckVersion > ss->version) {
-        /* Both sections use the same sentinel region. */
-        PRUint8 *downgrade_sentinel =
-            ss->ssl3.hs.server_random +
-            SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
-        if (!PORT_Memcmp(downgrade_sentinel,
-                         tls13_downgrade_random,
-                         sizeof(tls13_downgrade_random)) ||
-            !PORT_Memcmp(downgrade_sentinel,
-                         tls12_downgrade_random,
-                         sizeof(tls12_downgrade_random))) {
-            desc = illegal_parameter;
-            errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO;
-            goto alert_loser;
-        }
-    }
+    /* Disable this check for DTLS while we're on draft versions. */
+    if (ss->opt.enableHelloDowngradeCheck
+#ifdef DTLS_1_3_DRAFT_VERSION
+        && !IS_DTLS(ss)
 #endif
+            ) {
+        /* Check the ServerHello.random per [RFC 8446 Section 4.1.3].
+         *
+         * TLS 1.3 clients receiving a ServerHello indicating TLS 1.2 or below
+         * MUST check that the last 8 bytes are not equal to either of these
+         * values.  TLS 1.2 clients SHOULD also check that the last 8 bytes are
+         * not equal to the second value if the ServerHello indicates TLS 1.1 or
+         * below.  If a match is found, the client MUST abort the handshake with
+         * an "illegal_parameter" alert.
+         */
+        SSL3ProtocolVersion checkVersion =
+            ss->ssl3.downgradeCheckVersion ? ss->ssl3.downgradeCheckVersion
+                                           : ss->vrange.max;
+
+        if (checkVersion >= SSL_LIBRARY_VERSION_TLS_1_2 &&
+            checkVersion > ss->version) {
+            /* Both sections use the same sentinel region. */
+            PRUint8 *downgrade_sentinel =
+                ss->ssl3.hs.server_random +
+                SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
+            if (!PORT_Memcmp(downgrade_sentinel,
+                             tls13_downgrade_random,
+                             sizeof(tls13_downgrade_random)) ||
+                !PORT_Memcmp(downgrade_sentinel,
+                             tls12_downgrade_random,
+                             sizeof(tls12_downgrade_random))) {
+                desc = illegal_parameter;
+                errCode = SSL_ERROR_RX_MALFORMED_SERVER_HELLO;
+                goto alert_loser;
+            }
+        }
+    }
 
     /* Finally, now all the version-related checks have passed. */
     ss->ssl3.hs.preliminaryInfo |= ssl_preinfo_version;
     /* Update the write cipher spec to match the version. But not after
      * HelloRetryRequest, because cwSpec might be a 0-RTT cipher spec,
      * in which case this is a no-op. */
     if (!ss->firstHsDone && !isHelloRetry) {
         ssl_GetSpecWriteLock(ss);
@@ -8093,16 +8094,72 @@ ssl3_SelectServerCert(sslSocket *ss)
             ssl_SignatureSchemeToAuthType(ss->ssl3.hs.signatureScheme);
         return SECSuccess;
     }
 
     PORT_SetError(SSL_ERROR_NO_CYPHER_OVERLAP);
     return SECFailure;
 }
 
+static SECStatus
+ssl_GenerateServerRandom(sslSocket *ss)
+{
+    SECStatus rv = ssl3_GetNewRandom(ss->ssl3.hs.server_random);
+    if (rv != SECSuccess) {
+        return SECFailure;
+    }
+
+    if (ss->version == ss->vrange.max) {
+        return SECSuccess;
+    }
+#ifdef DTLS_1_3_DRAFT_VERSION
+    if (IS_DTLS(ss)) {
+        return SECSuccess;
+    }
+#endif
+
+    /*
+     * [RFC 8446 Section 4.1.3].
+     *
+     * TLS 1.3 servers which negotiate TLS 1.2 or below in response to a
+     * ClientHello MUST set the last 8 bytes of their Random value specially in
+     * their ServerHello.
+     *
+     * If negotiating TLS 1.2, TLS 1.3 servers MUST set the last 8 bytes of
+     * their Random value to the bytes:
+     *
+     *   44 4F 57 4E 47 52 44 01
+     *
+     * If negotiating TLS 1.1 or below, TLS 1.3 servers MUST, and TLS 1.2
+     * servers SHOULD, set the last 8 bytes of their ServerHello.Random value to
+     * the bytes:
+     *
+     *   44 4F 57 4E 47 52 44 00
+     */
+    PRUint8 *downgradeSentinel =
+        ss->ssl3.hs.server_random +
+        SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
+
+    switch (ss->vrange.max) {
+        case SSL_LIBRARY_VERSION_TLS_1_3:
+            PORT_Memcpy(downgradeSentinel,
+                        tls13_downgrade_random, sizeof(tls13_downgrade_random));
+            break;
+        case SSL_LIBRARY_VERSION_TLS_1_2:
+            PORT_Memcpy(downgradeSentinel,
+                        tls12_downgrade_random, sizeof(tls12_downgrade_random));
+            break;
+        default:
+            /* Do not change random. */
+            break;
+    }
+
+    return SECSuccess;
+}
+
 /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete
  * ssl3 Client Hello message.
  * Caller must hold Handshake and RecvBuf locks.
  */
 static SECStatus
 ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
 {
     sslSessionID *sid = NULL;
@@ -8287,66 +8344,16 @@ ssl3_HandleClientHello(sslSocket *ss, PR
         rv = SECITEM_CopyItem(NULL, &ss->ssl3.hs.fakeSid, &sidBytes);
         if (rv != SECSuccess) {
             desc = internal_error;
             errCode = PORT_GetError();
             goto alert_loser;
         }
     }
 
-    /* Generate the Server Random now so it is available
-     * when we process the ClientKeyShare in TLS 1.3 */
-    rv = ssl3_GetNewRandom(ss->ssl3.hs.server_random);
-    if (rv != SECSuccess) {
-        errCode = SSL_ERROR_GENERATE_RANDOM_FAILURE;
-        goto loser;
-    }
-
-#ifndef TLS_1_3_DRAFT_VERSION
-    /*
-     * [draft-ietf-tls-tls13-11 Section 6.3.1.1].
-     * TLS 1.3 server implementations which respond to a ClientHello with a
-     * client_version indicating TLS 1.2 or below MUST set the last eight
-     * bytes of their Random value to the bytes:
-     *
-     * 44 4F 57 4E 47 52 44 01
-     *
-     * TLS 1.2 server implementations which respond to a ClientHello with a
-     * client_version indicating TLS 1.1 or below SHOULD set the last eight
-     * bytes of their Random value to the bytes:
-     *
-     * 44 4F 57 4E 47 52 44 00
-     *
-     * TODO(ekr@rtfm.com): Note this change was not added in the SSLv2
-     * compat processing code since that will most likely be removed before
-     * we ship the final version of TLS 1.3. Bug 1306672.
-     */
-    if (ss->vrange.max > ss->version) {
-        PRUint8 *downgrade_sentinel =
-            ss->ssl3.hs.server_random +
-            SSL3_RANDOM_LENGTH - sizeof(tls13_downgrade_random);
-
-        switch (ss->vrange.max) {
-            case SSL_LIBRARY_VERSION_TLS_1_3:
-                PORT_Memcpy(downgrade_sentinel,
-                            tls13_downgrade_random,
-                            sizeof(tls13_downgrade_random));
-                break;
-            case SSL_LIBRARY_VERSION_TLS_1_2:
-                PORT_Memcpy(downgrade_sentinel,
-                            tls12_downgrade_random,
-                            sizeof(tls12_downgrade_random));
-                break;
-            default:
-                /* Do not change random. */
-                break;
-        }
-    }
-#endif
-
     /* If there is a cookie, then this is a second ClientHello (TLS 1.3). */
     if (ssl3_FindExtension(ss, ssl_tls13_cookie_xtn)) {
         ss->ssl3.hs.helloRetry = PR_TRUE;
     }
 
     if (ss->ssl3.hs.receivedCcs) {
         /* This is only valid if we sent HelloRetryRequest, so we should have
          * negotiated TLS 1.3 and there should be a cookie extension. */
@@ -9083,28 +9090,37 @@ loser:
 
 SECStatus
 ssl_ConstructServerHello(sslSocket *ss, PRBool helloRetry,
                          const sslBuffer *extensionBuf, sslBuffer *messageBuf)
 {
     SECStatus rv;
     SSL3ProtocolVersion version;
     sslSessionID *sid = ss->sec.ci.sid;
+    const PRUint8 *random;
 
     version = PR_MIN(ss->version, SSL_LIBRARY_VERSION_TLS_1_2);
     if (IS_DTLS(ss)) {
         version = dtls_TLSVersionToDTLSVersion(version);
     }
     rv = sslBuffer_AppendNumber(messageBuf, version, 2);
     if (rv != SECSuccess) {
         return SECFailure;
     }
-    /* Random already generated in ssl3_HandleClientHello */
-    rv = sslBuffer_Append(messageBuf, helloRetry ? ssl_hello_retry_random : ss->ssl3.hs.server_random,
-                          SSL3_RANDOM_LENGTH);
+
+    if (helloRetry) {
+        random = ssl_hello_retry_random;
+    } else {
+        rv = ssl_GenerateServerRandom(ss);
+        if (rv != SECSuccess) {
+            return SECFailure;
+        }
+        random = ss->ssl3.hs.server_random;
+    }
+    rv = sslBuffer_Append(messageBuf, random, SSL3_RANDOM_LENGTH);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
         if (sid) {
             rv = sslBuffer_AppendVariable(messageBuf, sid->u.ssl3.sessionID,
                                           sid->u.ssl3.sessionIDLength, 1);
--- a/lib/ssl/ssl3prot.h
+++ b/lib/ssl/ssl3prot.h
@@ -8,20 +8,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef __ssl3proto_h_
 #define __ssl3proto_h_
 
 typedef PRUint16 SSL3ProtocolVersion;
 /* version numbers are defined in sslproto.h */
 
-/* The TLS 1.3 draft version. Used to avoid negotiating
- * between incompatible pre-standard TLS 1.3 drafts.
- * TODO(ekr@rtfm.com): Remove when TLS 1.3 is published. */
-#define TLS_1_3_DRAFT_VERSION 28
+/* DTLS 1.3 is still a draft. */
+#define DTLS_1_3_DRAFT_VERSION 28
 
 typedef PRUint16 ssl3CipherSuite;
 /* The cipher suites are defined in sslproto.h */
 
 #define MAX_CERT_TYPES 10
 #define MAX_MAC_LENGTH 64
 #define MAX_PADDING_LENGTH 64
 #define MAX_KEY_LENGTH 64
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -261,16 +261,17 @@ typedef struct sslOptionsStr {
     unsigned int enableFallbackSCSV : 1;
     unsigned int enableServerDhe : 1;
     unsigned int enableExtendedMS : 1;
     unsigned int enableSignedCertTimestamps : 1;
     unsigned int requireDHENamedGroups : 1;
     unsigned int enable0RttData : 1;
     unsigned int enableTls13CompatMode : 1;
     unsigned int enableDtlsShortHeader : 1;
+    unsigned int enableHelloDowngradeCheck : 1;
 } sslOptions;
 
 typedef enum { sslHandshakingUndetermined = 0,
                sslHandshakingAsClient,
                sslHandshakingAsServer
 } sslHandshakingType;
 
 #define SSL_LOCK_RANK_SPEC 255
--- a/lib/ssl/sslsock.c
+++ b/lib/ssl/sslsock.c
@@ -77,17 +77,18 @@ static sslOptions ssl_defaults = {
     .reuseServerECDHEKey = PR_TRUE,
     .enableFallbackSCSV = PR_FALSE,
     .enableServerDhe = PR_TRUE,
     .enableExtendedMS = PR_FALSE,
     .enableSignedCertTimestamps = PR_FALSE,
     .requireDHENamedGroups = PR_FALSE,
     .enable0RttData = PR_FALSE,
     .enableTls13CompatMode = PR_FALSE,
-    .enableDtlsShortHeader = PR_FALSE
+    .enableDtlsShortHeader = PR_FALSE,
+    .enableHelloDowngradeCheck = PR_FALSE
 };
 
 /*
  * default range of enabled SSL/TLS protocols
  */
 static SSLVersionRange versions_defaults_stream = {
     SSL_LIBRARY_VERSION_TLS_1_0,
     SSL_LIBRARY_VERSION_TLS_1_2
@@ -816,16 +817,20 @@ SSL_OptionSet(PRFileDesc *fd, PRInt32 wh
         case SSL_ENABLE_TLS13_COMPAT_MODE:
             ss->opt.enableTls13CompatMode = val;
             break;
 
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             ss->opt.enableDtlsShortHeader = val;
             break;
 
+        case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
+            ss->opt.enableHelloDowngradeCheck = val;
+            break;
+
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     /* We can't use the macros for releasing the locks here,
      * because ss->opt.noLocks might have changed just above.
      * We must release these locks (monitors) here, if we aquired them above,
@@ -958,16 +963,19 @@ SSL_OptionGet(PRFileDesc *fd, PRInt32 wh
             val = ss->opt.recordSizeLimit;
             break;
         case SSL_ENABLE_TLS13_COMPAT_MODE:
             val = ss->opt.enableTls13CompatMode;
             break;
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             val = ss->opt.enableDtlsShortHeader;
             break;
+        case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
+            val = ss->opt.enableHelloDowngradeCheck;
+            break;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     ssl_ReleaseSSL3HandshakeLock(ss);
     ssl_Release1stHandshakeLock(ss);
 
@@ -1084,16 +1092,19 @@ SSL_OptionGetDefault(PRInt32 which, PRIn
             val = ssl_defaults.recordSizeLimit;
             break;
         case SSL_ENABLE_TLS13_COMPAT_MODE:
             val = ssl_defaults.enableTls13CompatMode;
             break;
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             val = ssl_defaults.enableDtlsShortHeader;
             break;
+        case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
+            val = ssl_defaults.enableHelloDowngradeCheck;
+            break;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             rv = SECFailure;
     }
 
     *pVal = val;
     return rv;
 }
@@ -1279,16 +1290,20 @@ SSL_OptionSetDefault(PRInt32 which, PRIn
         case SSL_ENABLE_TLS13_COMPAT_MODE:
             ssl_defaults.enableTls13CompatMode = val;
             break;
 
         case SSL_ENABLE_DTLS_SHORT_HEADER:
             ssl_defaults.enableDtlsShortHeader = val;
             break;
 
+        case SSL_ENABLE_HELLO_DOWNGRADE_CHECK:
+            ssl_defaults.enableHelloDowngradeCheck = val;
+            break;
+
         default:
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             return SECFailure;
     }
     return SECSuccess;
 }
 
 SECStatus
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -5268,31 +5268,31 @@ tls13_HandleEarlyApplicationData(sslSock
     PR_APPEND_LINK(&ed->link, &ss->ssl3.hs.bufferedEarlyData);
 
     origBuf->len = 0; /* So ssl3_GatherAppDataRecord will keep looping. */
 
     return SECSuccess;
 }
 
 PRUint16
-tls13_EncodeDraftVersion(SSL3ProtocolVersion version)
+tls13_EncodeDraftVersion(SSL3ProtocolVersion version, SSLProtocolVariant variant)
 {
-#ifdef TLS_1_3_DRAFT_VERSION
-    if (version == SSL_LIBRARY_VERSION_TLS_1_3) {
-        return 0x7f00 | TLS_1_3_DRAFT_VERSION;
+#ifdef DTLS_1_3_DRAFT_VERSION
+    if (version == SSL_LIBRARY_VERSION_TLS_1_3 &&
+        variant == ssl_variant_datagram) {
+        return 0x7f00 | DTLS_1_3_DRAFT_VERSION;
     }
 #endif
     return (PRUint16)version;
 }
 
 SECStatus
 tls13_ClientReadSupportedVersion(sslSocket *ss)
 {
     PRUint32 temp;
-    SSL3ProtocolVersion v;
     TLSExtension *versionExtension;
     SECItem it;
     SECStatus rv;
 
     /* Update the version based on the extension, as necessary. */
     versionExtension = ssl3_FindExtension(ss, ssl_tls13_supported_versions_xtn);
     if (!versionExtension) {
         return SECSuccess;
@@ -5304,39 +5304,25 @@ tls13_ClientReadSupportedVersion(sslSock
     rv = ssl3_ConsumeHandshakeNumber(ss, &temp, 2, &it.data, &it.len);
     if (rv != SECSuccess) {
         return SECFailure;
     }
     if (it.len) {
         FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter);
         return SECFailure;
     }
-    v = (SSL3ProtocolVersion)temp;
-
-    /* You cannot negotiate < TLS 1.3 with supported_versions. */
-    if (v < SSL_LIBRARY_VERSION_TLS_1_3) {
+
+    if (temp != tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3,
+                                         ss->protocolVariant)) {
+        /* You cannot negotiate < TLS 1.3 with supported_versions. */
         FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter);
         return SECFailure;
     }
 
-#ifdef TLS_1_3_DRAFT_VERSION
-    if (temp == SSL_LIBRARY_VERSION_TLS_1_3) {
-        FATAL_ERROR(ss, SSL_ERROR_UNSUPPORTED_VERSION, protocol_version);
-        return SECFailure;
-    }
-    if (temp == tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3)) {
-        v = SSL_LIBRARY_VERSION_TLS_1_3;
-    } else {
-        v = (SSL3ProtocolVersion)temp;
-    }
-#else
-    v = (SSL3ProtocolVersion)temp;
-#endif
-
-    ss->version = v;
+    ss->version = SSL_LIBRARY_VERSION_TLS_1_3;
     return SECSuccess;
 }
 
 /* Pick the highest version we support that is also advertised. */
 SECStatus
 tls13_NegotiateVersion(sslSocket *ss, const TLSExtension *supportedVersions)
 {
     PRUint16 version;
@@ -5350,17 +5336,17 @@ tls13_NegotiateVersion(sslSocket *ss, co
     if (rv != SECSuccess) {
         return SECFailure;
     }
     if (data.len || !versions.len || (versions.len & 1)) {
         FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter);
         return SECFailure;
     }
     for (version = ss->vrange.max; version >= ss->vrange.min; --version) {
-        PRUint16 wire = tls13_EncodeDraftVersion(version);
+        PRUint16 wire = tls13_EncodeDraftVersion(version, ss->protocolVariant);
         unsigned long offset;
 
         for (offset = 0; offset < versions.len; offset += 2) {
             PRUint16 supported =
                 (versions.data[offset] << 8) | versions.data[offset + 1];
             if (supported == wire) {
                 ss->version = version;
                 return SECSuccess;
--- a/lib/ssl/tls13con.h
+++ b/lib/ssl/tls13con.h
@@ -96,17 +96,18 @@ SECStatus tls13_ProtectRecord(sslSocket 
                               ssl3CipherSpec *cwSpec,
                               SSLContentType type,
                               const PRUint8 *pIn,
                               PRUint32 contentLen,
                               sslBuffer *wrBuf);
 PRInt32 tls13_Read0RttData(sslSocket *ss, void *buf, PRInt32 len);
 SECStatus tls13_HandleEarlyApplicationData(sslSocket *ss, sslBuffer *origBuf);
 PRBool tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid);
-PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version);
+PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version,
+                                  SSLProtocolVariant variant);
 SECStatus tls13_ClientReadSupportedVersion(sslSocket *ss);
 SECStatus tls13_NegotiateVersion(sslSocket *ss,
                                  const TLSExtension *supported_versions);
 
 PRBool tls13_IsReplay(const sslSocket *ss, const sslSessionID *sid);
 void tls13_AntiReplayRollover(PRTime now);
 
 SECStatus SSLExp_SetupAntiReplay(PRTime window, unsigned int k,
--- a/lib/ssl/tls13exthandle.c
+++ b/lib/ssl/tls13exthandle.c
@@ -746,17 +746,19 @@ tls13_ClientSendSupportedVersionsXtn(con
                 SSL_GETPID(), ss->fd));
 
     rv = sslBuffer_Skip(buf, 1, &lengthOffset);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     for (version = ss->vrange.max; version >= ss->vrange.min; --version) {
-        rv = sslBuffer_AppendNumber(buf, tls13_EncodeDraftVersion(version), 2);
+        PRUint16 wire = tls13_EncodeDraftVersion(version,
+                                                 ss->protocolVariant);
+        rv = sslBuffer_AppendNumber(buf, wire, 2);
         if (rv != SECSuccess) {
             return SECFailure;
         }
     }
 
     rv = sslBuffer_InsertLength(buf, lengthOffset, 1);
     if (rv != SECSuccess) {
         return SECFailure;
@@ -774,18 +776,19 @@ tls13_ServerSendSupportedVersionsXtn(con
 
     if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
         return SECSuccess;
     }
 
     SSL_TRC(3, ("%d: TLS13[%d]: server send supported_versions extension",
                 SSL_GETPID(), ss->fd));
 
-    rv = sslBuffer_AppendNumber(
-        buf, tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3), 2);
+    PRUint16 ver = tls13_EncodeDraftVersion(SSL_LIBRARY_VERSION_TLS_1_3,
+                                            ss->protocolVariant);
+    rv = sslBuffer_AppendNumber(buf, ver, 2);
     if (rv != SECSuccess) {
         return SECFailure;
     }
 
     *added = PR_TRUE;
     return SECSuccess;
 }