bug 125808, refresh trust and slot pointers of cert after token removal/insertion

4 files changed, 102 insertions, 47 deletions

diff --git a/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c
index 6cd9dfa53..0194b3083 100644
--- a/security/nss/lib/pk11wrap/pk11cert.c
+++ b/security/nss/lib/pk11wrap/pk11cert.c
@@ -101,10 +101,6 @@ static PRStatus convert_and_cache_cert(NSSCertificate *c, void *arg)
      * a CERTCertificate and fed into the callback.
      */
     nssrv = nssTrustDomain_AddCertsToCache(td, &c, 1);
-    if (!nssList_Get(nss3cb->cached, c)) {
-	nssCertificate_AddRef(c);
-	nssList_Add(nss3cb->cached, c);
-    }
     /* This is why the hack of copying the cert was done above.  The pointer
      * c passed to this function is provided by retrieve_cert.  That function
      * will destroy the pointer once this function returns.  Since c is a local
@@ -1426,16 +1422,14 @@ PK11_FindCertFromNickname(char *nickname, void *wincx) {
 	    search.cbarg = &token_cb;
 	    search.cached = certList;
 	    search.searchType = nssTokenSearchType_TokenOnly;
-	    /* then filter the list of cached certs for only those on the
-	     * token
-	     */
-	    nssCertificateList_DoCallback(certList, 
-	                                  token_callback, 
-	                                  &token_cb);
 	    /* now search the token */
 	    nssToken_TraverseCertificatesByNickname(token, NULL, 
 	                                            (NSSUTF8 *)nickname,
 	                                            &search);
