Bug 1375837 - Allow TLS 1.3 when renegotiation isn't enabled, r=ekr
authorMartin Thomson <martin.thomson@gmail.com>
Mon, 04 Sep 2017 15:05:11 +1000
changeset 13566 65d22612db0d887fa2b8d5bc6f612e1dfc9856e6
parent 13565 5d20e29c616a8c28ba5a2d44767d1e7d55448a20
child 13567 58cc6cee4bc115efb160f9744faf4fed3799a8df
push id2354
push usermartin.thomson@gmail.com
push dateWed, 06 Sep 2017 03:35:04 +0000
reviewersekr
bugs1375837
Bug 1375837 - Allow TLS 1.3 when renegotiation isn't enabled, r=ekr
gtests/ssl_gtest/manifest.mn
gtests/ssl_gtest/ssl_cert_ext_unittest.cc
gtests/ssl_gtest/ssl_dhe_unittest.cc
gtests/ssl_gtest/ssl_gtest.gyp
gtests/ssl_gtest/ssl_loopback_unittest.cc
gtests/ssl_gtest/ssl_renegotiation_unittest.cc
gtests/ssl_gtest/ssl_resumption_unittest.cc
gtests/ssl_gtest/ssl_staticrsa_unittest.cc
gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
gtests/ssl_gtest/ssl_version_unittest.cc
gtests/ssl_gtest/tls_agent.cc
gtests/ssl_gtest/tls_agent.h
lib/ssl/ssl3con.c
lib/ssl/ssl3exthandle.c
--- a/gtests/ssl_gtest/manifest.mn
+++ b/gtests/ssl_gtest/manifest.mn
@@ -28,16 +28,17 @@ CPPSRCS = \
       ssl_fuzz_unittest.cc \
       ssl_gather_unittest.cc \
       ssl_gtest.cc \
       ssl_hrr_unittest.cc \
       ssl_loopback_unittest.cc \
       ssl_misc_unittest.cc \
       ssl_record_unittest.cc \
       ssl_resumption_unittest.cc \
+      ssl_renegotiation_unittest.cc \
       ssl_skip_unittest.cc \
       ssl_staticrsa_unittest.cc \
       ssl_v2_client_hello_unittest.cc \
       ssl_version_unittest.cc \
       ssl_versionpolicy_unittest.cc \
       selfencrypt_unittest.cc \
       test_io.cc \
       tls_agent.cc \
--- a/gtests/ssl_gtest/ssl_cert_ext_unittest.cc
+++ b/gtests/ssl_gtest/ssl_cert_ext_unittest.cc
@@ -77,33 +77,30 @@ TEST_P(TlsConnectGenericPre13, SignedCer
   ScopedCERTCertificate cert;
   ScopedSECKEYPrivateKey priv;
   ASSERT_TRUE(TlsAgent::LoadCertificate(TlsAgent::kServerRsa, &cert, &priv));
   EXPECT_EQ(SECSuccess, SSL_ConfigSecureServerWithCertChain(
                             server_->ssl_fd(), cert.get(), nullptr, priv.get(),
                             ssl_kea_rsa));
   EXPECT_EQ(SECSuccess, SSL_SetSignedCertTimestamps(server_->ssl_fd(),
                                                     &kSctItem, ssl_kea_rsa));
-  EXPECT_EQ(SECSuccess,
-            SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
-                          PR_TRUE));
+
+  client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
   SignedCertificateTimestampsExtractor timestamps_extractor(client_);
 
   Connect();
 
   timestamps_extractor.assertTimestamps(kSctBuffer);
 }
 
 TEST_P(TlsConnectGeneric, SignedCertificateTimestampsSuccess) {
   EnsureTlsSetup();
   EXPECT_TRUE(
       server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kExtraSctData));
-  EXPECT_EQ(SECSuccess,
-            SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
-                          PR_TRUE));
+  client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
   SignedCertificateTimestampsExtractor timestamps_extractor(client_);
 
   Connect();
 
   timestamps_extractor.assertTimestamps(kSctBuffer);
 }
 
 // Test SSL_PeerSignedCertTimestamps returning zero-length SECItem
