Backout revision b33b017eede5, bug 1432144, r=franziskusNSS_3_36_BETA1

16 files changed, 209 insertions, 211 deletions

diff --git a/gtests/ssl_gtest/ssl_0rtt_unittest.cc b/gtests/ssl_gtest/ssl_0rtt_unittest.cc
index a5e57a69b..08781af71 100644
--- a/gtests/ssl_gtest/ssl_0rtt_unittest.cc
+++ b/gtests/ssl_gtest/ssl_0rtt_unittest.cc
@@ -518,7 +518,7 @@ TEST_P(TlsConnectTls13, SendTooMuchEarlyData) {
 TEST_P(TlsConnectTls13, ReceiveTooMuchEarlyData) {
   EnsureTlsSetup();
 
-  size_t limit = 5;
+  const size_t limit = 5;
   EXPECT_EQ(SECSuccess, SSL_SetMaxEarlyDataSize(server_->ssl_fd(), limit));
   SetupForZeroRtt();
 
@@ -548,9 +548,6 @@ 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());
   }
diff --git a/gtests/ssl_gtest/ssl_loopback_unittest.cc b/gtests/ssl_gtest/ssl_loopback_unittest.cc
index 1db1d7389..f1b78f52f 100644
--- a/gtests/ssl_gtest/ssl_loopback_unittest.cc
+++ b/gtests/ssl_gtest/ssl_loopback_unittest.cc
@@ -532,11 +532,6 @@ 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,
diff --git a/gtests/ssl_gtest/ssl_resumption_unittest.cc b/gtests/ssl_gtest/ssl_resumption_unittest.cc
index 49995fc6f..eb78c0585 100644
--- a/gtests/ssl_gtest/ssl_resumption_unittest.cc
+++ b/gtests/ssl_gtest/ssl_resumption_unittest.cc
@@ -1028,27 +1028,4 @@ 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
diff --git a/gtests/ssl_gtest/tls_agent.cc b/gtests/ssl_gtest/tls_agent.cc
index 2aa7b1998..2f71caedb 100644
--- a/gtests/ssl_gtest/tls_agent.cc
+++ b/gtests/ssl_gtest/tls_agent.cc
@@ -589,9 +589,7 @@ void TlsAgent::EnableFalseStart() {
   SetOption(SSL_ENABLE_FALSE_START, PR_TRUE);
 }
 
-void TlsAgent::ExpectResumption(bool expected) {
-  expect_resumption_ = expected;
-}
+void TlsAgent::ExpectResumption() { expect_resumption_ = true; }
 
 void TlsAgent::EnableAlpn(const uint8_t* val, size_t len) {
   EXPECT_TRUE(EnsureTlsSetup());
diff --git a/gtests/ssl_gtest/tls_agent.h b/gtests/ssl_gtest/tls_agent.h
index 6114b16d6..6cd6d5073 100644
--- a/gtests/ssl_gtest/tls_agent.h
+++ b/gtests/ssl_gtest/tls_agent.h
@@ -129,7 +129,7 @@ class TlsAgent : public PollTarget {
   void SetServerKeyBits(uint16_t bits);
   void ExpectReadWriteError();
   void EnableFalseStart();
-  void ExpectResumption(bool expected = true);
+  void ExpectResumption();
   void SkipVersionChecks();
   void SetSignatureSchemes(const SSLSignatureScheme* schemes, size_t count);
   void EnableAlpn(const uint8_t* val, size_t len);
diff --git a/gtests/ssl_gtest/tls_connect.h b/gtests/ssl_gtest/tls_connect.h
index 6a35fc78b..7dffe7f8a 100644
--- a/gtests/ssl_gtest/tls_connect.h
+++ b/gtests/ssl_gtest/tls_connect.h
@@ -313,7 +313,6 @@ 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:
diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index b7715378a..89fd06dfc 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -50,6 +50,7 @@ static SECStatus ssl3_SendServerHelloDone(sslSocket *ss);
 static SECStatus ssl3_SendServerKeyExchange(sslSocket *ss);
 static SECStatus ssl3_HandleClientHelloPart2(sslSocket *ss,
                                              SECItem *suites,
+                                             sslSessionID *sid,
                                              const PRUint8 *msg,
                                              unsigned int len);
 static SECStatus ssl3_HandleServerHelloPart2(sslSocket *ss,
@@ -2713,7 +2714,9 @@ SSL3_SendAlert(sslSocket *ss, SSL3AlertLevel level, SSL3AlertDescription desc)
         ssl_GetSSL3HandshakeLock(ss);
     }
     if (level == alert_fatal) {
-        ssl_UncacheSessionID(ss);
+        if (ss->sec.ci.sid) {
+            ssl_UncacheSessionID(ss);
+        }
     }
 
     rv = tls13_SetAlertCipherSpec(ss);
@@ -4525,6 +4528,7 @@ ssl_SetClientHelloSpecVersion(sslSocket *ss, ssl3CipherSpec *spec)
 SECStatus
 ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
 {
+    sslSessionID *sid;
     SECStatus rv;
     unsigned int i;
     unsigned int length;
@@ -4601,26 +4605,19 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
      * If we have an sid and it comes from an external cache, we use it. */
     if (ss->sec.ci.sid && ss->sec.ci.sid->cached == in_external_cache) {
         PORT_Assert(!ss->sec.isServer);
+        sid = ss->sec.ci.sid;
         SSL_TRC(3, ("%d: SSL3[%d]: using external resumption token in ClientHello",
                     SSL_GETPID(), ss->fd));
+    } else if (!ss->opt.noCache) {
+        /* We ignore ss->sec.ci.sid here, and use ssl_Lookup because Lookup
+         * handles expired entries and other details.
+         * XXX If we've been called from ssl_BeginClientHandshake, then
+         * this lookup is duplicative and wasteful.
+         */
+        sid = ssl_LookupSID(&ss->sec.ci.peer, ss->sec.ci.port, ss->peerID, ss->url);
     } else {
-        /* We allocated some sid but don't want to use it. */
-        if (ss->sec.ci.sid) {
-            ssl_FreeSID(ss->sec.ci.sid);
-            ss->sec.ci.sid = NULL;
-        }
-        if (!ss->opt.noCache) {
-            /* We ignore ss->sec.ci.sid here, and use ssl_Lookup because Lookup
-             * handles expired entries and other details.
-             * XXX If we've been called from ssl_BeginClientHandshake, then
-             * this lookup is duplicative and wasteful.
-             */
-            ss->sec.ci.sid = ssl_LookupSID(&ss->sec.ci.peer,
-                                           ss->sec.ci.port,
-                                           ss->peerID, ss->url);
-        }
+        sid = NULL;
     }
-    sslSessionID *sid = ss->sec.ci.sid;
 
     /* We can't resume based on a different token. If the sid exists,
      * make sure the token that holds the master secret still exists ...
@@ -4712,6 +4709,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
         if (!sidOK) {
             SSL_AtomicIncrementLong(&ssl3stats.sch_sid_cache_not_ok);
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
             sid = NULL;
         }
     }
@@ -4739,11 +4737,10 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
             version = ss->clientHelloVersion;
         }
 
-        rv = ssl3_NewSessionID(ss, PR_FALSE);
-        if (rv != SECSuccess) {
+        sid = ssl3_NewSessionID(ss, PR_FALSE);
+        if (!sid) {
             return SECFailure; /* memory error is set */
         }
-        sid = ss->sec.ci.sid;
         /* ss->version isn't set yet, but the sid needs a sane value. */
         sid->version = version;
     }
@@ -4756,6 +4753,11 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type)
     }
     ssl_ReleaseSpecWriteLock(ss);
 