+	    /* filter the list of cached certs for only those on the token */
+	    nssCertificateList_DoCallback(certList, 
+	                                  token_callback, 
+	                                  &token_cb);
 	}
 	if (certList) {
 	    nssList_Clear(certList, cert_destructor);
@@ -1458,12 +1452,12 @@ PK11_FindCertFromNickname(char *nickname, void *wincx) {
 	                                                          nickname, 
 	                                                          certList);
 		search.cached = certList;
-		nssCertificateList_DoCallback(certList, 
-		                              token_callback,
-		                              &token_cb);
 		nssToken_TraverseCertificatesByEmail(token, NULL, 
 		                                     (NSSASCII7 *)nickname,
 		                                     &search);
+		nssCertificateList_DoCallback(certList, 
+		                              token_callback,
+		                              &token_cb);
 	    }
 	    if (certList) {
 		nssList_Clear(certList, cert_destructor);
@@ -2733,18 +2727,18 @@ PK11_TraverseCertsForSubjectInSlot(CERTCertificate *cert, PK11SlotInfo *slot,
 	}
 	(void)nssTrustDomain_GetCertsForSubjectFromCache(td, &subject, 
 	                                                 subjectList);
-	filter_list_for_token_certs(subjectList, token);
 	/* set the search criteria */
 	search.callback = convert_and_cache_cert;
 	search.cbarg = &pk11cb;
 	search.cached = subjectList;
 	search.searchType = nssTokenSearchType_TokenOnly;
 	pk11cb.cached = subjectList;
-	nssrv = nssCertificateList_DoCallback(subjectList, 
-	                                      convert_cert, &pk11cb);
+	nssrv = nssToken_TraverseCertificatesBySubject(token, NULL, 
+	                                               &subject, &search);
 	if (nssrv == PR_SUCCESS) {
-	    nssrv = nssToken_TraverseCertificatesBySubject(token, NULL, 
-                                                           &subject, &search);
+	    filter_list_for_token_certs(subjectList, token);
+	    nssrv = nssCertificateList_DoCallback(subjectList, 
+	                                          convert_cert, &pk11cb);
 	}
     }
     if (subjectList) {
@@ -2820,18 +2814,18 @@ PK11_TraverseCertsForNicknameInSlot(SECItem *nickname, PK11SlotInfo *slot,
     } else {
 	nameList = nssList_Create(NULL, PR_FALSE);
 	(void)nssTrustDomain_GetCertsForNicknameFromCache(td, nick, nameList);
-	filter_list_for_token_certs(nameList, token);
 	/* set the search criteria */
 	search.callback = convert_and_cache_cert;
 	search.cbarg = &pk11cb;
 	search.cached = nameList;
 	search.searchType = nssTokenSearchType_TokenOnly;
 	pk11cb.cached = nameList;
-	nssrv = nssCertificateList_DoCallback(nameList, 
-	                                      convert_cert, &pk11cb);
+	nssrv = nssToken_TraverseCertificatesByNickname(token, NULL, 
+	                                                nick, &search);
 	if (nssrv == PR_SUCCESS) {
-	    nssrv = nssToken_TraverseCertificatesByNickname(token, NULL, 
-                                                            nick, &search);
+	    filter_list_for_token_certs(nameList, token);
+	    nssrv = nssCertificateList_DoCallback(nameList, 
+	                                          convert_cert, &pk11cb);
 	}
     }
     if (nameList) {
@@ -2891,17 +2885,17 @@ PK11_TraverseCertsInSlot(PK11SlotInfo *slot,
 	    return SECFailure;
 	}
 	(void *)nssTrustDomain_GetCertsFromCache(td, certList);
-	filter_list_for_token_certs(certList, tok);
 	/* set the search criteria */
 	search.callback = convert_and_cache_cert;
 	search.cbarg = &pk11cb;
 	search.cached = certList;
 	search.searchType = nssTokenSearchType_TokenOnly;
 	pk11cb.cached = certList;
-	nssrv = nssCertificateList_DoCallback(certList, 
-	                                      convert_cert, &pk11cb);
+	nssrv = nssToken_TraverseCertificates(tok, NULL, &search);
 	if (nssrv == PR_SUCCESS) {
-	    nssrv = nssToken_TraverseCertificates(tok, NULL, &search);
+	    filter_list_for_token_certs(certList, tok);
+	    nssrv = nssCertificateList_DoCallback(certList, 
+	                                          convert_cert, &pk11cb);
 	}
 	nssList_Clear(certList, cert_destructor);
 	nssList_Destroy(certList);
diff --git a/security/nss/lib/pki/pki3hack.c b/security/nss/lib/pki/pki3hack.c
index cc55218e2..ad9dbe5f0 100644
--- a/security/nss/lib/pki/pki3hack.c
+++ b/security/nss/lib/pki/pki3hack.c
@@ -100,6 +100,9 @@ STAN_GetDefaultCryptoToken
     return PK11Slot_GetNSSToken(pk11slot);
 }
 
+static CERTCertificate *
+stan_GetCERTCertificate(NSSCertificate *c, PRBool forceUpdate);
+
 /* stuff the cert in the global trust domain cache, and then add a reference
  * to remain with the token in a list.
  */
@@ -110,7 +113,7 @@ cache_token_cert(NSSCertificate *c, void *arg)
     NSSTrustDomain *td = STAN_GetDefaultTrustDomain();
     NSSCertificate *cp = nssCertificate_AddRef(c);
     if (nssList_Count(token->certList) > NSSTOKEN_MAX_LOCAL_CERTS) {
-	nssToken_DestroyCertList(token);
+	nssToken_DestroyCertList(token, PR_TRUE);
 	/* terminate the traversal */
 	return PR_FAILURE;
     }
@@ -122,8 +125,9 @@ cache_token_cert(NSSCertificate *c, void *arg)
     nssList_Add(token->certList, nssCertificate_AddRef(c));
     /* The cert needs to become external (made into a CERTCertificate)
      * in order for it to be properly released.
+     * Force an update of the nickname and slot fields.
      */
-    (void)STAN_GetCERTCertificate(c);
+    (void)stan_GetCERTCertificate(c, PR_TRUE);
     return PR_SUCCESS;
 }
 
