Bug 364684. Don't let the counter whose values are used for session object

handles overflow the sign bit.  Rename several softoken variables to
correctly identify their meaning.  r=rrelyea,wtchang
  tokenIDCount    -> sessionObjectHandleCount
  tokenHashTable  -> tokObjHashTable
  tokObjects      -> sessObjHashTable
  tokObjHashSize  -> sessObjHashSize

3 files changed, 87 insertions, 51 deletions

diff --git a/security/nss/lib/softoken/pkcs11.c b/security/nss/lib/softoken/pkcs11.c
index f913735a4..d33c9aa71 100644
--- a/security/nss/lib/softoken/pkcs11.c
+++ b/security/nss/lib/softoken/pkcs11.c
@@ -86,6 +86,7 @@ static char libraryDescription_space[33];
  * failure so that there are at most 60 login attempts per minute.
  */
 static PRIntervalTime loginWaitTime;
+static PRUint32       minSessionObjectHandle = 1U;
 
 #define __PASTE(x,y)    x##y
 
@@ -1402,7 +1403,7 @@ fail:
     return CKR_OK;
 }
 
-/* forward delcare the DES formating function for handleSecretKey */
+/* forward declare the DES formating function for handleSecretKey */
 void sftk_FormatDESKey(unsigned char *key, int length);
 static NSSLOWKEYPrivateKey *sftk_mkSecretKeyRep(SFTKObject *object);
 
