Fix for bug 137645 - cached certificate does not get its nickname updated after P12 import of matching user certificate

6 files changed, 27 insertions, 95 deletions

diff --git a/security/nss/lib/dev/devm.h b/security/nss/lib/dev/devm.h
index 69ec759ec..0dd0d5b36 100644
--- a/security/nss/lib/dev/devm.h
+++ b/security/nss/lib/dev/devm.h
@@ -208,7 +208,7 @@ nssTokenObjectCache_ImportObject
   CK_ULONG otlen
 );
 
-NSS_EXTERN PRStatus
+NSS_EXTERN void
 nssTokenObjectCache_RemoveObject
 (
   nssTokenObjectCache *cache,
diff --git a/security/nss/lib/dev/devtoken.c b/security/nss/lib/dev/devtoken.c
index 70f552d4b..a43ed43f4 100644
--- a/security/nss/lib/dev/devtoken.c
+++ b/security/nss/lib/dev/devtoken.c
@@ -282,7 +282,7 @@ nssToken_DeleteStoredObject
     nssSession *session = NULL;
     void *epv = nssToken_GetCryptokiEPV(instance->token);
     if (token->cache) {
-	status = nssTokenObjectCache_RemoveObject(token->cache, instance);
+	nssTokenObjectCache_RemoveObject(token->cache, instance);
     }
     if (instance->isTokenObject) {
        if (nssSession_IsReadWrite(token->defaultSession)) {
@@ -301,9 +301,7 @@ nssToken_DeleteStoredObject
     if (createdSession) {
 	nssSession_Destroy(session);
     }
-    if (ckrv != CKR_OK) {
-	return PR_FAILURE;
-    }
+    status = (ckrv == CKR_OK) ? PR_SUCCESS : PR_FAILURE;
     return status;
 }
 
@@ -592,6 +590,9 @@ nssToken_ImportCertificate
 	nssCKObject_SetAttributes(rvObject->handle, 
 	                          cert_tmpl, ctsize,
 	                          session, slot);
+	if (!rvObject->label && nickname) {
+	    rvObject->label = nssUTF8_Duplicate(nickname, NULL);
+	}
 	nssSession_Destroy(session);
 	nssSlot_Destroy(slot);
     } else {
diff --git a/security/nss/lib/dev/devutil.c b/security/nss/lib/dev/devutil.c
index 6e82574e7..99218f83d 100644
--- a/security/nss/lib/dev/devutil.c
+++ b/security/nss/lib/dev/devutil.c
@@ -1370,7 +1370,7 @@ nssTokenObjectCache_ImportObject
     return status;
 }
 
-NSS_IMPLEMENT PRStatus
+NSS_IMPLEMENT void
 nssTokenObjectCache_RemoveObject
 (
   nssTokenObjectCache *cache,
@@ -1408,7 +1408,6 @@ nssTokenObjectCache_RemoveObject
 	cache->objects[oType] = NULL;
     }
     PZ_Unlock(cache->lock);
-    return PR_SUCCESS;
 }
 
 /* XXX of course this doesn't belong here */
diff --git a/security/nss/lib/pkcs12/p12d.c b/security/nss/lib/pkcs12/p12d.c
index 93a7ff200..ca5017cc5 100644
--- a/security/nss/lib/pkcs12/p12d.c
+++ b/security/nss/lib/pkcs12/p12d.c
@@ -2240,7 +2240,7 @@ sec_pkcs12_validate_cert(sec_PKCS12SafeBag *cert,
     }
 
     cert->noInstall = PR_FALSE;
-    cert->removeExisting = PR_FALSE;
+    cert->unused = PR_FALSE;
     cert->problem = PR_FALSE;
     cert->error = 0;
 
@@ -2253,26 +2253,7 @@ sec_pkcs12_validate_cert(sec_PKCS12SafeBag *cert,
 	return;
     }
 
-    testCert = PK11_FindCertFromDERCert(cert->slot, leafCert, wincx);
     CERT_DestroyCertificate(leafCert);
-    /* if we can't find the certificate through the PKCS11 interface,
-     * we should check the cert database directly, if we are
-     * importing to an internal slot.
-     */
-    if(!testCert && PK11_IsInternal(cert->slot)) {
-	testCert = CERT_FindCertByDERCert(CERT_GetDefaultCertDB(),
-				 &cert->safeBagContent.certBag->value.x509Cert);
-    }
-
-    if(testCert) {
-	if(!testCert->nickname) {
-	    cert->removeExisting = PR_TRUE;
-	}
-	CERT_DestroyCertificate(testCert);
-	if(cert->noInstall && !cert->removeExisting) {
-	    return;
-	}
-    }
 
     sec_pkcs12_validate_cert_nickname(cert, key, nicknameCb, wincx);
 }
@@ -2320,59 +2301,6 @@ sec_pkcs12_validate_key_by_cert(sec_PKCS12SafeBag *cert, sec_PKCS12SafeBag *key,
 }
 
 static SECStatus
