From ce323f5d178aae5fc5768dd53b645566dffff9f6 Mon Sep 17 00:00:00 2001
From: "nelson%bolyard.com" <nelson%bolyard.com>
Date: Sat, 6 Jan 2007 06:05:50 +0000
Subject: [PATCH] Bug 321584. When importing a PKCS#12 file that has no
 friendly names, construct new friendly names, so the import will succeed.
 r=neil.williams

---
 security/nss/cmd/pk12util/pk12util.c | 30 ++++++++++++++++++----------
 security/nss/lib/pkcs12/p12d.c       | 16 +++++++++------
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/security/nss/cmd/pk12util/pk12util.c b/security/nss/cmd/pk12util/pk12util.c
index 6ef521bbe6f8..134a2e43c795 100644
--- a/security/nss/cmd/pk12util/pk12util.c
+++ b/security/nss/cmd/pk12util/pk12util.c
@@ -142,30 +142,40 @@ p12u_InitContext(PRBool fileImport, char *filename)
 SECItem *
 P12U_NicknameCollisionCallback(SECItem *old_nick, PRBool *cancel, void *wincx)
 {
-    if(cancel == NULL) {
-      pk12uErrno = PK12UERR_USER_CANCELLED;
-      return NULL;
+    char           *nick     = NULL;
+    SECItem        *ret_nick = NULL;
+    CERTCertificate* cert    = (CERTCertificate*)wincx;
+
+    if (!cancel || !cert) {
+	pk12uErrno = PK12UERR_USER_CANCELLED;
+	return NULL;
     }
 
     if (!old_nick)
-      fprintf(stdout, "pk12util: no nickname for cert...not handled\n");
+	fprintf(stdout, "pk12util: no nickname for cert in PKCS12 file.\n");
 
+#if 0
     /* XXX not handled yet  */
     *cancel = PR_TRUE;
     return NULL;
 
-#if 0
-    char *nick = NULL;
-    SECItem *ret_nick = NULL;
+#else
 
-    nick = strdup( DEFAULT_CERT_NICKNAME );
+    nick = CERT_MakeCANickname(cert); 
+    if (!nick) {
+    	return NULL;
+    }
 
-    if(old_nick && !PORT_Strcmp((char *)old_nick->data, nick)) {
+    if(old_nick && old_nick->data && old_nick->len &&
+       PORT_Strlen(nick) == old_nick->len &&
+       !PORT_Strncmp((char *)old_nick->data, nick, old_nick->len)) {
 	PORT_Free(nick);
+	PORT_SetError(SEC_ERROR_IO);
 	return NULL;
     }
 
-    ret_nick = (SECItem *)PORT_ZAlloc(sizeof(SECItem));
+    fprintf(stdout, "pk12util: using nickname: %s\n", nick);
+    ret_nick = PORT_ZNew(SECItem);
     if(ret_nick == NULL) {
 	PORT_Free(nick);
 	return NULL;
diff --git a/security/nss/lib/pkcs12/p12d.c b/security/nss/lib/pkcs12/p12d.c
index d37f1c1956ae..74d031a380e8 100644
--- a/security/nss/lib/pkcs12/p12d.c
+++ b/security/nss/lib/pkcs12/p12d.c
@@ -2296,9 +2296,9 @@ sec_pkcs12_validate_cert(sec_PKCS12SafeBag *cert,
 	return;
     }
 
-    CERT_DestroyCertificate(leafCert);
+    sec_pkcs12_validate_cert_nickname(cert, key, nicknameCb, (void *)leafCert);
 
-    sec_pkcs12_validate_cert_nickname(cert, key, nicknameCb, wincx);
+    CERT_DestroyCertificate(leafCert);
 }
 
 static void
@@ -2723,10 +2723,14 @@ SEC_PKCS12DecoderValidateBags(SEC_PKCS12DecoderContext *p12dcx,
 	i++;
     }
 
-    if(bagCnt == noInstallCnt) {
-	PORT_SetError(SEC_ERROR_PKCS12_DUPLICATE_DATA);
-	return SECFailure;
-    }
+    /* formerly was erroneous code here that assumed that if all bags
+     * failed to import, then the problem was duplicated data; 
+     * that is, it assume that the problem must be that the file had
+     * previously been successfully imported.  But importing a 
+     * previously imported file causes NO ERRORS at all, and this 
+     * false assumption caused real errors to be hidden behind false
+     * errors about duplicated data.
+     */
 
     if(probCnt) {
 	PORT_SetError(errorVal);