@@ -1787,9 +1788,11 @@ CK_RV
 sftk_handleObject(SFTKObject *object, SFTKSession *session)
 {
     SFTKSlot *slot = session->slot;
+    SFTKAttribute *attribute;
+    SFTKObject *duplicateObject = NULL;
+    CK_OBJECT_HANDLE handle;
     CK_BBOOL ckfalse = CK_FALSE;
     CK_BBOOL cktrue = CK_TRUE;
-    SFTKAttribute *attribute;
     CK_RV crv;
 
     /* make sure all the base object types are defined. If not set the
@@ -1815,11 +1818,37 @@ sftk_handleObject(SFTKObject *object, SFTKSession *session)
 	return CKR_SESSION_READ_ONLY;
     }
 	
-    /* PKCS #11 object ID's are unique for all objects on a
-     * token */
-    PZ_Lock(slot->objectLock);
-    object->handle = slot->tokenIDCount++;
-    PZ_Unlock(slot->objectLock);
+    /* Assign a unique SESSION object handle to every new object,
+     * whether it is a session object or a token object.  
+     * At this point, all new objects are structured as session objects.
+     * Objects with the CKA_TOKEN attribute true will be turned into 
+     * token objects and will have a token object handle assigned to 
+     * them by a call to sftk_mkHandle in the handler for each object 
+     * class, invoked below.
+     *
+     * It may be helpful to note/remember that 
+     * sftk_narrowToXxxObject uses sftk_isToken,
+     * sftk_isToken examines the sign bit of the object's handle, but
+     * sftk_isTrue(...,CKA_TOKEN) examines the CKA_TOKEN attribute.
+     */
+    do {
+	PRUint32 wrappedAround;
+
+	duplicateObject = NULL;
+	PZ_Lock(slot->objectLock);
+	wrappedAround = slot->sessionObjectHandleCount &  SFTK_TOKEN_MASK;
+	handle        = slot->sessionObjectHandleCount & ~SFTK_TOKEN_MASK;
+	if (!handle) /* don't allow zero handle */
+	    handle = minSessionObjectHandle;  
+	slot->sessionObjectHandleCount = (handle + 1U) | wrappedAround;
+	/* Is there already a session object with this handle? */
+	if (wrappedAround) {
+	    sftkqueue_find(duplicateObject, handle, slot->sessObjHashTable, \
+	                   slot->sessObjHashSize);
+	}
+	PZ_Unlock(slot->objectLock);
+    } while (duplicateObject != NULL);
+    object->handle = handle;
 
     /* get the object class */
     attribute = sftk_FindAttribute(object,CKA_CLASS);
@@ -1829,8 +1858,10 @@ sftk_handleObject(SFTKObject *object, SFTKSession *session)
     object->objclass = *(CK_OBJECT_CLASS *)attribute->attrib.pValue;
     sftk_FreeAttribute(attribute);
 
-    /* now handle the specific. Get a session handle for these functions
-     * to use */
+    /* Now handle the specific object class. 
+     * At this point, all objects are session objects, and the session
+     * number must be passed to the object class handlers.
+     */
     switch (object->objclass) {
     case CKO_DATA:
 	crv = sftk_handleDataObject(session,object);
@@ -1866,7 +1897,11 @@ sftk_handleObject(SFTKObject *object, SFTKSession *session)
 	return crv;
     }
 
-    /* now link the object into the slot and session structures */
+    /* Now link the object into the slot and session structures.
+     * If the object has a true CKA_TOKEN attribute, the above object
+     * class handlers will have set the sign bit in the object handle,
+     * causing the following test to be true.
+     */
     if (sftk_isToken(object->handle)) {
 	sftk_convertSessionToToken(object);
     } else {
@@ -2655,11 +2690,11 @@ SFTK_SlotInit(char *configdir,sftk_token_parameters *params, int moduleIndex)
 
     slot->optimizeSpace = params->optimizeSpace;
     if (slot->optimizeSpace) {
-	slot->tokObjHashSize = SPACE_TOKEN_OBJECT_HASH_SIZE;
+	slot->sessObjHashSize = SPACE_SESSION_OBJECT_HASH_SIZE;
 	slot->sessHashSize = SPACE_SESSION_HASH_SIZE;
 	slot->numSessionLocks = 1;
     } else {
-	slot->tokObjHashSize = TIME_TOKEN_OBJECT_HASH_SIZE;
+	slot->sessObjHashSize = TIME_SESSION_OBJECT_HASH_SIZE;
 	slot->sessHashSize = TIME_SESSION_HASH_SIZE;
 	slot->numSessionLocks = slot->sessHashSize/BUCKETS_PER_SESSION_LOCK;
     }
@@ -2685,16 +2720,16 @@ SFTK_SlotInit(char *configdir,sftk_token_parameters *params, int moduleIndex)
     slot->head = PORT_ZNewArray(SFTKSession *, slot->sessHashSize);
     if (slot->head == NULL) 
 	goto mem_loser;
-    slot->tokObjects = PORT_ZNewArray(SFTKObject *, slot->tokObjHashSize);
-    if (slot->tokObjects == NULL) 
+    slot->sessObjHashTable = PORT_ZNewArray(SFTKObject *, slot->sessObjHashSize);
+    if (slot->sessObjHashTable == NULL) 
 	goto mem_loser;
-    slot->tokenHashTable = PL_NewHashTable(64,sftk_HashNumber,PL_CompareValues,
+    slot->tokObjHashTable = PL_NewHashTable(64,sftk_HashNumber,PL_CompareValues,
 					SECITEM_HashCompare, NULL, 0);
-    if (slot->tokenHashTable == NULL) 
+    if (slot->tokObjHashTable == NULL) 
 	goto mem_loser;
 
     slot->sessionIDCount = 0;
-    slot->tokenIDCount = 1;
+    slot->sessionObjectHandleCount = minSessionObjectHandle;
     slot->slotID = slotID;
     sftk_setStringName(params->slotdes ? params->slotdes : 
 	      sftk_getDefSlotName(slotID), slot->slotDescription, 
@@ -2813,9 +2848,9 @@ SFTK_ShutdownSlot(SFTKSlot *slot)
 
     /* clear all objects.. session objects are cleared as a result of
      * closing all the sessions. We just need to clear the token object
-     * cache. slot->tokenHashTable guarentees we have the token 
+     * cache. slot->tokObjHashTable guarentees we have the token 
      * infrastructure set up. */
-    if (slot->tokenHashTable) {
+    if (slot->tokObjHashTable) {
 	SFTK_ClearTokenKeyHashTable(slot);
     }
 
@@ -2837,16 +2872,16 @@ SFTK_DestroySlotData(SFTKSlot *slot)
 
     SFTK_ShutdownSlot(slot);
 
-    if (slot->tokenHashTable) {
-	PL_HashTableDestroy(slot->tokenHashTable);
-	slot->tokenHashTable = NULL;
+    if (slot->tokObjHashTable) {
+	PL_HashTableDestroy(slot->tokObjHashTable);
+	slot->tokObjHashTable = NULL;
     }
 
-    if (slot->tokObjects) {
-	PORT_Free(slot->tokObjects);
-	slot->tokObjects = NULL;
+    if (slot->sessObjHashTable) {
+	PORT_Free(slot->sessObjHashTable);
+	slot->sessObjHashTable = NULL;
     }
-    slot->tokObjHashSize = 0;
+    slot->sessObjHashSize = 0;
 
     if (slot->head) {
 	PORT_Free(slot->head);
@@ -3439,15 +3474,15 @@ CK_RV NSC_InitToken(CK_SLOT_ID slotID,CK_CHAR_PTR pPin,
     /* first, delete all our loaded key and cert objects from our 
      * internal list. */
     PZ_Lock(slot->objectLock);
-    for (i=0; i < slot->tokObjHashSize; i++) {
+    for (i=0; i < slot->sessObjHashSize; i++) {
 	do {
-	    object = slot->tokObjects[i];
+	    object = slot->sessObjHashTable[i];
 	    /* hand deque */
 	    /* this duplicates function of NSC_close session functions, but 
 	     * because we know that we are freeing all the sessions, we can
 	     * do more efficient processing */
 	    if (object) {
-		slot->tokObjects[i] = object->next;
+		slot->sessObjHashTable[i] = object->next;
 
 		if (object->next) object->next->prev = NULL;
 		object->next = object->prev = NULL;
@@ -5115,9 +5150,9 @@ CK_RV NSC_FindObjectsInit(CK_SESSION_HANDLE hSession,
     
     /* build list of found objects in the session */
     if (!tokenOnly) {
-	crv = sftk_searchObjectList(search, slot->tokObjects, 
-				slot->tokObjHashSize, slot->objectLock, 
-					pTemplate, ulCount, isLoggedIn);
+	crv = sftk_searchObjectList(search, slot->sessObjHashTable, 
+				    slot->sessObjHashSize, slot->objectLock, 
+				    pTemplate, ulCount, isLoggedIn);
     }
     if (crv != CKR_OK) {
 	goto loser;
diff --git a/security/nss/lib/softoken/pkcs11i.h b/security/nss/lib/softoken/pkcs11i.h
index 6f77994fd..cadcb6ffe 100644
--- a/security/nss/lib/softoken/pkcs11i.h
+++ b/security/nss/lib/softoken/pkcs11i.h
@@ -88,10 +88,10 @@
  * NSS is a shared library.
  */
 #define SPACE_ATTRIBUTE_HASH_SIZE 32 
-#define SPACE_TOKEN_OBJECT_HASH_SIZE 32
+#define SPACE_SESSION_OBJECT_HASH_SIZE 32
 #define SPACE_SESSION_HASH_SIZE 32
 #define TIME_ATTRIBUTE_HASH_SIZE 32
-#define TIME_TOKEN_OBJECT_HASH_SIZE 1024
+#define TIME_SESSION_OBJECT_HASH_SIZE 1024
 #define TIME_SESSION_HASH_SIZE 1024
 #define MAX_OBJECT_LIST_SIZE 800  
 				  /* how many objects to keep on the free list
@@ -319,8 +319,9 @@ struct SFTKSessionStr {
  *
  * The array of sessionLock's protect the session hash table (head[])
  * as well as the reference count of session objects in that bucket
- * (head[]->refCount),  objectLock protects all elements of the token
- * object hash table (tokObjects[], tokenIDCount, and tokenHashTable),
+ * (head[]->refCount),  objectLock protects all elements of the slot's
+ * object hash tables (sessObjHashTable[] and tokObjHashTable), and
+ * sessionObjectHandleCount.
  * slotLock protects the remaining protected elements:
  * password, isLoggedIn, ssoLoggedIn, and sessionCount,
  * and pwCheckLock serializes the key database password checks in
@@ -366,11 +367,11 @@ struct SFTKSlotStr {
     int			sessionCount;           /* variable - reset */
     PRInt32             rwSessionCount;    	/* set by atomic operations */
                                           	/* (reset) */
-    int			tokenIDCount;      	/* variable - perserved */
+    PRUint32		sessionObjectHandleCount; /* variable - preserved */
     int			index;			/* invariant */
-    PLHashTable		*tokenHashTable;	/* invariant */
-    SFTKObject		**tokObjects;		/* variable - reset */
-    unsigned int	tokObjHashSize;		/* invariant */
+    PLHashTable		*tokObjHashTable;	/* invariant */
+    SFTKObject		**sessObjHashTable;	/* variable - reset */
+    unsigned int	sessObjHashSize;	/* invariant */
     SFTKSession		**head;			/* variable -reset */
     unsigned int	sessHashSize;		/* invariant */
     char		tokDescription[33];	/* per load */
diff --git a/security/nss/lib/softoken/pkcs11u.c b/security/nss/lib/softoken/pkcs11u.c
index 9790f0ca3..a575bd13f 100644
--- a/security/nss/lib/softoken/pkcs11u.c
+++ b/security/nss/lib/softoken/pkcs11u.c
@@ -2093,8 +2093,8 @@ sftk_deleteTokenKeyByHandle(SFTKSlot *slot, CK_OBJECT_HANDLE handle)
    SECItem *item;
    PRBool rem;
 
-   item = (SECItem *)PL_HashTableLookup(slot->tokenHashTable, (void *)handle);
-   rem = PL_HashTableRemove(slot->tokenHashTable,(void *)handle) ;
+   item = (SECItem *)PL_HashTableLookup(slot->tokObjHashTable, (void *)handle);
+   rem = PL_HashTableRemove(slot->tokObjHashTable,(void *)handle) ;
    if (rem && item) {
 	SECITEM_FreeItem(item,PR_TRUE);
    }
@@ -2117,7 +2117,7 @@ sftk_addTokenKeyByHandle(SFTKSlot *slot, CK_OBJECT_HANDLE handle, SECItem *key)
     if (item == NULL) {
 	return SECFailure;
     }
-    entry = PL_HashTableAdd(slot->tokenHashTable,(void *)handle,item);
+    entry = PL_HashTableAdd(slot->tokObjHashTable,(void *)handle,item);
     if (entry == NULL) {
 	SECITEM_FreeItem(item,PR_TRUE);
 	return SECFailure;
@@ -2129,7 +2129,7 @@ sftk_addTokenKeyByHandle(SFTKSlot *slot, CK_OBJECT_HANDLE handle, SECItem *key)
 static SECItem *
 sftk_lookupTokenKeyByHandle(SFTKSlot *slot, CK_OBJECT_HANDLE handle)
 {
-    return (SECItem *)PL_HashTableLookup(slot->tokenHashTable, (void *)handle);
+    return (SECItem *)PL_HashTableLookup(slot->tokObjHashTable, (void *)handle);
 }
 
 /*
@@ -2161,7 +2161,7 @@ SFTK_ClearTokenKeyHashTable(SFTKSlot *slot)
 {
     sftk_tokenKeyLock(slot);
     PORT_Assert(!slot->present);
-    PL_HashTableEnumerateEntries(slot->tokenHashTable, sftk_freeHashItem, NULL);
+    PL_HashTableEnumerateEntries(slot->tokObjHashTable, sftk_freeHashItem, NULL);
     sftk_tokenKeyUnlock(slot);
     return CKR_OK;
 }
@@ -2408,14 +2408,14 @@ static SFTKObject *
 sftk_ObjectFromHandleOnSlot(CK_OBJECT_HANDLE handle, SFTKSlot *slot)
 {
     SFTKObject *object;
-    PRUint32 index = sftk_hash(handle, slot->tokObjHashSize);
+    PRUint32 index = sftk_hash(handle, slot->sessObjHashSize);
 
     if (sftk_isToken(handle)) {
 	return sftk_NewTokenObject(slot, NULL, handle);
     }
 
     PZ_Lock(slot->objectLock);
-    sftkqueue_find2(object, handle, index, slot->tokObjects);
+    sftkqueue_find2(object, handle, index, slot->sessObjHashTable);
     if (object) {
 	sftk_ReferenceObject(object);
     }
@@ -2468,10 +2468,10 @@ sftk_FreeObject(SFTKObject *object)
 void
 sftk_AddSlotObject(SFTKSlot *slot, SFTKObject *object)
 {
-    PRUint32 index = sftk_hash(object->handle, slot->tokObjHashSize);
+    PRUint32 index = sftk_hash(object->handle, slot->sessObjHashSize);
     sftkqueue_init_element(object);
     PZ_Lock(slot->objectLock);
-    sftkqueue_add2(object, object->handle, index, slot->tokObjects);
+    sftkqueue_add2(object, object->handle, index, slot->sessObjHashTable);
     PZ_Unlock(slot->objectLock);
 }
 
@@ -2505,7 +2505,7 @@ sftk_DeleteObject(SFTKSession *session, SFTKObject *object)
     NSSLOWCERTCertificate *cert;
     NSSLOWCERTCertTrust tmptrust;
     PRBool isKrl;
-    PRUint32 index = sftk_hash(object->handle, slot->tokObjHashSize);
+    PRUint32 index = sftk_hash(object->handle, slot->sessObjHashSize);
 
   /* Handle Token case */
     if (so && so->session) {
@@ -2514,7 +2514,7 @@ sftk_DeleteObject(SFTKSession *session, SFTKObject *object)
 	sftkqueue_delete(&so->sessionList,0,session->objects,0);
 	PZ_Unlock(session->objectLock);
 	PZ_Lock(slot->objectLock);
-	sftkqueue_delete2(object, object->handle, index, slot->tokObjects);
+	sftkqueue_delete2(object, object->handle, index, slot->sessObjHashTable);
 	PZ_Unlock(slot->objectLock);
 	sftkqueue_clear_deleted_element(object);
 	sftk_FreeObject(object); /* reduce it's reference count */