From 4171be6338d6aa25be8618517eb4c9f80a7d1d27 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Mon, 11 Sep 2017 12:12:30 +1000 Subject: [PATCH] Bug 1398647 - Remove the SECItem used for "storing" the master secret, r=ttaubert --HG-- branch : NSS_TLS13_DRAFT19_BRANCH extra : rebase_source : 7b6b118d9a116e1803db64ad29f4e0c1e4ebfe1b extra : amend_source : 6285cc56284e96566fdf653475fb04526fd11ddb extra : source : 1430f8033e9f639ec6912b78b52652948ba5d57a --- lib/ssl/ssl3con.c | 446 ++++++++++++++++------------------------ lib/ssl/ssl3ecc.c | 4 +- lib/ssl/ssl3exthandle.c | 57 ++--- lib/ssl/sslimpl.h | 8 +- lib/ssl/sslinfo.c | 2 +- lib/ssl/tls13con.c | 4 - 6 files changed, 202 insertions(+), 319 deletions(-) diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 5bf4b1cdd4..e86988d7a2 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -40,8 +40,10 @@ static PK11SymKey *ssl3_GenerateRSAPMS(sslSocket *ss, ssl3CipherSpec *spec, PK11SlotInfo *serverKeySlot); -static SECStatus ssl3_DeriveMasterSecret(sslSocket *ss, PK11SymKey *pms); -static SECStatus ssl3_DeriveConnectionKeys(sslSocket *ss); +static SECStatus ssl3_ComputeMasterSecret(sslSocket *ss, PK11SymKey *pms, + PK11SymKey **msp); +static SECStatus ssl3_DeriveConnectionKeys(sslSocket *ss, + PK11SymKey *masterSecret); static SECStatus ssl3_HandshakeFailure(sslSocket *ss); static SECStatus ssl3_SendCertificate(sslSocket *ss); static SECStatus ssl3_SendCertificateRequest(sslSocket *ss); @@ -1562,8 +1564,6 @@ ssl3_DestroyCipherSpec(ssl3CipherSpec *spec, PRBool freeSrvName) PK11_FreeSymKey(spec->master_secret); spec->master_secret = NULL; } - spec->msItem.data = NULL; - spec->msItem.len = 0; ssl3_CleanupKeyMaterial(&spec->client); ssl3_CleanupKeyMaterial(&spec->server); spec->destroyCompressContext = NULL; @@ -1962,9 +1962,8 @@ ssl3_ChaCha20Poly1305(ssl3KeyMaterial *keys, PRBool doDecrypt, * Caller holds Spec write lock. */ static SECStatus -ssl3_InitPendingContexts(sslSocket *ss) +ssl3_InitPendingContexts(sslSocket *ss, ssl3CipherSpec *pwSpec) { - ssl3CipherSpec *pwSpec; const ssl3BulkCipherDef *cipher_def; PK11Context *serverContext = NULL; PK11Context *clientContext = NULL; @@ -1980,7 +1979,6 @@ ssl3_InitPendingContexts(sslSocket *ss) PORT_Assert(ss->opt.noLocks || ssl_HaveSpecWriteLock(ss)); PORT_Assert(ss->ssl3.prSpec == ss->ssl3.pwSpec); - pwSpec = ss->ssl3.pwSpec; cipher_def = pwSpec->cipher_def; macLength = pwSpec->mac_size; calg = cipher_def->calg; @@ -2123,73 +2121,80 @@ ssl3_InitPendingContexts(sslSocket *ss) * ssl3_HandleServerHello (for session restart) * ssl3_HandleClientHello (for session restart) * Sets error code, but caller probably should override to disambiguate. - * NULL pms means re-use old master_secret. * - * If the old master secret is reused, pms is NULL and the master secret is - * already in pwSpec->master_secret. + * If |secret| is a master secret from a previous connection is reused, |derive| + * is PR_FALSE. If the secret is a pre-master secret, then |derive| is PR_TRUE + * and the master secret is derived from |secret|. */ SECStatus -ssl3_InitPendingCipherSpec(sslSocket *ss, PK11SymKey *pms) +ssl3_InitPendingCipherSpecs(sslSocket *ss, PK11SymKey *secret, PRBool derive) { + PK11SymKey *masterSecret; ssl3CipherSpec *pwSpec; - ssl3CipherSpec *cwSpec; SECStatus rv; PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); + PORT_Assert(secret); ssl_GetSpecWriteLock(ss); /**************************************/ PORT_Assert(ss->ssl3.prSpec == ss->ssl3.pwSpec); - + PORT_Assert(ss->ssl3.pwSpec); + PORT_Assert(ss->ssl3.cwSpec->epoch == ss->ssl3.crSpec->epoch); pwSpec = ss->ssl3.pwSpec; - cwSpec = ss->ssl3.cwSpec; - if (pms || (!pwSpec->msItem.len && !pwSpec->master_secret)) { - rv = ssl3_DeriveMasterSecret(ss, pms); + if (ss->ssl3.cwSpec->epoch == PR_UINT16_MAX) { + /* The problem here is that we have rehandshaked too many + * times (you are not allowed to wrap the epoch). The + * spec says you should be discarding the connection + * and start over, so not much we can do here. */ + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + goto loser; + } + + if (derive) { + rv = ssl3_ComputeMasterSecret(ss, secret, &masterSecret); if (rv != SECSuccess) { - goto done; /* err code set by ssl3_DeriveMasterSecret */ + goto loser; } + } else { + masterSecret = secret; } - if (pwSpec->master_secret) { - rv = ssl3_DeriveConnectionKeys(ss); - if (rv == SECSuccess) { - rv = ssl3_InitPendingContexts(ss); + + PORT_Assert(masterSecret); + rv = ssl3_DeriveConnectionKeys(ss, masterSecret); + if (rv != SECSuccess) { + if (derive) { + /* masterSecret was created here. */ + PK11_FreeSymKey(masterSecret); } - } else { - PORT_Assert(pwSpec->master_secret); - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - rv = SECFailure; + goto loser; } + + pwSpec->master_secret = masterSecret; + rv = ssl3_InitPendingContexts(ss, ss->ssl3.pwSpec); if (rv != SECSuccess) { - goto done; + goto loser; } - /* Generic behaviors -- common to all crypto methods */ - if (!IS_DTLS(ss)) { - pwSpec->read_seq_num = pwSpec->write_seq_num = 0; - } else { - if (cwSpec->epoch == PR_UINT16_MAX) { - /* The problem here is that we have rehandshaked too many - * times (you are not allowed to wrap the epoch). The - * spec says you should be discarding the connection - * and start over, so not much we can do here. */ - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - rv = SECFailure; - goto done; - } + /* With keys created, get ready to read and write records. */ + if (IS_DTLS(ss)) { /* The sequence number has the high 16 bits as the epoch. */ - pwSpec->epoch = cwSpec->epoch + 1; - pwSpec->read_seq_num = pwSpec->write_seq_num = - (sslSequenceNumber)pwSpec->epoch << 48; + pwSpec->epoch = ss->ssl3.cwSpec->epoch + 1; + pwSpec->read_seq_num = pwSpec->write_seq_num = (sslSequenceNumber)pwSpec->epoch << 48; dtls_InitRecvdRecords(&pwSpec->recvdRecords); + } else { + pwSpec->read_seq_num = pwSpec->write_seq_num = 0; } -done: ssl_ReleaseSpecWriteLock(ss); /******************************/ - if (rv != SECSuccess) - ssl_MapLowLevelError(SSL_ERROR_SESSION_KEY_GEN_FAILURE); - return rv; + return SECSuccess; + +loser: + ssl_ReleaseSpecWriteLock(ss); /******************************/ + ssl_MapLowLevelError(SSL_ERROR_SESSION_KEY_GEN_FAILURE); + return SECFailure; } /* @@ -3856,36 +3861,6 @@ ssl3_ComputeMasterSecret(sslSocket *ss, PK11SymKey *pms, } } -/* This method uses PKCS11 to derive the MS from the PMS, where PMS -** is a PKCS11 symkey. We call ssl3_ComputeMasterSecret to do the -** computations and then modify the pwSpec->state as a side effect. -** -** This is used in all cases except the "triple bypass" with RSA key -** exchange. -** -** Called from ssl3_InitPendingCipherSpec. prSpec is pwSpec. -*/ -static SECStatus -ssl3_DeriveMasterSecret(sslSocket *ss, PK11SymKey *pms) -{ - SECStatus rv; - PK11SymKey *ms = NULL; - ssl3CipherSpec *pwSpec = ss->ssl3.pwSpec; - - PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); - PORT_Assert(ss->opt.noLocks || ssl_HaveSpecWriteLock(ss)); - PORT_Assert(ss->ssl3.prSpec == ss->ssl3.pwSpec); - - if (pms) { - rv = ssl3_ComputeMasterSecret(ss, pms, &ms); - pwSpec->master_secret = ms; - if (rv != SECSuccess) - return rv; - } - - return SECSuccess; -} - /* * Derive encryption and MAC Keys (and IVs) from master secret * Sets a useful error code when returning SECFailure. @@ -3902,7 +3877,7 @@ ssl3_DeriveMasterSecret(sslSocket *ss, PK11SymKey *pms) * */ static SECStatus -ssl3_DeriveConnectionKeys(sslSocket *ss) +ssl3_DeriveConnectionKeys(sslSocket *ss, PK11SymKey *masterSecret) { ssl3CipherSpec *pwSpec = ss->ssl3.pwSpec; unsigned char *cr = (unsigned char *)&ss->ssl3.hs.client_random; @@ -3928,11 +3903,8 @@ ssl3_DeriveConnectionKeys(sslSocket *ss) PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); PORT_Assert(ss->opt.noLocks || ssl_HaveSpecWriteLock(ss)); PORT_Assert(ss->ssl3.prSpec == ss->ssl3.pwSpec); + PORT_Assert(masterSecret); - if (!pwSpec->master_secret) { - PORT_SetError(SSL_ERROR_SESSION_KEY_GEN_FAILURE); - return SECFailure; - } /* * generate the key material */ @@ -3986,7 +3958,7 @@ ssl3_DeriveConnectionKeys(sslSocket *ss) /* CKM_SSL3_KEY_AND_MAC_DERIVE is defined to set ENCRYPT, DECRYPT, and * DERIVE by DEFAULT */ - symKey = PK11_Derive(pwSpec->master_secret, key_derive, ¶ms, + symKey = PK11_Derive(masterSecret, key_derive, ¶ms, bulk_mechanism, CKA_ENCRYPT, keySize); if (!symKey) { ssl_MapLowLevelError(SSL_ERROR_SESSION_KEY_GEN_FAILURE); @@ -5021,7 +4993,7 @@ ssl3_SendClientHello(sslSocket *ss, sslClientHelloType type) } /* Check that we can recover the master secret. */ - if (sidOK && sid->u.ssl3.keys.msIsWrapped) { + if (sidOK) { PK11SlotInfo *slot = NULL; if (sid->u.ssl3.masterValid) { slot = SECMOD_LookupSlot(sid->u.ssl3.masterModuleID, @@ -6000,7 +5972,7 @@ ssl3_SendRSAClientKeyExchange(sslSocket *ss, SECKEYPublicKey *svrPubKey) goto loser; /* err set by ssl3_AppendHandshake* */ } - rv = ssl3_InitPendingCipherSpec(ss, pms); + rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE); PK11_FreeSymKey(pms); pms = NULL; @@ -6143,7 +6115,7 @@ ssl3_SendDHClientKeyExchange(sslSocket *ss, SECKEYPublicKey *svrPubKey) goto loser; /* err set by ssl3_AppendBufferToHandshake */ } - rv = ssl3_InitPendingCipherSpec(ss, pms); + rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE); if (rv != SECSuccess) { ssl_MapLowLevelError(SSL_ERROR_CLIENT_KEY_EXCHANGE_FAILURE); goto loser; @@ -6770,6 +6742,51 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length) return SECFailure; } +static SECStatus +ssl3_UnwrapMasterSecretClient(sslSocket *ss, sslSessionID *sid, PK11SymKey **ms) +{ + PK11SlotInfo *slot; + PK11SymKey *wrapKey; + CK_FLAGS keyFlags = 0; + SECItem wrappedMS = { + siBuffer, + sid->u.ssl3.keys.wrapped_master_secret, + sid->u.ssl3.keys.wrapped_master_secret_len + }; + + /* unwrap master secret */ + slot = SECMOD_LookupSlot(sid->u.ssl3.masterModuleID, + sid->u.ssl3.masterSlotID); + if (slot == NULL) { + return SECFailure; + } + if (!PK11_IsPresent(slot)) { + PK11_FreeSlot(slot); + return SECFailure; + } + wrapKey = PK11_GetWrapKey(slot, sid->u.ssl3.masterWrapIndex, + sid->u.ssl3.masterWrapMech, + sid->u.ssl3.masterWrapSeries, + ss->pkcs11PinArg); + PK11_FreeSlot(slot); + if (wrapKey == NULL) { + return SECFailure; + } + + if (ss->version > SSL_LIBRARY_VERSION_3_0) { /* isTLS */ + keyFlags = CKF_SIGN | CKF_VERIFY; + } + + *ms = PK11_UnwrapSymKeyWithFlags(wrapKey, sid->u.ssl3.masterWrapMech, + NULL, &wrappedMS, CKM_SSL3_MASTER_KEY_DERIVE, + CKA_DERIVE, sizeof(SSL3MasterSecret), keyFlags); + PK11_FreeSymKey(wrapKey); + if (!*ms) { + return SECFailure; + } + return SECSuccess; +} + static SECStatus ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes, int *retErrCode) @@ -6819,9 +6836,7 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes, goto alert_loser; } do { - ssl3CipherSpec *pwSpec = ss->ssl3.pwSpec; - - SECItem wrappedMS; /* wrapped master secret. */ + PK11SymKey *masterSecret; /* [draft-ietf-tls-session-hash-06; Section 5.3] * @@ -6856,59 +6871,9 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes, ss->sec.originalKeaGroup = ssl_LookupNamedGroup(sid->keaGroup); ss->sec.signatureScheme = sid->sigScheme; - if (sid->u.ssl3.keys.msIsWrapped) { - PK11SlotInfo *slot; - PK11SymKey *wrapKey; /* wrapping key */ - CK_FLAGS keyFlags = 0; - - /* unwrap master secret */ - slot = SECMOD_LookupSlot(sid->u.ssl3.masterModuleID, - sid->u.ssl3.masterSlotID); - if (slot == NULL) { - break; /* not considered an error. */ - } - if (!PK11_IsPresent(slot)) { - PK11_FreeSlot(slot); - break; /* not considered an error. */ - } - wrapKey = PK11_GetWrapKey(slot, sid->u.ssl3.masterWrapIndex, - sid->u.ssl3.masterWrapMech, - sid->u.ssl3.masterWrapSeries, - ss->pkcs11PinArg); - PK11_FreeSlot(slot); - if (wrapKey == NULL) { - break; /* not considered an error. */ - } - - if (ss->version > SSL_LIBRARY_VERSION_3_0) { /* isTLS */ - keyFlags = - CKF_SIGN | CKF_VERIFY; - } - - wrappedMS.data = sid->u.ssl3.keys.wrapped_master_secret; - wrappedMS.len = sid->u.ssl3.keys.wrapped_master_secret_len; - pwSpec->master_secret = - PK11_UnwrapSymKeyWithFlags(wrapKey, sid->u.ssl3.masterWrapMech, - NULL, &wrappedMS, CKM_SSL3_MASTER_KEY_DERIVE, - CKA_DERIVE, sizeof(SSL3MasterSecret), keyFlags); - errCode = PORT_GetError(); - PK11_FreeSymKey(wrapKey); - if (pwSpec->master_secret == NULL) { - break; /* errorCode set just after call to UnwrapSymKey. */ - } - } else { - /* need to import the raw master secret to session object */ - PK11SlotInfo *slot = PK11_GetInternalSlot(); - wrappedMS.data = sid->u.ssl3.keys.wrapped_master_secret; - wrappedMS.len = sid->u.ssl3.keys.wrapped_master_secret_len; - pwSpec->master_secret = - PK11_ImportSymKey(slot, CKM_SSL3_MASTER_KEY_DERIVE, - PK11_OriginUnwrap, CKA_ENCRYPT, - &wrappedMS, NULL); - PK11_FreeSlot(slot); - if (pwSpec->master_secret == NULL) { - break; - } + rv = ssl3_UnwrapMasterSecretClient(ss, sid, &masterSecret); + if (rv != SECSuccess) { + break; /* not considered an error */ } /* Got a Match */ @@ -6930,8 +6895,8 @@ ssl3_HandleServerHelloPart2(sslSocket *ss, const SECItem *sidBytes, ss->sec.peerCert = CERT_DupCertificate(sid->peerCert); } - /* NULL value for PMS because we are reusing the old MS */ - rv = ssl3_InitPendingCipherSpec(ss, NULL); + /* We are re-using the old MS, so no need to derive again. */ + rv = ssl3_InitPendingCipherSpecs(ss, masterSecret, PR_FALSE); if (rv != SECSuccess) { goto alert_loser; /* err code was set */ } @@ -8611,6 +8576,38 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) return SECFailure; } +static SECStatus +ssl3_UnwrapMasterSecretServer(sslSocket *ss, sslSessionID *sid, PK11SymKey **ms) +{ + PK11SymKey *wrapKey; + CK_FLAGS keyFlags = 0; + SECItem wrappedMS = { + siBuffer, + sid->u.ssl3.keys.wrapped_master_secret, + sid->u.ssl3.keys.wrapped_master_secret_len + }; + + wrapKey = ssl3_GetWrappingKey(ss, NULL, sid->u.ssl3.masterWrapMech, + ss->pkcs11PinArg); + if (!wrapKey) { + return SECFailure; + } + + if (ss->version > SSL_LIBRARY_VERSION_3_0) { /* isTLS */ + keyFlags = CKF_SIGN | CKF_VERIFY; + } + + /* unwrap the master secret. */ + *ms = PK11_UnwrapSymKeyWithFlags(wrapKey, sid->u.ssl3.masterWrapMech, + NULL, &wrappedMS, CKM_SSL3_MASTER_KEY_DERIVE, + CKA_DERIVE, sizeof(SSL3MasterSecret), keyFlags); + PK11_FreeSymKey(wrapKey); + if (!*ms) { + return SECFailure; + } + return SECSuccess; +} + static SECStatus ssl3_HandleClientHelloPart2(sslSocket *ss, SECItem *suites, @@ -8619,7 +8616,6 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, const PRUint8 *msg, unsigned int len) { - PRBool haveSpecWriteLock = PR_FALSE; PRBool haveXmitBufLock = PR_FALSE; int errCode = SSL_ERROR_RX_MALFORMED_CLIENT_HELLO; SSL3AlertDescription desc = illegal_parameter; @@ -8745,8 +8741,7 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, */ if (sid != NULL) do { - ssl3CipherSpec *pwSpec; - SECItem wrappedMS; /* wrapped key */ + PK11SymKey *masterSecret; if (sid->version != ss->version || sid->u.ssl3.cipherSuite != ss->ssl3.hs.cipher_suite || @@ -8799,54 +8794,13 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, } ss->sec.ci.sid = NULL; } - /* we need to resurrect the master secret.... */ - - ssl_GetSpecWriteLock(ss); - haveSpecWriteLock = PR_TRUE; - pwSpec = ss->ssl3.pwSpec; - if (sid->u.ssl3.keys.msIsWrapped) { - PK11SymKey *wrapKey; /* wrapping key */ - CK_FLAGS keyFlags = 0; - - wrapKey = ssl3_GetWrappingKey(ss, NULL, - sid->u.ssl3.masterWrapMech, - ss->pkcs11PinArg); - if (!wrapKey) { - /* we have a SID cache entry, but no wrapping key for it??? */ - break; - } - - if (ss->version > SSL_LIBRARY_VERSION_3_0) { /* isTLS */ - keyFlags = CKF_SIGN | CKF_VERIFY; - } - - wrappedMS.data = sid->u.ssl3.keys.wrapped_master_secret; - wrappedMS.len = sid->u.ssl3.keys.wrapped_master_secret_len; - /* unwrap the master secret. */ - pwSpec->master_secret = - PK11_UnwrapSymKeyWithFlags(wrapKey, sid->u.ssl3.masterWrapMech, - NULL, &wrappedMS, CKM_SSL3_MASTER_KEY_DERIVE, - CKA_DERIVE, sizeof(SSL3MasterSecret), keyFlags); - PK11_FreeSymKey(wrapKey); - if (pwSpec->master_secret == NULL) { - break; /* not an error */ - } - } else { - /* need to import the raw master secret to session object */ - PK11SlotInfo *slot; - wrappedMS.data = sid->u.ssl3.keys.wrapped_master_secret; - wrappedMS.len = sid->u.ssl3.keys.wrapped_master_secret_len; - slot = PK11_GetInternalSlot(); - pwSpec->master_secret = - PK11_ImportSymKey(slot, CKM_SSL3_MASTER_KEY_DERIVE, - PK11_OriginUnwrap, CKA_ENCRYPT, &wrappedMS, - NULL); - PK11_FreeSlot(slot); - if (pwSpec->master_secret == NULL) { - break; /* not an error */ - } + /* we need to resurrect the master secret.... */ + 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); @@ -8902,13 +8856,8 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, goto loser; } - if (haveSpecWriteLock) { - ssl_ReleaseSpecWriteLock(ss); - haveSpecWriteLock = PR_FALSE; - } - - /* NULL value for PMS because we are re-using the old MS */ - rv = ssl3_InitPendingCipherSpec(ss, NULL); + /* We are re-using the old MS, so no need to derive again. */ + rv = ssl3_InitPendingCipherSpecs(ss, masterSecret, PR_FALSE); if (rv != SECSuccess) { errCode = PORT_GetError(); goto loser; @@ -8933,11 +8882,6 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, return SECSuccess; } while (0); - if (haveSpecWriteLock) { - ssl_ReleaseSpecWriteLock(ss); - haveSpecWriteLock = PR_FALSE; - } - if (sid) { /* we had a sid, but it's no longer valid, free it */ ss->statelessResume = PR_FALSE; SSL_AtomicIncrementLong(&ssl3stats.hch_sid_cache_not_ok); @@ -9003,10 +8947,6 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, return SECSuccess; alert_loser: - if (haveSpecWriteLock) { - ssl_ReleaseSpecWriteLock(ss); - haveSpecWriteLock = PR_FALSE; - } (void)SSL3_SendAlert(ss, alert_fatal, desc); /* FALLTHRU */ loser: @@ -9015,10 +8955,6 @@ ssl3_HandleClientHelloPart2(sslSocket *ss, ssl_FreeSID(sid); } - if (haveSpecWriteLock) { - ssl_ReleaseSpecWriteLock(ss); - } - if (haveXmitBufLock) { ssl_ReleaseXmitBufLock(ss); } @@ -9978,7 +9914,7 @@ ssl3_HandleRSAClientKeyExchange(sslSocket *ss, } /* This step will derive the MS from the PMS, among other things. */ - rv = ssl3_InitPendingCipherSpec(ss, currentPms); + rv = ssl3_InitPendingCipherSpecs(ss, currentPms, PR_TRUE); PK11_FreeSymKey(currentPms); if (rv != SECSuccess) { @@ -10043,7 +9979,7 @@ ssl3_HandleDHClientKeyExchange(sslSocket *ss, return SECFailure; } - rv = ssl3_InitPendingCipherSpec(ss, pms); + rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE); PK11_FreeSymKey(pms); ssl_FreeEphemeralKeyPairs(ss); return rv; @@ -11054,40 +10990,39 @@ ssl3_TLSPRFWithMasterSecret(sslSocket *ss, ssl3CipherSpec *spec, const unsigned char *val, unsigned int valLen, unsigned char *out, unsigned int outLen) { - SECStatus rv = SECSuccess; + SECItem param = { siBuffer, NULL, 0 }; + CK_MECHANISM_TYPE mech = CKM_TLS_PRF_GENERAL; + PK11Context *prf_context; + unsigned int retLen; + SECStatus rv; - if (spec->master_secret) { - SECItem param = { siBuffer, NULL, 0 }; - CK_MECHANISM_TYPE mech = CKM_TLS_PRF_GENERAL; - PK11Context *prf_context; - unsigned int retLen; + if (!spec->master_secret) { + PORT_Assert(spec->master_secret); + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return SECFailure; + } - if (spec->version >= SSL_LIBRARY_VERSION_TLS_1_2) { - /* Bug 1312976 non-SHA256 exporters are broken. */ - if (ssl3_GetPrfHashMechanism(ss) != CKM_SHA256) { - PORT_Assert(0); - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - return SECFailure; - } - mech = CKM_NSS_TLS_PRF_GENERAL_SHA256; - } - prf_context = PK11_CreateContextBySymKey(mech, CKA_SIGN, - spec->master_secret, ¶m); - if (!prf_context) + if (spec->version >= SSL_LIBRARY_VERSION_TLS_1_2) { + /* Bug 1312976 non-SHA256 exporters are broken. */ + if (ssl3_GetPrfHashMechanism(ss) != CKM_SHA256) { + PORT_Assert(0); + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); return SECFailure; + } + mech = CKM_NSS_TLS_PRF_GENERAL_SHA256; + } + prf_context = PK11_CreateContextBySymKey(mech, CKA_SIGN, + spec->master_secret, ¶m); + if (!prf_context) + return SECFailure; - rv = PK11_DigestBegin(prf_context); - rv |= PK11_DigestOp(prf_context, (unsigned char *)label, labelLen); - rv |= PK11_DigestOp(prf_context, val, valLen); - rv |= PK11_DigestFinal(prf_context, out, &retLen, outLen); - PORT_Assert(rv != SECSuccess || retLen == outLen); + rv = PK11_DigestBegin(prf_context); + rv |= PK11_DigestOp(prf_context, (unsigned char *)label, labelLen); + rv |= PK11_DigestOp(prf_context, val, valLen); + rv |= PK11_DigestFinal(prf_context, out, &retLen, outLen); + PORT_Assert(rv != SECSuccess || retLen == outLen); - PK11_DestroyContext(prf_context, PR_TRUE); - } else { - PORT_Assert(spec->master_secret); - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - rv = SECFailure; - } + PK11_DestroyContext(prf_context, PR_TRUE); return rv; } @@ -11523,7 +11458,7 @@ ssl3_HandleFinished(sslSocket *ss, PRUint8 *b, PRUint32 length) SECStatus ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret) { - SECStatus rv; + PORT_Assert(secret); /* fill in the sid */ sid->u.ssl3.cipherSuite = ss->ssl3.hs.cipher_suite; @@ -11555,26 +11490,8 @@ ssl3_FillInCachedSID(sslSocket *ss, sslSessionID *sid, PK11SymKey *secret) } } - ssl_GetSpecReadLock(ss); /*************************************/ - /* 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, - ss->ssl3.crSpec->msItem.data, ss->ssl3.crSpec->msItem.len); - sid->u.ssl3.masterValid = PR_TRUE; - sid->u.ssl3.keys.msIsWrapped = PR_FALSE; - rv = SECSuccess; - } else { - PORT_Assert(secret); - rv = ssl3_CacheWrappedSecret(ss, ss->sec.ci.sid, secret); - sid->u.ssl3.keys.msIsWrapped = PR_TRUE; - } - ssl_ReleaseSpecReadLock(ss); /*************************************/ - - return rv; + return ssl3_CacheWrappedSecret(ss, ss->sec.ci.sid, secret); } /* The return type is SECStatus instead of void because this function needs @@ -12849,9 +12766,6 @@ ssl3_InitCipherSpec(ssl3CipherSpec *spec) spec->mac_size = 0; spec->master_secret = NULL; - spec->msItem.data = NULL; - spec->msItem.len = 0; - spec->client.write_key = NULL; spec->client.write_mac_key = NULL; spec->client.write_mac_context = NULL; diff --git a/lib/ssl/ssl3ecc.c b/lib/ssl/ssl3ecc.c index 66dda086cd..a30d99623c 100644 --- a/lib/ssl/ssl3ecc.c +++ b/lib/ssl/ssl3ecc.c @@ -232,7 +232,7 @@ ssl3_SendECDHClientKeyExchange(sslSocket *ss, SECKEYPublicKey *svrPubKey) goto loser; /* err set by ssl3_AppendHandshake* */ } - rv = ssl3_InitPendingCipherSpec(ss, pms); + rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE); if (rv != SECSuccess) { ssl_MapLowLevelError(SSL_ERROR_CLIENT_KEY_EXCHANGE_FAILURE); goto loser; @@ -313,7 +313,7 @@ ssl3_HandleECDHClientKeyExchange(sslSocket *ss, PRUint8 *b, return SECFailure; } - rv = ssl3_InitPendingCipherSpec(ss, pms); + rv = ssl3_InitPendingCipherSpecs(ss, pms, PR_TRUE); PK11_FreeSymKey(pms); if (rv != SECSuccess) { /* error code set by ssl3_InitPendingCipherSpec */ diff --git a/lib/ssl/ssl3exthandle.c b/lib/ssl/ssl3exthandle.c index c3d32f6960..b52b79a4ec 100644 --- a/lib/ssl/ssl3exthandle.c +++ b/lib/ssl/ssl3exthandle.c @@ -654,7 +654,7 @@ ssl3_ClientHandleStatusRequestXtn(const sslSocket *ss, TLSExtensionData *xtnData } PRUint32 ssl_ticket_lifetime = 2 * 24 * 60 * 60; /* 2 days in seconds */ -#define TLS_EX_SESS_TICKET_VERSION (0x0106) +#define TLS_EX_SESS_TICKET_VERSION (0x0107) /* * Called from ssl3_SendNewSessionTicket, tls13_SendNewSessionTicket @@ -667,13 +667,12 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, SECStatus rv; sslBuffer plaintext = { NULL, 0, 0 }; SECItem ticket_buf = { 0, NULL, 0 }; - PRBool ms_is_wrapped; + sslSessionID sid; unsigned char wrapped_ms[SSL3_MASTER_SECRET_LENGTH]; SECItem ms_item = { 0, NULL, 0 }; PRTime now; SECItem *srvName = NULL; - CK_MECHANISM_TYPE msWrapMech = 0; /* dummy default value, - * must be >= 0 */ + CK_MECHANISM_TYPE msWrapMech; SECItem *alpnSelection = NULL; PRUint32 ticketAgeBaseline; @@ -683,33 +682,23 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss)); PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss)); - 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 = 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; + /* Extract the master secret wrapped. */ - PORT_Memset(&sid, 0, sizeof(sslSessionID)); + PORT_Memset(&sid, 0, sizeof(sslSessionID)); - 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; - memcpy(wrapped_ms, sid.u.ssl3.keys.wrapped_master_secret, - sid.u.ssl3.keys.wrapped_master_secret_len); - ms_item.data = wrapped_ms; - ms_item.len = sid.u.ssl3.keys.wrapped_master_secret_len; - msWrapMech = sid.u.ssl3.masterWrapMech; - } else { - /* TODO: else send an empty ticket. */ + 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; - } - ms_is_wrapped = PR_TRUE; + memcpy(wrapped_ms, sid.u.ssl3.keys.wrapped_master_secret, + sid.u.ssl3.keys.wrapped_master_secret_len); + ms_item.data = wrapped_ms; + ms_item.len = sid.u.ssl3.keys.wrapped_master_secret_len; + msWrapMech = sid.u.ssl3.masterWrapMech; + } else { + /* TODO: else send an empty ticket. */ + goto loser; } /* Prep to send negotiated name */ srvName = &ss->sec.ci.sid->u.ssl3.srvName; @@ -779,9 +768,6 @@ ssl3_EncodeSessionTicket(sslSocket *ss, const NewSessionTicket *ticket, goto loser; /* master_secret */ - rv = sslBuffer_AppendNumber(&plaintext, ms_is_wrapped, 1); - if (rv != SECSuccess) - goto loser; rv = sslBuffer_AppendNumber(&plaintext, msWrapMech, 4); if (rv != SECSuccess) goto loser; @@ -1052,14 +1038,6 @@ ssl_ParseSessionTicket(sslSocket *ss, const SECItem *decryptedTicket, } /* Read the master secret (and how it is wrapped). */ - rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 1, &buffer, &len); - if (rv != SECSuccess) { - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - return SECFailure; - } - PORT_Assert(temp == PR_TRUE || temp == PR_FALSE); - parsedTicket->ms_is_wrapped = (PRBool)temp; - rv = ssl3_ExtConsumeHandshakeNumber(ss, &temp, 4, &buffer, &len); if (rv != SECSuccess) { PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); @@ -1230,7 +1208,6 @@ ssl_CreateSIDFromTicket(sslSocket *ss, const SECItem *rawTicket, parsedTicket->master_secret, parsedTicket->ms_length); sid->u.ssl3.keys.wrapped_master_secret_len = parsedTicket->ms_length; sid->u.ssl3.masterWrapMech = parsedTicket->msWrapMech; - sid->u.ssl3.keys.msIsWrapped = parsedTicket->ms_is_wrapped; sid->u.ssl3.masterValid = PR_TRUE; sid->u.ssl3.keys.resumable = PR_TRUE; sid->u.ssl3.keys.extendedMasterSecretUsed = parsedTicket->extendedMasterSecretUsed; diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index 30ca051a72..18b371446a 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -411,7 +411,6 @@ typedef PRUint16 DTLSEpoch; typedef struct { PRUint8 wrapped_master_secret[48]; PRUint16 wrapped_master_secret_len; - PRUint8 msIsWrapped; PRUint8 resumable; PRUint8 extendedMasterSecretUsed; } ssl3SidKeys; /* 53 bytes */ @@ -420,9 +419,6 @@ typedef struct { PK11SymKey *write_key; PK11SymKey *write_mac_key; PK11Context *write_mac_context; - SECItem write_key_item; - SECItem write_iv_item; - SECItem write_mac_key_item; PRUint8 write_iv[MAX_IV_LENGTH]; } ssl3KeyMaterial; @@ -499,7 +495,6 @@ struct ssl3CipherSpecStr { SSL3ProtocolVersion version; ssl3KeyMaterial client; ssl3KeyMaterial server; - SECItem msItem; DTLSEpoch epoch; DTLSRecvdRecords recvdRecords; /* The number of 0-RTT bytes that can be sent or received in TLS 1.3. This @@ -1358,7 +1353,8 @@ extern int ssl_Do1stHandshake(sslSocket *ss); extern void ssl_ChooseSessionIDProcs(sslSecurityInfo *sec); -extern void ssl3_InitCipherSpec(ssl3CipherSpec *spec); +extern SECStatus ssl3_InitPendingCipherSpecs(sslSocket *ss, PK11SymKey *secret, + PRBool derive); extern sslSessionID *ssl3_NewSessionID(sslSocket *ss, PRBool is_server); extern sslSessionID *ssl_LookupSID(const PRIPv6Addr *addr, PRUint16 port, const char *peerID, const char *urlSvrName); diff --git a/lib/ssl/sslinfo.c b/lib/ssl/sslinfo.c index a8b4f2f268..6d8caec6fa 100644 --- a/lib/ssl/sslinfo.c +++ b/lib/ssl/sslinfo.c @@ -510,7 +510,7 @@ SSL_ExportKeyingMaterial(PRFileDesc *fd, * secret is available and we have sent ChangeCipherSpec. */ ssl_GetSpecReadLock(ss); - if (!ss->ssl3.cwSpec->master_secret && !ss->ssl3.cwSpec->msItem.len) { + if (!ss->ssl3.cwSpec->master_secret) { PORT_SetError(SSL_ERROR_HANDSHAKE_NOT_COMPLETED); rv = SECFailure; } else { diff --git a/lib/ssl/tls13con.c b/lib/ssl/tls13con.c index 7099ea7e31..15c56ef8d1 100644 --- a/lib/ssl/tls13con.c +++ b/lib/ssl/tls13con.c @@ -640,10 +640,6 @@ tls13_RecoverWrappedSharedSecret(sslSocket *ss, sslSessionID *sid) SSL_TRC(3, ("%d: TLS13[%d]: recovering static secret (%s)", SSL_GETPID(), ss->fd, SSL_ROLE(ss))); - if (!sid->u.ssl3.keys.msIsWrapped) { - PORT_Assert(0); /* I think this can't happen. */ - return SECFailure; - } /* Now find the hash used as the PRF for the previous handshake. */ hashType = tls13_GetHashForCipherSuite(sid->u.ssl3.cipherSuite);