Bug 1411475 - Google Hack, r=ekr

This makes the TLS 1.3 handshake look like TLS 1.2.

The trickiest part here is in 0-RTT.  I've chosen to remember that the
alternative handshake was used and send a ChangeCipherSpec if the previous
session used the alternative AND if the client enables the alternative.

This assumes that a server will commit to supporting - and selecting - this
alternative handshake type for as long as it supports 0-RTT from sessions that
have the alternative handshake type.  That is, if you negotiate the alternative
handshake and the server supports 0-RTT, then it will not just support TLS 1.3
for the duration of the ticket, but also the alternative handshake type.  A
client can disable the alternative handshake because the version in the
ClientHello indicates whether the client intended to send a CCS, but the server
cannot refuse to pick it if the client offers.

Of course, if we agree that the final TLS 1.3 is in this form, we don't have a
problem, it's only an issue because we need to switch-hit.

I chose to remove the Facebook alternative content type hack as all signs
indicate that it doesn't help.

--HG--
branch : NSS_TLS13_DRAFT19_BRANCH
extra : rebase_source : cba8b9be8726f29acf742d225693a70af10ac5ca

cmd/tstclnt/tstclnt.c

@@ -252,7 +252,7 @@ PrintParameterUsage(void)
                     "%-20s The following values are valid:\n"
                     "%-20s P256, P384, P521, x25519, FF2048, FF3072, FF4096, FF6144, FF8192\n",
             "-I", "", "");
-    fprintf(stderr, "%-20s Enable alternate content type for TLS 1.3 ServerHello\n", "-X alt-server-hello");
+    fprintf(stderr, "%-20s Enable alternative TLS 1.3 handshake\n", "-X alt-server-hello");
 }
 
 static void