+    if (ss->sec.ci.sid != NULL) {
+        ssl_FreeSID(ss->sec.ci.sid); /* decrement ref count, free if zero */
+    }
+    ss->sec.ci.sid = sid;
+
     /* HACK for SCSV in SSL 3.0.  On initial handshake, prepend SCSV,
      * only if TLS is disabled.
      */
@@ -5013,6 +5015,7 @@ loser:
 static SECStatus
 ssl3_HandleHelloRequest(sslSocket *ss)
 {
+    sslSessionID *sid = ss->sec.ci.sid;
     SECStatus rv;
 
     SSL_TRC(3, ("%d: SSL3[%d]: handle hello_request handshake",
@@ -5035,7 +5038,12 @@ ssl3_HandleHelloRequest(sslSocket *ss)
         return SECFailure;
     }
 
-    ssl_UncacheSessionID(ss);
+    if (sid) {
+        ssl_UncacheSessionID(ss);
+        ssl_FreeSID(sid);
+        ss->sec.ci.sid = NULL;
+    }
+
     if (IS_DTLS(ss)) {
         dtls_RehandshakeCleanup(ss);
     }
@@ -6607,13 +6615,13 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes,
     /* throw the old one away */
     sid->u.ssl3.keys.resumable = PR_FALSE;
     ssl_UncacheSessionID(ss);
+    ssl_FreeSID(sid);
 
     /* get a new sid */
-    rv = ssl3_NewSessionID(ss, PR_FALSE);
-    if (rv != SECSuccess) {
+    ss->sec.ci.sid = sid = ssl3_NewSessionID(ss, PR_FALSE);
+    if (sid == NULL) {
         goto alert_loser; /* memory error is set. */
     }
-    sid = ss->sec.ci.sid;
 
     sid->version = ss->version;
     sid->u.ssl3.sessionIDLength = sidBytes->len;
@@ -7481,15 +7489,14 @@ ssl3_ServerNameCompare(const SECItem *name1, const SECItem *name2)
  *  ssl3_HandleClientHello()
  *  ssl3_HandleV2ClientHello()
  */
-SECStatus
+sslSessionID *
 ssl3_NewSessionID(sslSocket *ss, PRBool is_server)
 {
     sslSessionID *sid;
 
     sid = PORT_ZNew(sslSessionID);
-    if (sid == NULL) {
-        return SECFailure;
-    }
+    if (sid == NULL)
+        return sid;
 
     if (is_server) {
         const SECItem *srvName;
@@ -7503,7 +7510,7 @@ ssl3_NewSessionID(sslSocket *ss, PRBool is_server)
         ssl_ReleaseSpecReadLock(ss); /************************************/
         if (rv != SECSuccess) {
             PORT_Free(sid);
-            return SECFailure;
+            return NULL;
         }
     }
     sid->peerID = (ss->peerID == NULL) ? NULL : PORT_Strdup(ss->peerID);
@@ -7531,16 +7538,10 @@ ssl3_NewSessionID(sslSocket *ss, PRBool is_server)
         if (rv != SECSuccess) {
             ssl_FreeSID(sid);
             ssl_MapLowLevelError(SSL_ERROR_GENERATE_RANDOM_FAILURE);
-            return SECFailure;
+            return NULL;
         }
     }
-
-    /* Destroy any sid we might have had. */
-    ssl_UncacheSessionID(ss);
-    PORT_Assert(!ss->sec.ci.sid);
-
-    ss->sec.ci.sid = sid;
-    return SECSuccess;
+    return sid;
 }
 
 /* Called from:  ssl3_HandleClientHello, ssl3_HandleV2ClientHello */
@@ -7864,6 +7865,7 @@ ssl3_SelectServerCert(sslSocket *ss)
 static SECStatus
 ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
 {
+    sslSessionID *sid = NULL;
     PRUint32 tmp;
     unsigned int i;
     SECStatus rv;
@@ -8194,7 +8196,6 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
      * (2) the client support the session ticket extension, but sent an
      * empty ticket.
      */
-    sslSessionID *sid = ss->sec.ci.sid;
     if ((ss->version < SSL_LIBRARY_VERSION_TLS_1_3) &&
         (!ssl3_ExtensionNegotiated(ss, ssl_session_ticket_xtn) ||
          ss->xtnData.emptySessionTicket)) {
@@ -8205,31 +8206,20 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
                         ss->sec.ci.peer.pr_s6_addr32[2],
                         ss->sec.ci.peer.pr_s6_addr32[3]));
             if (ssl_sid_lookup) {
-                ssl_UncacheSessionID(ss);
-                ss->sec.ci.sid = sid = (*ssl_sid_lookup)(&ss->sec.ci.peer,
-                                                         sidBytes.data,
-                                                         sidBytes.len,
-                                                         ss->dbHandle);
+                sid = (*ssl_sid_lookup)(&ss->sec.ci.peer, sidBytes.data,
+                                        sidBytes.len, ss->dbHandle);
             } else {
                 errCode = SSL_ERROR_SERVER_CACHE_NOT_CONFIGURED;
                 goto loser;
             }
-        } else {
-            /* Free a potentially leftover session ID from a previous handshake. */
-            ssl_UncacheSessionID(ss);
-            sid = NULL;
         }
     } else if (ss->statelessResume) {
         /* Fill in the client's session ID if doing a stateless resume.
          * (When doing stateless resumes, server echos client's SessionID.)
          * This branch also handles TLS 1.3 resumption-PSK.
          */
+        sid = ss->sec.ci.sid;
         PORT_Assert(sid != NULL); /* Should have already been filled in.*/
-        if (!sid) {
-            desc = handshake_failure;
-            errCode = SEC_ERROR_LIBRARY_FAILURE;
-            goto alert_loser;
-        }
 
         if (sidBytes.len > 0 && sidBytes.len <= SSL3_SESSIONID_BYTES) {
             sid->u.ssl3.sessionIDLength = sidBytes.len;
@@ -8239,10 +8229,13 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
         } else {
             sid->u.ssl3.sessionIDLength = 0;
         }
-    } else {
-        /* Free a potentially leftover session ID from a previous handshake. */
-        ssl_UncacheSessionID(ss);
-        sid = NULL;
+        ss->sec.ci.sid = NULL;
+    }
+
+    /* Free a potentially leftover session ID from a previous handshake. */
+    if (ss->sec.ci.sid) {
+        ssl_FreeSID(ss->sec.ci.sid);
+        ss->sec.ci.sid = NULL;
     }
 
     if (sid != NULL) {
@@ -8260,6 +8253,7 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
 
             SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_not_ok);
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
             sid = NULL;
         }
     }
