Bug 1286140: HelloRetryRequest, r=ekr

--HG-- extra : rebase_source : 1e1b30a40bbcca8b7715133510fab5e25dac9729
sailfishos-mirror · Sep 12, 2016 · bb2b3c6 · bb2b3c6
1 parent 2240c2e
commit bb2b3c6
Show file tree

Hide file tree

Showing 30 changed files with 1,080 additions and 410 deletions.
diff --git a/cmd/lib/basicutil.c b/cmd/lib/basicutil.c
@@ -687,12 +687,7 @@ static unsigned char
 nibble(char c)
 {
     c = PORT_Tolower(c);
-    return (c >= '0' && c <= '9') ? c - '0' : (c >=
-                                                   'a' &&
-                                               c <=
-                                                   'f')
-                                                  ? c - 'a' + 10
-                                                  : -1;
+    return (c >= '0' && c <= '9') ? c - '0' : (c >= 'a' && c <= 'f') ? c - 'a' + 10 : -1;
 }
 
 SECStatus

diff --git a/external_tests/ssl_gtest/manifest.mn b/external_tests/ssl_gtest/manifest.mn
@@ -24,6 +24,7 @@ CPPSRCS = \
       ssl_ems_unittest.cc \
       ssl_extension_unittest.cc \
       ssl_gtest.cc \
+      ssl_hrr_unittest.cc \
       ssl_loopback_unittest.cc \
       ssl_record_unittest.cc \
       ssl_resumption_unittest.cc \