@@ -1183,7 +1183,7 @@ run_client(void)
 
     /* Alternate ServerHello content type (TLS 1.3 only) */
     if (enableAltServerHello) {
-        rv = SSL_UseAltServerHelloType(s, PR_TRUE);
+        rv = SSL_UseAltHandshakeType(s, PR_TRUE);
         if (rv != SECSuccess) {
             SECU_PrintError(progName, "error enabling alternate ServerHello type");
             error = 1;

gtests/ssl_gtest/manifest.mn

@@ -15,6 +15,7 @@ CPPSRCS = \
       bloomfilter_unittest.cc \
       ssl_0rtt_unittest.cc \
       ssl_agent_unittest.cc \
+      ssl_alths_unittest.cc \
       ssl_auth_unittest.cc \
       ssl_cert_ext_unittest.cc \
       ssl_ciphersuite_unittest.cc \

gtests/ssl_gtest/ssl_alths_unittest.cc

@@ -0,0 +1,177 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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 <memory>
+#include <vector>
+#include "ssl.h"
+#include "sslerr.h"
+#include "sslproto.h"
+
+#include "gtest_utils.h"
+#include "tls_connect.h"
+#include "tls_filter.h"
+#include "tls_parser.h"
+
+namespace nss_test {
+
+static const uint32_t kServerHelloVersionAlt = SSL_LIBRARY_VERSION_TLS_1_2;
+static const uint32_t kServerHelloVersionRegular =
+    0x7f00 | TLS_1_3_DRAFT_VERSION;
+
+class AltHandshakeTest : public TlsConnectStreamTls13 {
+ protected:
+  void SetUp() {
+    TlsConnectStreamTls13::SetUp();
+    client_ccs_recorder_ =
+        std::make_shared<TlsRecordRecorder>(kTlsChangeCipherSpecType);
+    server_ccs_recorder_ =
+        std::make_shared<TlsRecordRecorder>(kTlsChangeCipherSpecType);
+    server_hello_recorder_ =
+        std::make_shared<TlsInspectorRecordHandshakeMessage>(
+            kTlsHandshakeServerHello);
+  }
+
+  void SetAltHandshakeTypeEnabled() {
+    client_->SetAltHandshakeTypeEnabled();
+    server_->SetAltHandshakeTypeEnabled();
+  }
+
+  void InstallFilters() {
+    client_->SetPacketFilter(client_ccs_recorder_);
+    auto chain = std::make_shared<ChainedPacketFilter>(ChainedPacketFilterInit(
+        {server_ccs_recorder_, server_hello_recorder_}));
+    server_->SetPacketFilter(chain);
+  }
+
+  void CheckServerHelloVersion(uint32_t server_hello_version) {
+    uint32_t ver;
+    ASSERT_TRUE(server_hello_recorder_->buffer().Read(0, 2, &ver));
+    ASSERT_EQ(server_hello_version, ver);
+  }
+
+  void CheckForRegularHandshake() {
+    EXPECT_EQ(0U, client_ccs_recorder_->count());
+    EXPECT_EQ(0U, server_ccs_recorder_->count());
+    CheckServerHelloVersion(kServerHelloVersionRegular);
+  }
+
+  void CheckForAltHandshake() {
+    EXPECT_EQ(1U, client_ccs_recorder_->count());
+    EXPECT_EQ(1U, server_ccs_recorder_->count());
+    CheckServerHelloVersion(kServerHelloVersionAlt);
+  }
+
+  std::shared_ptr<TlsRecordRecorder> client_ccs_recorder_;
+  std::shared_ptr<TlsRecordRecorder> server_ccs_recorder_;
+  std::shared_ptr<TlsInspectorRecordHandshakeMessage> server_hello_recorder_;
+};
+
+TEST_F(AltHandshakeTest, ClientOnly) {
+  client_->SetAltHandshakeTypeEnabled();
+  InstallFilters();
+  Connect();
+  CheckForRegularHandshake();
+}
+
+TEST_F(AltHandshakeTest, ServerOnly) {
+  server_->SetAltHandshakeTypeEnabled();
+  InstallFilters();
+  Connect();
+  CheckForRegularHandshake();
+}
+
+TEST_F(AltHandshakeTest, Enabled) {
+  SetAltHandshakeTypeEnabled();
+  InstallFilters();
+  Connect();
+  CheckForAltHandshake();
+}
+
+TEST_F(AltHandshakeTest, ZeroRtt) {
+  SetAltHandshakeTypeEnabled();
+  SetupForZeroRtt();
+  SetAltHandshakeTypeEnabled();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+
+  InstallFilters();
+
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(true, true);
+  Handshake();
+  ExpectEarlyDataAccepted(true);
+  CheckConnected();
+
+  CheckForAltHandshake();
+}
+
+// Neither client nor server has the extension prior to resumption, so the
+// client doesn't send a CCS before its 0-RTT data.
+TEST_F(AltHandshakeTest, DisabledBeforeZeroRtt) {
+  SetupForZeroRtt();
+  SetAltHandshakeTypeEnabled();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+
+  InstallFilters();
+
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(true, true);
+  Handshake();
+  ExpectEarlyDataAccepted(true);
+  CheckConnected();
+
+  EXPECT_EQ(0U, client_ccs_recorder_->count());
+  EXPECT_EQ(1U, server_ccs_recorder_->count());
+  CheckServerHelloVersion(kServerHelloVersionAlt);
+}
+
+// Both use the alternative in the initial handshake but only the server enables
+// it on resumption.
+TEST_F(AltHandshakeTest, ClientDisabledAfterZeroRtt) {
+  SetAltHandshakeTypeEnabled();
+  SetupForZeroRtt();
+  server_->SetAltHandshakeTypeEnabled();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+
+  InstallFilters();
+
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(true, true);
+  Handshake();
+  ExpectEarlyDataAccepted(true);
+  CheckConnected();
+
+  CheckForRegularHandshake();
+}
+
+// If the alternative handshake isn't negotiated after 0-RTT, and the client has
+// it enabled, it will send a ChangeCipherSpec.  The server chokes on it if it
+// hasn't negotiated the alternative handshake.
+TEST_F(AltHandshakeTest, ServerDisabledAfterZeroRtt) {
+  SetAltHandshakeTypeEnabled();
+  SetupForZeroRtt();
+  client_->SetAltHandshakeTypeEnabled();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+
+  client_->Handshake();  // Send ClientHello (and CCS)
+
+  server_->Handshake();  // Consume the ClientHello, which is OK.
+  client_->ExpectResumption();
+  client_->Handshake();  // Read the server handshake.
+  EXPECT_EQ(TlsAgent::STATE_CONNECTED, client_->state());
+
+  // Now the server reads the CCS instead of more handshake messages.
+  ExpectAlert(server_, kTlsAlertBadRecordMac);
+  server_->Handshake();
+  EXPECT_EQ(TlsAgent::STATE_ERROR, server_->state());
+  client_->Handshake();  // Consume the alert.
+  EXPECT_EQ(TlsAgent::STATE_ERROR, client_->state());
+}
+
+}  // nss_test

gtests/ssl_gtest/ssl_extension_unittest.cc

@@ -1041,10 +1041,6 @@ TEST_P(TlsBogusExtensionTest13, AddBogusExtensionHelloRetryRequest) {
   Run(kTlsHandshakeHelloRetryRequest);
 }
 
-TEST_P(TlsBogusExtensionTest13, AddVersionExtensionServerHello) {
-  Run(kTlsHandshakeServerHello, ssl_tls13_supported_versions_xtn);
-}
-
 TEST_P(TlsBogusExtensionTest13, AddVersionExtensionEncryptedExtensions) {
   Run(kTlsHandshakeEncryptedExtensions, ssl_tls13_supported_versions_xtn);
 }

gtests/ssl_gtest/ssl_gtest.gyp

@@ -16,6 +16,7 @@
         'selfencrypt_unittest.cc',
         'ssl_0rtt_unittest.cc',
         'ssl_agent_unittest.cc',
+        'ssl_alths_unittest.cc',
         'ssl_auth_unittest.cc',
         'ssl_cert_ext_unittest.cc',
         'ssl_ciphersuite_unittest.cc',

gtests/ssl_gtest/ssl_loopback_unittest.cc

@@ -11,7 +11,6 @@
 #include "ssl.h"
 #include "sslerr.h"
 #include "sslproto.h"
-#include "ssl3prot.h"
 
 extern "C" {
 // This is not something that should make you happy.
@@ -104,9 +103,9 @@ TEST_P(TlsConnectGeneric, CaptureAlertServer) {
   auto alert_recorder = std::make_shared<TlsAlertRecorder>();
   server_->SetPacketFilter(alert_recorder);
 
-  ConnectExpectAlert(server_, kTlsAlertIllegalParameter);
+  ConnectExpectAlert(server_, kTlsAlertDecodeError);
   EXPECT_EQ(kTlsAlertFatal, alert_recorder->level());
-  EXPECT_EQ(kTlsAlertIllegalParameter, alert_recorder->description());
+  EXPECT_EQ(kTlsAlertDecodeError, alert_recorder->description());
 }
 
 TEST_P(TlsConnectGenericPre13, CaptureAlertClient) {
@@ -452,42 +451,6 @@ TEST_F(TlsConnectTest, ConnectSSLv3ClientAuth) {
   CheckKeys(ssl_kea_rsa, ssl_grp_none, ssl_auth_rsa_decrypt, ssl_sig_none);
 }
 
-TEST_F(TlsConnectStreamTls13, ClientAltHandshakeType) {
-  client_->SetAltHandshakeTypeEnabled();
-  auto filter = std::make_shared<TlsHeaderRecorder>();
-  server_->SetPacketFilter(filter);
-  Connect();
-  ASSERT_EQ(kTlsHandshakeType, filter->header(0)->content_type());
-}
-
-TEST_F(TlsConnectStreamTls13, ServerAltHandshakeType) {
-  server_->SetAltHandshakeTypeEnabled();
-  auto filter = std::make_shared<TlsHeaderRecorder>();
-  server_->SetPacketFilter(filter);
-  Connect();
-  ASSERT_EQ(kTlsHandshakeType, filter->header(0)->content_type());
-}
-
-TEST_F(TlsConnectStreamTls13, BothAltHandshakeType) {
-  client_->SetAltHandshakeTypeEnabled();
-  server_->SetAltHandshakeTypeEnabled();
-  auto header_filter = std::make_shared<TlsHeaderRecorder>();
-  auto sh_filter = std::make_shared<TlsInspectorRecordHandshakeMessage>(
-      kTlsHandshakeServerHello);
-  std::vector<std::shared_ptr<PacketFilter>> filters = {header_filter,
-                                                        sh_filter};
-  auto chained = std::make_shared<ChainedPacketFilter>(filters);
-  server_->SetPacketFilter(chained);
-  header_filter->SetAgent(server_.get());
-  header_filter->EnableDecryption();
-  Connect();
-  ASSERT_EQ(kTlsAltHandshakeType, header_filter->header(0)->content_type());
-  ASSERT_EQ(kTlsHandshakeType, header_filter->header(1)->content_type());
-  uint32_t ver;
-  ASSERT_TRUE(sh_filter->buffer().Read(0, 2, &ver));
-  ASSERT_EQ((uint32_t)(0x7a00 | TLS_1_3_DRAFT_VERSION), ver);
-}
-
 static size_t ExpectedCbcLen(size_t in, size_t hmac = 20, size_t block = 16) {
   // MAC-then-Encrypt expansion formula:
   return ((in + hmac + (block - 1)) / block) * block;

gtests/ssl_gtest/tls_agent.cc

@@ -380,7 +380,7 @@ void TlsAgent::Set0RttEnabled(bool en) {
 void TlsAgent::SetAltHandshakeTypeEnabled() {
   EXPECT_TRUE(EnsureTlsSetup());
 
-  SECStatus rv = SSL_UseAltServerHelloType(ssl_fd(), true);
+  SECStatus rv = SSL_UseAltHandshakeType(ssl_fd(), PR_TRUE);
   EXPECT_EQ(SECSuccess, rv);
 }