@@ -8270,9 +8264,10 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
     }
 
     if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) {
-        rv = tls13_HandleClientHelloPart2(ss, &suites, savedMsg, savedLen);
+        rv = tls13_HandleClientHelloPart2(ss, &suites, sid, savedMsg, savedLen);
     } else {
-        rv = ssl3_HandleClientHelloPart2(ss, &suites, savedMsg, savedLen);
+        rv = ssl3_HandleClientHelloPart2(ss, &suites, sid,
+                                         savedMsg, savedLen);
     }
     if (rv != SECSuccess) {
         errCode = PORT_GetError();
@@ -8289,11 +8284,10 @@ loser:
 }
 
 static SECStatus
-ssl3_UnwrapMasterSecretServer(sslSocket *ss, PK11SymKey **ms)
+ssl3_UnwrapMasterSecretServer(sslSocket *ss, sslSessionID *sid, PK11SymKey **ms)
 {
     PK11SymKey *wrapKey;
     CK_FLAGS keyFlags = 0;
-    sslSessionID *sid = ss->sec.ci.sid;
     SECItem wrappedMS = {
         siBuffer,
         sid->u.ssl3.keys.wrapped_master_secret,
@@ -8324,6 +8318,7 @@ ssl3_UnwrapMasterSecretServer(sslSocket *ss, PK11SymKey **ms)
 static SECStatus
 ssl3_HandleClientHelloPart2(sslSocket *ss,
                             SECItem *suites,
+                            sslSessionID *sid,
                             const PRUint8 *msg,
                             unsigned int len)
 {
@@ -8333,7 +8328,6 @@ ssl3_HandleClientHelloPart2(sslSocket *ss,
     SECStatus rv;
     unsigned int i;
     unsigned int j;
-    sslSessionID *sid = ss->sec.ci.sid;
 
     rv = ssl_HashHandshakeMessage(ss, ssl_hs_client_hello, msg, len);
     if (rv != SECSuccess) {
@@ -8463,12 +8457,22 @@ cipher_found:
                 }
             }
 
+            if (ss->sec.ci.sid) {
+                ssl_UncacheSessionID(ss);
+                PORT_Assert(ss->sec.ci.sid != sid); /* should be impossible, but ... */
+                if (ss->sec.ci.sid != sid) {
+                    ssl_FreeSID(ss->sec.ci.sid);
+                }
+                ss->sec.ci.sid = NULL;
+            }
+
             /* we need to resurrect the master secret.... */
-            rv = ssl3_UnwrapMasterSecretServer(ss, &masterSecret);
+            rv = ssl3_UnwrapMasterSecretServer(ss, sid, &masterSecret);
             if (rv != SECSuccess) {
                 break; /* not an error */
             }
 
+            ss->sec.ci.sid = sid;
             if (sid->peerCert != NULL) {
                 ss->sec.peerCert = CERT_DupCertificate(sid->peerCert);
             }
@@ -8551,6 +8555,7 @@ cipher_found:
         ss->statelessResume = PR_FALSE;
         SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_not_ok);
         ssl_UncacheSessionID(ss);
+        ssl_FreeSID(sid);
         sid = NULL;
     }
     SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_misses);
@@ -8584,12 +8589,12 @@ cipher_found:
         goto alert_loser;
     }
 