diff --git a/external_tests/ssl_gtest/ssl_0rtt_unittest.cc b/external_tests/ssl_gtest/ssl_0rtt_unittest.cc
@@ -22,20 +22,19 @@ extern "C" {
 
 namespace nss_test {
 
-TEST_F(TlsConnectTest, DamageSecretHandleZeroRttClientFinished) {
+TEST_P(TlsConnectTls13, ZeroRtt) {
   SetupForZeroRtt();
   client_->Set0RttEnabled(true);
   server_->Set0RttEnabled(true);
-  client_->SetPacketFilter(new AfterRecordN(
-      client_, server_,
-      0,  // ClientHello.
-      [this]() { SSLInt_DamageEarlyTrafficSecret(server_->ssl_fd()); }));
-  ConnectExpectFail();
-  client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
-  server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(true);
+  Handshake();
+  ExpectEarlyDataAccepted(true);
+  CheckConnected();
+  SendReceive();
 }
 
-TEST_F(TlsConnectTest, ZeroRttServerRejectByOption) {
+TEST_P(TlsConnectTls13, ZeroRttServerRejectByOption) {
   SetupForZeroRtt();
   client_->Set0RttEnabled(true);
   ExpectResumption(RESUME_TICKET);
@@ -45,7 +44,7 @@ TEST_F(TlsConnectTest, ZeroRttServerRejectByOption) {
   SendReceive();
 }
 
-TEST_F(TlsConnectTest, ZeroRttServerForgetTicket) {
+TEST_P(TlsConnectTls13, ZeroRttServerForgetTicket) {
   SetupForZeroRtt();
   client_->Set0RttEnabled(true);
   server_->Set0RttEnabled(true);
@@ -58,11 +57,7 @@ TEST_F(TlsConnectTest, ZeroRttServerForgetTicket) {
   SendReceive();
 }
 
-TEST_F(TlsConnectTest, ZeroRttServerOnly) {
-  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
-                           SSL_LIBRARY_VERSION_TLS_1_3);
+TEST_P(TlsConnectTls13, ZeroRttServerOnly) {
   ExpectResumption(RESUME_NONE);
   server_->Set0RttEnabled(true);
   client_->StartConnect();
@@ -84,19 +79,7 @@ TEST_F(TlsConnectTest, ZeroRttServerOnly) {
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign);
 }
 
-TEST_F(TlsConnectTest, ZeroRtt) {
-  SetupForZeroRtt();
-  client_->Set0RttEnabled(true);
-  server_->Set0RttEnabled(true);
-  ExpectResumption(RESUME_TICKET);
-  ZeroRttSendReceive(true);
-  Handshake();
-  ExpectEarlyDataAccepted(true);
-  CheckConnected();
-  SendReceive();
-}
-
-TEST_F(TlsConnectTest, TestTls13ZeroRttAlpn) {
+TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpn) {
   EnableAlpn();
   SetupForZeroRtt();
   EnableAlpn();
@@ -114,28 +97,9 @@ TEST_F(TlsConnectTest, TestTls13ZeroRttAlpn) {
   CheckAlpn("a");
 }
 
-// Remove the old ALPN value and so the client will not offer ALPN.
-TEST_F(TlsConnectTest, TestTls13ZeroRttAlpnChangeBoth) {
-  EnableAlpn();
-  SetupForZeroRtt();
-  static const uint8_t alpn[] = {0x01, 0x62};  // "b"
-  EnableAlpn(alpn, sizeof(alpn));
-  client_->Set0RttEnabled(true);
-  server_->Set0RttEnabled(true);
-  ExpectResumption(RESUME_TICKET);
-  ZeroRttSendReceive(false, [this]() {
-    client_->CheckAlpn(SSL_NEXT_PROTO_NO_SUPPORT);
-    return false;
-  });
-  Handshake();
-  CheckConnected();
-  SendReceive();
-  CheckAlpn("b");
-}
-
 // Have the server negotiate a different ALPN value, and therefore
 // reject 0-RTT.
-TEST_F(TlsConnectTest, TestTls13ZeroRttAlpnChangeServer) {
+TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpnChangeServer) {
   EnableAlpn();
   SetupForZeroRtt();
   static const uint8_t client_alpn[] = {0x01, 0x61, 0x01, 0x62};  // "a", "b"
@@ -159,7 +123,7 @@ TEST_F(TlsConnectTest, TestTls13ZeroRttAlpnChangeServer) {
 // Stomp the ALPN on the client after sending the ClientHello so
 // that the server selection appears to be incorrect. The client
 // should then fail the connection.
-TEST_F(TlsConnectTest, TestTls13ZeroRttNoAlpnServer) {
+TEST_P(TlsConnectTls13, TestTls13ZeroRttNoAlpnServer) {
   EnableAlpn();
   SetupForZeroRtt();
   client_->Set0RttEnabled(true);
@@ -181,7 +145,7 @@ TEST_F(TlsConnectTest, TestTls13ZeroRttNoAlpnServer) {
 // Set up with no ALPN and then set the client so it thinks it has ALPN.
 // The server responds without the extension and the client returns an
 // error.
-TEST_F(TlsConnectTest, TestTls13ZeroRttNoAlpnClient) {
+TEST_P(TlsConnectTls13, TestTls13ZeroRttNoAlpnClient) {
   SetupForZeroRtt();
   client_->Set0RttEnabled(true);
   server_->Set0RttEnabled(true);
@@ -197,4 +161,36 @@ TEST_F(TlsConnectTest, TestTls13ZeroRttNoAlpnClient) {
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
+// Remove the old ALPN value and so the client will not offer early data.
+TEST_P(TlsConnectTls13, TestTls13ZeroRttAlpnChangeBoth) {
+  EnableAlpn();
+  SetupForZeroRtt();
+  static const uint8_t alpn[] = {0x01, 0x62};  // "b"
+  EnableAlpn(alpn, sizeof(alpn));
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+  ExpectResumption(RESUME_TICKET);
+  ZeroRttSendReceive(false, [this]() {
+    client_->CheckAlpn(SSL_NEXT_PROTO_NO_SUPPORT);
+    return false;
+  });
+  Handshake();
+  CheckConnected();
+  SendReceive();
+  CheckAlpn("b");
+}
+
+TEST_F(TlsConnectTest, DamageSecretHandleZeroRttClientFinished) {
+  SetupForZeroRtt();
+  client_->Set0RttEnabled(true);
+  server_->Set0RttEnabled(true);
+  client_->SetPacketFilter(new AfterRecordN(
+      client_, server_,
+      0,  // ClientHello.
+      [this]() { SSLInt_DamageEarlyTrafficSecret(server_->ssl_fd()); }));
+  ConnectExpectFail();
+  client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
+  server_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
+}
+
 }  // namespace nss_test
diff --git a/external_tests/ssl_gtest/ssl_damage_unittest.cc b/external_tests/ssl_gtest/ssl_damage_unittest.cc
@@ -55,7 +55,7 @@ TEST_F(TlsConnectTest, DamageSecretHandleServerFinished) {
       [this]() { SSLInt_DamageHsTrafficSecret(client_->ssl_fd()); }));
   ConnectExpectFail();
   client_->CheckErrorCode(SSL_ERROR_BAD_HANDSHAKE_HASH_VALUE);
-  server_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT);
+  server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ);
 }
 
 }  // namespace nspr_test
diff --git a/external_tests/ssl_gtest/ssl_dhe_unittest.cc b/external_tests/ssl_gtest/ssl_dhe_unittest.cc
@@ -534,10 +534,10 @@ TEST_P(TlsConnectTls13, ResumeFfdhe) {
   ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET);
   EnableOnlyDheCiphers();
   TlsExtensionCapture* clientCapture =
-      new TlsExtensionCapture(kTlsExtensionPreSharedKey);
+      new TlsExtensionCapture(ssl_tls13_pre_shared_key_xtn);
   client_->SetPacketFilter(clientCapture);
   TlsExtensionCapture* serverCapture =
-      new TlsExtensionCapture(kTlsExtensionPreSharedKey);
+      new TlsExtensionCapture(ssl_tls13_pre_shared_key_xtn);
   server_->SetPacketFilter(serverCapture);
   ExpectResumption(RESUME_TICKET);
   Connect();

diff --git a/external_tests/ssl_gtest/ssl_drop_unittest.cc b/external_tests/ssl_gtest/ssl_drop_unittest.cc
@@ -20,25 +20,6 @@ extern "C" {
 
 namespace nss_test {
 
-// This class selectively drops complete writes.  This relies on the fact that
-// writes in libssl are on record boundaries.
-class SelectiveDropFilter : public PacketFilter, public PollTarget {
- public:
-  SelectiveDropFilter(uint32_t pattern) : pattern_(pattern), counter_(0) {}
-
- protected:
-  virtual Action Filter(const DataBuffer& input, DataBuffer* output) override {
-    if (counter_ >= 32) {
-      return KEEP;
-    }
-    return ((1 << counter_++) & pattern_) ? DROP : KEEP;
-  }
-
- private:
-  const uint32_t pattern_;
-  uint8_t counter_;
-};
-
 TEST_P(TlsConnectDatagram, DropClientFirstFlightOnce) {
   client_->SetPacketFilter(new SelectiveDropFilter(0x1));
   Connect();

diff --git a/external_tests/ssl_gtest/ssl_ecdh_unittest.cc b/external_tests/ssl_gtest/ssl_ecdh_unittest.cc
@@ -51,20 +51,32 @@ TEST_P(TlsConnectGeneric, ConnectEcdhe) {
 // If we pick a 256-bit cipher suite and use a P-384 certificate, the server
 // should choose P-384 for key exchange too.  Only valid for TLS >=1.2 because
 // we don't have 256-bit ciphers before then.
-// TODO: Re-enable for 1.3 when Bug 1286140 lands.
-TEST_P(TlsConnectTls12, ConnectEcdheP384) {
+TEST_P(TlsConnectTls12Plus, ConnectEcdheP384) {
   Reset(TlsAgent::kServerEcdsa384);
   ConnectWithCipherSuite(TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256);
   CheckKeys(ssl_kea_ecdh, ssl_auth_ecdsa, 384);
 }
 
-TEST_P(TlsConnectGeneric, ConnectEcdheP384) {
+TEST_P(TlsConnectGeneric, ConnectEcdheP384Client) {
   EnsureTlsSetup();
   client_->ConfigNamedGroup(ssl_grp_ec_secp256r1, false);
   Connect();
   CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign, 384);
 }
 
+// This causes a HelloRetryRequest in TLS 1.3.  Earlier versions don't care.
+TEST_P(TlsConnectGeneric, ConnectEcdheP384Server) {
+  EnsureTlsSetup();
+  auto hrr_capture =
+      new TlsInspectorRecordHandshakeMessage(kTlsHandshakeHelloRetryRequest);
+  server_->SetPacketFilter(hrr_capture);
+  server_->ConfigNamedGroup(ssl_grp_ec_secp256r1, false);
+  Connect();
+  CheckKeys(ssl_kea_ecdh, ssl_auth_rsa_sign, 384);
+  EXPECT_EQ(version_ == SSL_LIBRARY_VERSION_TLS_1_3,
+            hrr_capture->buffer().len() != 0);
+}
+
 // This enables only P-256 on the client and disables it on the server.
 // This test will fail when we add other groups that identify as ECDHE.
 TEST_P(TlsConnectGeneric, ConnectEcdheGroupMismatch) {

diff --git a/external_tests/ssl_gtest/ssl_extension_unittest.cc b/external_tests/ssl_gtest/ssl_extension_unittest.cc
@@ -542,8 +542,8 @@ TEST_P(TlsExtensionTest13, ModifyDraftVersionAndFail) {
   EXPECT_EQ(SSL_ERROR_UNSUPPORTED_VERSION, server_->error_code());
 }
 
-// This test only works with TLS because the MAC error causes a
-// timeout on the server.
+// This test only works in stream mode because the MAC error causes a timeout on
+// the server with datagram.
 TEST_F(TlsExtensionTest13Stream, DropServerKeyShare) {
   EnsureTlsSetup();
   server_->SetPacketFilter(new TlsExtensionDropper(ssl_tls13_key_share_xtn));