@@ -151,7 +155,6 @@ static void remove_token_instance(NSSCertificate *c, NSSToken *token)
 
 static PRBool instance_destructor(NSSCertificate *c, NSSToken *token)
 {
-    CERTCertificate *cert = STAN_GetCERTCertificate(c);
     remove_token_instance(c, token);
     if (nssList_Count(c->object.instanceList) == 0) {
 	return PR_TRUE;
@@ -160,7 +163,7 @@ static PRBool instance_destructor(NSSCertificate *c, NSSToken *token)
 }
 
 NSS_IMPLEMENT void
-nssCertificateList_DestroyTokenCerts(nssList *certList, NSSToken *token)
+destroy_token_certs(nssList *certList, NSSToken *token, PRBool renewInstances)
 {
     nssListIterator *certs;
     NSSCertificate *cert;
@@ -171,9 +174,12 @@ nssCertificateList_DestroyTokenCerts(nssList *certList, NSSToken *token)
          cert  = (NSSCertificate *)nssListIterator_Next(certs))
     {
 	removeIt = instance_destructor(cert, token);
-	if (removeIt) {
+	if (removeIt || !renewInstances) {
 	    nssList_Remove(certList, cert);
 	    CERT_DestroyCertificate(STAN_GetCERTCertificate(cert));
+	} else {
+	    /* force an update of the nickname and slot fields of the cert */
+	    (void)stan_GetCERTCertificate(cert, PR_TRUE);
 	}
     }
     nssListIterator_Finish(certs);
@@ -181,6 +187,12 @@ nssCertificateList_DestroyTokenCerts(nssList *certList, NSSToken *token)
 }
 
 NSS_IMPLEMENT void
+nssCertificateList_DestroyTokenCerts(nssList *certList, NSSToken *token)
+{
+    destroy_token_certs(certList, token, PR_FALSE);
+}
+
+NSS_IMPLEMENT void
 nssCertificateList_RemoveTokenCerts(nssList *certList, NSSToken *token)
 {
     nssListIterator *certs;
@@ -194,6 +206,9 @@ nssCertificateList_RemoveTokenCerts(nssList *certList, NSSToken *token)
 	removeIt = instance_destructor(cert, token);
 	if (removeIt) {
 	    nssList_Remove(certList, cert);
+	} else {
+	    /* force an update of the nickname and slot fields of the cert */
+	    (void)stan_GetCERTCertificate(cert, PR_TRUE);
 	}
     }
     nssListIterator_Finish(certs);
@@ -202,12 +217,12 @@ nssCertificateList_RemoveTokenCerts(nssList *certList, NSSToken *token)
 
 /* destroy the list of certs on a token */
 NSS_IMPLEMENT void
-nssToken_DestroyCertList(NSSToken *token)
+nssToken_DestroyCertList(NSSToken *token, PRBool renewInstances)
 {
     if (!token->certList) {
 	return;
     }
-    nssCertificateList_DestroyTokenCerts(token->certList, token);
+    destroy_token_certs(token->certList, token, renewInstances);
     nssList_Clear(token->certList, NULL);
     /* leave the list non-null to prevent it from being searched */
 }
@@ -261,7 +276,7 @@ nssToken_SearchCerts
 	*notPresentOpt = PR_FALSE;
     }
     if (!nssToken_IsPresent(token)) {
-	nssToken_DestroyCertList(token); /* will free cached certs */
+	nssToken_DestroyCertList(token, PR_TRUE); /* will free cached certs */
 	if (notPresentOpt) {
 	    *notPresentOpt = PR_TRUE;
 	}
@@ -346,7 +361,7 @@ NSS_IMPLEMENT void
 STAN_DestroyNSSToken(NSSToken *token)
 {
     if (token->certList) {
-	nssToken_DestroyCertList(token);
+	nssToken_DestroyCertList(token, PR_FALSE);
     }
     nssToken_Destroy(token);
 }
@@ -689,6 +704,24 @@ static int nsstoken_get_trust_order(NSSToken *token)
     return module->trustOrder;
 }
 