-    rv = ssl3_NewSessionID(ss, PR_TRUE);
-    if (rv != SECSuccess) {
+    sid = ssl3_NewSessionID(ss, PR_TRUE);
+    if (sid == NULL) {
         errCode = PORT_GetError();
         goto loser; /* memory error is set. */
     }
-    sid = ss->sec.ci.sid;
+    ss->sec.ci.sid = sid;
 
     sid->u.ssl3.keys.extendedMasterSecretUsed =
         ssl3_ExtensionNegotiated(ss, ssl_extended_master_secret_xtn);
@@ -8614,7 +8619,11 @@ alert_loser:
     (void)SSL3_SendAlert(ss, alert_fatal, desc);
 /* FALLTHRU */
 loser:
-    ssl_UncacheSessionID(ss);
+    if (sid && sid != ss->sec.ci.sid) {
+        ssl_UncacheSessionID(ss);
+        ssl_FreeSID(sid);
+    }
+
     if (haveXmitBufLock) {
         ssl_ReleaseXmitBufLock(ss);
     }
@@ -8631,6 +8640,7 @@ SECStatus
 ssl3_HandleV2ClientHello(sslSocket *ss, unsigned char *buffer, unsigned int length,
                          PRUint8 padding)
 {
+    sslSessionID *sid = NULL;
     unsigned char *suites;
     unsigned char *random;
     SSL3ProtocolVersion version;
@@ -8797,12 +8807,13 @@ suite_found:
 
     /* we don't even search for a cache hit here.  It's just a miss. */
     SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_misses);
-    PORT_Assert(!ss->sec.ci.sid);
-    rv = ssl3_NewSessionID(ss, PR_TRUE);
-    if (rv != SECSuccess) {
+    sid = ssl3_NewSessionID(ss, PR_TRUE);
+    if (sid == NULL) {
         errCode = PORT_GetError();
         goto loser; /* memory error is set. */
     }
+    ss->sec.ci.sid = sid;
+    /* do not worry about memory leak of sid since it now belongs to ci */
 
     /* We have to update the handshake hashes before we can send stuff */
     rv = ssl3_UpdateHandshakeHashes(ss, buffer, length);
@@ -10865,7 +10876,6 @@ fail:
 
 /* wrap the master secret, and put it into the SID.
  * Caller holds the Spec read lock.
- * sid here can be a local SID that's not stored in ss.
  */
 SECStatus
 ssl3_CacheWrappedSecret(sslSocket *ss, sslSessionID *sid,
@@ -10954,6 +10964,7 @@ ssl3_CacheWrappedSecret(sslSocket *ss, sslSessionID *sid,
 static SECStatus
 ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length)
 {
+    sslSessionID *sid = ss->sec.ci.sid;
     SECStatus rv = SECSuccess;
     PRBool isServer = ss->sec.isServer;
     PRBool isTLS;
@@ -11097,9 +11108,8 @@ xmit_loser:
         return rv;
     }
 
-    PORT_Assert(ss->sec.ci.sid);
-    if (ss->sec.ci.sid->cached == never_cached && !ss->opt.noCache) {
-        rv = ssl3_FillInCachedSID(ss, ss->ssl3.crSpec->masterSecret);
+    if (sid->cached == never_cached && !ss->opt.noCache) {
+        rv = ssl3_FillInCachedSID(ss, sid, ss->ssl3.crSpec->masterSecret);
 
         /* If the wrap failed, we don't cache the sid.
          * The connection continues normally however.
@@ -11123,10 +11133,9 @@ xmit_loser:
 }
 
 SECStatus
-ssl3_FillInCachedSID(sslSocket *ss, PK11SymKey *secret)
+ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret)
 {
     PORT_Assert(secret);
-    sslSessionID *sid = ss->sec.ci.sid;
 
     /* fill in the sid */
     sid->u.ssl3.cipherSuite = ss->ssl3.hs.cipher_suite;
@@ -12635,6 +12644,7 @@ ssl3_InitSocketPolicy(sslSocket *ss)
 SECStatus
 ssl3_RedoHandshake(sslSocket *ss, PRBool flushCache)
 {
+    sslSessionID *sid = ss->sec.ci.sid;
     SECStatus rv;
 
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
@@ -12658,8 +12668,10 @@ ssl3_RedoHandshake(sslSocket *ss, PRBool flushCache)
         return SECFailure;
     }
 
-    if (flushCache) {
+    if (sid && flushCache) {
         ssl_UncacheSessionID(ss); /* remove it from whichever cache it's in. */
+        ssl_FreeSID(sid);         /* dec ref count and free if zero. */
+        ss->sec.ci.sid = NULL;
     }
 
     ssl_GetXmitBufLock(ss); /**************************************/
diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c
index 11568def9..e6388945e 100644
--- a/lib/ssl/ssl3exthandle.c
+++ b/lib/ssl/ssl3exthandle.c
@@ -667,7 +667,6 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket,
     SECStatus rv;
     sslBuffer plaintext = SSL_BUFFER_EMPTY;
     SECItem ticket_buf = { 0, NULL, 0 };
-    /* This SID is NOT the one in ss and only used in this function. */
     sslSessionID sid;
     unsigned char wrapped_ms[SSL3_MASTER_SECRET_LENGTH];
     SECItem ms_item = { 0, NULL, 0 };
@@ -1156,13 +1155,15 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket,
 
 static SECStatus
 ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket,
-                        SessionTicket *parsedTicket)
+                        SessionTicket *parsedTicket, sslSessionID **out)
 {
-    SECStatus rv = ssl3_NewSessionID(ss, PR_TRUE);
-    if (rv != SECSuccess) {
-        return rv;
+    sslSessionID *sid;
+    SECStatus rv;
+
+    sid = ssl3_NewSessionID(ss, PR_TRUE);
+    if (sid == NULL) {
+        return SECFailure;
     }
-    sslSessionID *sid = ss->sec.ci.sid;
 
     /* Copy over parameters. */
     sid->version = parsedTicket->ssl_version;
@@ -1226,10 +1227,11 @@ ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket,
         }
     }
 
+    *out = sid;
     return SECSuccess;
 
 loser:
-    ssl_UncacheSessionID(ss);
+    ssl_FreeSID(sid);
     return SECFailure;
 }
 
@@ -1240,9 +1242,15 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, const SECItem *ticket,
 {
     SECItem decryptedTicket = { siBuffer, NULL, 0 };
     SessionTicket parsedTicket;
+    sslSessionID *sid = NULL;
     SECStatus rv;
 
-    ssl_UncacheSessionID(ss);
+    if (ss->sec.ci.sid != NULL) {
+        ssl_UncacheSessionID(ss);
+        ssl_FreeSID(ss->sec.ci.sid);
+        ss->sec.ci.sid = NULL;
+    }
+
     if (!SECITEM_AllocItem(NULL, &decryptedTicket, ticket->len)) {
         return SECFailure;
     }
@@ -1281,7 +1289,7 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, const SECItem *ticket,
     if (parsedTicket.timestamp + ssl_ticket_lifetime * PR_USEC_PER_SEC >
         ssl_TimeUsec()) {
 
-        rv = ssl_CreateSIDFromTicket(ss, ticket, &parsedTicket);
+        rv = ssl_CreateSIDFromTicket(ss, ticket, &parsedTicket, &sid);
         if (rv != SECSuccess) {
             goto loser; /* code already set */
         }
@@ -1294,6 +1302,7 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, const SECItem *ticket,
         }
 
         ss->statelessResume = PR_TRUE;
+        ss->sec.ci.sid = sid;
 
         /* We have the baseline value for the obfuscated ticket age here.  Save
          * that in xtnData temporarily.  This value is updated in
@@ -1306,7 +1315,9 @@ ssl3_ProcessSessionTicketCommon(sslSocket *ss, const SECItem *ticket,
     return SECSuccess;
 
 loser:
-    ssl_UncacheSessionID(ss);
+    if (sid) {
+        ssl_FreeSID(sid);
+    }
     SECITEM_ZfreeItem(&decryptedTicket, PR_FALSE);
     PORT_Memset(&parsedTicket, 0, sizeof(parsedTicket));
     return SECFailure;
diff --git a/lib/ssl/sslcon.c b/lib/ssl/sslcon.c
index 03288df66..bc63e1537 100644
--- a/lib/ssl/sslcon.c
+++ b/lib/ssl/sslcon.c
@@ -119,6 +119,7 @@ ssl_CheckConfigSanity(sslSocket *ss)
 SECStatus
 ssl_BeginClientHandshake(sslSocket *ss)
 {
+    sslSessionID *sid = NULL;
     SECStatus rv;
 
     PORT_Assert(ss->opt.noLocks || ssl_Have1stHandshakeLock(ss));
@@ -154,16 +155,14 @@ ssl_BeginClientHandshake(sslSocket *ss)
 
     SSL_TRC(3, ("%d: SSL[%d]: sending client-hello", SSL_GETPID(), ss->fd));
 
-    sslSessionID *sid = NULL;
     /* If there's an sid set from an external cache, use it. */
     if (ss->sec.ci.sid && ss->sec.ci.sid->cached == in_external_cache) {
         sid = ss->sec.ci.sid;
         SSL_TRC(3, ("%d: SSL[%d]: using external token", SSL_GETPID(), ss->fd));
     } else if (!ss->opt.noCache) {
         /* Try to find server in our session-id cache */
-        ssl_UncacheSessionID(ss);
-        ss->sec.ci.sid = sid = ssl_LookupSID(&ss->sec.ci.peer, ss->sec.ci.port,
-                                             ss->peerID, ss->url);
+        sid = ssl_LookupSID(&ss->sec.ci.peer, ss->sec.ci.port, ss->peerID,
+                            ss->url);
     }
 
     if (sid) {
@@ -172,11 +171,12 @@ ssl_BeginClientHandshake(sslSocket *ss)
             ss->sec.localCert = CERT_DupCertificate(sid->localCert);
         } else {
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
             sid = NULL;
         }
     }
     if (!sid) {
-        ss->sec.ci.sid = sid = PORT_ZNew(sslSessionID);
+        sid = PORT_ZNew(sslSessionID);
         if (!sid) {
             goto loser;
         }
@@ -191,6 +191,7 @@ ssl_BeginClientHandshake(sslSocket *ss)
             sid->urlSvrName = PORT_Strdup(ss->url);
         }
     }
+    ss->sec.ci.sid = sid;
 
     PORT_Assert(sid != NULL);
 
diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h
index 0e0403c16..10d0333d9 100644
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -1157,7 +1157,7 @@ extern int ssl_Do1stHandshake(sslSocket *ss);
 
 extern SECStatus ssl3_InitPendingCipherSpecs(sslSocket *ss, PK11SymKey *secret,
                                              PRBool derive);
-extern SECStatus ssl3_NewSessionID(sslSocket *ss, PRBool is_server);
+extern sslSessionID *ssl3_NewSessionID(sslSocket *ss, PRBool is_server);
 extern sslSessionID *ssl_LookupSID(const PRIPv6Addr *addr, PRUint16 port,
                                    const char *peerID, const char *urlSvrName);
 extern void ssl_FreeSID(sslSessionID *sid);
@@ -1621,7 +1621,8 @@ PK11SymKey *ssl3_GetWrappingKey(sslSocket *ss,
                                 PK11SlotInfo *masterSecretSlot,
                                 CK_MECHANISM_TYPE masterWrapMech,
                                 void *pwArg);
-SECStatus ssl3_FillInCachedSID(sslSocket *ss, PK11SymKey *secret);
+SECStatus ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid,
+                               PK11SymKey *secret);
 const ssl3CipherSuiteDef *ssl_LookupCipherSuiteDef(ssl3CipherSuite suite);
 SECStatus ssl3_SelectServerCert(sslSocket *ss);
 SECStatus ssl_PickSignatureScheme(sslSocket *ss,
diff --git a/lib/ssl/sslnonce.c b/lib/ssl/sslnonce.c
index 18cd9b9d3..f79c23fc7 100644
--- a/lib/ssl/sslnonce.c
+++ b/lib/ssl/sslnonce.c
@@ -1127,21 +1127,19 @@ ssl_CacheSessionID(sslSocket *ss)
 void
 ssl_UncacheSessionID(sslSocket *ss)
 {
+    if (ss->opt.noCache) {
+        return;
+    }
 
     sslSecurityInfo *sec = &ss->sec;
     PORT_Assert(sec);
 
     if (sec->ci.sid) {
-        if (!ss->opt.noCache) {
-            if (sec->isServer) {
-                ssl_ServerUncacheSessionID(sec->ci.sid);
-            } else if (!ss->resumptionTokenCallback) {
-                LockAndUncacheSID(sec->ci.sid);
-            }
+        if (sec->isServer) {
+            ssl_ServerUncacheSessionID(sec->ci.sid);
+        } else if (!ss->resumptionTokenCallback) {
+            LockAndUncacheSID(sec->ci.sid);
         }
-        PORT_Assert(sec->ci.sid->references == 1);
-        ssl_FreeSID(sec->ci.sid);
-        sec->ci.sid = NULL;
     }
 }
 
diff --git a/lib/ssl/sslsock.c b/lib/ssl/sslsock.c
index fc39e2ad3..e08d5e232 100644
--- a/lib/ssl/sslsock.c
+++ b/lib/ssl/sslsock.c
@@ -4040,13 +4040,13 @@ SSLExp_SetResumptionToken(PRFileDesc *fd, const PRUint8 *token,
 
     PRINT_BUF(50, (ss, "incoming resumption token", token, len));
 
-    SECStatus rv = ssl3_NewSessionID(ss, PR_FALSE);
-    if (rv != SECSuccess) {
+    ss->sec.ci.sid = ssl3_NewSessionID(ss, PR_FALSE);
+    if (!ss->sec.ci.sid) {
         goto done;
     }
 
     /* Populate NewSessionTicket values */
-    rv = ssl_DecodeResumptionToken(ss->sec.ci.sid, token, len);
+    SECStatus rv = ssl_DecodeResumptionToken(ss->sec.ci.sid, token, len);
     if (rv != SECSuccess) {
         // If decoding fails, we assume the token is bad.
         PORT_SetError(SSL_ERROR_BAD_RESUMPTION_TOKEN_ERROR);
@@ -4066,7 +4066,8 @@ SSLExp_SetResumptionToken(PRFileDesc *fd, const PRUint8 *token,
     /* Use the sid->cached as marker that this is from an external cache and
      * we don't have to look up anything in the NSS internal cache. */
     ss->sec.ci.sid->cached = in_external_cache;
-    ss->sec.ci.sid->references = 1;
+    // This has to be 2 to not free this in sendClientHello.
+    ss->sec.ci.sid->references = 2;
     ss->sec.ci.sid->lastAccessTime = ssl_TimeSec();
 
     ssl_ReleaseSSL3HandshakeLock(ss);
diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c
index 654616125..c06acc83a 100644
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -61,7 +61,8 @@ static SECStatus
 tls13_SendCertificateVerify(sslSocket *ss, SECKEYPrivateKey *privKey);
 static SECStatus tls13_HandleCertificateVerify(
     sslSocket *ss, PRUint8 *b, PRUint32 length);
-static SECStatus tls13_RecoverWrappedSharedSecret(sslSocket *ss);
+static SECStatus tls13_RecoverWrappedSharedSecret(sslSocket *ss,
+                                                  sslSessionID *sid);
 static SECStatus
 tls13_DeriveSecretWrap(sslSocket *ss, PK11SymKey *key,
                        const char *prefix,
@@ -456,12 +457,14 @@ tls13_SetupClientHello(sslSocket *ss)
     if (ss->statelessResume) {
         SECStatus rv;
 
-        PORT_Assert(sid);
-        rv = tls13_RecoverWrappedSharedSecret(ss);
+        PORT_Assert(ss->sec.ci.sid);
+        rv = tls13_RecoverWrappedSharedSecret(ss, ss->sec.ci.sid);
         if (rv != SECSuccess) {
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
             SSL_AtomicIncrementLong(&ssl3stats->sch_sid_cache_not_ok);
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(ss->sec.ci.sid);
+            ss->sec.ci.sid = NULL;
             return SECFailure;
         }
 
@@ -857,12 +860,11 @@ tls13_HandlePostHelloHandshakeMessage(sslSocket *ss, PRUint8 *b, PRUint32 length
 }
 
 static SECStatus
-tls13_RecoverWrappedSharedSecret(sslSocket *ss)
+tls13_RecoverWrappedSharedSecret(sslSocket *ss, sslSessionID *sid)
 {
     PK11SymKey *wrapKey; /* wrapping key */
     SECItem wrappedMS = { siBuffer, NULL, 0 };
     SSLHashType hashType;
-    sslSessionID *sid = ss->sec.ci.sid;
 
     SSL_TRC(3, ("%d: TLS13[%d]: recovering static secret (%s)",
                 SSL_GETPID(), ss->fd, SSL_ROLE(ss)));
@@ -1169,13 +1171,12 @@ tls13_ComputeFinalSecrets(sslSocket *ss)
 }
 
 static void
-tls13_RestoreCipherInfo(sslSocket *ss)
+tls13_RestoreCipherInfo(sslSocket *ss, sslSessionID *sid)
 {
     /* Set these to match the cached value.
      * TODO(ekr@rtfm.com): Make a version with the "true" values.
      * Bug 1256137.
      */
-    sslSessionID *sid = ss->sec.ci.sid;
     ss->sec.authType = sid->authType;
     ss->sec.authKeyBits = sid->authKeyBits;
     ss->sec.originalKeaGroup = ssl_LookupNamedGroup(sid->keaGroup);
@@ -1184,10 +1185,9 @@ tls13_RestoreCipherInfo(sslSocket *ss)
 
 /* Check whether resumption-PSK is allowed. */
 static PRBool
-tls13_CanResume(sslSocket *ss)
+tls13_CanResume(sslSocket *ss, const sslSessionID *sid)
 {
     const sslServerCert *sc;
-    sslSessionID *sid = ss->sec.ci.sid;
 
     if (!sid) {
         return PR_FALSE;
@@ -1214,9 +1214,8 @@ tls13_CanResume(sslSocket *ss)
 }
 
 static PRBool
-tls13_CanNegotiateZeroRtt(sslSocket *ss)
+tls13_CanNegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid)
 {
-    sslSessionID *sid = ss->sec.ci.sid;
     PORT_Assert(ss->ssl3.hs.zeroRttState == ssl_0rtt_sent);
 
     if (!sid)
@@ -1234,7 +1233,7 @@ tls13_CanNegotiateZeroRtt(sslSocket *ss)
                             &sid->u.ssl3.alpnSelection) != 0)
         return PR_FALSE;
 
-    if (tls13_IsReplay(ss)) {
+    if (tls13_IsReplay(ss, sid)) {
         return PR_FALSE;
     }
 
@@ -1251,10 +1250,10 @@ tls13_CanNegotiateZeroRtt(sslSocket *ss)
  * 5. We negotiated the same ALPN value as in the ticket.
  */
 static void
-tls13_NegotiateZeroRtt(sslSocket *ss)
+tls13_NegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid)
 {
     SSL_TRC(3, ("%d: TLS13[%d]: negotiate 0-RTT %p",
-                SSL_GETPID(), ss->fd, ss->sec.ci.sid));
+                SSL_GETPID(), ss->fd, sid));
 
     /* tls13_ServerHandleEarlyDataXtn sets this to ssl_0rtt_sent, so this will
      * be ssl_0rtt_none unless early_data is present. */
@@ -1272,7 +1271,7 @@ tls13_NegotiateZeroRtt(sslSocket *ss)
         return;
     }
 
-    if (!tls13_CanNegotiateZeroRtt(ss)) {
+    if (!tls13_CanNegotiateZeroRtt(ss, sid)) {
         SSL_TRC(3, ("%d: TLS13[%d]: ignore 0-RTT",
                     SSL_GETPID(), ss->fd));
         ss->ssl3.hs.zeroRttState = ssl_0rtt_ignored;
@@ -1575,6 +1574,7 @@ tls13_NegotiateAuthentication(sslSocket *ss)
 SECStatus
 tls13_HandleClientHelloPart2(sslSocket *ss,
                              const SECItem *suites,
+                             sslSessionID *sid,
                              const PRUint8 *msg,
                              unsigned int len)
 {
@@ -1663,12 +1663,12 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
 
     /* Check if we could in principle resume. */
     if (ss->statelessResume) {
-        PORT_Assert(ss->sec.ci.sid);
-        if (!ss->sec.ci.sid) {
+        PORT_Assert(sid);
+        if (!sid) {
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
-            goto loser;
+            return SECFailure;
         }
-        if (!tls13_CanResume(ss)) {
+        if (!tls13_CanResume(ss, sid)) {
             ss->statelessResume = PR_FALSE;
         }
     }
@@ -1718,7 +1718,10 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
         goto loser;
     }
     if (hrr) {
-        ssl_UncacheSessionID(ss);
+        if (sid) { /* Free the sid. */
+            ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
+        }
         PORT_Assert(ss->ssl3.hs.helloRetry);
         return SECSuccess;
     }
@@ -1730,7 +1733,6 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
     }
 
     if (ss->statelessResume) {
-        sslSessionID *sid = ss->sec.ci.sid;
         /* We are now committed to trying to resume. */
         PORT_Assert(sid);
 
@@ -1746,13 +1748,13 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
                                                 sid->namedCurve);
         PORT_Assert(ss->sec.serverCert);
 
-        rv = tls13_RecoverWrappedSharedSecret(ss);
+        rv = tls13_RecoverWrappedSharedSecret(ss, sid);
         if (rv != SECSuccess) {
             SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_not_ok);
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
             goto loser;
         }
-        tls13_RestoreCipherInfo(ss);
+        tls13_RestoreCipherInfo(ss, sid);
 
         ss->sec.localCert = CERT_DupCertificate(ss->sec.serverCert->serverCert);
         if (sid->peerCert != NULL) {
@@ -1763,20 +1765,22 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
             ss, &ss->xtnData,
             ssl_tls13_pre_shared_key_xtn, tls13_ServerSendPreSharedKeyXtn);
 
-        tls13_NegotiateZeroRtt(ss);
+        tls13_NegotiateZeroRtt(ss, sid);
     } else {
-        if (ss->sec.ci.sid) { /* we had a sid, but it's no longer valid, free it */
+        if (sid) { /* we had a sid, but it's no longer valid, free it */
             SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_not_ok);
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
+            sid = NULL;
         }
-        tls13_NegotiateZeroRtt(ss);
+        tls13_NegotiateZeroRtt(ss, NULL);
     }
 
     /* Need to compute early secrets. */
     rv = tls13_ComputeEarlySecrets(ss);
     if (rv != SECSuccess) {
         FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
-        goto loser;
+        return SECFailure;
     }
 
     /* Now that we have the binder key check the binder. */
@@ -1822,20 +1826,24 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
         SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_hits);
         SSL_AtomicIncrementLong(&ssl3stats->hch_sid_stateless_resumes);
     } else {
-        if (ss->sec.ci.sid) {
+        if (sid) {
             /* We had a sid, but it's no longer valid, free it. */
             SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_not_ok);
             ssl_UncacheSessionID(ss);
+            ssl_FreeSID(sid);
         } else {
             SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_misses);
         }
 
-        rv = ssl3_NewSessionID(ss, PR_TRUE);
-        if (rv != SECSuccess) {
+        sid = ssl3_NewSessionID(ss, PR_TRUE);
+        if (!sid) {
             FATAL_ERROR(ss, PORT_GetError(), internal_error);
-            goto loser;
+            return SECFailure;
         }
     }
+    /* Take ownership of the session. */
+    ss->sec.ci.sid = sid;
+    sid = NULL;
 
     if (ss->ssl3.hs.zeroRttState == ssl_0rtt_accepted) {
         rv = tls13_DeriveEarlySecrets(ss);
@@ -1856,7 +1864,10 @@ tls13_HandleClientHelloPart2(sslSocket *ss,
     return SECSuccess;
 
 loser:
-    ssl_UncacheSessionID(ss);
+    if (sid) {
+        ssl_UncacheSessionID(ss);
+        ssl_FreeSID(sid);
+    }
     return SECFailure;
 }
 
@@ -2483,6 +2494,7 @@ SECStatus
 tls13_HandleServerHelloPart2(sslSocket *ss)
 {
     SECStatus rv;
+    sslSessionID *sid = ss->sec.ci.sid;
     SSL3Statistics *ssl3stats = SSL_GetStatistics();
 
     if (ssl3_ExtensionNegotiated(ss, ssl_tls13_pre_shared_key_xtn)) {
@@ -2498,7 +2510,7 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
 
     if (ss->statelessResume) {
         if (tls13_GetHash(ss) !=
-            tls13_GetHashForCipherSuite(ss->sec.ci.sid->u.ssl3.cipherSuite)) {
+            tls13_GetHashForCipherSuite(sid->u.ssl3.cipherSuite)) {
             FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO,
                         illegal_parameter);
             return SECFailure;
@@ -2512,9 +2524,9 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
     if (ss->statelessResume) {
         /* PSK */
         ss->ssl3.hs.kea_def_mutable.authKeyType = ssl_auth_psk;
-        tls13_RestoreCipherInfo(ss);
-        if (ss->sec.ci.sid->peerCert) {
-            ss->sec.peerCert = CERT_DupCertificate(ss->sec.ci.sid->peerCert);
+        tls13_RestoreCipherInfo(ss, sid);
+        if (sid->peerCert) {
+            ss->sec.peerCert = CERT_DupCertificate(sid->peerCert);
         }
 
         SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_hits);
@@ -2524,7 +2536,7 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
         if (ssl3_ExtensionAdvertised(ss, ssl_tls13_pre_shared_key_xtn)) {
             SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_misses);
         }
-        if (ss->sec.ci.sid->cached == in_client_cache) {
+        if (sid->cached == in_client_cache) {
             /* If we tried to resume and failed, let's not try again. */
             ssl_UncacheSessionID(ss);
         }
@@ -2544,23 +2556,18 @@ tls13_HandleServerHelloPart2(sslSocket *ss)
 
     /* Discard current SID and make a new one, though it may eventually
      * end up looking a lot like the old one.
-     * Note that we don't uncache here. By freeing the sid here the uncache
-     * in ssl3_NewSessionID has no effect.
      */
-    if (ss->sec.ci.sid) {
-        ssl_FreeSID(ss->sec.ci.sid);
-        ss->sec.ci.sid = NULL;
-    }
-    rv = ssl3_NewSessionID(ss, PR_FALSE);
-    if (rv != SECSuccess) {
+    ssl_FreeSID(sid);
+    ss->sec.ci.sid = sid = ssl3_NewSessionID(ss, PR_FALSE);
+    if (sid == NULL) {
         FATAL_ERROR(ss, PORT_GetError(), internal_error);
         return SECFailure;
     }
     if (ss->statelessResume) {
         PORT_Assert(ss->sec.peerCert);
-        ss->sec.ci.sid->peerCert = CERT_DupCertificate(ss->sec.peerCert);
+        sid->peerCert = CERT_DupCertificate(ss->sec.peerCert);
     }
-    ss->sec.ci.sid->version = ss->version;
+    sid->version = ss->version;
 
     rv = tls13_HandleServerKeyShare(ss);
     if (rv != SECSuccess) {
@@ -4656,22 +4663,24 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length)
         /* Replace a previous session ticket when
          * we receive a second NewSessionTicket message. */
         if (ss->sec.ci.sid->cached == in_client_cache) {
-            /* We need the cert in the new sid. */
-            PORT_Assert(ss->sec.ci.sid->peerCert);
-            CERTCertificate *tmp = CERT_DupCertificate(ss->sec.ci.sid->peerCert);
-            if (!tmp) {
+            /* Create a new session ID. */
+            sslSessionID *sid = ssl3_NewSessionID(ss, PR_FALSE);
+            if (!sid) {
                 return SECFailure;
             }
 
-            /* Create a new session ID. The old one is being destroyed. */
-            rv = ssl3_NewSessionID(ss, PR_FALSE);
-            if (rv != SECSuccess) {
-                CERT_DestroyCertificate(tmp);
+            /* Copy over the peerCert. */
+            PORT_Assert(ss->sec.ci.sid->peerCert);
+            sid->peerCert = CERT_DupCertificate(ss->sec.ci.sid->peerCert);
+            if (!sid->peerCert) {
+                ssl_FreeSID(sid);
                 return SECFailure;
             }
 
-            /* "Copy" over the peerCert. */
-            ss->sec.ci.sid->peerCert = tmp;
+            /* Destroy the old SID. */
+            ssl_UncacheSessionID(ss);
+            ssl_FreeSID(ss->sec.ci.sid);
+            ss->sec.ci.sid = sid;
         }
 
         ssl3_SetSIDSessionTicket(ss->sec.ci.sid, &ticket);
@@ -4688,7 +4697,7 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length)
             return SECFailure;
         }
 
-        rv = ssl3_FillInCachedSID(ss, secret);
+        rv = ssl3_FillInCachedSID(ss, ss->sec.ci.sid, secret);
         PK11_FreeSymKey(secret);
         if (rv != SECSuccess) {
             return SECFailure;
@@ -5013,11 +5022,10 @@ tls13_UnprotectRecord(sslSocket *ss,
  * Called from tls13_ClientSendEarlyDataXtn().
  */
 PRBool
-tls13_ClientAllow0Rtt(const sslSocket *ss)
+tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid)
 {
     /* We checked that the cipher suite was still allowed back in
      * ssl3_SendClientHello. */
-    const sslSessionID *sid = ss->sec.ci.sid;
     if (sid->version < SSL_LIBRARY_VERSION_TLS_1_3)
         return PR_FALSE;
     if (ss->ssl3.hs.helloRetry)
diff --git a/lib/ssl/tls13con.h b/lib/ssl/tls13con.h
index dc936bbb7..1aaffb651 100644
--- a/lib/ssl/tls13con.h
+++ b/lib/ssl/tls13con.h
@@ -70,6 +70,7 @@ PRBool tls13_PskSuiteEnabled(sslSocket *ss);
 SECStatus tls13_WriteExtensionsWithBinder(sslSocket *ss, sslBuffer *extensions);
 SECStatus tls13_HandleClientHelloPart2(sslSocket *ss,
                                        const SECItem *suites,
+                                       sslSessionID *sid,
                                        const PRUint8 *msg,
                                        unsigned int len);
 SECStatus tls13_HandleServerHelloPart2(sslSocket *ss);
@@ -98,12 +99,12 @@ SECStatus tls13_ProtectRecord(sslSocket *ss,
                               sslBuffer *wrBuf);
 PRInt32 tls13_Read0RttData(sslSocket *ss, void *buf, PRInt32 len);
 SECStatus tls13_HandleEarlyApplicationData(sslSocket *ss, sslBuffer *origBuf);
-PRBool tls13_ClientAllow0Rtt(const sslSocket *ss);
+PRBool tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid);
 PRUint16 tls13_EncodeDraftVersion(SSL3ProtocolVersion version);
 SECStatus tls13_NegotiateVersion(sslSocket *ss,
                                  const TLSExtension *supported_versions);
 
-PRBool tls13_IsReplay(const sslSocket *ss);
+PRBool tls13_IsReplay(const sslSocket *ss, const sslSessionID *sid);
 void tls13_AntiReplayRollover(PRTime now);
 
 SECStatus SSLExp_SetupAntiReplay(PRTime window, unsigned int k,
diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c
index 95792a1ce..899f23827 100644
--- a/lib/ssl/tls13exthandle.c
+++ b/lib/ssl/tls13exthandle.c
@@ -627,7 +627,7 @@ SECStatus
 tls13_ClientSendEarlyDataXtn(const sslSocket *ss, TLSExtensionData *xtnData,
                              sslBuffer *buf, PRBool *added)
 {
-    if (!tls13_ClientAllow0Rtt(ss)) {
+    if (!tls13_ClientAllow0Rtt(ss, ss->sec.ci.sid)) {
         return SECSuccess;
     }
 
diff --git a/lib/ssl/tls13replay.c b/lib/ssl/tls13replay.c
index 78cf4477b..b090f9bca 100644
--- a/lib/ssl/tls13replay.c
+++ b/lib/ssl/tls13replay.c
@@ -188,17 +188,16 @@ tls13_AntiReplayUpdate()
 }
 
 PRBool
-tls13_InWindow(const sslSocket *ss)
+tls13_InWindow(const sslSocket *ss, const sslSessionID *sid)
 {
     PRInt32 timeDelta;
-    PORT_Assert(ss->sec.ci.sid);
 
     /* Calculate the difference between the client's view of the age of the
      * ticket (in |ss->xtnData.ticketAge|) and the server's view, which we now
      * calculate.  The result should be close to zero.  timeDelta is signed to
      * make the comparisons below easier. */
     timeDelta = ss->xtnData.ticketAge -
-                ((ssl_TimeUsec() - ss->sec.ci.sid->creationTime) / PR_USEC_PER_MSEC);
+                ((ssl_TimeUsec() - sid->creationTime) / PR_USEC_PER_MSEC);
 
     /* Only allow the time delta to be at most half of our window.  This is
      * symmetrical, though it doesn't need to be; this assumes that clock errors
@@ -231,7 +230,7 @@ tls13_InWindow(const sslSocket *ss)
  * replay.  In that case, we reject 0-RTT unnecessarily, but that's OK because
  * no client expects 0-RTT to work every time. */
 PRBool
-tls13_IsReplay(const sslSocket *ss)
+tls13_IsReplay(const sslSocket *ss, const sslSessionID *sid)
 {
     PRBool replay;
     unsigned int size;
@@ -246,7 +245,7 @@ tls13_IsReplay(const sslSocket *ss)
         return PR_TRUE;
     }
 
-    if (!tls13_InWindow(ss)) {
+    if (!tls13_InWindow(ss, sid)) {
         return PR_TRUE;
     }