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

@@ -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;

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);