From ecfa56ea1a97f6f4e80480fbd4d0efc30ada677a Mon Sep 17 00:00:00 2001 From: Kai Engert Date: Tue, 27 Feb 2018 13:16:29 +0100 Subject: [PATCH] Backout revision b33b017eede5, bug 1432144, r=franziskus --- gtests/ssl_gtest/ssl_0rtt_unittest.cc | 5 +- gtests/ssl_gtest/ssl_loopback_unittest.cc | 5 - gtests/ssl_gtest/ssl_resumption_unittest.cc | 23 --- gtests/ssl_gtest/tls_agent.cc | 4 +- gtests/ssl_gtest/tls_agent.h | 2 +- gtests/ssl_gtest/tls_connect.h | 1 - lib/ssl/ssl3con.c | 164 +++++++++++--------- lib/ssl/ssl3exthandle.c | 31 ++-- lib/ssl/sslcon.c | 11 +- lib/ssl/sslimpl.h | 5 +- lib/ssl/sslnonce.c | 16 +- lib/ssl/sslsock.c | 9 +- lib/ssl/tls13con.c | 128 ++++++++------- lib/ssl/tls13con.h | 5 +- lib/ssl/tls13exthandle.c | 2 +- lib/ssl/tls13replay.c | 9 +- 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 a5e57a69bb..08781af711 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 1db1d7389a..f1b78f52fd 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 49995fc6f5..eb78c05851 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 2aa7b19982..2f71caedb0 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 6114b16d63..6cd6d5073b 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 6a35fc78b6..7dffe7f8aa 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 b7715378a1..89fd06dfca 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 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) 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 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) } 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 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, } } + 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 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, 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 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, 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 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, (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 @@ ssl3_HandleV2ClientHello(sslSocket *ss, unsigned char *buffer, unsigned int leng /* 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 @@ ssl3_SendFinished(sslSocket *ss, PRInt32 flags) /* 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 @@ ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length) 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 @@ ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length) } 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 11568def9f..e6388945e1 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 03288df66e..bc63e15371 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 0e0403c161..10d0333d9e 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 18cd9b9d3b..f79c23fc72 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 fc39e2ad31..e08d5e2326 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 6546161258..c06acc83a9 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 dc936bbb7a..1aaffb651d 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 95792a1cee..899f238276 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 78cf4477b0..b090f9bca7 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; }