diff --git a/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp b/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp index 1e685b97ae..b3b6de22b1 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp +++ b/security/nss/gtests/mozpkix_gtest/pkixbuild_tests.cpp @@ -151,14 +151,11 @@ private: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, - Time validityBeginning, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { - // All of the certificates in this test for which this is called have a - // validity period that begins "one day before now". - EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning); return Success; } @@ -304,14 +301,11 @@ public: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, - Time validityBeginning, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { - // All of the certificates in this test for which this is called have a - // validity period that begins "one day before now". - EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning); return Success; } @@ -328,8 +322,9 @@ public: { } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { ADD_FAILURE(); @@ -449,14 +444,11 @@ public: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, - Time validityBeginning, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { - // All of the certificates in this test for which this is called have a - // validity period that begins "one day before now". - EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning); return Success; } @@ -676,14 +668,11 @@ private: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, - Time validityBeginning, Duration, + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, /*optional*/ const Input*) override { - // All of the certificates in this test for which this is called have a - // validity period that begins "one day before now". - EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning); return Success; } @@ -738,8 +727,8 @@ class RevokedEndEntityTrustDomain final : public MultiplePathTrustDomain { public: Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time, - Time, Duration, /*optional*/ const Input*, - /*optional*/ const Input*) override + Duration, /*optional*/ const Input*, + /*optional*/ const Input*, /*optional*/ const Input*) override { if (endEntityOrCA == EndEntityOrCA::MustBeEndEntity) { return Result::ERROR_REVOKED_CERTIFICATE; @@ -843,14 +832,11 @@ private: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, - Time validityBeginning, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { - // All of the certificates in this test for which this is called have a - // validity period that begins "one day before now". - EXPECT_EQ(TimeFromEpochInSeconds(oneDayBeforeNow), validityBeginning); return Success; } diff --git a/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp b/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp index 63eb32c469..98c755920f 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp +++ b/security/nss/gtests/mozpkix_gtest/pkixcert_extension_tests.cpp @@ -69,8 +69,9 @@ private: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, - /*optional*/ const Input*, /*optional*/ const Input*) + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, + /*optional*/ const Input*) override { return Success; diff --git a/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp b/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp index ba8dee3c68..5d5eeb3ff4 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp +++ b/security/nss/gtests/mozpkix_gtest/pkixcert_signature_algorithm_tests.cpp @@ -91,8 +91,8 @@ private: return checker.Check(issuerCert, nullptr, keepGoing); } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, - const Input*, const Input*) override + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + const Input*, const Input*, const Input*) override { return Success; } diff --git a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp index edeae1612c..055593fb23 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp +++ b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckExtendedKeyUsage_tests.cpp @@ -557,8 +557,8 @@ private: return checker.Check(derCert, nullptr, keepGoing); } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, - const Input*, const Input*) override + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + const Input*, const Input*, const Input*) override { return Success; } diff --git a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp index f5501537b3..126709b5ec 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp +++ b/security/nss/gtests/mozpkix_gtest/pkixcheck_CheckSignatureAlgorithm_tests.cpp @@ -301,7 +301,8 @@ public: return Success; } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, /*optional*/ const Input*) override { diff --git a/security/nss/gtests/mozpkix_gtest/pkixgtest.h b/security/nss/gtests/mozpkix_gtest/pkixgtest.h index 9897f5433b..6c4bc0ae46 100644 --- a/security/nss/gtests/mozpkix_gtest/pkixgtest.h +++ b/security/nss/gtests/mozpkix_gtest/pkixgtest.h @@ -99,7 +99,8 @@ class EverythingFailsByDefaultTrustDomain : public TrustDomain { Result::FATAL_ERROR_LIBRARY_FAILURE); } - Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Time, Duration, + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, Duration, + /*optional*/ const Input*, /*optional*/ const Input*, /*optional*/ const Input*) override { ADD_FAILURE(); diff --git a/security/nss/lib/certdb/crl.c b/security/nss/lib/certdb/crl.c index 64c60493f9..63adcad46d 100644 --- a/security/nss/lib/certdb/crl.c +++ b/security/nss/lib/certdb/crl.c @@ -1391,7 +1391,6 @@ TokenCRLStillExists(CERTSignedCrl* crl) arena = NSSArena_Create(); PORT_Assert(arena); if (!arena) { - (void)nssToken_Destroy(instance.token); return PR_FALSE; } @@ -1413,7 +1412,6 @@ TokenCRLStillExists(CERTSignedCrl* crl) xstatus = PR_FALSE; } NSSArena_Destroy(arena); - (void)nssToken_Destroy(instance.token); return xstatus; } diff --git a/security/nss/lib/certdb/stanpcertdb.c b/security/nss/lib/certdb/stanpcertdb.c index e255df1053..8e1cf279ab 100644 --- a/security/nss/lib/certdb/stanpcertdb.c +++ b/security/nss/lib/certdb/stanpcertdb.c @@ -299,15 +299,9 @@ __CERT_AddTempCertToPerm(CERTCertificate *cert, char *nickname, /* Import the perm instance onto the internal token */ slot = PK11_GetInternalKeySlot(); internal = PK11Slot_GetNSSToken(slot); - if (!internal) { - PK11_FreeSlot(slot); - PORT_SetError(SEC_ERROR_NO_TOKEN); - return SECFailure; - } permInstance = nssToken_ImportCertificate( internal, NULL, NSSCertificateType_PKIX, &c->id, stanNick, &c->encoding, &c->issuer, &c->subject, &c->serial, cert->emailAddr, PR_TRUE); - (void)nssToken_Destroy(internal); nss_ZFreeIf(stanNick); stanNick = NULL; PK11_FreeSlot(slot); diff --git a/security/nss/lib/dev/devtoken.c b/security/nss/lib/dev/devtoken.c index d155d3b2ba..207b0bc163 100644 --- a/security/nss/lib/dev/devtoken.c +++ b/security/nss/lib/dev/devtoken.c @@ -46,6 +46,13 @@ nssToken_Remove( nssTokenObjectCache_Clear(tok->cache); } +NSS_IMPLEMENT void +NSSToken_Destroy( + NSSToken *tok) +{ + (void)nssToken_Destroy(tok); +} + NSS_IMPLEMENT NSSToken * nssToken_AddRef( NSSToken *tok) @@ -982,9 +989,8 @@ sha1_hash(NSSItem *input, NSSItem *output) NSSToken *token = PK11Slot_GetNSSToken(internal); ap = NSSAlgorithmAndParameters_CreateSHA1Digest(NULL); (void)nssToken_Digest(token, NULL, ap, input, output, NULL); + PK11_FreeSlot(token->pk11slot); nss_ZFreeIf(ap); - (void)nssToken_Destroy(token); - PK11_FreeSlot(internal); } static void @@ -995,9 +1001,8 @@ md5_hash(NSSItem *input, NSSItem *output) NSSToken *token = PK11Slot_GetNSSToken(internal); ap = NSSAlgorithmAndParameters_CreateMD5Digest(NULL); (void)nssToken_Digest(token, NULL, ap, input, output, NULL); + PK11_FreeSlot(token->pk11slot); nss_ZFreeIf(ap); - (void)nssToken_Destroy(token); - PK11_FreeSlot(internal); } static CK_TRUST diff --git a/security/nss/lib/dev/devutil.c b/security/nss/lib/dev/devutil.c index a6bbb93207..74c32f088f 100644 --- a/security/nss/lib/dev/devutil.c +++ b/security/nss/lib/dev/devutil.c @@ -56,7 +56,7 @@ nssCryptokiObject_Destroy( nssCryptokiObject *object) { if (object) { - (void)nssToken_Destroy(object->token); + nssToken_Destroy(object->token); nss_ZFreeIf(object->label); nss_ZFreeIf(object); } @@ -150,12 +150,19 @@ nssTokenArray_Destroy( if (tokens) { NSSToken **tokenp; for (tokenp = tokens; *tokenp; tokenp++) { - (void)nssToken_Destroy(*tokenp); + nssToken_Destroy(*tokenp); } nss_ZFreeIf(tokens); } } +NSS_IMPLEMENT void +NSSTokenArray_Destroy( + NSSToken **tokens) +{ + nssTokenArray_Destroy(tokens); +} + NSS_IMPLEMENT void nssCryptokiObjectArray_Destroy( nssCryptokiObject **objects) @@ -358,7 +365,7 @@ create_object( /* The cache is tied to the token, and therefore the objects * in it should not hold references to the token. */ - (void)nssToken_Destroy(object->token); + nssToken_Destroy(object->token); rvCachedObject->object = object; rvCachedObject->attributes = nss_ZNEWARRAY(arena, CK_ATTRIBUTE, numTypes); if (!rvCachedObject->attributes) { @@ -561,7 +568,7 @@ get_token_objects_for_cache( &numObjects, &status); if (status != PR_SUCCESS) { - nssCryptokiObjectArray_Destroy(objects); + nss_ZFreeIf(objects); return status; } for (i = 0; i < numObjects; i++) { @@ -577,8 +584,7 @@ get_token_objects_for_cache( } else { PRUint32 j; for (j = 0; j < i; j++) { - /* Any token references that were removed in successful loop iterations - * need to be restored before we call nssCryptokiObjectArray_Destroy */ + /* sigh */ nssToken_AddRef(cache->objects[objectType][j]->object->token); nssArena_Destroy(cache->objects[objectType][j]->arena); } diff --git a/security/nss/lib/mozpkix/include/pkix/pkixtypes.h b/security/nss/lib/mozpkix/include/pkix/pkixtypes.h index 8f149129fe..5360a4aa1a 100644 --- a/security/nss/lib/mozpkix/include/pkix/pkixtypes.h +++ b/security/nss/lib/mozpkix/include/pkix/pkixtypes.h @@ -277,10 +277,10 @@ class TrustDomain { virtual Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID& certID, Time time, - Time validityBeginning, Duration validityDuration, /*optional*/ const Input* stapledOCSPresponse, - /*optional*/ const Input* aiaExtension) = 0; + /*optional*/ const Input* aiaExtension, + /*optional*/ const Input* sctExtension) = 0; // Check that the given digest algorithm is acceptable for use in signatures. // diff --git a/security/nss/lib/mozpkix/lib/pkixbuild.cpp b/security/nss/lib/mozpkix/lib/pkixbuild.cpp index e15e738063..a23e621449 100644 --- a/security/nss/lib/mozpkix/lib/pkixbuild.cpp +++ b/security/nss/lib/mozpkix/lib/pkixbuild.cpp @@ -251,9 +251,9 @@ PathBuildingStep::Check(Input potentialIssuerDER, } Duration validityDuration(notAfter, notBefore); rv = trustDomain.CheckRevocation(subject.endEntityOrCA, certID, time, - notBefore, validityDuration, - stapledOCSPResponse, - subject.GetAuthorityInfoAccess()); + validityDuration, stapledOCSPResponse, + subject.GetAuthorityInfoAccess(), + subject.GetSignedCertificateTimestamps()); if (rv != Success) { // Since this is actually a problem with the current subject certificate // (rather than the issuer), it doesn't make sense to keep going; all diff --git a/security/nss/lib/pk11wrap/pk11auth.c b/security/nss/lib/pk11wrap/pk11auth.c index b7d3b40cc8..6a3ba40d95 100644 --- a/security/nss/lib/pk11wrap/pk11auth.c +++ b/security/nss/lib/pk11wrap/pk11auth.c @@ -4,8 +4,6 @@ /* * This file deals with PKCS #11 passwords and authentication. */ -#include "dev.h" -#include "dev3hack.h" #include "seccomon.h" #include "secmod.h" #include "secmodi.h" @@ -639,11 +637,8 @@ PK11_DoPassword(PK11SlotInfo *slot, CK_SESSION_HANDLE session, } if (rv == SECSuccess) { if (!contextSpecific && !PK11_IsFriendly(slot)) { - NSSToken *token = PK11Slot_GetNSSToken(slot); - if (token) { - nssTrustDomain_UpdateCachedTokenCerts(token->trustDomain, token); - (void)nssToken_Destroy(token); - } + nssTrustDomain_UpdateCachedTokenCerts(slot->nssToken->trustDomain, + slot->nssToken); } } else if (!attempt) PORT_SetError(SEC_ERROR_BAD_PASSWORD); diff --git a/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c index 974dea558e..8ff55fce7d 100644 --- a/security/nss/lib/pk11wrap/pk11cert.c +++ b/security/nss/lib/pk11wrap/pk11cert.c @@ -240,17 +240,16 @@ pk11_fastCert(PK11SlotInfo *slot, CK_OBJECT_HANDLE certID, NSSCertificate *c; nssCryptokiObject *co = NULL; nssPKIObject *pkio; + NSSToken *token; NSSTrustDomain *td = STAN_GetDefaultTrustDomain(); /* Get the cryptoki object from the handle */ - NSSToken *token = PK11Slot_GetNSSToken(slot); - if (!token || !token->defaultSession) { - (void)nssToken_Destroy(token); /* null token is ok */ + token = PK11Slot_GetNSSToken(slot); + if (token && token->defaultSession) { + co = nssCryptokiObject_Create(token, token->defaultSession, certID); + } else { PORT_SetError(SEC_ERROR_NO_TOKEN); - return NULL; } - co = nssCryptokiObject_Create(token, token->defaultSession, certID); - (void)nssToken_Destroy(token); if (!co) { return NULL; } @@ -753,7 +752,7 @@ find_certs_from_uri(const char *uriString, void *wincx) nssPKIObjectCollection_AddInstances(collection, instances, 0); nss_ZFreeIf(instances); } - (void)nssToken_Destroy(*tok); + nssToken_Destroy(*tok); } nss_ZFreeIf(tokens); nssList_Destroy(certList); @@ -862,7 +861,9 @@ find_certs_from_nickname(const char *nickname, void *wincx) } else { slot = PK11_GetInternalKeySlot(); token = PK11Slot_GetNSSToken(slot); - if (!token) { + if (token) { + nssToken_AddRef(token); + } else { PORT_SetError(SEC_ERROR_NO_TOKEN); } } @@ -926,7 +927,7 @@ find_certs_from_nickname(const char *nickname, void *wincx) } loser: if (token) { - (void)nssToken_Destroy(token); + nssToken_Destroy(token); } if (slot) { PK11_FreeSlot(slot); @@ -1126,15 +1127,15 @@ PK11_ImportCert(PK11SlotInfo *slot, CERTCertificate *cert, PRStatus status; NSSCertificate *c; nssCryptokiObject *keyobj, *certobj; - NSSToken *token = NULL; + NSSToken *token = PK11Slot_GetNSSToken(slot); + SECItem *keyID = pk11_mkcertKeyID(cert); char *emailAddr = NULL; nssCertificateStoreTrace lockTrace = { NULL, NULL, PR_FALSE, PR_FALSE }; nssCertificateStoreTrace unlockTrace = { NULL, NULL, PR_FALSE, PR_FALSE }; - SECItem *keyID = pk11_mkcertKeyID(cert); + if (keyID == NULL) { goto loser; /* error code should be set already */ } - token = PK11Slot_GetNSSToken(slot); if (!token) { PORT_SetError(SEC_ERROR_NO_TOKEN); goto loser; @@ -1227,12 +1228,8 @@ PK11_ImportCert(PK11SlotInfo *slot, CERTCertificate *cert, (void)STAN_ForceCERTCertificateUpdate(c); nssCertificate_Destroy(c); SECITEM_FreeItem(keyID, PR_TRUE); - (void)nssToken_Destroy(token); return SECSuccess; loser: - if (token) { - (void)nssToken_Destroy(token); - } CERT_MapStanError(); SECITEM_FreeItem(keyID, PR_TRUE); if (PORT_GetError() != SEC_ERROR_TOKEN_NOT_LOGGED_IN) { @@ -1511,7 +1508,7 @@ PK11_FindCertByIssuerAndSNOnToken(PK11SlotInfo *slot, NSSCertificate *cert = NULL; NSSDER issuer, serial; NSSTrustDomain *td = STAN_GetDefaultTrustDomain(); - NSSToken *token = NULL; + NSSToken *token = slot->nssToken; nssSession *session; nssCryptokiObject *instance = NULL; nssPKIObject *object = NULL; @@ -1526,18 +1523,12 @@ PK11_FindCertByIssuerAndSNOnToken(PK11SlotInfo *slot, return NULL; } - token = PK11Slot_GetNSSToken(slot); - if (!token) { + /* Paranoia */ + if (token == NULL) { PORT_SetError(SEC_ERROR_NO_TOKEN); return NULL; } - session = nssToken_GetDefaultSession(token); /* non-owning */ - if (!session) { - (void)nssToken_Destroy(token); - return NULL; - } - /* PKCS#11 needs to use DER-encoded serial numbers. Create a * CERTIssuerAndSN that actually has the encoded value and pass that * to PKCS#11 (and the crypto context). @@ -1546,17 +1537,20 @@ PK11_FindCertByIssuerAndSNOnToken(PK11SlotInfo *slot, &issuerSN->serialNumber, SEC_ASN1_GET(SEC_IntegerTemplate)); if (!derSerial) { - (void)nssToken_Destroy(token); return NULL; } NSSITEM_FROM_SECITEM(&issuer, &issuerSN->derIssuer); NSSITEM_FROM_SECITEM(&serial, derSerial); + session = nssToken_GetDefaultSession(token); + if (!session) { + goto loser; + } + instance = nssToken_FindCertificateByIssuerAndSerialNumber(token, session, &issuer, &serial, nssTokenSearchType_TokenForced, &status); - (void)nssToken_Destroy(token); SECITEM_FreeItem(derSerial, PR_TRUE); if (!instance) { @@ -2226,22 +2220,16 @@ PK11_TraverseCertsForSubjectInSlot(CERTCertificate *cert, PK11SlotInfo *slot, td = STAN_GetDefaultTrustDomain(); NSSITEM_FROM_SECITEM(&subject, &cert->derSubject); token = PK11Slot_GetNSSToken(slot); - if (!token) { - return SECSuccess; - } if (!nssToken_IsPresent(token)) { - (void)nssToken_Destroy(token); return SECSuccess; } collection = nssCertificateCollection_Create(td, NULL); if (!collection) { - (void)nssToken_Destroy(token); return SECFailure; } subjectList = nssList_Create(NULL, PR_FALSE); if (!subjectList) { nssPKIObjectCollection_Destroy(collection); - (void)nssToken_Destroy(token); return SECFailure; } (void)nssTrustDomain_GetCertsForSubjectFromCache(td, &subject, @@ -2256,7 +2244,6 @@ PK11_TraverseCertsForSubjectInSlot(CERTCertificate *cert, PK11SlotInfo *slot, certs = nssPKIObjectCollection_GetCertificates(collection, NULL, 0, NULL); nssPKIObjectCollection_Destroy(collection); - (void)nssToken_Destroy(token); if (certs) { CERTCertificate *oldie; NSSCertificate **cp; @@ -2290,8 +2277,7 @@ PK11_TraverseCertsForNicknameInSlot(SECItem *nickname, PK11SlotInfo *slot, nssList *nameList = NULL; nssTokenSearchType tokenOnly = nssTokenSearchType_TokenOnly; token = PK11Slot_GetNSSToken(slot); - if (!token || !nssToken_IsPresent(token)) { - (void)nssToken_Destroy(token); + if (!nssToken_IsPresent(token)) { return SECSuccess; } if (nickname->data[nickname->len - 1] != '\0') { @@ -2321,7 +2307,6 @@ PK11_TraverseCertsForNicknameInSlot(SECItem *nickname, PK11SlotInfo *slot, certs = nssPKIObjectCollection_GetCertificates(collection, NULL, 0, NULL); nssPKIObjectCollection_Destroy(collection); - (void)nssToken_Destroy(token); if (certs) { CERTCertificate *oldie; NSSCertificate **cp; @@ -2341,7 +2326,6 @@ PK11_TraverseCertsForNicknameInSlot(SECItem *nickname, PK11SlotInfo *slot, nss_ZFreeIf(nick); return (nssrv == PR_SUCCESS) ? SECSuccess : SECFailure; loser: - (void)nssToken_Destroy(token); if (created) { nss_ZFreeIf(nick); } @@ -2367,22 +2351,16 @@ PK11_TraverseCertsInSlot(PK11SlotInfo *slot, NSSCertificate **certs; nssTokenSearchType tokenOnly = nssTokenSearchType_TokenOnly; tok = PK11Slot_GetNSSToken(slot); - if (!tok) { - return SECSuccess; - } if (!nssToken_IsPresent(tok)) { - (void)nssToken_Destroy(tok); return SECSuccess; } collection = nssCertificateCollection_Create(td, NULL); if (!collection) { - (void)nssToken_Destroy(tok); return SECFailure; } certList = nssList_Create(NULL, PR_FALSE); if (!certList) { nssPKIObjectCollection_Destroy(collection); - (void)nssToken_Destroy(tok); return SECFailure; } (void)nssTrustDomain_GetCertsFromCache(td, certList); @@ -2395,7 +2373,6 @@ PK11_TraverseCertsInSlot(PK11SlotInfo *slot, certs = nssPKIObjectCollection_GetCertificates(collection, NULL, 0, NULL); nssPKIObjectCollection_Destroy(collection); - (void)nssToken_Destroy(tok); if (certs) { CERTCertificate *oldie; NSSCertificate **cp; @@ -2435,6 +2412,7 @@ PK11_FindCertFromDERCertItem(PK11SlotInfo *slot, const SECItem *inDerCert, SECStatus rv; CERTCertificate *cert = NULL; + tok = PK11Slot_GetNSSToken(slot); NSSITEM_FROM_SECITEM(&derCert, inDerCert); rv = pk11_AuthenticateUnfriendly(slot, PR_TRUE, wincx); if (rv != SECSuccess) { @@ -2442,14 +2420,8 @@ PK11_FindCertFromDERCertItem(PK11SlotInfo *slot, const SECItem *inDerCert, return NULL; } - tok = PK11Slot_GetNSSToken(slot); - if (!tok) { - PK11_FreeSlot(slot); - return NULL; - } co = nssToken_FindCertificateByEncodedCertificate(tok, NULL, &derCert, nssTokenSearchType_TokenOnly, NULL); - (void)nssToken_Destroy(tok); if (co) { cert = PK11_MakeCertFromHandle(slot, co->handle, NULL); diff --git a/security/nss/lib/pk11wrap/pk11nobj.c b/security/nss/lib/pk11wrap/pk11nobj.c index 725abab174..41d1d96883 100644 --- a/security/nss/lib/pk11wrap/pk11nobj.c +++ b/security/nss/lib/pk11wrap/pk11nobj.c @@ -411,17 +411,12 @@ PK11_FindCrlByName(PK11SlotInfo **slot, CK_OBJECT_HANDLE *crlHandle, nssPKIObjectCollection *collection; nssTokenSearchType tokenOnly = nssTokenSearchType_TokenOnly; NSSToken *token = PK11Slot_GetNSSToken(*slot); - if (!token) { - goto loser; - } collection = nssCRLCollection_Create(td, NULL); if (!collection) { - (void)nssToken_Destroy(token); goto loser; } instances = nssToken_FindCRLsBySubject(token, NULL, &subject, tokenOnly, 0, NULL); - (void)nssToken_Destroy(token); nssPKIObjectCollection_AddInstances(collection, instances, 0); nss_ZFreeIf(instances); crls = nssPKIObjectCollection_GetCRLs(collection, NULL, 0, NULL); @@ -485,21 +480,16 @@ PK11_PutCrl(PK11SlotInfo *slot, SECItem *crl, SECItem *name, char *url, int type) { NSSItem derCRL, derSubject; - NSSToken *token; + NSSToken *token = PK11Slot_GetNSSToken(slot); nssCryptokiObject *object; PRBool isKRL = (type == SEC_CRL_TYPE) ? PR_FALSE : PR_TRUE; CK_OBJECT_HANDLE rvH; NSSITEM_FROM_SECITEM(&derSubject, name); NSSITEM_FROM_SECITEM(&derCRL, crl); - token = PK11Slot_GetNSSToken(slot); - if (!token) { - PORT_SetError(SEC_ERROR_NO_TOKEN); - return CK_INVALID_HANDLE; - } + object = nssToken_ImportCRL(token, NULL, &derSubject, &derCRL, isKRL, url, PR_TRUE); - (void)nssToken_Destroy(token); if (object) { rvH = object->handle; @@ -518,8 +508,8 @@ SECStatus SEC_DeletePermCRL(CERTSignedCrl *crl) { PRStatus status; - nssCryptokiObject *object; NSSToken *token; + nssCryptokiObject *object; PK11SlotInfo *slot = crl->slot; if (slot == NULL) { @@ -528,17 +518,13 @@ SEC_DeletePermCRL(CERTSignedCrl *crl) PORT_SetError(SEC_ERROR_CRL_INVALID); return SECFailure; } - token = PK11Slot_GetNSSToken(slot); - if (!token) { - return SECFailure; - } + object = nss_ZNEW(NULL, nssCryptokiObject); if (!object) { - (void)nssToken_Destroy(token); return SECFailure; } - object->token = token; /* object takes ownership */ + object->token = nssToken_AddRef(token); object->handle = crl->pkcs11ID; object->isTokenObject = PR_TRUE; diff --git a/security/nss/lib/pk11wrap/pk11slot.c b/security/nss/lib/pk11wrap/pk11slot.c index e7ff3dd008..c44ed9b49e 100644 --- a/security/nss/lib/pk11wrap/pk11slot.c +++ b/security/nss/lib/pk11wrap/pk11slot.c @@ -359,24 +359,19 @@ PK11_NewSlotInfo(SECMODModule *mod) PK11SlotInfo *slot; slot = (PK11SlotInfo *)PORT_Alloc(sizeof(PK11SlotInfo)); - if (slot == NULL) { + if (slot == NULL) return slot; + + slot->sessionLock = mod->isThreadSafe ? PZ_NewLock(nssILockSession) : mod->refLock; + if (slot->sessionLock == NULL) { + PORT_Free(slot); + return NULL; } slot->freeListLock = PZ_NewLock(nssILockFreelist); if (slot->freeListLock == NULL) { - PORT_Free(slot); - return NULL; - } - slot->nssTokenLock = PZ_NewLock(nssILockOther); - if (slot->nssTokenLock == NULL) { - PZ_DestroyLock(slot->freeListLock); - PORT_Free(slot); - return NULL; - } - slot->sessionLock = mod->isThreadSafe ? PZ_NewLock(nssILockSession) : mod->refLock; - if (slot->sessionLock == NULL) { - PZ_DestroyLock(slot->nssTokenLock); - PZ_DestroyLock(slot->freeListLock); + if (mod->isThreadSafe) { + PZ_DestroyLock(slot->sessionLock); + } PORT_Free(slot); return NULL; } @@ -464,10 +459,6 @@ PK11_DestroySlot(PK11SlotInfo *slot) PZ_DestroyLock(slot->freeListLock); slot->freeListLock = NULL; } - if (slot->nssTokenLock) { - PZ_DestroyLock(slot->nssTokenLock); - slot->nssTokenLock = NULL; - } /* finally Tell our parent module that we've gone away so it can unload */ if (slot->module) { @@ -1266,7 +1257,6 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts) CK_RV crv; SECStatus rv; PRStatus status; - NSSToken *nssToken; /* set the slot flags to the current token values */ if (!slot->isThreadSafe) @@ -1304,9 +1294,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts) slot->maxPassword = slot->tokenInfo.ulMaxPinLen; PORT_Memcpy(slot->serial, slot->tokenInfo.serialNumber, sizeof(slot->serial)); - nssToken = PK11Slot_GetNSSToken(slot); - nssToken_UpdateName(nssToken); /* null token is OK */ - (void)nssToken_Destroy(nssToken); + nssToken_UpdateName(slot->nssToken); slot->defRWSession = (PRBool)((!slot->readOnly) && (slot->tokenInfo.ulMaxSessionCount == 1)); @@ -1374,9 +1362,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts) PK11_ExitSlotMonitor(slot); } - nssToken = PK11Slot_GetNSSToken(slot); - status = nssToken_Refresh(nssToken); /* null token is OK */ - (void)nssToken_Destroy(nssToken); + status = nssToken_Refresh(slot->nssToken); if (status != PR_SUCCESS) return SECFailure; @@ -1610,11 +1596,8 @@ pk11_IsPresentCertLoad(PK11SlotInfo *slot, PRBool loadCerts) return PR_TRUE; } - NSSToken *nssToken = PK11Slot_GetNSSToken(slot); - if (nssToken) { - PRBool present = nssToken_IsPresent(nssToken); - (void)nssToken_Destroy(nssToken); - return present; + if (slot->nssToken) { + return nssToken_IsPresent(slot->nssToken); } /* removable slots have a flag that says they are present */ @@ -2651,44 +2634,20 @@ PK11_ResetToken(PK11SlotInfo *slot, char *sso_pwd) PORT_SetError(PK11_MapError(crv)); return SECFailure; } - NSSToken *token = PK11Slot_GetNSSToken(slot); - if (token) { - nssTrustDomain_UpdateCachedTokenCerts(token->trustDomain, token); - (void)nssToken_Destroy(token); - } + nssTrustDomain_UpdateCachedTokenCerts(slot->nssToken->trustDomain, + slot->nssToken); return SECSuccess; } - void PK11Slot_SetNSSToken(PK11SlotInfo *sl, NSSToken *nsst) { - NSSToken *old; - if (nsst) { - nsst = nssToken_AddRef(nsst); - } - - PZ_Lock(sl->nssTokenLock); - old = sl->nssToken; sl->nssToken = nsst; - PZ_Unlock(sl->nssTokenLock); - - if (old) { - (void)nssToken_Destroy(old); - } } NSSToken * PK11Slot_GetNSSToken(PK11SlotInfo *sl) { - NSSToken *rv = NULL; - - PZ_Lock(sl->nssTokenLock); - if (sl->nssToken) { - rv = nssToken_AddRef(sl->nssToken); - } - PZ_Unlock(sl->nssTokenLock); - - return rv; + return sl->nssToken; } /* diff --git a/security/nss/lib/pk11wrap/pk11util.c b/security/nss/lib/pk11wrap/pk11util.c index 7356f6b249..906a2f7d55 100644 --- a/security/nss/lib/pk11wrap/pk11util.c +++ b/security/nss/lib/pk11wrap/pk11util.c @@ -13,7 +13,6 @@ #include "pki3hack.h" #include "secerr.h" #include "dev.h" -#include "dev3hack.h" #include "utilpars.h" #include "pkcs11uri.h" @@ -1267,14 +1266,8 @@ SECMOD_WaitForAnyTokenEvent(SECMODModule *mod, unsigned long flags, } /* if we are in the delay period for the "isPresent" call, reset * the delay since we know things have probably changed... */ - if (slot) { - NSSToken *nssToken = PK11Slot_GetNSSToken(slot); - if (nssToken) { - if (nssToken->slot) { - nssSlot_ResetDelay(nssToken->slot); - } - (void)nssToken_Destroy(nssToken); - } + if (slot && slot->nssToken && slot->nssToken->slot) { + nssSlot_ResetDelay(slot->nssToken->slot); } return slot; @@ -1507,12 +1500,8 @@ SECMOD_OpenNewSlot(SECMODModule *mod, const char *moduleSpec) if (slot) { /* if we are in the delay period for the "isPresent" call, reset * the delay since we know things have probably changed... */ - NSSToken *nssToken = PK11Slot_GetNSSToken(slot); - if (nssToken) { - if (nssToken->slot) { - nssSlot_ResetDelay(nssToken->slot); - } - (void)nssToken_Destroy(nssToken); + if (slot->nssToken && slot->nssToken->slot) { + nssSlot_ResetDelay(slot->nssToken->slot); } /* force the slot info structures to properly reset */ (void)PK11_IsPresent(slot); @@ -1642,12 +1631,8 @@ SECMOD_CloseUserDB(PK11SlotInfo *slot) PR_smprintf_free(sendSpec); /* if we are in the delay period for the "isPresent" call, reset * the delay since we know things have probably changed... */ - NSSToken *nssToken = PK11Slot_GetNSSToken(slot); - if (nssToken) { - if (nssToken->slot) { - nssSlot_ResetDelay(nssToken->slot); - } - (void)nssToken_Destroy(nssToken); + if (slot->nssToken && slot->nssToken->slot) { + nssSlot_ResetDelay(slot->nssToken->slot); /* force the slot info structures to properly reset */ (void)PK11_IsPresent(slot); } diff --git a/security/nss/lib/pk11wrap/secmodti.h b/security/nss/lib/pk11wrap/secmodti.h index 004f40f867..4af20100db 100644 --- a/security/nss/lib/pk11wrap/secmodti.h +++ b/security/nss/lib/pk11wrap/secmodti.h @@ -107,7 +107,6 @@ struct PK11SlotInfoStr { unsigned int lastState; /* for Stan */ NSSToken *nssToken; - PZLock *nssTokenLock; /* the tokeninfo struct */ CK_TOKEN_INFO tokenInfo; /* fast mechanism lookup */ diff --git a/security/nss/lib/pki/pki3hack.c b/security/nss/lib/pki/pki3hack.c index 5556cd1769..7fe9113e4a 100644 --- a/security/nss/lib/pki/pki3hack.c +++ b/security/nss/lib/pki/pki3hack.c @@ -72,16 +72,12 @@ STAN_InitTokenForSlotInfo(NSSTrustDomain *td, PK11SlotInfo *slot) } } token = nssToken_CreateFromPK11SlotInfo(td, slot); + PK11Slot_SetNSSToken(slot, token); + /* Don't add nonexistent token to TD's token list */ if (token) { - /* PK11Slot_SetNSSToken increments the refcount on |token| to 2 */ - PK11Slot_SetNSSToken(slot, token); - - /* we give our reference to |td->tokenList| */ NSSRWLock_LockWrite(td->tokensLock); nssList_Add(td->tokenList, token); NSSRWLock_UnlockWrite(td->tokensLock); - } else { - PK11Slot_SetNSSToken(slot, NULL); } return PR_SUCCESS; } @@ -192,8 +188,7 @@ STAN_RemoveModuleFromDefaultTrustDomain( nssList_Remove(td->tokenList, token); NSSRWLock_UnlockWrite(td->tokensLock); PK11Slot_SetNSSToken(module->slots[i], NULL); - (void)nssToken_Destroy(token); /* for the |td->tokenList| reference */ - (void)nssToken_Destroy(token); /* for our PK11Slot_GetNSSToken reference */ + nssToken_Destroy(token); } } NSSRWLock_LockWrite(td->tokensLock); @@ -1081,11 +1076,7 @@ STAN_GetNSSCertificate(CERTCertificate *cc) nssArena_Destroy(arena); return NULL; } - instance->token = PK11Slot_GetNSSToken(cc->slot); - if (!instance->token) { - nssArena_Destroy(arena); - return NULL; - } + instance->token = nssToken_AddRef(PK11Slot_GetNSSToken(cc->slot)); instance->handle = cc->pkcs11ID; instance->isTokenObject = PR_TRUE; if (cc->nickname) { @@ -1278,10 +1269,6 @@ STAN_ChangeCertTrust(CERTCertificate *cc, CERTCertTrust *trust) NSSASCII7 *email = c->email; tok = PK11Slot_GetNSSToken(slot); PK11_FreeSlot(slot); - if (!tok) { - nssrv = PR_FAILURE; - goto done; - } newInstance = nssToken_ImportCertificate(tok, NULL, NSSCertificateType_PKIX, @@ -1296,7 +1283,6 @@ STAN_ChangeCertTrust(CERTCertificate *cc, CERTCertTrust *trust) nss_ZFreeIf(nickname); nickname = NULL; if (!newInstance) { - (void)nssToken_Destroy(tok); nssrv = PR_FAILURE; goto done; } @@ -1308,7 +1294,6 @@ STAN_ChangeCertTrust(CERTCertificate *cc, CERTCertTrust *trust) nssTrust->codeSigning, nssTrust->emailProtection, nssTrust->stepUpApproved, PR_TRUE); - (void)nssToken_Destroy(tok); } if (newInstance) { nssCryptokiObject_Destroy(newInstance); diff --git a/security/nss/lib/pki/trustdomain.c b/security/nss/lib/pki/trustdomain.c index c8f9f1d5b9..151b888750 100644 --- a/security/nss/lib/pki/trustdomain.c +++ b/security/nss/lib/pki/trustdomain.c @@ -11,7 +11,6 @@ #endif /* PKIM_H */ #include "cert.h" -#include "dev3hack.h" #include "pki3hack.h" #include "pk11pub.h" #include "nssrwlk.h" @@ -62,14 +61,11 @@ static void token_destructor(void *t) { NSSToken *tok = (NSSToken *)t; - /* Remove the token list's reference to the token */ - (void)nssToken_Destroy(tok); - - /* Signal that the slot should not give out any more references to the - * token. The token might still have a positive refcount after this call. - * The token has a reference to the slot, so the slot will not be destroyed - * until after the token's refcount drops to 0. */ - PK11Slot_SetNSSToken(tok->pk11slot, NULL); + /* The token holds the first/last reference to the slot. + * When the token is actually destroyed (ref count == 0), + * the slot will also be destroyed. + */ + nssToken_Destroy(tok); } NSS_IMPLEMENT PRStatus @@ -131,6 +127,7 @@ nssTrustDomain_GetActiveSlots( return NULL; } nssList_GetArray(td->tokenList, (void **)tokens, count); + NSSRWLock_UnlockRead(td->tokensLock); count = 0; for (tp = tokens; *tp; tp++) { NSSSlot *slot = nssToken_GetSlot(*tp); @@ -140,7 +137,6 @@ nssTrustDomain_GetActiveSlots( nssSlot_Destroy(slot); } } - NSSRWLock_UnlockRead(td->tokensLock); nss_ZFreeIf(tokens); if (!count) { nss_ZFreeIf(slots); @@ -473,7 +469,7 @@ nssTrustDomain_FindCertificatesByNickname( numRemaining, &status); } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); if (status != PR_SUCCESS) { errors++; continue; @@ -622,7 +618,7 @@ nssTrustDomain_FindCertificatesBySubject( numRemaining, &status); } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); if (status != PR_SUCCESS) { errors++; continue; @@ -783,7 +779,7 @@ nssTrustDomain_FindCertificateByIssuerAndSerialNumber( tokenOnly, &status); } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); if (status != PR_SUCCESS) { continue; } @@ -1026,7 +1022,7 @@ NSSTrustDomain_TraverseCertificates( collector, collection); } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); } } @@ -1080,7 +1076,7 @@ nssTrustDomain_FindTrustForCertificate( nssCryptokiObject_Destroy(to); } } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); } } if (pkio) { @@ -1130,7 +1126,7 @@ nssTrustDomain_FindCRLsBySubject( instances = nssToken_FindCRLsBySubject(token, session, subject, tokenOnly, 0, &status); } - (void)nssToken_Destroy(token); + nssToken_Destroy(token); if (status == PR_SUCCESS) { /* add the found CRL's to the collection */ status = nssPKIObjectCollection_AddInstances(collection, diff --git a/security/nss/lib/softoken/pkcs11.c b/security/nss/lib/softoken/pkcs11.c index 366522d463..5708815ce3 100644 --- a/security/nss/lib/softoken/pkcs11.c +++ b/security/nss/lib/softoken/pkcs11.c @@ -1607,7 +1607,7 @@ sftk_handleObject(SFTKObject *object, SFTKSession *session) PZ_Unlock(slot->slotLock); /* don't create a private object if we aren't logged in */ - if (!isLoggedIn && needLogin && (sftk_isTrue(object, CKA_PRIVATE))) { + if (!isLoggedIn && needLogin && sftk_isTrue(object, CKA_PRIVATE)) { return CKR_USER_NOT_LOGGED_IN; } @@ -4674,7 +4674,7 @@ NSC_GetAttributeValue(CK_SESSION_HANDLE hSession, PZ_Unlock(slot->slotLock); /* don't read a private object if we aren't logged in */ - if (!isLoggedIn && needLogin && (sftk_isTrue(object, CKA_PRIVATE))) { + if (!isLoggedIn && needLogin && sftk_isTrue(object, CKA_PRIVATE)) { sftk_FreeObject(object); return CKR_USER_NOT_LOGGED_IN; } @@ -4745,7 +4745,7 @@ NSC_SetAttributeValue(CK_SESSION_HANDLE hSession, PZ_Unlock(slot->slotLock); /* don't modify a private object if we aren't logged in */ - if (!isLoggedIn && needLogin && (sftk_isTrue(object, CKA_PRIVATE))) { + if (!isLoggedIn && needLogin && sftk_isTrue(object, CKA_PRIVATE)) { sftk_FreeSession(session); sftk_FreeObject(object); return CKR_USER_NOT_LOGGED_IN; diff --git a/security/nss/lib/ssl/ssl3ext.c b/security/nss/lib/ssl/ssl3ext.c index 87bdd87f4d..7e674f0e03 100644 --- a/security/nss/lib/ssl/ssl3ext.c +++ b/security/nss/lib/ssl/ssl3ext.c @@ -528,23 +528,12 @@ ssl3_HandleParsedExtensions(sslSocket *ss, SSLHandshakeType message) if (allowNotOffered) { continue; /* Skip over unknown extensions. */ } - /* RFC8446 Section 4.2 - Implementations MUST NOT send extension responses if - * the remote endpoint did not send the corresponding extension request ... - * Upon receiving such an extension, an endpoint MUST abort the handshake with - * an "unsupported_extension" alert. */ - SSL_TRC(3, ("%d: TLS13: unknown extension %d in message %d", - SSL_GETPID(), extension, message)); - tls13_FatalError(ss, SSL_ERROR_RX_UNEXPECTED_EXTENSION, - unsupported_extension); - return SECFailure; + /* Fall through. */ case tls13_extension_disallowed: - /* RFC8446 Section 4.2 - If an implementation receives an extension which it - * recognizes and which is not specified for the message in which it appears, - * it MUST abort the handshake with an "illegal_parameter" alert. */ - SSL_TRC(3, ("%d: TLS13: disallowed extension %d in message %d", + SSL_TRC(3, ("%d: TLS13: unexpected extension %d in message %d", SSL_GETPID(), extension, message)); tls13_FatalError(ss, SSL_ERROR_EXTENSION_DISALLOWED_FOR_VERSION, - illegal_parameter); + unsupported_extension); return SECFailure; } }