@@ -115,19 +112,17 @@ TEST_P(TlsConnectGeneric, SignedCertific
   SignedCertificateTimestampsExtractor timestamps_extractor(client_);
 
   Connect();
   timestamps_extractor.assertTimestamps(DataBuffer());
 }
 
 TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveServer) {
   EnsureTlsSetup();
-  EXPECT_EQ(SECSuccess,
-            SSL_OptionSet(client_->ssl_fd(), SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
-                          PR_TRUE));
+  client_->SetOption(SSL_ENABLE_SIGNED_CERT_TIMESTAMPS, PR_TRUE);
   SignedCertificateTimestampsExtractor timestamps_extractor(client_);
 
   Connect();
   timestamps_extractor.assertTimestamps(DataBuffer());
 }
 
 TEST_P(TlsConnectGeneric, SignedCertificateTimestampsInactiveBoth) {
   EnsureTlsSetup();
@@ -168,42 +163,39 @@ TEST_P(TlsConnectGeneric, OcspNotRequest
   EXPECT_TRUE(
       server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kOcspExtraData));
   Connect();
 }
 
 // Even if the client asks, the server has nothing unless it is configured.
 TEST_P(TlsConnectGeneric, OcspNotProvided) {
   EnsureTlsSetup();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
+  client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
   client_->SetAuthCertificateCallback(CheckNoOCSP);
   Connect();
 }
 
 TEST_P(TlsConnectGenericPre13, OcspMangled) {
   EnsureTlsSetup();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
+  client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
   EXPECT_TRUE(
       server_->ConfigServerCert(TlsAgent::kServerRsa, true, &kOcspExtraData));
 
   static const uint8_t val[] = {1};
   auto replacer = std::make_shared<TlsExtensionReplacer>(
       ssl_cert_status_xtn, DataBuffer(val, sizeof(val)));
   server_->SetPacketFilter(replacer);
   ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
   client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_SERVER_HELLO);
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
 TEST_P(TlsConnectGeneric, OcspSuccess) {
   EnsureTlsSetup();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
+  client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
   auto capture_ocsp =
       std::make_shared<TlsExtensionCapture>(ssl_cert_status_xtn);
   server_->SetPacketFilter(capture_ocsp);
 
   // The value should be available during the AuthCertificateCallback
   client_->SetAuthCertificateCallback([](TlsAgent* agent, bool checksig,
                                          bool isServer) -> SECStatus {
     const SECItemArray* ocsp = SSL_PeerStapledOCSPResponses(agent->ssl_fd());
@@ -220,18 +212,17 @@ TEST_P(TlsConnectGeneric, OcspSuccess) {
   Connect();
   // In TLS 1.3, the server doesn't provide a visible ServerHello extension.
   // For earlier versions, the extension is just empty.
   EXPECT_EQ(0U, capture_ocsp->extension().len());
 }
 
 TEST_P(TlsConnectGeneric, OcspHugeSuccess) {
   EnsureTlsSetup();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_ENABLE_OCSP_STAPLING, PR_TRUE));
+  client_->SetOption(SSL_ENABLE_OCSP_STAPLING, PR_TRUE);
 
   uint8_t hugeOcspValue[16385];
   memset(hugeOcspValue, 0xa1, sizeof(hugeOcspValue));
   const SECItem hugeOcspItems[] = {
       {siBuffer, const_cast<uint8_t*>(hugeOcspValue), sizeof(hugeOcspValue)}};
   const SECItemArray hugeOcspResponses = {const_cast<SECItem*>(hugeOcspItems),
                                           PR_ARRAY_SIZE(hugeOcspItems)};
   const SSLExtraServerCertData hugeOcspExtraData = {
--- a/gtests/ssl_gtest/ssl_dhe_unittest.cc
+++ b/gtests/ssl_gtest/ssl_dhe_unittest.cc
@@ -54,18 +54,17 @@ TEST_P(TlsConnectTls13, SharesForBothEcd
   CheckGroups(groups_capture->extension(), track_group_type);
   CheckShares(shares_capture->extension(), track_group_type);
   EXPECT_TRUE(ec) << "Should include an EC group and share";
   EXPECT_TRUE(dh) << "Should include an FFDHE group and share";
 }
 
 TEST_P(TlsConnectGeneric, ConnectFfdheClient) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   auto groups_capture =
       std::make_shared<TlsExtensionCapture>(ssl_supported_groups_xtn);
   auto shares_capture =
       std::make_shared<TlsExtensionCapture>(ssl_tls13_key_share_xtn);
   std::vector<std::shared_ptr<PacketFilter>> captures = {groups_capture,
                                                          shares_capture};
   client_->SetPacketFilter(std::make_shared<ChainedPacketFilter>(captures));
 
@@ -85,18 +84,17 @@ TEST_P(TlsConnectGeneric, ConnectFfdheCl
   }
 }
 
 // Requiring the FFDHE extension on the server alone means that clients won't be
 // able to connect using a DHE suite.  They should still connect in TLS 1.3,
 // because the client automatically sends the supported groups extension.
 TEST_P(TlsConnectGenericPre13, ConnectFfdheServer) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(server_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  server_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
 
   if (version_ >= SSL_LIBRARY_VERSION_TLS_1_3) {
     Connect();
     CheckKeys(ssl_kea_dh, ssl_auth_rsa_sign);
   } else {
     ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
     client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
     server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
@@ -121,18 +119,17 @@ class TlsDheServerKeyExchangeDamager : p
   }
 };
 
 // Changing the prime in the server's key share results in an error.  This will
 // invalidate the signature over the ServerKeyShare. That's ok, NSS won't check
 // the signature until everything else has been checked.
 TEST_P(TlsConnectGenericPre13, DamageServerKeyShare) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   server_->SetPacketFilter(std::make_shared<TlsDheServerKeyExchangeDamager>());
 
   ConnectExpectAlert(client_, kTlsAlertIllegalParameter);
 
   client_->CheckErrorCode(SSL_ERROR_WEAK_SERVER_EPHEMERAL_DH_KEY);
   server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
 }
 
@@ -284,18 +281,17 @@ class TlsDamageDHYTest
  public:
   TlsDamageDHYTest()
       : TlsConnectTestBase(std::get<0>(GetParam()), std::get<1>(GetParam())) {}
 };
 
 TEST_P(TlsDamageDHYTest, DamageServerY) {
   EnableOnlyDheCiphers();
   if (std::get<3>(GetParam())) {
-    EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                        SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+    client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   }
   TlsDheSkeChangeY::ChangeYTo change = std::get<2>(GetParam());
   server_->SetPacketFilter(
       std::make_shared<TlsDheSkeChangeYServer>(change, true));
 
   if (change == TlsDheSkeChangeY::kYZeroPad) {
     ExpectAlert(client_, kTlsAlertDecryptError);
   } else {
@@ -315,18 +311,17 @@ TEST_P(TlsDamageDHYTest, DamageServerY) 
     client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_DHE_KEY_SHARE);
     server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
   }
 }
 
 TEST_P(TlsDamageDHYTest, DamageClientY) {
   EnableOnlyDheCiphers();
   if (std::get<3>(GetParam())) {
-    EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                        SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+    client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   }
   // The filter on the server is required to capture the prime.
   auto server_filter =
       std::make_shared<TlsDheSkeChangeYServer>(TlsDheSkeChangeY::kYZero, false);
   server_->SetPacketFilter(server_filter);
 
   // The client filter does the damage.
   TlsDheSkeChangeY::ChangeYTo change = std::get<2>(GetParam());
@@ -440,18 +435,17 @@ TEST_P(TlsConnectGenericPre13, PadDheP) 
 
 // The server should not pick the weak DH group if the client includes FFDHE
 // named groups in the supported_groups extension. The server then picks a
 // commonly-supported named DH group and this connects.
 //
 // Note: This test case can take ages to generate the weak DH key.
 TEST_P(TlsConnectGenericPre13, WeakDHGroup) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   EXPECT_EQ(SECSuccess,
             SSL_EnableWeakDHEPrimeGroup(server_->ssl_fd(), PR_TRUE));
 
   Connect();
 }
 
 TEST_P(TlsConnectGeneric, Ffdhe3072) {
   EnableOnlyDheCiphers();
@@ -491,18 +485,17 @@ TEST_P(TlsConnectTls13, NamedGroupMismat
   client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
 }
 
 // Even though the client doesn't have DHE groups enabled the server assumes it
 // does. The client requires named groups and thus does not accept FF3072 as
 // custom group in contrast to the previous test.
 TEST_P(TlsConnectGenericPre13, RequireNamedGroupsMismatchPre13) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   static const std::vector<SSLNamedGroup> server_groups = {ssl_grp_ffdhe_3072};
   static const std::vector<SSLNamedGroup> client_groups = {ssl_grp_ec_secp256r1,
                                                            ssl_grp_ffdhe_2048};
   server_->ConfigNamedGroups(server_groups);
   client_->ConfigNamedGroups(client_groups);
 
   ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
   server_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP);
