Bug 444850 ? NSS misbehaves badly in the presence of a disabled PKCS#11 slot

r=julien.pierre, rrelyea

6 files changed, 103 insertions, 68 deletions

diff --git a/security/nss/lib/dev/devslot.c b/security/nss/lib/dev/devslot.c
index 21344e6eb..4ba45bfc2 100644
--- a/security/nss/lib/dev/devslot.c
+++ b/security/nss/lib/dev/devslot.c
@@ -50,6 +50,8 @@ static const char CVS_ID[] = "@(#) $RCSfile$ $Revision$ $Date$";
 #include "ckhelper.h"
 #endif /* CKHELPER_H */
 
+#include "pk11pub.h"
+
 /* measured in seconds */
 #define NSSSLOT_TOKEN_DELAY_TIME 1
 
@@ -163,13 +165,13 @@ nssSlot_IsTokenPresent (
     nssSession *session;
     CK_SLOT_INFO slotInfo;
     void *epv;
-    /* permanent slots are always present */
+    /* permanent slots are always present unless they're disabled */
     if (nssSlot_IsPermanent(slot)) {
-	return PR_TRUE;
+	return !PK11_IsDisabled(slot->pk11slot);
     }
     /* avoid repeated calls to check token status within set interval */
     if (within_token_delay_period(slot)) {
-	return (PRBool)((slot->ckFlags & CKF_TOKEN_PRESENT) != 0);
+	return ((slot->ckFlags & CKF_TOKEN_PRESENT) != 0);
     }
 
     /* First obtain the slot info */
@@ -188,18 +190,20 @@ nssSlot_IsTokenPresent (
     /* check for the presence of the token */
     if ((slot->ckFlags & CKF_TOKEN_PRESENT) == 0) {
 	if (!slot->token) {
-	    /* token was ne'er present */
+	    /* token was never present */
 	    return PR_FALSE;
 	}
 	session = nssToken_GetDefaultSession(slot->token);
-	nssSession_EnterMonitor(session);
-	/* token is not present */
-	if (session->handle != CK_INVALID_SESSION) {
-	    /* session is valid, close and invalidate it */
-	    CKAPI(epv)->C_CloseSession(session->handle);
-	    session->handle = CK_INVALID_SESSION;
+	if (session) {
+	    nssSession_EnterMonitor(session);
+	    /* token is not present */
+	    if (session->handle != CK_INVALID_SESSION) {
+		/* session is valid, close and invalidate it */
+		CKAPI(epv)->C_CloseSession(session->handle);
+		session->handle = CK_INVALID_SESSION;
+	    }
+	    nssSession_ExitMonitor(session);
 	}
-	nssSession_ExitMonitor(session);
 	if (slot->token->base.name[0] != 0) {
 	    /* notify the high-level cache that the token is removed */
 	    slot->token->base.name[0] = 0; /* XXX */
@@ -214,36 +218,36 @@ nssSlot_IsTokenPresent (
      * has been removed and reinserted.
      */
     session = nssToken_GetDefaultSession(slot->token);
-    nssSession_EnterMonitor(session);
-    if (session->handle != CK_INVALID_SESSION) {
-	CK_SESSION_INFO sessionInfo;
-	ckrv = CKAPI(epv)->C_GetSessionInfo(session->handle, &sessionInfo);
-	if (ckrv != CKR_OK) {
-	    /* session is screwy, close and invalidate it */
-	    CKAPI(epv)->C_CloseSession(session->handle);
-	    session->handle = CK_INVALID_SESSION;
-	}
-    }
-    nssSession_ExitMonitor(session);
-    /* token not removed, finished */
-    if (session->handle != CK_INVALID_SESSION) {
-	return PR_TRUE;
-    } else {
-	/* the token has been removed, and reinserted, or the slot contains
-	 * a token it doesn't recognize. invalidate all the old
-	 * information we had on this token, if we can't refresh, clear
-	 * the present flag */
-	nssToken_NotifyCertsNotVisible(slot->token);
-	nssToken_Remove(slot->token);
-	/* token has been removed, need to refresh with new session */
-	nssrv = nssSlot_Refresh(slot);
-	if (nssrv != PR_SUCCESS) {
-	    slot->token->base.name[0] = 0; /* XXX */
-	    slot->ckFlags &= ~CKF_TOKEN_PRESENT;
-	    return PR_FALSE;
+    if (session) {
+	nssSession_EnterMonitor(session);
+	if (session->handle != CK_INVALID_SESSION) {
+	    CK_SESSION_INFO sessionInfo;
+	    ckrv = CKAPI(epv)->C_GetSessionInfo(session->handle, &sessionInfo);
+	    if (ckrv != CKR_OK) {
+		/* session is screwy, close and invalidate it */
+		CKAPI(epv)->C_CloseSession(session->handle);
+		session->handle = CK_INVALID_SESSION;
+	    }
 	}
-	return PR_TRUE;
+	nssSession_ExitMonitor(session);
+	/* token not removed, finished */
+	if (session->handle != CK_INVALID_SESSION)
+	    return PR_TRUE;
+    } 
+    /* the token has been removed, and reinserted, or the slot contains
+     * a token it doesn't recognize. invalidate all the old
+     * information we had on this token, if we can't refresh, clear
+     * the present flag */
+    nssToken_NotifyCertsNotVisible(slot->token);
+    nssToken_Remove(slot->token);
+    /* token has been removed, need to refresh with new session */
+    nssrv = nssSlot_Refresh(slot);
+    if (nssrv != PR_SUCCESS) {
+        slot->token->base.name[0] = 0; /* XXX */
+        slot->ckFlags &= ~CKF_TOKEN_PRESENT;
+        return PR_FALSE;
     }
+    return PR_TRUE;
 }
 
 NSS_IMPLEMENT void *
diff --git a/security/nss/lib/dev/devtoken.c b/security/nss/lib/dev/devtoken.c
index 192611e40..7eb17b9d2 100644
--- a/security/nss/lib/dev/devtoken.c
+++ b/security/nss/lib/dev/devtoken.c
@@ -307,6 +307,7 @@ find_objects (
     nssSession *session = (sessionOpt) ? sessionOpt : tok->defaultSession;
 
     /* Don't ask the module to use an invalid session handle. */
+    PORT_Assert(session->handle != CK_INVALID_SESSION);
     if (session->handle == CK_INVALID_SESSION) {
 	ckrv = CKR_SESSION_HANDLE_INVALID;
 	goto loser;                
diff --git a/security/nss/lib/pk11wrap/dev3hack.c b/security/nss/lib/pk11wrap/dev3hack.c
index 575c589f4..c7a586305 100644
--- a/security/nss/lib/pk11wrap/dev3hack.c
+++ b/security/nss/lib/pk11wrap/dev3hack.c
@@ -56,21 +56,23 @@ static const char CVS_ID[] = "@(#) $RCSfile$ $Revision$ $Date$";
 
 #include "pk11func.h"
 #include "secmodti.h"
+#include "secerr.h"
 
 NSS_IMPLEMENT nssSession *
 nssSession_ImportNSS3Session(NSSArena *arenaOpt,
                              CK_SESSION_HANDLE session, 
                              PZLock *lock, PRBool rw)
 {
-    nssSession *rvSession;
-    rvSession = nss_ZNEW(arenaOpt, nssSession);
-    if (!rvSession) {
-        return NULL;
+    nssSession *rvSession = NULL;
+    if (session != CK_INVALID_SESSION) {
+	rvSession = nss_ZNEW(arenaOpt, nssSession);
+	if (rvSession) {
+	    rvSession->handle = session;
+	    rvSession->lock = lock;
+	    rvSession->ownLock = PR_FALSE;
+	    rvSession->isRW = rw;
+	}
     }
-    rvSession->handle = session;
-    rvSession->lock = lock;
-    rvSession->ownLock = PR_FALSE;
-    rvSession->isRW = rw;
     return rvSession;
 }
 
@@ -160,11 +162,17 @@ nssSlot_CreateFromPK11SlotInfo(NSSTrustDomain *td, PK11SlotInfo *nss3slot)
     return rvSlot;
 }
 
-NSS_IMPLEMENT NSSToken *
+NSSToken *
 nssToken_CreateFromPK11SlotInfo(NSSTrustDomain *td, PK11SlotInfo *nss3slot)
 {
     NSSToken *rvToken;
     NSSArena *arena;
+
+    /* Don't create a token object for a disabled slot */
+    if (nss3slot->disabled) {
+	PORT_SetError(SEC_ERROR_NO_TOKEN);
+	return NULL;
+    }
     arena = nssArena_Create();
     if (!arena) {
 	return NULL;
@@ -176,6 +184,10 @@ nssToken_CreateFromPK11SlotInfo(NSSTrustDomain *td, PK11SlotInfo *nss3slot)
     }
     rvToken->base.refCount = 1;
     rvToken->base.lock = PZ_NewLock(nssILockOther);
+    if (!rvToken->base.lock) {
+	nssArena_Destroy(arena);
+	return NULL;
+    }
     rvToken->base.arena = arena;
     rvToken->pk11slot = nss3slot;
     rvToken->epv = nss3slot->functionList;
@@ -183,26 +195,28 @@ nssToken_CreateFromPK11SlotInfo(NSSTrustDomain *td, PK11SlotInfo *nss3slot)
                                                        nss3slot->session,
                                                        nss3slot->sessionLock,
                                                        nss3slot->defRWSession);
-    /* The above test was used in 3.4, for this cache have it always on */
+    /* continue, even if rvToken->defaultSession is NULL */
     if (!PK11_IsInternal(nss3slot) && PK11_IsHW(nss3slot)) {
 	rvToken->cache = nssTokenObjectCache_Create(rvToken, 
 	                                            PR_TRUE, PR_TRUE, PR_TRUE);
-	if (!rvToken->cache) {
-	    nssArena_Destroy(arena);
-	    return (NSSToken *)NULL;
-	}
+	if (!rvToken->cache)
+	    goto loser;
     }
     rvToken->trustDomain = td;
     /* Grab the token name from the PKCS#11 fixed-length buffer */
     rvToken->base.name = nssUTF8_Duplicate(nss3slot->token_name,td->arena);
     rvToken->slot = nssSlot_CreateFromPK11SlotInfo(td, nss3slot);
     if (!rvToken->slot) {
-        nssArena_Destroy(arena);
-        return (NSSToken *)NULL;
+        goto loser;
     }
     rvToken->slot->token = rvToken;
-    rvToken->defaultSession->slot = rvToken->slot;
+    if (rvToken->defaultSession)
+	rvToken->defaultSession->slot = rvToken->slot;
     return rvToken;
+loser:
+    PZ_DestroyLock(rvToken->base.lock);
+    nssArena_Destroy(arena);
+    return NULL;
 }
 
 NSS_IMPLEMENT void
@@ -241,11 +255,12 @@ nssToken_Refresh(NSSToken *token)
 	return PR_SUCCESS;
     }
     nss3slot = token->pk11slot;
-    token->defaultSession = nssSession_ImportNSS3Session(token->slot->base.arena,
-                                                       nss3slot->session,
-                                                       nss3slot->sessionLock,
-                                                       nss3slot->defRWSession);
-    return PR_SUCCESS;
+    token->defaultSession = 
+    	nssSession_ImportNSS3Session(token->slot->base.arena,
+				     nss3slot->session,
+				     nss3slot->sessionLock,
+				     nss3slot->defRWSession);
+    return token->defaultSession ? PR_SUCCESS : PR_FAILURE;
 }
 
 NSS_IMPLEMENT PRStatus
diff --git a/security/nss/lib/pk11wrap/pk11slot.c b/security/nss/lib/pk11wrap/pk11slot.c
index a53aead0a..c619ae003 100644
--- a/security/nss/lib/pk11wrap/pk11slot.c
+++ b/security/nss/lib/pk11wrap/pk11slot.c
@@ -1081,6 +1081,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts)
     CK_RV crv;
     char *tmp;
     SECStatus rv;
+    PRStatus status;
 
     /* set the slot flags to the current token values */
     if (!slot->isThreadSafe) PK11_EnterSlotMonitor(slot);
@@ -1177,7 +1178,9 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts)
 	if (!slot->isThreadSafe) PK11_ExitSlotMonitor(slot);
     }
 
-    nssToken_Refresh(slot->nssToken);
+    status = nssToken_Refresh(slot->nssToken);
+    if (status != PR_SUCCESS)
+    	return SECFailure;
 
     if (!(slot->isInternal) && (slot->hasRandom)) {
 	/* if this slot has a random number generater, use it to add entropy
@@ -1209,7 +1212,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBool loadCerts)
 	    PK11_ExitSlotMonitor(int_slot);
 	    if (crv == CKR_OK) {
 	        PK11_EnterSlotMonitor(slot);
-		PK11_GETTAB(slot)->C_SeedRandom(slot->session,
+		crv = PK11_GETTAB(slot)->C_SeedRandom(slot->session,
 					random_bytes, sizeof(random_bytes));
 	        PK11_ExitSlotMonitor(slot);
 	    }
diff --git a/security/nss/lib/pki/pki3hack.c b/security/nss/lib/pki/pki3hack.c
index 5e42bc157..6171f69bb 100644
--- a/security/nss/lib/pki/pki3hack.c
+++ b/security/nss/lib/pki/pki3hack.c
@@ -104,9 +104,12 @@ STAN_InitTokenForSlotInfo(NSSTrustDomain *td, PK11SlotInfo *slot)
     }
     token = nssToken_CreateFromPK11SlotInfo(td, slot);
     PK11Slot_SetNSSToken(slot, token);
-    NSSRWLock_LockWrite(td->tokensLock);
-    nssList_Add(td->tokenList, token);
-    NSSRWLock_UnlockWrite(td->tokensLock);
+    /* Don't add non-existent token to TD's token list */
+    if (token) {
+	NSSRWLock_LockWrite(td->tokensLock);
+	nssList_Add(td->tokenList, token);
+	NSSRWLock_UnlockWrite(td->tokensLock);
+    }
     return PR_SUCCESS;
 }
 
diff --git a/security/nss/lib/pki/trustdomain.c b/security/nss/lib/pki/trustdomain.c
index a2c7de668..ddd8df2a5 100644
--- a/security/nss/lib/pki/trustdomain.c
+++ b/security/nss/lib/pki/trustdomain.c
@@ -52,7 +52,7 @@ static const char CVS_ID[] = "@(#) $RCSfile$ $Revision$ $Date$";
 
 #include "cert.h"
 #include "pki3hack.h"
-
+#include "pk11pub.h"
 #include "nssrwlk.h"
 
 #define NSSTRUSTDOMAIN_DEFAULT_CACHE_SIZE 32
@@ -169,9 +169,18 @@ nssTrustDomain_GetActiveSlots (
     NSSRWLock_UnlockRead(td->tokensLock);
     count = 0;
     for (tp = tokens; *tp; tp++) {
-	slots[count++] = nssToken_GetSlot(*tp);
+        NSSSlot * slot = nssToken_GetSlot(*tp);
+        if (!PK11_IsDisabled(slot->pk11slot)) {
+            slots[count++] = slot;
+        } else {
+	    nssSlot_Destroy(slot);
+	}
     }
     nss_ZFreeIf(tokens);
+    if (!count) {
+	nss_ZFreeIf(slots);
+    	slots = NULL;
+    }
     return slots;
 }