diff options
author | Kai Engert <kaie@kuix.de> | 2018-02-27 13:16:29 +0100 |
---|---|---|
committer | Kai Engert <kaie@kuix.de> | 2018-02-27 13:16:29 +0100 |
commit | 562c6c5484ab5cf9e4bd668d140f4ae1bd655627 (patch) | |
tree | 245ca025f6112f22888a4d2ddcef2ae340538539 /lib | |
parent | d329040789c889cd83372835f32f3859105f4bd0 (diff) | |
download | nss-hg-562c6c5484ab5cf9e4bd668d140f4ae1bd655627.tar.gz |
Backout revision b33b017eede5, bug 1432144, r=franziskusNSS_3_36_BETA1
Diffstat (limited to 'lib')
-rw-r--r-- | lib/ssl/ssl3con.c | 164 | ||||
-rw-r--r-- | lib/ssl/ssl3exthandle.c | 31 | ||||
-rw-r--r-- | lib/ssl/sslcon.c | 11 | ||||
-rw-r--r-- | lib/ssl/sslimpl.h | 5 | ||||
-rw-r--r-- | lib/ssl/sslnonce.c | 16 | ||||
-rw-r--r-- | lib/ssl/sslsock.c | 9 | ||||
-rw-r--r-- | lib/ssl/tls13con.c | 128 | ||||
-rw-r--r-- | lib/ssl/tls13con.h | 5 | ||||
-rw-r--r-- | lib/ssl/tls13exthandle.c | 2 | ||||
-rw-r--r-- | lib/ssl/tls13replay.c | 9 |
10 files changed, 206 insertions, 174 deletions
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; } |