@@ -520,18 +513,17 @@ TEST_P(TlsConnectGenericPre13, Preferred
   client_->CheckKEA(ssl_kea_dh, ssl_grp_ffdhe_3072, 3072);
   server_->CheckKEA(ssl_kea_dh, ssl_grp_ffdhe_3072, 3072);
   client_->CheckAuthType(ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
   server_->CheckAuthType(ssl_auth_rsa_sign, ssl_sig_rsa_pss_sha256);
 }
 
 TEST_P(TlsConnectGenericPre13, MismatchDHE) {
   EnableOnlyDheCiphers();
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(client_->ssl_fd(),
-                                      SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE));
+  client_->SetOption(SSL_REQUIRE_DH_NAMED_GROUPS, PR_TRUE);
   static const SSLDHEGroupType serverGroups[] = {ssl_ff_dhe_3072_group};
   EXPECT_EQ(SECSuccess, SSL_DHEGroupPrefSet(server_->ssl_fd(), serverGroups,
                                             PR_ARRAY_SIZE(serverGroups)));
   static const SSLDHEGroupType clientGroups[] = {ssl_ff_dhe_2048_group};
   EXPECT_EQ(SECSuccess, SSL_DHEGroupPrefSet(client_->ssl_fd(), clientGroups,
                                             PR_ARRAY_SIZE(clientGroups)));
 
   ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
--- a/gtests/ssl_gtest/ssl_gtest.gyp
+++ b/gtests/ssl_gtest/ssl_gtest.gyp
@@ -29,16 +29,17 @@
         'ssl_fragment_unittest.cc',
         'ssl_gather_unittest.cc',
         'ssl_gtest.cc',
         'ssl_hrr_unittest.cc',
         'ssl_loopback_unittest.cc',
         'ssl_misc_unittest.cc',
         'ssl_record_unittest.cc',
         'ssl_resumption_unittest.cc',
+        'ssl_renegotiation_unittest.cc',
         'ssl_skip_unittest.cc',
         'ssl_staticrsa_unittest.cc',
         'ssl_v2_client_hello_unittest.cc',
         'ssl_version_unittest.cc',
         'ssl_versionpolicy_unittest.cc',
         'test_io.cc',
         'tls_agent.cc',
         'tls_connect.cc',
--- a/gtests/ssl_gtest/ssl_loopback_unittest.cc
+++ b/gtests/ssl_gtest/ssl_loopback_unittest.cc
@@ -163,34 +163,16 @@ TEST_P(TlsConnectGeneric, ConnectAlpnClo
 
 TEST_P(TlsConnectDatagram, ConnectSrtp) {
   EnableSrtp();
   Connect();
   CheckSrtp();
   SendReceive();
 }
 
-// 1.3 is disabled in the next few tests because we don't
-// presently support resumption in 1.3.
-TEST_P(TlsConnectStreamPre13, ConnectAndClientRenegotiate) {
-  Connect();
-  server_->PrepareForRenegotiate();
-  client_->StartRenegotiate();
-  Handshake();
-  CheckConnected();
-}
-
-TEST_P(TlsConnectStreamPre13, ConnectAndServerRenegotiate) {
-  Connect();
-  client_->PrepareForRenegotiate();
-  server_->StartRenegotiate();
-  Handshake();
-  CheckConnected();
-}
-
 TEST_P(TlsConnectGeneric, ConnectSendReceive) {
   Connect();
   SendReceive();
 }
 
 // The next two tests takes advantage of the fact that we
 // automatically read the first 1024 bytes, so if
 // we provide 1200 bytes, they overrun the read buffer
@@ -225,18 +207,18 @@ TEST_P(TlsConnectStream, ShortRead) {
   client_->ReadBytes();
   ASSERT_EQ(50U, client_->received_bytes());
 }
 
 // We enable compression via the API but it's disabled internally,
 // so we should never get it.
 TEST_P(TlsConnectGeneric, ConnectWithCompressionEnabled) {
   EnsureTlsSetup();
-  client_->EnableCompression();
-  server_->EnableCompression();
+  client_->SetOption(SSL_ENABLE_DEFLATE, PR_TRUE);
+  server_->SetOption(SSL_ENABLE_DEFLATE, PR_TRUE);
   Connect();
   EXPECT_FALSE(client_->is_compressed());
   SendReceive();
 }
 
 TEST_P(TlsConnectDatagram, TestDtlsHolddownExpiry) {
   Connect();
   std::cerr << "Expiring holddown timer\n";
new file mode 100644
--- /dev/null
+++ b/gtests/ssl_gtest/ssl_renegotiation_unittest.cc
@@ -0,0 +1,144 @@
+/* -*- 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 <functional>
+#include <memory>
+#include "secerr.h"
+#include "ssl.h"
+#include "sslerr.h"
+#include "sslproto.h"
+
+#include "gtest_utils.h"
+#include "tls_connect.h"
+
+namespace nss_test {
+
+// 1.3 is disabled in the next few tests because we don't
+// presently support resumption in 1.3.
+TEST_P(TlsConnectStreamPre13, RenegotiateClient) {
+  Connect();
+  server_->PrepareForRenegotiate();
+  client_->StartRenegotiate();
+  Handshake();
+  CheckConnected();
+}
+
+TEST_P(TlsConnectStreamPre13, RenegotiateServer) {
+  Connect();
+  client_->PrepareForRenegotiate();
+  server_->StartRenegotiate();
+  Handshake();
+  CheckConnected();
+}
+
+// The renegotiation options shouldn't cause an error if TLS 1.3 is chosen.
+TEST_F(TlsConnectTest, RenegotiationConfigTls13) {
+  EnsureTlsSetup();
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetOption(SSL_ENABLE_RENEGOTIATION, SSL_RENEGOTIATE_UNRESTRICTED);
+  server_->SetOption(SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
+  Connect();
+  SendReceive();
+  CheckKeys();
+}
+
+TEST_P(TlsConnectStream, ConnectTls10AndServerRenegotiateHigher) {
+  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
+    return;
+  }
+  // Set the client so it will accept any version from 1.0
+  // to |version_|.
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
+                           SSL_LIBRARY_VERSION_TLS_1_0);
+  // Reset version so that the checks succeed.
+  uint16_t test_version = version_;
+  version_ = SSL_LIBRARY_VERSION_TLS_1_0;
+  Connect();
+
+  // Now renegotiate, with the server being set to do
+  // |version_|.
+  client_->PrepareForRenegotiate();
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
+  // Reset version and cipher suite so that the preinfo callback
+  // doesn't fail.
+  server_->ResetPreliminaryInfo();
+  server_->StartRenegotiate();
+
+  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    ExpectAlert(server_, kTlsAlertUnexpectedMessage);
+  } else {
+    ExpectAlert(client_, kTlsAlertIllegalParameter);
+  }
+
+  Handshake();
+  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    // In TLS 1.3, the server detects this problem.
+    client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
+    server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
+  } else {
+    client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+    server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  }
+}
+
+TEST_P(TlsConnectStream, ConnectTls10AndClientRenegotiateHigher) {
+  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
+    return;
+  }
+  // Set the client so it will accept any version from 1.0
+  // to |version_|.
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
+                           SSL_LIBRARY_VERSION_TLS_1_0);
+  // Reset version so that the checks succeed.
+  uint16_t test_version = version_;
+  version_ = SSL_LIBRARY_VERSION_TLS_1_0;
+  Connect();
+
+  // Now renegotiate, with the server being set to do
+  // |version_|.
+  server_->PrepareForRenegotiate();
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
+  // Reset version and cipher suite so that the preinfo callback
+  // doesn't fail.
+  server_->ResetPreliminaryInfo();
+  client_->StartRenegotiate();
+  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    ExpectAlert(server_, kTlsAlertUnexpectedMessage);
+  } else {
+    ExpectAlert(client_, kTlsAlertIllegalParameter);
+  }
+  Handshake();
+  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+    // In TLS 1.3, the server detects this problem.
+    client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
+    server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
+  } else {
+    client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+    server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
+  }
+}
+
+TEST_F(TlsConnectTest, Tls13RejectsRehandshakeClient) {
+  EnsureTlsSetup();
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  SECStatus rv = SSL_ReHandshake(client_->ssl_fd(), PR_TRUE);
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
+}
+
+TEST_F(TlsConnectTest, Tls13RejectsRehandshakeServer) {
+  EnsureTlsSetup();
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
+  Connect();
+  SECStatus rv = SSL_ReHandshake(server_->ssl_fd(), PR_TRUE);
+  EXPECT_EQ(SECFailure, rv);
+  EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
+}
+
+}  // namespace nss_test
--- a/gtests/ssl_gtest/ssl_resumption_unittest.cc
+++ b/gtests/ssl_gtest/ssl_resumption_unittest.cc
@@ -350,33 +350,28 @@ TEST_P(TlsConnectGenericPre13, ConnectEc
   // Make sure they are the same.
   EXPECT_EQ(dhe1.public_key_.len(), dhe2.public_key_.len());
   EXPECT_TRUE(!memcmp(dhe1.public_key_.data(), dhe2.public_key_.data(),
                       dhe1.public_key_.len()));
 }
 
 // This test parses the ServerKeyExchange, which isn't in 1.3
 TEST_P(TlsConnectGenericPre13, ConnectEcdheTwiceNewKey) {
-  server_->EnsureTlsSetup();
-  SECStatus rv =
-      SSL_OptionSet(server_->ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
+  server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
   auto i1 = std::make_shared<TlsInspectorRecordHandshakeMessage>(
       kTlsHandshakeServerKeyExchange);
   server_->SetPacketFilter(i1);
   Connect();
   CheckKeys();
   TlsServerKeyExchangeEcdhe dhe1;
   EXPECT_TRUE(dhe1.Parse(i1->buffer()));
 
   // Restart
   Reset();
-  server_->EnsureTlsSetup();
-  rv = SSL_OptionSet(server_->ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
+  server_->SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
   auto i2 = std::make_shared<TlsInspectorRecordHandshakeMessage>(
       kTlsHandshakeServerKeyExchange);
   server_->SetPacketFilter(i2);
   ConfigureSessionCache(RESUME_NONE, RESUME_NONE);
   Connect();
   CheckKeys();
 
   TlsServerKeyExchangeEcdhe dhe2;
--- a/gtests/ssl_gtest/ssl_staticrsa_unittest.cc
+++ b/gtests/ssl_gtest/ssl_staticrsa_unittest.cc
@@ -66,17 +66,17 @@ TEST_P(TlsConnectStreamPre13, ConnectSta
 
 // Test that a PMS with a bogus version number is ignored when
 // rollback detection is disabled. This is a positive control for
 // ConnectStaticRSABogusPMSVersionDetect.
 TEST_P(TlsConnectGenericPre13, ConnectStaticRSABogusPMSVersionIgnore) {
   EnableOnlyStaticRsaCiphers();
   client_->SetPacketFilter(
       std::make_shared<TlsInspectorClientHelloVersionChanger>(server_));
-  server_->DisableRollbackDetection();
+  server_->SetOption(SSL_ROLLBACK_DETECTION, PR_FALSE);
   Connect();
 }
 
 // This test is stream so we can catch the bad_record_mac alert.
 TEST_P(TlsConnectStreamPre13, ConnectExtendedMasterSecretStaticRSABogusCKE) {
   EnableOnlyStaticRsaCiphers();
   EnableExtendedMasterSecret();
   auto inspect = std::make_shared<TlsInspectorReplaceHandshakeMessage>(
@@ -97,13 +97,13 @@ TEST_P(TlsConnectStreamPre13,
 }
 
 TEST_P(TlsConnectStreamPre13,
        ConnectExtendedMasterSecretStaticRSABogusPMSVersionIgnore) {
   EnableOnlyStaticRsaCiphers();
   EnableExtendedMasterSecret();
   client_->SetPacketFilter(
       std::make_shared<TlsInspectorClientHelloVersionChanger>(server_));
-  server_->DisableRollbackDetection();
+  server_->SetOption(SSL_ROLLBACK_DETECTION, PR_FALSE);
   Connect();
 }
 
 }  // namespace nspr_test
--- a/gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
+++ b/gtests/ssl_gtest/ssl_v2_client_hello_unittest.cc
@@ -148,23 +148,16 @@ class SSLv2ClientHelloTestF : public Tls
       : TlsConnectTestBase(variant, version), filter_(nullptr) {}
 
   void SetUp() {
     TlsConnectTestBase::SetUp();
     filter_ = std::make_shared<SSLv2ClientHelloFilter>(client_, version_);
     client_->SetPacketFilter(filter_);
   }
 
-  void RequireSafeRenegotiation() {
-    server_->EnsureTlsSetup();
-    SECStatus rv =
-        SSL_OptionSet(server_->ssl_fd(), SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
-    EXPECT_EQ(rv, SECSuccess);
-  }
-
   void SetExpectedVersion(uint16_t version) {
     TlsConnectTestBase::SetExpectedVersion(version);
     filter_->SetVersion(version);
   }
 
   void SetAvailableCipherSuite(uint16_t cipher) {
     filter_->SetCipherSuites(std::vector<uint16_t>(1, cipher));
   }
@@ -314,26 +307,26 @@ TEST_P(SSLv2ClientHelloTest, BigClientRa
 
   ConnectExpectAlert(server_, kTlsAlertIllegalParameter);
   EXPECT_EQ(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, server_->error_code());
 }
 
 // Connection must fail if we require safe renegotiation but the client doesn't
 // include TLS_EMPTY_RENEGOTIATION_INFO_SCSV in the list of cipher suites.
 TEST_P(SSLv2ClientHelloTest, RequireSafeRenegotiation) {
-  RequireSafeRenegotiation();
+  server_->SetOption(SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
   SetAvailableCipherSuite(TLS_DHE_RSA_WITH_AES_128_CBC_SHA);
   ConnectExpectAlert(server_, kTlsAlertHandshakeFailure);
   EXPECT_EQ(SSL_ERROR_UNSAFE_NEGOTIATION, server_->error_code());
 }
 
 // Connection must succeed when requiring safe renegotiation and the client
 // includes TLS_EMPTY_RENEGOTIATION_INFO_SCSV in the list of cipher suites.
 TEST_P(SSLv2ClientHelloTest, RequireSafeRenegotiationWithSCSV) {
-  RequireSafeRenegotiation();
+  server_->SetOption(SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
   std::vector<uint16_t> cipher_suites = {TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
                                          TLS_EMPTY_RENEGOTIATION_INFO_SCSV};
   SetAvailableCipherSuites(cipher_suites);
   Connect();
 }
 
 // Connect to the server with TLS 1.1, signalling that this is a fallback from
 // a higher version. As the server doesn't support anything higher than TLS 1.1
--- a/gtests/ssl_gtest/ssl_version_unittest.cc
+++ b/gtests/ssl_gtest/ssl_version_unittest.cc
@@ -123,22 +123,22 @@ TEST_F(TlsConnectTest, TestFallbackFromT
   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());
 }
 #endif
 
 TEST_P(TlsConnectGeneric, TestFallbackSCSVVersionMatch) {
-  client_->SetFallbackSCSVEnabled(true);
+  client_->SetOption(SSL_ENABLE_FALLBACK_SCSV, PR_TRUE);
   Connect();
 }
 
 TEST_P(TlsConnectGenericPre13, TestFallbackSCSVVersionMismatch) {
-  client_->SetFallbackSCSVEnabled(true);
+  client_->SetOption(SSL_ENABLE_FALLBACK_SCSV, PR_TRUE);
   server_->SetVersionRange(version_, version_ + 1);
   ConnectExpectAlert(server_, kTlsAlertInappropriateFallback);
   client_->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.
@@ -150,112 +150,16 @@ TEST_F(TlsConnectTest, DisallowSSLv3Hell
   EnsureTlsSetup();
   rv = SSL_VersionRangeSet(client_->ssl_fd(), &vrange);
   EXPECT_EQ(SECFailure, rv);
 
   rv = SSL_VersionRangeSet(server_->ssl_fd(), &vrange);
   EXPECT_EQ(SECFailure, rv);
 }
 
-TEST_P(TlsConnectStream, ConnectTls10AndServerRenegotiateHigher) {
-  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
-    return;
-  }
-  // Set the client so it will accept any version from 1.0
-  // to |version_|.
-  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
-                           SSL_LIBRARY_VERSION_TLS_1_0);
-  // Reset version so that the checks succeed.
-  uint16_t test_version = version_;
-  version_ = SSL_LIBRARY_VERSION_TLS_1_0;
-  Connect();
-
-  // Now renegotiate, with the server being set to do
-  // |version_|.
-  client_->PrepareForRenegotiate();
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
-  // Reset version and cipher suite so that the preinfo callback
-  // doesn't fail.
-  server_->ResetPreliminaryInfo();
-  server_->StartRenegotiate();
-
-  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-    ExpectAlert(server_, kTlsAlertUnexpectedMessage);
-  } else {
-    ExpectAlert(client_, kTlsAlertIllegalParameter);
-  }
-
-  Handshake();
-  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-    // In TLS 1.3, the server detects this problem.
-    client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
-    server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
-  } else {
-    client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
-    server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
-  }
-}
-
-TEST_P(TlsConnectStream, ConnectTls10AndClientRenegotiateHigher) {
-  if (version_ == SSL_LIBRARY_VERSION_TLS_1_0) {
-    return;
-  }
-  // Set the client so it will accept any version from 1.0
-  // to |version_|.
-  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, version_);
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0,
-                           SSL_LIBRARY_VERSION_TLS_1_0);
-  // Reset version so that the checks succeed.
-  uint16_t test_version = version_;
-  version_ = SSL_LIBRARY_VERSION_TLS_1_0;
-  Connect();
-
-  // Now renegotiate, with the server being set to do
-  // |version_|.
-  server_->PrepareForRenegotiate();
-  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_0, test_version);
-  // Reset version and cipher suite so that the preinfo callback
-  // doesn't fail.
-  server_->ResetPreliminaryInfo();
-  client_->StartRenegotiate();
-  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-    ExpectAlert(server_, kTlsAlertUnexpectedMessage);
-  } else {
-    ExpectAlert(client_, kTlsAlertIllegalParameter);
-  }
-  Handshake();
-  if (test_version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-    // In TLS 1.3, the server detects this problem.
-    client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
-    server_->CheckErrorCode(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED);
-  } else {
-    client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
-    server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT);
-  }
-}
-
-TEST_F(TlsConnectTest, Tls13RejectsRehandshakeClient) {
-  EnsureTlsSetup();
-  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
-  Connect();
-  SECStatus rv = SSL_ReHandshake(client_->ssl_fd(), PR_TRUE);
-  EXPECT_EQ(SECFailure, rv);
-  EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
-}
-
-TEST_F(TlsConnectTest, Tls13RejectsRehandshakeServer) {
-  EnsureTlsSetup();
-  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3);
-  Connect();
-  SECStatus rv = SSL_ReHandshake(server_->ssl_fd(), PR_TRUE);
-  EXPECT_EQ(SECFailure, rv);
-  EXPECT_EQ(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, PORT_GetError());
-}
-
 TEST_P(TlsConnectGeneric, AlertBeforeServerHello) {
   EnsureTlsSetup();
   client_->ExpectReceiveAlert(kTlsAlertUnrecognizedName, kTlsAlertWarning);
   client_->StartConnect();
   server_->StartConnect();
   client_->Handshake();  // Send ClientHello.
   static const uint8_t kWarningAlert[] = {kTlsAlertWarning,
                                           kTlsAlertUnrecognizedName};
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -254,23 +254,20 @@ bool TlsAgent::GetPeerChainLength(size_t
   return true;
 }
 
 void TlsAgent::CheckCipherSuite(uint16_t cipher_suite) {
   EXPECT_EQ(csinfo_.cipherSuite, cipher_suite);
 }
 
 void TlsAgent::RequestClientAuth(bool requireAuth) {
-  EXPECT_TRUE(EnsureTlsSetup());
   ASSERT_EQ(SERVER, role_);
 
-  EXPECT_EQ(SECSuccess,
-            SSL_OptionSet(ssl_fd(), SSL_REQUEST_CERTIFICATE, PR_TRUE));
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(ssl_fd(), SSL_REQUIRE_CERTIFICATE,
-                                      requireAuth ? PR_TRUE : PR_FALSE));
+  SetOption(SSL_REQUEST_CERTIFICATE, PR_TRUE);
+  SetOption(SSL_REQUIRE_CERTIFICATE, requireAuth ? PR_TRUE : PR_FALSE);
 
   EXPECT_EQ(SECSuccess, SSL_AuthCertificateHook(
                             ssl_fd(), &TlsAgent::ClientAuthenticated, this));
   expect_client_auth_ = true;
 }
 
 void TlsAgent::StartConnect(PRFileDesc* model) {
   EXPECT_TRUE(EnsureTlsSetup(model));
@@ -372,45 +369,18 @@ void TlsAgent::EnableSingleCipher(uint16
 }
 
 void TlsAgent::ConfigNamedGroups(const std::vector<SSLNamedGroup>& groups) {
   EXPECT_TRUE(EnsureTlsSetup());
   SECStatus rv = SSL_NamedGroupConfig(ssl_fd(), &groups[0], groups.size());
   EXPECT_EQ(SECSuccess, rv);
 }
 
-void TlsAgent::SetSessionTicketsEnabled(bool en) {
-  EXPECT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_ENABLE_SESSION_TICKETS,
-                               en ? PR_TRUE : PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
-}
-
-void TlsAgent::SetSessionCacheEnabled(bool en) {
-  EXPECT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_NO_CACHE, en ? PR_FALSE : PR_TRUE);
-  EXPECT_EQ(SECSuccess, rv);
-}
-
 void TlsAgent::Set0RttEnabled(bool en) {
-  EXPECT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv =
-      SSL_OptionSet(ssl_fd(), SSL_ENABLE_0RTT_DATA, en ? PR_TRUE : PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
-}
-
-void TlsAgent::SetFallbackSCSVEnabled(bool en) {
-  EXPECT_TRUE(role_ == CLIENT && EnsureTlsSetup());
-
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_ENABLE_FALLBACK_SCSV,
-                               en ? PR_TRUE : PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
+  SetOption(SSL_ENABLE_0RTT_DATA, en ? PR_TRUE : PR_FALSE);
 }
 
 void TlsAgent::SetShortHeadersEnabled() {
   EXPECT_TRUE(EnsureTlsSetup());
 
   SECStatus rv = SSLInt_EnableShortHeaders(ssl_fd());
   EXPECT_EQ(SECSuccess, rv);
 }
@@ -572,26 +542,25 @@ void TlsAgent::CheckAuthType(SSLAuthType
 }
 
 void TlsAgent::EnableFalseStart() {
   EXPECT_TRUE(EnsureTlsSetup());
 
   falsestart_enabled_ = true;
   EXPECT_EQ(SECSuccess, SSL_SetCanFalseStartCallback(
                             ssl_fd(), CanFalseStartCallback, this));
-  EXPECT_EQ(SECSuccess,
-            SSL_OptionSet(ssl_fd(), SSL_ENABLE_FALSE_START, PR_TRUE));
+  SetOption(SSL_ENABLE_FALSE_START, PR_TRUE);
 }
 
 void TlsAgent::ExpectResumption() { expect_resumption_ = true; }
 
 void TlsAgent::EnableAlpn(const uint8_t* val, size_t len) {
   EXPECT_TRUE(EnsureTlsSetup());
 
-  EXPECT_EQ(SECSuccess, SSL_OptionSet(ssl_fd(), SSL_ENABLE_ALPN, PR_TRUE));
+  SetOption(SSL_ENABLE_ALPN, PR_TRUE);
   EXPECT_EQ(SECSuccess, SSL_SetNextProtoNego(ssl_fd(), val, len));
 }
 
 void TlsAgent::CheckAlpn(SSLNextProtoState expected_state,
                          const std::string& expected) const {
   SSLNextProtoState state;
   char chosen[10];
   unsigned int chosen_len;
@@ -776,22 +745,17 @@ void TlsAgent::Connected() {
   PRBool short_headers;
   rv = SSLInt_UsingShortHeaders(ssl_fd(), &short_headers);
   EXPECT_EQ(SECSuccess, rv);
   EXPECT_EQ((PRBool)expect_short_headers_, short_headers);
   SetState(STATE_CONNECTED);
 }
 
 void TlsAgent::EnableExtendedMasterSecret() {
-  ASSERT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv =
-      SSL_OptionSet(ssl_fd(), SSL_ENABLE_EXTENDED_MASTER_SECRET, PR_TRUE);
-
-  ASSERT_EQ(SECSuccess, rv);
+  SetOption(SSL_ENABLE_EXTENDED_MASTER_SECRET, PR_TRUE);
 }
 
 void TlsAgent::CheckExtendedMasterSecret(bool expected) {
   if (version() >= SSL_LIBRARY_VERSION_TLS_1_3) {
     expected = PR_TRUE;
   }
   ASSERT_EQ(expected, info_.extendedMasterSecretUsed != PR_FALSE)
       << "unexpected extended master secret state for " << name_;
@@ -804,31 +768,16 @@ void TlsAgent::CheckEarlyDataAccepted(bo
   ASSERT_EQ(expected, info_.earlyDataAccepted != PR_FALSE)
       << "unexpected early data state for " << name_;
 }
 
 void TlsAgent::CheckSecretsDestroyed() {
   ASSERT_EQ(PR_TRUE, SSLInt_CheckSecretsDestroyed(ssl_fd()));
 }
 
-void TlsAgent::DisableRollbackDetection() {
-  ASSERT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_ROLLBACK_DETECTION, PR_FALSE);
-
-  ASSERT_EQ(SECSuccess, rv);
-}
-
-void TlsAgent::EnableCompression() {
-  ASSERT_TRUE(EnsureTlsSetup());
-
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_ENABLE_DEFLATE, PR_TRUE);
-  ASSERT_EQ(SECSuccess, rv);
-}
-
 void TlsAgent::SetDowngradeCheckVersion(uint16_t version) {
   ASSERT_TRUE(EnsureTlsSetup());
 
   SECStatus rv = SSL_SetDowngradeCheckVersion(ssl_fd(), version);
   ASSERT_EQ(SECSuccess, rv);
 }
 
 void TlsAgent::Handshake() {
@@ -954,33 +903,30 @@ void TlsAgent::ReadBytes(size_t amount) 
     LOGV("Re-arming");
     Poller::Instance()->Wait(READABLE_EVENT, adapter_, this,
                              &TlsAgent::ReadableCallback);
   }
 }
 
 void TlsAgent::ResetSentBytes() { send_ctr_ = 0; }
 
-void TlsAgent::ConfigureSessionCache(SessionResumptionMode mode) {
-  EXPECT_TRUE(EnsureTlsSetup());
+void TlsAgent::SetOption(int32_t option, int value) {
+  ASSERT_TRUE(EnsureTlsSetup());
+  EXPECT_EQ(SECSuccess, SSL_OptionSet(ssl_fd(), option, value));
+}
 
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_NO_CACHE,
-                               mode & RESUME_SESSIONID ? PR_FALSE : PR_TRUE);
-  EXPECT_EQ(SECSuccess, rv);
-
-  rv = SSL_OptionSet(ssl_fd(), SSL_ENABLE_SESSION_TICKETS,
-                     mode & RESUME_TICKET ? PR_TRUE : PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
+void TlsAgent::ConfigureSessionCache(SessionResumptionMode mode) {
+  SetOption(SSL_NO_CACHE, mode & RESUME_SESSIONID ? PR_FALSE : PR_TRUE);
+  SetOption(SSL_ENABLE_SESSION_TICKETS,
+            mode & RESUME_TICKET ? PR_TRUE : PR_FALSE);
 }
 
 void TlsAgent::DisableECDHEServerKeyReuse() {
-  ASSERT_TRUE(EnsureTlsSetup());
   ASSERT_EQ(TlsAgent::SERVER, role_);
-  SECStatus rv = SSL_OptionSet(ssl_fd(), SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
-  EXPECT_EQ(SECSuccess, rv);
+  SetOption(SSL_REUSE_SERVER_ECDHE_KEY, PR_FALSE);
 }
 
 static const std::string kTlsRolesAllArr[] = {"CLIENT", "SERVER"};
 ::testing::internal::ParamGenerator<std::string>
     TlsAgentTestBase::kTlsRolesAll = ::testing::ValuesIn(kTlsRolesAllArr);
 
 void TlsAgentTestBase::SetUp() {
   SSL_ConfigServerSessionIDCache(1024, 0, 0, g_working_dir_path.c_str());
--- a/gtests/ssl_gtest/tls_agent.h
+++ b/gtests/ssl_gtest/tls_agent.h
@@ -116,19 +116,18 @@ class TlsAgent : public PollTarget {
   bool ConfigServerCert(const std::string& name, bool updateKeyBits = false,
                         const SSLExtraServerCertData* serverCertData = nullptr);
   bool ConfigServerCertWithChain(const std::string& name);
   bool EnsureTlsSetup(PRFileDesc* modelSocket = nullptr);
 
   void SetupClientAuth();
   void RequestClientAuth(bool requireAuth);
 
+  void SetOption(int32_t option, int value);
   void ConfigureSessionCache(SessionResumptionMode mode);
-  void SetSessionTicketsEnabled(bool en);
-  void SetSessionCacheEnabled(bool en);
   void Set0RttEnabled(bool en);
   void SetFallbackSCSVEnabled(bool en);
   void SetShortHeadersEnabled();
   void SetAltHandshakeTypeEnabled();
   void SetVersionRange(uint16_t minver, uint16_t maxver);
   void GetVersionRange(uint16_t* minver, uint16_t* maxver);
   void CheckPreliminaryInfo();
   void ResetPreliminaryInfo();
@@ -152,18 +151,16 @@ class TlsAgent : public PollTarget {
   void SendBuffer(const DataBuffer& buf);
   // Send data directly to the underlying socket, skipping the TLS layer.
   void SendDirect(const DataBuffer& buf);
   void ReadBytes(size_t max = 16384U);
   void ResetSentBytes();  // Hack to test drops.
   void EnableExtendedMasterSecret();
   void CheckExtendedMasterSecret(bool expected);
   void CheckEarlyDataAccepted(bool expected);
-  void DisableRollbackDetection();
-  void EnableCompression();
   void SetDowngradeCheckVersion(uint16_t version);
   void CheckSecretsDestroyed();
   void ConfigNamedGroups(const std::vector<SSLNamedGroup>& groups);
   void DisableECDHEServerKeyReuse();
   bool GetPeerChainLength(size_t* count);
   void CheckCipherSuite(uint16_t cipher_suite);
 
   const std::string& name() const { return name_; }
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -8534,39 +8534,43 @@ ssl3_HandleClientHello(sslSocket *ss, PR
             if (suite_i == TLS_EMPTY_RENEGOTIATION_INFO_SCSV) {
                 PRUint8 *b2 = (PRUint8 *)emptyRIext;
                 PRUint32 L2 = sizeof emptyRIext;
                 (void)ssl3_HandleExtensions(ss, &b2, &L2, client_hello);
                 break;
             }
         }
     }
+
     /* This is a second check for TLS 1.3 and re-handshake to stop us
      * from re-handshake up to TLS 1.3, so it happens after version
      * negotiation. */
-    if (ss->firstHsDone && ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-        desc = unexpected_message;
-        errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED;
-        goto alert_loser;
-    }
-    if (ss->firstHsDone &&
-        (ss->opt.enableRenegotiation == SSL_RENEGOTIATE_REQUIRES_XTN ||
-         ss->opt.enableRenegotiation == SSL_RENEGOTIATE_TRANSITIONAL) &&
-        !ssl3_ExtensionNegotiated(ss, ssl_renegotiation_info_xtn)) {
-        desc = no_renegotiation;
-        level = alert_warning;
-        errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED;
-        goto alert_loser;
-    }
-    if ((ss->opt.requireSafeNegotiation ||
-         (ss->firstHsDone && ss->peerRequestedProtection)) &&
-        !ssl3_ExtensionNegotiated(ss, ssl_renegotiation_info_xtn)) {
-        desc = handshake_failure;
-        errCode = SSL_ERROR_UNSAFE_NEGOTIATION;
-        goto alert_loser;
+    if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
+        if (ss->firstHsDone) {
+            desc = unexpected_message;
+            errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED;
+            goto alert_loser;
+        }
+    } else {
+        if (ss->firstHsDone &&
+            (ss->opt.enableRenegotiation == SSL_RENEGOTIATE_REQUIRES_XTN ||
+             ss->opt.enableRenegotiation == SSL_RENEGOTIATE_TRANSITIONAL) &&
+            !ssl3_ExtensionNegotiated(ss, ssl_renegotiation_info_xtn)) {
+            desc = no_renegotiation;
+            level = alert_warning;
+            errCode = SSL_ERROR_RENEGOTIATION_NOT_ALLOWED;
+            goto alert_loser;
+        }
+        if ((ss->opt.requireSafeNegotiation ||
+             (ss->firstHsDone && ss->peerRequestedProtection)) &&
+            !ssl3_ExtensionNegotiated(ss, ssl_renegotiation_info_xtn)) {
+            desc = handshake_failure;
+            errCode = SSL_ERROR_UNSAFE_NEGOTIATION;
+            goto alert_loser;
+        }
     }
 
     /* We do stateful resumes only if we are in TLS < 1.3 and
      * either of the following conditions are satisfied:
      * (1) the client does not support the session ticket extension, or
      * (2) the client support the session ticket extension, but sent an
      * empty ticket.
      */
--- a/lib/ssl/ssl3exthandle.c
+++ b/lib/ssl/ssl3exthandle.c
@@ -1492,22 +1492,22 @@ ssl3_SendRenegotiationInfoXtn(
     const sslSocket *ss,
     TLSExtensionData *xtnData,
     PRBool append,
     PRUint32 maxBytes)
 {
     PRInt32 len = 0;
     PRInt32 needed;
 
-    /* In draft-ietf-tls-renegotiation-03, it is NOT RECOMMENDED to send
-     * both the SCSV and the empty RI, so when we send SCSV in
-     * the initial handshake, we don't also send RI.
+    /* In RFC 5746, it is NOT RECOMMENDED to send both the SCSV and the empty
+     * RI, so when we send SCSV in the initial handshake, we don't also send RI.
      */
-    if (!ss || ss->ssl3.hs.sendingSCSV)
+    if (ss->ssl3.hs.sendingSCSV) {
         return 0;
+    }
     if (ss->firstHsDone) {
         len = ss->sec.isServer ? ss->ssl3.hs.finishedBytes * 2
                                : ss->ssl3.hs.finishedBytes;
     }
     needed = 5 + len;
     if (maxBytes < (PRUint32)needed) {
         return 0;
     }