-sec_pkcs12_remove_existing_cert(sec_PKCS12SafeBag *cert, 
-				void *wincx)
-{
-    SECItem *derCert = NULL;
-    CERTCertificate *tempCert = NULL;
-    CK_OBJECT_HANDLE certObj;
-    PRBool removed = PR_FALSE;
-
-    if(!cert) {
-	return SECFailure;
-    }
-
-    PORT_Assert(cert->removeExisting);
-
-    cert->removeExisting = PR_FALSE;
-    derCert = &cert->safeBagContent.certBag->value.x509Cert;
-    tempCert = CERT_DecodeDERCertificate(derCert, PR_FALSE, NULL);
-    if(!tempCert) {
-	return SECFailure;
-    }
-
-    certObj = PK11_FindCertInSlot(cert->slot, tempCert, wincx);
-    CERT_DestroyCertificate(tempCert);
-    tempCert = NULL;
-
-    if(certObj != CK_INVALID_HANDLE) {
-	PK11_DestroyObject(cert->slot, certObj);
-	removed = PR_TRUE;
-    } else if(PK11_IsInternal(cert->slot)) {
-	tempCert = CERT_FindCertByDERCert(CERT_GetDefaultCertDB(), derCert);
-	if(tempCert) {
-	    if(SEC_DeletePermCertificate(tempCert) == SECSuccess) {
-		removed = PR_TRUE;
-	    } 
-	    CERT_DestroyCertificate(tempCert);
-	    tempCert = NULL;
-	}
-    }
-
-    if(!removed) {
-	cert->problem = PR_TRUE;
-	cert->error = SEC_ERROR_NO_MEMORY;
-	cert->noInstall = PR_TRUE;
-    }
-	
-    if(tempCert) {
-	CERT_DestroyCertificate(tempCert);
-    }
-
-    return ((removed) ? SECSuccess : SECFailure);
-}
-
-static SECStatus
 sec_pkcs12_add_cert(sec_PKCS12SafeBag *cert, PRBool keyExists, void *wincx)
 {
     SECItem *derCert, *nickName;
@@ -2388,15 +2316,8 @@ sec_pkcs12_add_cert(sec_PKCS12SafeBag *cert, PRBool keyExists, void *wincx)
     }
 
     derCert = &cert->safeBagContent.certBag->value.x509Cert;
-    if(cert->removeExisting) {
-	if(sec_pkcs12_remove_existing_cert(cert, wincx) 
-			!= SECSuccess) {
-	    return SECFailure;
-	}
-	cert->removeExisting = PR_FALSE;
-    }
 
-    PORT_Assert(!cert->problem && !cert->removeExisting && !cert->noInstall);
+    PORT_Assert(!cert->problem && !cert->noInstall);
 
     nickName = sec_pkcs12_get_nickname(cert);
     if(nickName) {
@@ -2442,12 +2363,6 @@ sec_pkcs12_add_key(sec_PKCS12SafeBag *key, SECItem *publicValue,
 	return SECFailure;
     }
 
-    if(key->removeExisting) {
-	key->problem = PR_TRUE;
-	key->error = SEC_ERROR_PKCS12_UNABLE_TO_IMPORT_KEY;
-	return SECFailure;
-    }
-
     if(key->problem || key->noInstall) {
 	return SECSuccess;
     }
diff --git a/security/nss/lib/pkcs12/p12t.h b/security/nss/lib/pkcs12/p12t.h
index 6b9d3da1b..74e803c60 100644
--- a/security/nss/lib/pkcs12/p12t.h
+++ b/security/nss/lib/pkcs12/p12t.h
@@ -111,7 +111,7 @@ struct sec_PKCS12SafeBagStr {
     unsigned int nAttribs;
 
     /* used for validation/importing */
-    PRBool problem, noInstall, validated, hasKey, removeExisting, installed;
+    PRBool problem, noInstall, validated, hasKey, unused, installed;
     int error;
 
     PRBool swapUnicodeBytes;
diff --git a/security/nss/lib/pki/pkibase.c b/security/nss/lib/pki/pkibase.c
index 8479df315..626dc6c90 100644
--- a/security/nss/lib/pki/pkibase.c
+++ b/security/nss/lib/pki/pkibase.c
@@ -145,6 +145,23 @@ nssPKIObject_AddInstance
 	for (i=0; i<object->numInstances; i++) {
 	    if (nssCryptokiObject_Equal(object->instances[i], instance)) {
 		PZ_Unlock(object->lock);
+		if (instance->label) {
+		    if (!object->instances[i]->label ||
+		        !nssUTF8_Equal(instance->label,
+		                       object->instances[i]->label, NULL))
+		    {
+			/* Either the old instance did not have a label,
+			 * or the label has changed.
+			 */
+			nss_ZFreeIf(object->instances[i]->label);
+			object->instances[i]->label = instance->label;
+			instance->label = NULL;
+		    }
+		} else if (object->instances[i]->label) {
+		    /* The old label was removed */
+		    nss_ZFreeIf(object->instances[i]->label);
+		    object->instances[i]->label = NULL;
+		}
 		nssCryptokiObject_Destroy(instance);
 		return PR_SUCCESS;
 	    }