From 33dcc1b4e8aa91a016e5b5375874ef93a6dc46f7 Mon Sep 17 00:00:00 2001 From: "rrelyea%redhat.com" Date: Tue, 21 Mar 2006 02:28:48 +0000 Subject: [PATCH] Bug 273637 3 locks in softoken have unsafe initialization r=alexi r=julien git-svn-id: svn://10.0.0.236/trunk@192673 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/security/nss/lib/softoken/keydb.c | 11 ++--- mozilla/security/nss/lib/softoken/pcertdb.c | 54 ++++++++++++--------- mozilla/security/nss/lib/softoken/pkcs11.c | 7 ++- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/mozilla/security/nss/lib/softoken/keydb.c b/mozilla/security/nss/lib/softoken/keydb.c index 24a5e10cd90..0b8fa4e6a16 100644 --- a/mozilla/security/nss/lib/softoken/keydb.c +++ b/mozilla/security/nss/lib/softoken/keydb.c @@ -34,7 +34,7 @@ * the terms of any one of the MPL, the GPL or the LGPL. * * ***** END LICENSE BLOCK ***** */ -/* $Id: keydb.c,v 1.40 2005-09-28 17:12:17 relyea%netscape.com Exp $ */ +/* $Id: keydb.c,v 1.41 2006-03-21 02:28:48 rrelyea%redhat.com Exp $ */ #include "lowkeyi.h" #include "seccomon.h" @@ -52,12 +52,8 @@ #include "nsslocks.h" #include "keydbi.h" +#include "softoken.h" -#ifdef NSS_ENABLE_ECC -extern SECStatus EC_FillParams(PRArenaPool *arena, - const SECItem *encodedParams, - ECParams *params); -#endif /* * Record keys for keydb @@ -2046,6 +2042,9 @@ seckey_decrypt_private_key(NSSLOWKEYEncryptedPrivateKeyInfo *epki, rv = EC_FillParams(permarena, &pk->u.ec.ecParams.DEREncoding, &pk->u.ec.ecParams); + if (rv != SECSuccess) + goto loser; + /* * NOTE: Encoding of the publicValue is optional * so we need to be able to regenerate the publicValue diff --git a/mozilla/security/nss/lib/softoken/pcertdb.c b/mozilla/security/nss/lib/softoken/pcertdb.c index 7fb8d6514b0..b54d024a83f 100644 --- a/mozilla/security/nss/lib/softoken/pcertdb.c +++ b/mozilla/security/nss/lib/softoken/pcertdb.c @@ -37,7 +37,7 @@ /* * Permanent Certificate database handling code * - * $Id: pcertdb.c,v 1.57 2006-03-09 23:38:57 nelson%bolyard.com Exp $ + * $Id: pcertdb.c,v 1.58 2006-03-21 02:28:48 rrelyea%redhat.com Exp $ */ #include "prtime.h" @@ -91,6 +91,9 @@ static int entryListCount = 0; * a global lock to make the database thread safe. */ static PZLock *dbLock = NULL; +static PZLock *certRefCountLock = NULL; +static PZLock *certTrustLock = NULL; +static PZLock *freeListLock = NULL; void certdb_InitDBLock(NSSLOWCERTCertDBHandle *handle) @@ -99,8 +102,31 @@ certdb_InitDBLock(NSSLOWCERTCertDBHandle *handle) nss_InitLock(&dbLock, nssILockCertDB); PORT_Assert(dbLock != NULL); } +} - return; +SECStatus +nsslowcert_InitLocks(void) +{ + if (freeListLock == NULL) { + nss_InitLock(&freeListLock, nssILockRefLock); + if (freeListLock == NULL) { + return SECFailure; + } + } + if (certRefCountLock == NULL) { + nss_InitLock(&certRefCountLock, nssILockRefLock); + if (certRefCountLock == NULL) { + return SECFailure; + } + } + if (certTrustLock == NULL ) { + nss_InitLock(&certTrustLock, nssILockCertDB); + if (certTrustLock == NULL) { + return SECFailure; + } + } + + return SECSuccess; } /* @@ -133,7 +159,6 @@ nsslowcert_UnlockDB(NSSLOWCERTCertDBHandle *handle) return; } -static PZLock *certRefCountLock = NULL; /* * Acquire the cert reference count lock @@ -144,10 +169,7 @@ static PZLock *certRefCountLock = NULL; static void nsslowcert_LockCertRefCount(NSSLOWCERTCertificate *cert) { - if ( certRefCountLock == NULL ) { - nss_InitLock(&certRefCountLock, nssILockRefLock); - PORT_Assert(certRefCountLock != NULL); - } + PORT_Assert(certRefCountLock != NULL); PZ_Lock(certRefCountLock); return; @@ -170,8 +192,6 @@ nsslowcert_UnlockCertRefCount(NSSLOWCERTCertificate *cert) return; } -static PZLock *certTrustLock = NULL; - /* * Acquire the cert trust lock * There is currently one global lock for all certs, but I'm putting a cert @@ -181,11 +201,8 @@ static PZLock *certTrustLock = NULL; void nsslowcert_LockCertTrust(NSSLOWCERTCertificate *cert) { - if ( certTrustLock == NULL ) { - nss_InitLock(&certTrustLock, nssILockCertDB); - PORT_Assert(certTrustLock != NULL); - } - + PORT_Assert(certTrustLock != NULL); + PZ_Lock(certTrustLock); return; } @@ -207,7 +224,6 @@ nsslowcert_UnlockCertTrust(NSSLOWCERTCertificate *cert) return; } -static PZLock *freeListLock = NULL; /* * Acquire the cert reference count lock @@ -218,10 +234,7 @@ static PZLock *freeListLock = NULL; static void nsslowcert_LockFreeList(void) { - if ( freeListLock == NULL ) { - nss_InitLock(&freeListLock, nssILockRefLock); - PORT_Assert(freeListLock != NULL); - } + PORT_Assert(freeListLock != NULL); PZ_Lock(freeListLock); return; @@ -5316,9 +5329,6 @@ nsslowcert_SaveSMimeProfile(NSSLOWCERTCertDBHandle *dbhandle, char *emailAddr, return(rv); } -/* If the freeListLock doesn't exist when this function is called, -** this function will create it, use it 3 times, and delete it. -*/ void nsslowcert_DestroyFreeLists(void) { diff --git a/mozilla/security/nss/lib/softoken/pkcs11.c b/mozilla/security/nss/lib/softoken/pkcs11.c index 82e580ca349..466b777d4f6 100644 --- a/mozilla/security/nss/lib/softoken/pkcs11.c +++ b/mozilla/security/nss/lib/softoken/pkcs11.c @@ -2974,7 +2974,6 @@ CK_RV nsc_CommonInitialize(CK_VOID_PTR pReserved, PRBool isFIPS) loginWaitTime = PR_SecondsToInterval(1); } - rv = secoid_Init(); if (rv != SECSuccess) { crv = CKR_DEVICE_ERROR; @@ -2988,6 +2987,12 @@ CK_RV nsc_CommonInitialize(CK_VOID_PTR pReserved, PRBool isFIPS) } RNG_SystemInfoForRNG(); + rv = nsslowcert_InitLocks(); + if (rv != SECSuccess) { + crv = CKR_DEVICE_ERROR; + return crv; + } + /* NOTE: * we should be getting out mutexes from this list, not statically binding