Bug 1432144 - clean-up sid handling, r=mt

Summary:
SIDs usage is pretty messy. In this patch I move all *sid to point to ss->sec.ci.sid (unless the SID is purely local to the function).
This allows us to free sids when uncaching them.

Reviewers: mt

Reviewed By: mt

Bug #: 1432144

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

--HG--
extra : amend_source : 5ef18a287bc484ad89ca1832b34a80b9156fc2a0
extra : histedit_source : fe16f957386bde7340a9e9c03f58cbf5eeecf4ca%2C54da085800cd3ff79b1a3eeb01918a25382c7a6f

gtests/ssl_gtest/ssl_0rtt_unittest.cc

@@ -519,7 +519,7 @@ TEST_P(TlsConnectTls13, SendTooMuchEarlyData) {
 TEST_P(TlsConnectTls13, ReceiveTooMuchEarlyData) {
   EnsureTlsSetup();
 
-  const size_t limit = 5;
+  size_t limit = 5;
   EXPECT_EQ(SECSuccess, SSL_SetMaxEarlyDataSize(server_->ssl_fd(), limit));
   SetupForZeroRtt();
 
@@ -549,6 +549,9 @@ TEST_P(TlsConnectTls13, ReceiveTooMuchEarlyData) {
   server_->Handshake();  // This reads the early data and maybe throws an error.
   if (variant_ == ssl_variant_stream) {
     server_->CheckErrorCode(SSL_ERROR_TOO_MUCH_EARLY_DATA);
+    // We drop the SID when sending the alert such that max_early_data_size is 0
+    // here.
+    limit = 0;
   } else {
     EXPECT_EQ(TlsAgent::STATE_CONNECTING, server_->state());
   }

gtests/ssl_gtest/ssl_loopback_unittest.cc

@@ -530,6 +530,11 @@ INSTANTIATE_TEST_CASE_P(
                        TlsConnectTestBase::kTlsV11V12));
 INSTANTIATE_TEST_CASE_P(Pre13StreamOnly, TlsConnectStreamPre13,
                         TlsConnectTestBase::kTlsV10ToV12);
+INSTANTIATE_TEST_CASE_P(
+    Pre13Stream, TlsConnectStreamResumptionPre13,
+    ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream,
+                       TlsConnectTestBase::kTlsV10ToV12,
+                       ::testing::Values(true, false)));
 
 INSTANTIATE_TEST_CASE_P(Version12Plus, TlsConnectTls12Plus,
                         ::testing::Combine(TlsConnectTestBase::kTlsVariantsAll,

gtests/ssl_gtest/ssl_resumption_unittest.cc

@@ -1035,4 +1035,27 @@ TEST_P(TlsConnectGenericResumption, ConnectResumeClientAuth) {
   SendReceive();
 }
 
+// Renegotiate a resumed session.
+TEST_P(TlsConnectStreamResumptionPre13, ConnectResumeRenegotiateClient) {
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  Connect();
+  SendReceive();
+
+  Reset();
+  ConfigureSessionCache(RESUME_BOTH, RESUME_BOTH);
+  ExpectResumption(RESUME_TICKET);
+  Connect();
+
+  // Disable resumption and prepare for renegotiation.
+  server_->ExpectResumption(false);
+  server_->PrepareForRenegotiate();
+  client_->ExpectResumption(false);
+  client_->StartRenegotiate();
+  Handshake();
+  // Don't CheckConnected its logic doesn't work in this case.
+  // It assumes a certain number of SIDs, resumed sessions, and cache
+  // hits/misses.
+  SendReceive();
+}
+
 }  // namespace nss_test

gtests/ssl_gtest/tls_agent.cc

@@ -588,7 +588,9 @@ void TlsAgent::EnableFalseStart() {
   SetOption(SSL_ENABLE_FALSE_START, PR_TRUE);
 }
 
-void TlsAgent::ExpectResumption() { expect_resumption_ = true; }
+void TlsAgent::ExpectResumption(bool expected) {
+  expect_resumption_ = expected;
+}
 
 void TlsAgent::EnableAlpn(const uint8_t* val, size_t len) {
   EXPECT_TRUE(EnsureTlsSetup());

gtests/ssl_gtest/tls_agent.h

@@ -135,7 +135,7 @@ class TlsAgent : public PollTarget {
   void SetServerKeyBits(uint16_t bits);
   void ExpectReadWriteError();
   void EnableFalseStart();
-  void ExpectResumption();
+  void ExpectResumption(bool expected = true);
   void SkipVersionChecks();
   void SetSignatureSchemes(const SSLSignatureScheme* schemes, size_t count);
   void EnableAlpn(const uint8_t* val, size_t len);

gtests/ssl_gtest/tls_connect.h

@@ -313,6 +313,7 @@ class TlsConnectDatagramPre13 : public TlsConnectDatagram {
 
 // A variant that is used only with Pre13.
 class TlsConnectGenericPre13 : public TlsConnectGeneric {};
+class TlsConnectStreamResumptionPre13 : public TlsConnectGenericResumption {};
 
 class TlsKeyExchangeTest : public TlsConnectGeneric {
  protected: