Bug 1166338 - Change SSL_REUSE_SERVER_ECDHE_KEY default to false. r=djackson

Differential Revision: https://phabricator.services.mozilla.com/D143514

6 files changed, 8 insertions, 12 deletions

diff --git a/gtests/ssl_gtest/ssl_fuzz_unittest.cc b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
index 4718dca17..ef6f7602c 100644
--- a/gtests/ssl_gtest/ssl_fuzz_unittest.cc
+++ b/gtests/ssl_gtest/ssl_fuzz_unittest.cc
@@ -57,7 +57,6 @@ FUZZ_P(TlsFuzzTest, DeterministicExporter) {
 
   Reset();
   ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
-  DisableECDHEServerKeyReuse();
 
   // Reset the RNG state.
   EXPECT_EQ(SECSuccess, RNG_RandomUpdate(NULL, 0));
@@ -71,7 +70,6 @@ FUZZ_P(TlsFuzzTest, DeterministicExporter) {
 
   Reset();
   ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
-  DisableECDHEServerKeyReuse();
 
   // Reset the RNG state.
   EXPECT_EQ(SECSuccess, RNG_RandomUpdate(NULL, 0));
@@ -97,7 +95,6 @@ FUZZ_P(TlsFuzzTest, DeterministicTranscript) {
   for (size_t i = 0; i < 5; i++) {
     Reset();
     ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
-    DisableECDHEServerKeyReuse();
 
     DataBuffer buffer;
     MakeTlsFilter<TlsConversationRecorder>(client_, buffer);
diff --git a/gtests/ssl_gtest/ssl_resumption_unittest.cc b/gtests/ssl_gtest/ssl_resumption_unittest.cc
index 53cddfe74..2e23fc096 100644
--- a/gtests/ssl_gtest/ssl_resumption_unittest.cc
+++ b/gtests/ssl_gtest/ssl_resumption_unittest.cc
@@ -457,10 +457,10 @@ TEST_P(TlsConnectGeneric, ServerSNICertTypeSwitch) {
   EXPECT_TRUE(SECITEM_ItemsAreEqual(&cert1->derCert, &cert2->derCert));
 }
 
-// Prior to TLS 1.3, we were not fully ephemeral; though 1.3 fixes that
 TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceReuseKey) {
   auto filter = MakeTlsFilter<TlsHandshakeRecorder>(
       server_, kTlsHandshakeServerKeyExchange);
+  EnableECDHEServerKeyReuse();
   Connect();
   CheckKeys();
   TlsServerKeyExchangeEcdhe dhe1;
@@ -468,6 +468,7 @@ TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceReuseKey) {
 
   // Restart
   Reset();
+  EnableECDHEServerKeyReuse();
   auto filter2 = MakeTlsFilter<TlsHandshakeRecorder>(
       server_, kTlsHandshakeServerKeyExchange);
   ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
@@ -485,7 +486,6 @@ TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceReuseKey) {
 
 // This test parses the ServerKeyExchange, which isn't in 1.3
 TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceNewKey) {
-  server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
   auto filter = MakeTlsFilter<TlsHandshakeRecorder>(
       server_, kTlsHandshakeServerKeyExchange);
   Connect();
@@ -495,7 +495,6 @@ TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceNewKey) {
 
   // Restart
   Reset();
-  server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
   auto filter2 = MakeTlsFilter<TlsHandshakeRecorder>(
       server_, kTlsHandshakeServerKeyExchange);
   ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
diff --git a/gtests/ssl_gtest/tls_agent.cc b/gtests/ssl_gtest/tls_agent.cc
index cfc1320b2..650be8449 100644
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -1172,9 +1172,9 @@ void TlsAgent::ConfigureSessionCache(SessionResumptionMode mode) {
             mode & RESUME_TICKET ? PR_TRUE : PR_FALSE);
 }
 
-void TlsAgent::DisableECDHEServerKeyReuse() {
+void TlsAgent::EnableECDHEServerKeyReuse() {
   ASSERT_EQ(TlsAgent::SERVER, role_);
-  SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
+  SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_TRUE);
 }
 
 static const std::string kTlsRolesAllArr[] = {"CLIENT", "SERVER"};
diff --git a/gtests/ssl_gtest/tls_agent.h b/gtests/ssl_gtest/tls_agent.h
index 21a6c0475..8b54155a5 100644
--- a/gtests/ssl_gtest/tls_agent.h
+++ b/gtests/ssl_gtest/tls_agent.h
@@ -193,7 +193,7 @@ class TlsAgent : public PollTarget {
   void SetDowngradeCheckVersion(uint16_t version);
   void CheckSecretsDestroyed();
   void ConfigNamedGroups(const std::vector<SSLNamedGroup>& groups);
-  void DisableECDHEServerKeyReuse();
+  void EnableECDHEServerKeyReuse();
   bool GetPeerChainLength(size_t* count);
   void CheckCipherSuite(uint16_t cipher_suite);
   void SetResumptionTokenCallback();
diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc
index 50a4d9661..fd10e34a7 100644
--- a/gtests/ssl_gtest/tls_connect.cc
+++ b/gtests/ssl_gtest/tls_connect.cc
@@ -900,8 +900,8 @@ void TlsConnectTestBase::CheckEarlyDataAccepted() {
   server_->CheckEarlyDataAccepted(expect_early_data_accepted_);
 }
 
-void TlsConnectTestBase::DisableECDHEServerKeyReuse() {
-  server_->DisableECDHEServerKeyReuse();
+void TlsConnectTestBase::EnableECDHEServerKeyReuse() {
+  server_->EnableECDHEServerKeyReuse();
 }
 
 void TlsConnectTestBase::SkipVersionChecks() {
diff --git a/gtests/ssl_gtest/tls_connect.h b/gtests/ssl_gtest/tls_connect.h
index d4d42a596..6a4795f83 100644
--- a/gtests/ssl_gtest/tls_connect.h
+++ b/gtests/ssl_gtest/tls_connect.h
@@ -134,7 +134,7 @@ class TlsConnectTestBase : public ::testing::Test {
   void Receive(size_t amount);
   void ExpectExtendedMasterSecret(bool expected);
   void ExpectEarlyDataAccepted(bool expected);
-  void DisableECDHEServerKeyReuse();
+  void EnableECDHEServerKeyReuse();
   void SkipVersionChecks();
 
   // Move the DTLS timers for both endpoints to pop the next timer.