diff --git a/security/nss/lib/pk11wrap/pk11akey.c b/security/nss/lib/pk11wrap/pk11akey.c index 2400beebfe6..e937140cbad 100644 --- a/security/nss/lib/pk11wrap/pk11akey.c +++ b/security/nss/lib/pk11wrap/pk11akey.c @@ -959,12 +959,17 @@ PK11_GenerateKeyPair(PK11SlotInfo *slot,CK_MECHANISM_TYPE type, haslock = PK11_RWSessionHasLock(slot,session_handle); restore = PR_TRUE; } else { - PK11_EnterSlotMonitor(slot); /* gross!! */ session_handle = slot->session; + if (session_handle != CK_INVALID_SESSION) + PK11_EnterSlotMonitor(slot); restore = PR_FALSE; haslock = PR_TRUE; } + if (session_handle == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return NULL; + } crv = PK11_GETTAB(slot)->C_GenerateKeyPair(session_handle, &mechanism, pubTemplate,pubCount,privTemplate,privCount,&pubID,&privID); @@ -1640,6 +1645,10 @@ PK11_ConvertSessionPrivKeyToTokenPrivKey(SECKEYPrivateKey *privk, void* wincx) PK11_Authenticate(slot, PR_TRUE, wincx); rwsession = PK11_GetRWSession(slot); + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return NULL; + } crv = PK11_GETTAB(slot)->C_CopyObject(rwsession, privk->pkcs11ID, template, 1, &newKeyID); PK11_RestoreROSession(slot, rwsession); @@ -1652,6 +1661,7 @@ PK11_ConvertSessionPrivKeyToTokenPrivKey(SECKEYPrivateKey *privk, void* wincx) return PK11_MakePrivKey(slot, nullKey /*KeyType*/, PR_FALSE /*isTemp*/, newKeyID, NULL /*wincx*/); } + /* * destroy a private key if there are no matching certs. * this function also frees the privKey structure. diff --git a/security/nss/lib/pk11wrap/pk11auth.c b/security/nss/lib/pk11wrap/pk11auth.c index 18dc088060a..3d52ad0b9e9 100644 --- a/security/nss/lib/pk11wrap/pk11auth.c +++ b/security/nss/lib/pk11wrap/pk11auth.c @@ -316,7 +316,10 @@ PK11_CheckSSOPassword(PK11SlotInfo *slot, char *ssopw) /* get a rwsession */ rwsession = PK11_GetRWSession(slot); - if (rwsession == CK_INVALID_SESSION) return rv; + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return rv; + } if (slot->protectedAuthPath) { len = 0; @@ -383,7 +386,11 @@ PK11_InitPin(PK11SlotInfo *slot,char *ssopw, char *userpw) /* get a rwsession */ rwsession = PK11_GetRWSession(slot); - if (rwsession == CK_INVALID_SESSION) goto done; + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + slot->lastLoginCheck = 0; + return rv; + } if (slot->protectedAuthPath) { len = 0; @@ -443,6 +450,10 @@ PK11_ChangePW(PK11SlotInfo *slot,char *oldpw, char *newpw) /* get a rwsession */ rwsession = PK11_GetRWSession(slot); + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return rv; + } crv = PK11_GETTAB(slot)->C_SetPIN(rwsession, (unsigned char *)oldpw,oldLen,(unsigned char *)newpw,newLen); diff --git a/security/nss/lib/pk11wrap/pk11obj.c b/security/nss/lib/pk11wrap/pk11obj.c index eba924bc88e..e0a193ee384 100644 --- a/security/nss/lib/pk11wrap/pk11obj.c +++ b/security/nss/lib/pk11wrap/pk11obj.c @@ -97,6 +97,10 @@ PK11_DestroyTokenObject(PK11SlotInfo *slot,CK_OBJECT_HANDLE object) { rwsession = PK11_GetRWSession(slot); + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return SECFailure; + } crv = PK11_GETTAB(slot)->C_DestroyObject(rwsession,object); if (crv != CKR_OK) { @@ -210,6 +214,9 @@ PK11_GetAttributes(PRArenaPool *arena,PK11SlotInfo *slot, /* make pedantic happy... note that it's only used arena != NULL */ void *mark = NULL; CK_RV crv; + PORT_Assert(slot->session != CK_INVALID_SESSION); + if (slot->session == CK_INVALID_SESSION) + return CKR_SESSION_HANDLE_INVALID; /* * first get all the lengths of the parameters. @@ -319,6 +326,10 @@ PK11_SetObjectNickname(PK11SlotInfo *slot, CK_OBJECT_HANDLE id, PK11_SETATTRS(&setTemplate, CKA_LABEL, (CK_CHAR *) nickname, len); rwsession = PK11_GetRWSession(slot); + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return SECFailure; + } crv = PK11_GETTAB(slot)->C_SetAttributeValue(rwsession, id, &setTemplate, 1); PK11_RestoreROSession(slot, rwsession); @@ -389,7 +400,12 @@ PK11_CreateNewObject(PK11SlotInfo *slot, CK_SESSION_HANDLE session, rwsession = PK11_GetRWSession(slot); } else if (rwsession == CK_INVALID_SESSION) { rwsession = slot->session; - PK11_EnterSlotMonitor(slot); + if (rwsession != CK_INVALID_SESSION) + PK11_EnterSlotMonitor(slot); + } + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return SECFailure; } crv = PK11_GETTAB(slot)->C_CreateObject(rwsession, theTemplate, count,objectID); @@ -924,11 +940,17 @@ PK11_UnwrapPrivKey(PK11SlotInfo *slot, PK11SymKey *wrappingKey, if (newKey) { if (perm) { /* Get RW Session will either lock the monitor if necessary, - * or return a thread safe session handle. */ + * or return a thread safe session handle, or fail. */ rwsession = PK11_GetRWSession(slot); } else { rwsession = slot->session; - PK11_EnterSlotMonitor(slot); + if (rwsession != CK_INVALID_SESSION) + PK11_EnterSlotMonitor(slot); + } + if (rwsession == CK_INVALID_SESSION) { + PK11_FreeSymKey(newKey); + PORT_SetError(SEC_ERROR_BAD_DATA); + return NULL; } crv = PK11_GETTAB(slot)->C_UnwrapKey(rwsession, &mechanism, newKey->objectID, @@ -1105,7 +1127,7 @@ PK11_FindGenericObjects(PK11SlotInfo *slot, CK_OBJECT_CLASS objClass) CK_ATTRIBUTE template[1]; CK_ATTRIBUTE *attrs = template; CK_OBJECT_HANDLE *objectIDs = NULL; - PK11GenericObject *lastObj, *obj; + PK11GenericObject *lastObj = NULL, *obj; PK11GenericObject *firstObj = NULL; int i, count = 0; diff --git a/security/nss/lib/pk11wrap/pk11skey.c b/security/nss/lib/pk11wrap/pk11skey.c index 696382354b0..537c845edc3 100644 --- a/security/nss/lib/pk11wrap/pk11skey.c +++ b/security/nss/lib/pk11wrap/pk11skey.c @@ -85,18 +85,26 @@ pk11_getKeyFromList(PK11SlotInfo *slot) { symKey->next = NULL; if ((symKey->series != slot->series) || (!symKey->sessionOwner)) symKey->session = pk11_GetNewSession(slot,&symKey->sessionOwner); - return symKey; + PORT_Assert(symKey->session != CK_INVALID_SESSION); + if (symKey->session != CK_INVALID_SESSION) + return symKey; + PK11_FreeSymKey(symKey); } symKey = PORT_New(PK11SymKey); if (symKey == NULL) { return NULL; } - symKey->session = pk11_GetNewSession(slot,&symKey->sessionOwner); symKey->next = NULL; - return symKey; + symKey->session = pk11_GetNewSession(slot,&symKey->sessionOwner); + PORT_Assert(symKey->session != CK_INVALID_SESSION); + if (symKey->session != CK_INVALID_SESSION) + return symKey; + PK11_FreeSymKey(symKey); + return NULL; } +/* Caller MUST hold slot->freeListLock (or ref count == 0?) !! */ void PK11_CleanKeyList(PK11SlotInfo *slot) { @@ -105,6 +113,8 @@ PK11_CleanKeyList(PK11SlotInfo *slot) while (slot->freeSymKeysHead) { symKey = slot->freeSymKeysHead; slot->freeSymKeysHead = symKey->next; +/* XXX Perhaps this should be: +** if (symKey->sessionOwner) */ pk11_CloseSession(slot, symKey->session,symKey->sessionOwner); PORT_Free(symKey); }; @@ -127,6 +137,11 @@ pk11_CreateSymKey(PK11SlotInfo *slot, CK_MECHANISM_TYPE type, PRBool owner, if (symKey == NULL) { return NULL; } + if (symKey->session == CK_INVALID_SESSION) { + PK11_FreeSymKey(symKey); + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + return NULL; + } symKey->type = type; symKey->data.type = siBuffer; @@ -851,11 +866,18 @@ PK11_TokenKeyGenWithFlags(PK11SlotInfo *slot, CK_MECHANISM_TYPE type, /* Get session and perform locking */ if (isToken) { PK11_Authenticate(symKey->slot,PR_TRUE,wincx); - session = PK11_GetRWSession(symKey->slot); /* Should always be original slot */ + /* Should always be original slot */ + session = PK11_GetRWSession(symKey->slot); symKey->owner = PR_FALSE; } else { session = symKey->session; - pk11_EnterKeyMonitor(symKey); + if (session != CK_INVALID_SESSION) + pk11_EnterKeyMonitor(symKey); + } + if (session == CK_INVALID_SESSION) { + PK11_FreeSymKey(symKey); + PORT_SetError(SEC_ERROR_BAD_DATA); + return NULL; } crv = PK11_GETTAB(symKey->slot)->C_GenerateKey(session, @@ -931,6 +953,10 @@ PK11_ConvertSessionSymKeyToTokenSymKey(PK11SymKey *symk, void *wincx) PK11_Authenticate(slot, PR_TRUE, wincx); rwsession = PK11_GetRWSession(slot); + if (rwsession == CK_INVALID_SESSION) { + PORT_SetError(SEC_ERROR_BAD_DATA); + return NULL; + } crv = PK11_GETTAB(slot)->C_CopyObject(rwsession, symk->objectID, template, 1, &newKeyID); PK11_RestoreROSession(slot, rwsession); @@ -1274,7 +1300,8 @@ pk11_DeriveWithTemplate( PK11SymKey *baseKey, CK_MECHANISM_TYPE derive, newBaseKey = pk11_CopyToSlot (newSlot, derive, CKA_DERIVE, baseKey); PK11_FreeSlot(newSlot); - if (newBaseKey == NULL) return NULL; + if (newBaseKey == NULL) + return NULL; baseKey = newBaseKey; slot = baseKey->slot; } @@ -1304,15 +1331,21 @@ pk11_DeriveWithTemplate( PK11SymKey *baseKey, CK_MECHANISM_TYPE derive, pk11_EnterKeyMonitor(symKey); session = symKey->session; } - crv = PK11_GETTAB(slot)->C_DeriveKey(session, &mechanism, - baseKey->objectID, keyTemplate, templateCount, &symKey->objectID); - if (isPerm) { - PK11_RestoreROSession(slot, session); + if (session == CK_INVALID_SESSION) { + if (!isPerm) + pk11_ExitKeyMonitor(symKey); + crv = CKR_SESSION_HANDLE_INVALID; } else { - pk11_ExitKeyMonitor(symKey); + crv = PK11_GETTAB(slot)->C_DeriveKey(session, &mechanism, + baseKey->objectID, keyTemplate, templateCount, &symKey->objectID); + if (isPerm) { + PK11_RestoreROSession(slot, session); + } else { + pk11_ExitKeyMonitor(symKey); + } } - - if (newBaseKey) PK11_FreeSymKey(newBaseKey); + if (newBaseKey) + PK11_FreeSymKey(newBaseKey); if (crv != CKR_OK) { PK11_FreeSymKey(symKey); return NULL; @@ -1821,11 +1854,16 @@ pk11_AnyUnwrapKey(PK11SlotInfo *slot, CK_OBJECT_HANDLE wrappingKey, pk11_EnterKeyMonitor(symKey); rwsession = symKey->session; } - crv = PK11_GETTAB(slot)->C_UnwrapKey(rwsession,&mechanism,wrappingKey, + PORT_Assert(rwsession != CK_INVALID_SESSION); + if (rwsession == CK_INVALID_SESSION) + crv = CKR_SESSION_HANDLE_INVALID; + else + crv = PK11_GETTAB(slot)->C_UnwrapKey(rwsession,&mechanism,wrappingKey, wrappedKey->data, wrappedKey->len, keyTemplate, templateCount, &symKey->objectID); if (isPerm) { - PK11_RestoreROSession(slot, rwsession); + if (rwsession != CK_INVALID_SESSION) + PK11_RestoreROSession(slot, rwsession); } else { pk11_ExitKeyMonitor(symKey); } diff --git a/security/nss/lib/pk11wrap/pk11slot.c b/security/nss/lib/pk11wrap/pk11slot.c index 11c73d9f8c9..975d56e78cb 100644 --- a/security/nss/lib/pk11wrap/pk11slot.c +++ b/security/nss/lib/pk11wrap/pk11slot.c @@ -666,36 +666,53 @@ CK_SESSION_HANDLE PK11_GetRWSession(PK11SlotInfo *slot) { CK_SESSION_HANDLE rwsession; CK_RV crv; + PRBool haveMonitor = PR_FALSE; - if (!slot->isThreadSafe || slot->defRWSession) PK11_EnterSlotMonitor(slot); - if (slot->defRWSession) return slot->session; + if (!slot->isThreadSafe || slot->defRWSession) { + PK11_EnterSlotMonitor(slot); + haveMonitor = PR_TRUE; + } + if (slot->defRWSession) { + PORT_Assert(slot->session != CK_INVALID_SESSION); + if (slot->session != CK_INVALID_SESSION) + return slot->session; + } crv = PK11_GETTAB(slot)->C_OpenSession(slot->slotID, CKF_RW_SESSION|CKF_SERIAL_SESSION, slot, pk11_notify,&rwsession); - if (crv == CKR_SESSION_COUNT) { - PK11_GETTAB(slot)->C_CloseSession(slot->session); - slot->session = CK_INVALID_SESSION; - crv = PK11_GETTAB(slot)->C_OpenSession(slot->slotID, - CKF_RW_SESSION|CKF_SERIAL_SESSION, - slot,pk11_notify,&rwsession); - } - if (crv != CKR_OK) { + PORT_Assert(rwsession != CK_INVALID_SESSION || crv != CKR_OK); + if (crv != CKR_OK || rwsession == CK_INVALID_SESSION) { + if (crv == CKR_OK) + crv = CKR_DEVICE_ERROR; + if (haveMonitor) + PK11_ExitSlotMonitor(slot); PORT_SetError(PK11_MapError(crv)); - if (slot->session == CK_INVALID_SESSION) { - PK11_GETTAB(slot)->C_OpenSession(slot->slotID,CKF_SERIAL_SESSION, - slot,pk11_notify,&slot->session); - } - if (!slot->isThreadSafe) PK11_ExitSlotMonitor(slot); return CK_INVALID_SESSION; } - + if (slot->defRWSession) { /* we have the monitor */ + slot->session = rwsession; + } return rwsession; } PRBool -PK11_RWSessionHasLock(PK11SlotInfo *slot,CK_SESSION_HANDLE session_handle) { - return (PRBool)(!slot->isThreadSafe || slot->defRWSession); +PK11_RWSessionHasLock(PK11SlotInfo *slot,CK_SESSION_HANDLE session_handle) +{ + PRBool hasLock; + hasLock = (PRBool)(!slot->isThreadSafe || + (slot->defRWSession && slot->session != CK_INVALID_SESSION)); + return hasLock; +} + +static PRBool +pk11_RWSessionIsDefault(PK11SlotInfo *slot,CK_SESSION_HANDLE rwsession) +{ + PRBool isDefault; + isDefault = (PRBool)(slot->session == rwsession && + slot->defRWSession && + slot->session != CK_INVALID_SESSION); + return isDefault; } /* @@ -706,16 +723,14 @@ PK11_RWSessionHasLock(PK11SlotInfo *slot,CK_SESSION_HANDLE session_handle) { void PK11_RestoreROSession(PK11SlotInfo *slot,CK_SESSION_HANDLE rwsession) { - if (slot->defRWSession) { - PK11_ExitSlotMonitor(slot); - return; + PORT_Assert(rwsession != CK_INVALID_SESSION); + if (rwsession != CK_INVALID_SESSION) { + PRBool doExit = PK11_RWSessionHasLock(slot, rwsession); + if (!pk11_RWSessionIsDefault(slot, rwsession)) + PK11_GETTAB(slot)->C_CloseSession(rwsession); + if (doExit) + PK11_ExitSlotMonitor(slot); } - PK11_GETTAB(slot)->C_CloseSession(rwsession); - if (slot->session == CK_INVALID_SESSION) { - PK11_GETTAB(slot)->C_OpenSession(slot->slotID,CKF_SERIAL_SESSION, - slot,pk11_notify,&slot->session); - } - if (!slot->isThreadSafe) PK11_ExitSlotMonitor(slot); } /************************************************************ diff --git a/security/nss/lib/pk11wrap/pk11util.c b/security/nss/lib/pk11wrap/pk11util.c index 87d5e26183e..8c017ac86d3 100644 --- a/security/nss/lib/pk11wrap/pk11util.c +++ b/security/nss/lib/pk11wrap/pk11util.c @@ -861,7 +861,7 @@ SECMOD_UpdateSlotList(SECMODModule *mod) CK_ULONG count; int i, oldCount; PRBool freeRef = PR_FALSE; - void *mark; + void *mark = NULL; CK_ULONG *slotIDs = NULL; PK11SlotInfo **newSlots = NULL; PK11SlotInfo **oldSlots = NULL;