+/* check all cert instances for private key */
+static PRBool is_user_cert(NSSCertificate *c, CERTCertificate *cc)
+{
+    PRBool isUser = PR_FALSE;
+    nssCryptokiInstance *instance;
+    nssListIterator *instances = c->object.instances;
+    for (instance  = (nssCryptokiInstance *)nssListIterator_Start(instances);
+         instance != (nssCryptokiInstance *)NULL;
+         instance  = (nssCryptokiInstance *)nssListIterator_Next(instances)) 
+    {
+	if (PK11_IsUserCert(instance->token->pk11slot, cc, instance->handle)) {
+	    isUser = PR_TRUE;
+	}
+    }
+    nssListIterator_Finish(instances);
+    return isUser;
+}
+
 CERTCertTrust * 
 nssTrust_GetCERTCertTrustForCert(NSSCertificate *c, CERTCertificate *cc)
 {
@@ -738,7 +771,7 @@ nssTrust_GetCERTCertTrustForCert(NSSCertificate *c, CERTCertificate *cc)
     nssListIterator_Destroy(tokens);
     rvTrust = cert_trust_from_stan_trust(&t, cc->arena);
     if (!rvTrust) return NULL;
-    if (cc->slot && PK11_IsUserCert(cc->slot, cc, cc->pkcs11ID)) {
+    if (is_user_cert(c, cc)) {
 	rvTrust->sslFlags |= CERTDB_USER;
 	rvTrust->emailFlags |= CERTDB_USER;
 	rvTrust->objectSigningFlags |= CERTDB_USER;
@@ -749,20 +782,38 @@ nssTrust_GetCERTCertTrustForCert(NSSCertificate *c, CERTCertificate *cc)
 static nssCryptokiInstance *
 get_cert_instance(NSSCertificate *c)
 {
-    nssCryptokiInstance *instance;
+    nssCryptokiInstance *instance, *ci;
+    nssListIterator *instances = c->object.instances;
     instance = NULL;
-    nssList_GetArray(c->object.instanceList, (void **)&instance, 1);
+    for (ci  = (nssCryptokiInstance *)nssListIterator_Start(instances);
+         ci != (nssCryptokiInstance *)NULL;
+         ci  = (nssCryptokiInstance *)nssListIterator_Next(instances)) 
+    {
+	if (!instance) {
+	    instance = ci;
+	} else {
+	    /* This only really works for two instances...  But 3.4 can't
+	     * handle more anyway.  The logic is, if there are multiple
+	     * instances, prefer the one that is not internal (e.g., on
+	     * a hardware device.
+	     */
+	    if (PK11_IsInternal(instance->token->pk11slot)) {
+		instance = ci;
+	    }
+	}
+    }
+    nssListIterator_Finish(instances);
     return instance;
 }
 
 static void
-fill_CERTCertificateFields(NSSCertificate *c, CERTCertificate *cc)
+fill_CERTCertificateFields(NSSCertificate *c, CERTCertificate *cc, PRBool forced)
 {
     NSSTrust *nssTrust;
     NSSCryptoContext *context = c->object.cryptoContext;
     nssCryptokiInstance *instance = get_cert_instance(c);
     /* fill other fields needed by NSS3 functions using CERTCertificate */
-    if (!cc->nickname && c->nickname) {
+    if ((!cc->nickname && c->nickname) || forced) {
 	PRStatus nssrv;
 	int nicklen, tokenlen, len;
 	NSSUTF8 *tokenName = NULL;
@@ -812,8 +863,8 @@ fill_CERTCertificateFields(NSSCertificate *c, CERTCertificate *cc)
     cc->nssCertificate = c;
 }
 
-NSS_EXTERN CERTCertificate *
-STAN_GetCERTCertificate(NSSCertificate *c)
+static CERTCertificate *
+stan_GetCERTCertificate(NSSCertificate *c, PRBool forceUpdate)
 {
     nssDecodedCert *dc;
     CERTCertificate *cc;
@@ -826,8 +877,8 @@ STAN_GetCERTCertificate(NSSCertificate *c)
     }
     cc = (CERTCertificate *)dc->data;
     if (cc) {
-	if (!cc->nssCertificate) {
-	    fill_CERTCertificateFields(c, cc);
+	if (!cc->nssCertificate || forceUpdate) {
+	    fill_CERTCertificateFields(c, cc, forceUpdate);
 	} else if (!cc->trust && !c->object.cryptoContext) {
 	    /* if it's a perm cert, it might have been stored before the
 	     * trust, so look for the trust again.  But a temp cert can be
@@ -839,6 +890,12 @@ STAN_GetCERTCertificate(NSSCertificate *c)
     return cc;
 }
 
+NSS_IMPLEMENT CERTCertificate *
+STAN_GetCERTCertificate(NSSCertificate *c)
+{
+    return stan_GetCERTCertificate(c, PR_FALSE);
+}
+
 static CK_TRUST
 get_stan_trust(unsigned int t, PRBool isClientAuth) 
 {
diff --git a/security/nss/lib/pki/pki3hack.h b/security/nss/lib/pki/pki3hack.h
index 3bbe62716..b2d908b6f 100644
--- a/security/nss/lib/pki/pki3hack.h
+++ b/security/nss/lib/pki/pki3hack.h
@@ -81,8 +81,12 @@ nssToken_SearchCerts
   PRBool *notPresentOpt
 );
 
+/* renewInstances -- if the cached token certs have multiple instances,
+ * don't destroy them.  If this parameter is false, they will be destroyed
+ * anyway (used for clean shutdown).
+ */
 NSS_EXTERN void
-nssToken_DestroyCertList(NSSToken *token);
+nssToken_DestroyCertList(NSSToken *token, PRBool renewInstances);
 
 NSS_EXTERN void
 nssCertificateList_DestroyTokenCerts(nssList *certList, NSSToken *token);
diff --git a/security/nss/lib/pki/trustdomain.c b/security/nss/lib/pki/trustdomain.c
index 0d33e086d..1e3ca163a 100644
--- a/security/nss/lib/pki/trustdomain.c
+++ b/security/nss/lib/pki/trustdomain.c
@@ -515,7 +515,7 @@ static PRBool cert_token_not_present(NSSCertificate *c)
          instance  = (nssCryptokiInstance *)nssListIterator_Next(instances))
     {
 	if (!nssToken_IsPresent(instance->token)) {
-	    nssToken_DestroyCertList(instance->token);
+	    nssToken_DestroyCertList(instance->token, PR_TRUE);
 	    nssList_Remove(c->object.instanceList, instance);
 	} else {
 	    freeIt = PR_FALSE;