diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c index 2187547c14..823d61cd1b 100644 --- a/gtests/ssl_gtest/libssl_internals.c +++ b/gtests/ssl_gtest/libssl_internals.c @@ -136,7 +136,6 @@ PRBool SSLInt_CheckSecretsDestroyed(PRFileDesc *fd) { } CHECK_SECRET(currentSecret); - CHECK_SECRET(resumptionMasterSecret); CHECK_SECRET(dheSecret); CHECK_SECRET(clientEarlyTrafficSecret); CHECK_SECRET(clientHsTrafficSecret); diff --git a/gtests/ssl_gtest/tls_filter.cc b/gtests/ssl_gtest/tls_filter.cc index 8c0a3367ad..ceec618812 100644 --- a/gtests/ssl_gtest/tls_filter.cc +++ b/gtests/ssl_gtest/tls_filter.cc @@ -470,6 +470,9 @@ static bool FindNewSessionTicketExtensions(TlsParser* parser, if (!parser->Skip(8)) { // lifetime, age add return false; } + if (!parser->SkipVariable(1)) { // ticket_nonce + return false; + } if (!parser->SkipVariable(2)) { // ticket return false; } diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 3412c03ee9..01d3bbdcdd 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -10145,7 +10145,8 @@ ssl3_SendNewSessionTicket(sslSocket *ss) SECStatus rv; NewSessionTicket nticket = { 0 }; - rv = ssl3_EncodeSessionTicket(ss, &nticket, &ticket); + rv = ssl3_EncodeSessionTicket(ss, &nticket, &ticket, + ss->ssl3.pwSpec->master_secret); if (rv != SECSuccess) goto loser; @@ -11229,8 +11230,8 @@ ssl3_SendFinished(sslSocket *ss, PRInt32 flags) * Caller holds the Spec read lock. */ SECStatus -ssl3_CacheWrappedMasterSecret(sslSocket *ss, sslSessionID *sid, - ssl3CipherSpec *spec) +ssl3_CacheWrappedSecret(sslSocket *ss, sslSessionID *sid, + PK11SymKey *secret) { PK11SymKey *wrappingKey = NULL; PK11SlotInfo *symKeySlot; @@ -11239,7 +11240,7 @@ ssl3_CacheWrappedMasterSecret(sslSocket *ss, sslSessionID *sid, PRBool isServer = ss->sec.isServer; CK_MECHANISM_TYPE mechanism = CKM_INVALID_MECHANISM; - symKeySlot = PK11_GetSlotFromKey(spec->master_secret); + symKeySlot = PK11_GetSlotFromKey(secret); if (!isServer) { int wrapKeyIndex; int incarnation; @@ -11300,7 +11301,7 @@ ssl3_CacheWrappedMasterSecret(sslSocket *ss, sslSessionID *sid, wmsItem.data = sid->u.ssl3.keys.wrapped_master_secret; wmsItem.len = sizeof sid->u.ssl3.keys.wrapped_master_secret; rv = PK11_WrapSymKey(mechanism, NULL, wrappingKey, - spec->master_secret, &wmsItem); + secret, &wmsItem); /* rv is examined below. */ sid->u.ssl3.keys.wrapped_master_secret_len = wmsItem.len; PK11_FreeSymKey(wrappingKey); @@ -11450,7 +11451,7 @@ ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length, } if (sid->cached == never_cached && !ss->opt.noCache) { - rv = ssl3_FillInCachedSID(ss, sid); + rv = ssl3_FillInCachedSID(ss, sid, ss->ssl3.crSpec->master_secret); /* If the wrap failed, we don't cache the sid. * The connection continues normally however. @@ -11474,7 +11475,7 @@ ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length, } SECStatus -ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid) +ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret) { SECStatus rv; @@ -11506,6 +11507,7 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid) /* Copy the master secret (wrapped or unwrapped) into the sid */ if (ss->ssl3.crSpec->msItem.len && ss->ssl3.crSpec->msItem.data) { + PORT_Assert(ss->version < SSL_LIBRARY_VERSION_TLS_1_3); sid->u.ssl3.keys.wrapped_master_secret_len = ss->ssl3.crSpec->msItem.len; memcpy(sid->u.ssl3.keys.wrapped_master_secret, @@ -11514,8 +11516,8 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid) sid->u.ssl3.keys.msIsWrapped = PR_FALSE; rv = SECSuccess; } else { - rv = ssl3_CacheWrappedMasterSecret(ss, ss->sec.ci.sid, - ss->ssl3.crSpec); + PORT_Assert(secret); + rv = ssl3_CacheWrappedSecret(ss, ss->sec.ci.sid, secret); sid->u.ssl3.keys.msIsWrapped = PR_TRUE; } ssl_ReleaseSpecReadLock(ss); /*************************************/ diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c index 46136c8d2d..cac2e9aa71 100644 --- a/lib/ssl/ssl3exthandle.c +++ b/lib/ssl/ssl3exthandle.c @@ -662,7 +662,7 @@ PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */ SECStatus ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, - SECItem *ticket_data) + SECItem *ticket_data, PK11SymKey *secret) { SECStatus rv; SECItem plaintext; @@ -677,7 +677,6 @@ ssl3_EncodeSessionTicket(sslSocket *ss, SECItem *srvName = NULL; CK_MECHANISM_TYPE msWrapMech = 0; /* dummy default value, * must be >= 0 */ - ssl3CipherSpec *spec; SECItem *alpnSelection = NULL; SSL_TRC(3, ("%d: SSL3[%d]: send session_ticket handshake", @@ -690,22 +689,20 @@ ssl3_EncodeSessionTicket(sslSocket *ss, cert_length = 2 + ss->sec.ci.sid->peerCert->derCert.len; } - if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3) { - spec = ss->ssl3.cwSpec; - } else { - spec = ss->ssl3.pwSpec; - } - if (spec->msItem.len && spec->msItem.data) { + if (ss->ssl3.pwSpec->msItem.len && ss->ssl3.pwSpec->msItem.data) { + PORT_Assert(ss->version < SSL_LIBRARY_VERSION_TLS_1_3); /* The master secret is available unwrapped. */ - ms_item.data = spec->msItem.data; - ms_item.len = spec->msItem.len; + ms_item.data = ss->ssl3.pwSpec->msItem.data; + ms_item.len = ss->ssl3.pwSpec->msItem.len; ms_is_wrapped = PR_FALSE; } else { /* Extract the master secret wrapped. */ sslSessionID sid; + PORT_Memset(&sid, 0, sizeof(sslSessionID)); - rv = ssl3_CacheWrappedMasterSecret(ss, &sid, spec); + PORT_Assert(secret); + rv = ssl3_CacheWrappedSecret(ss, &sid, secret); if (rv == SECSuccess) { if (sid.u.ssl3.keys.wrapped_master_secret_len > sizeof(wrapped_ms)) goto loser; diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index bf1ea658a7..eb6dec444c 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -1683,8 +1683,8 @@ extern SECStatus ssl3_SignHashes(sslSocket *ss, SSL3Hashes *hash, SECKEYPrivateKey *key, SECItem *buf); extern SECStatus ssl3_VerifySignedHashes(sslSocket *ss, SSLSignatureScheme scheme, SSL3Hashes *hash, SECItem *buf); -extern SECStatus ssl3_CacheWrappedMasterSecret( - sslSocket *ss, sslSessionID *sid, ssl3CipherSpec *spec); +extern SECStatus ssl3_CacheWrappedSecret(sslSocket *ss, sslSessionID *sid, + PK11SymKey *secret); extern void ssl3_FreeSniNameArray(TLSExtensionData *xtnData); /* Hello Extension related routines. */ @@ -1692,7 +1692,7 @@ extern void ssl3_SetSIDSessionTicket(sslSessionID *sid, /*in/out*/ NewSessionTicket *session_ticket); SECStatus ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket_input, - SECItem *ticket_data); + SECItem *ticket_data, PK11SymKey *secret); SECStatus ssl_MaybeSetSelfEncryptKeyPair(const sslKeyPair *keyPair); SECStatus ssl_GetSelfEncryptKeys(sslSocket *ss, unsigned char *keyName, @@ -1813,7 +1813,8 @@ PK11SymKey *ssl3_GetWrappingKey(sslSocket *ss, PK11SlotInfo *masterSecretSlot, CK_MECHANISM_TYPE masterWrapMech, void *pwArg); -SECStatus ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid); +SECStatus ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, + PK11SymKey *secret); const ssl3CipherSuiteDef *ssl_LookupCipherSuiteDef(ssl3CipherSuite suite); const ssl3BulkCipherDef * ssl_GetBulkCipherDef(const ssl3CipherSuiteDef *cipher_def); diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index f7cf81168b..ba5f13ffa1 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -125,6 +125,7 @@ const char kHkdfLabelApplicationTrafficSecret[] = "ap traffic"; const char kHkdfLabelFinishedSecret[] = "finished"; const char kHkdfLabelResumptionMasterSecret[] = "res master"; const char kHkdfLabelExporterMasterSecret[] = "exp master"; +const char kHkdfLabelResumption[] = "resumption"; const char kHkdfPurposeKey[] = "key"; const char kHkdfPurposeIv[] = "iv"; @@ -791,9 +792,8 @@ tls13_ComputeEarlySecrets(sslSocket *ss) if (rv != SECSuccess) { return SECFailure; } - } else { - PORT_Assert(!ss->ssl3.hs.resumptionMasterSecret); } + PORT_Assert(!ss->ssl3.hs.resumptionMasterSecret); return SECSuccess; } @@ -915,30 +915,19 @@ static SECStatus tls13_ComputeFinalSecrets(sslSocket *ss) { SECStatus rv; - PK11SymKey *resumptionMasterSecret = NULL; PORT_Assert(!ss->ssl3.crSpec->master_secret); PORT_Assert(!ss->ssl3.cwSpec->master_secret); rv = tls13_DeriveSecretWrap(ss, ss->ssl3.hs.currentSecret, NULL, kHkdfLabelResumptionMasterSecret, - &resumptionMasterSecret); + &ss->ssl3.hs.resumptionMasterSecret); PK11_FreeSymKey(ss->ssl3.hs.currentSecret); ss->ssl3.hs.currentSecret = NULL; if (rv != SECSuccess) { return SECFailure; } - /* This is pretty gross. TLS 1.3 uses a number of master secrets: - * The master secret to generate the keys and then the resumption - * master secret for future connections. To make this work without - * refactoring too much of the SSLv3 code, we store the RMS in - * |crSpec->master_secret| and |cwSpec->master_secret|. - */ - ss->ssl3.crSpec->master_secret = resumptionMasterSecret; - ss->ssl3.cwSpec->master_secret = - PK11_ReferenceSymKey(ss->ssl3.crSpec->master_secret); - return SECSuccess; } @@ -3849,6 +3838,7 @@ tls13_SendClientSecondRound(sslSocket *ss) * struct { * uint32 ticket_lifetime; * uint32 ticket_age_add; + * opaque ticket_nonce<1..255>; * opaque ticket<1..2^16-1>; * TicketExtension extensions<0..2^16-2>; * } NewSessionTicket; @@ -3860,10 +3850,12 @@ SECStatus tls13_SendNewSessionTicket(sslSocket *ss) { PRUint16 message_length; + PK11SymKey *secret; SECItem ticket_data = { 0, NULL, 0 }; SECStatus rv; NewSessionTicket ticket = { 0 }; PRUint32 max_early_data_size_len = 0; + ticket.flags = 0; if (ss->opt.enable0RttData) { ticket.flags |= ticket_allow_early_data; @@ -3871,13 +3863,26 @@ tls13_SendNewSessionTicket(sslSocket *ss) } ticket.ticket_lifetime_hint = ssl_ticket_lifetime; - rv = ssl3_EncodeSessionTicket(ss, &ticket, &ticket_data); + rv = tls13_HkdfExpandLabel(ss->ssl3.hs.resumptionMasterSecret, + tls13_GetHash(ss), + NULL, 0, + kHkdfLabelResumption, + strlen(kHkdfLabelResumption), + tls13_GetHkdfMechanism(ss), + tls13_GetHashSize(ss), &secret); + if (rv != SECSuccess) { + goto loser; + } + + rv = ssl3_EncodeSessionTicket(ss, &ticket, &ticket_data, secret); + PK11_FreeSymKey(secret); if (rv != SECSuccess) goto loser; message_length = 4 + /* lifetime */ 4 + /* ticket_age_add */ + 1 + /* ticket_nonce length */ 2 + max_early_data_size_len + /* max_early_data_size_len */ 2 + /* ticket length */ ticket_data.len; @@ -3902,6 +3907,11 @@ tls13_SendNewSessionTicket(sslSocket *ss) if (rv != SECSuccess) goto loser; + /* An empty nonce. */ + rv = ssl3_AppendHandshakeVariable(ss, NULL, 0, 1); + if (rv != SECSuccess) + goto loser; + /* Encode the ticket. */ rv = ssl3_AppendHandshakeVariable( ss, ticket_data.data, ticket_data.len, 2); @@ -3946,6 +3956,7 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) PRUint32 utmp; NewSessionTicket ticket = { 0 }; SECItem data; + SECItem ticket_nonce; SECItem ticket_data; SSL_TRC(3, ("%d: TLS13[%d]: handle new session ticket message", @@ -3980,6 +3991,14 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) } ticket.ticket_age_add = PR_ntohl(utmp); + /* The nonce. */ + rv = ssl3_ConsumeHandshakeVariable(ss, &ticket_nonce, 1, &b, &length); + if (rv != SECSuccess) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_NEW_SESSION_TICKET, + decode_error); + return SECFailure; + } + /* Get the ticket value. */ rv = ssl3_ConsumeHandshakeVariable(ss, &ticket_data, 2, &b, &length); if (rv != SECSuccess || !ticket_data.len) { @@ -4009,6 +4028,8 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) } if (!ss->opt.noCache) { + PK11SymKey *secret; + PORT_Assert(ss->sec.ci.sid); rv = SECITEM_CopyItem(NULL, &ticket.ticket, &ticket_data); if (rv != SECSuccess) { @@ -4045,9 +4066,22 @@ tls13_HandleNewSessionTicket(sslSocket *ss, PRUint8 *b, PRUint32 length) ssl3_SetSIDSessionTicket(ss->sec.ci.sid, &ticket); PORT_Assert(!ticket.ticket.data); - rv = ssl3_FillInCachedSID(ss, ss->sec.ci.sid); - if (rv != SECSuccess) + rv = tls13_HkdfExpandLabel(ss->ssl3.hs.resumptionMasterSecret, + tls13_GetHash(ss), + ticket_nonce.data, ticket_nonce.len, + kHkdfLabelResumption, + strlen(kHkdfLabelResumption), + tls13_GetHkdfMechanism(ss), + tls13_GetHashSize(ss), &secret); + if (rv != SECSuccess) { + return SECFailure; + } + + rv = ssl3_FillInCachedSID(ss, ss->sec.ci.sid, secret); + PK11_FreeSymKey(secret); + if (rv != SECSuccess) { return SECFailure; + } /* Cache the session. */ ss->sec.cache(ss->sec.ci.sid);