Bugzilla bug 298045: when importing a package of CA certs, display and ask

the user to trust the root most CA (instead of the leaf most CA) in the
package.  Also make sure that the cert is indeed a CA cert.  r=kaie,relyea.
sr=sfraser.

security/manager/ssl/src/nsNSSCertificateDB.cpp

@@ -262,13 +262,13 @@ nsNSSCertificateDB::handleCACertDownload(nsIArray *x509Certs,
   // certs which may or may not be a chained list of certs.  Until
   // the day we can design some solid UI for the general case, we'll
   // code to the > 90% case.  That case is where a CA sends down a
-  // list that is a chain up to its root in either ascending or 
-  // descending order.  What we're gonna do is compare the first 
-  // 2 entries, if the first was signed by the second, we assume
-  // the leaf cert is the first cert and display it.  If the second
-  // cert was signed by the first cert, then we assume the first cert
-  // is the root and the last cert in the array is the leaf.  In this
-  // case we display the last cert.
+  // list that is a hierarchy whose root is either the first or 
+  // the last cert.  What we're gonna do is compare the first 
+  // 2 entries, if the second was signed by the first, we assume
+  // the root cert is the first cert and display it.  Otherwise,
+  // we compare the last 2 entries, if the second to last cert was
+  // signed by the last cert, then we assume the last cert is the
+  // root and display it.
 
   nsNSSShutDownPreventionLock locker;
 
@@ -287,35 +287,37 @@ nsNSSCertificateDB::handleCACertDownload(nsIArray *x509Certs,
     selCertIndex = 0;
     certToShow = do_QueryElementAt(x509Certs, selCertIndex);
   } else {
-    nsCOMPtr<nsIX509Cert> cert0;
-    nsCOMPtr<nsIX509Cert> cert1;
+    nsCOMPtr<nsIX509Cert> cert0;    // first cert
+    nsCOMPtr<nsIX509Cert> cert1;    // second cert
+    nsCOMPtr<nsIX509Cert> certn_2;  // second to last cert
+    nsCOMPtr<nsIX509Cert> certn_1;  // last cert
 
     cert0 = do_QueryElementAt(x509Certs, 0);
     cert1 = do_QueryElementAt(x509Certs, 1);
+    certn_2 = do_QueryElementAt(x509Certs, numCerts-2);
+    certn_1 = do_QueryElementAt(x509Certs, numCerts-1);
 
     nsXPIDLString cert0SubjectName;
-    nsXPIDLString cert0IssuerName;
-    nsXPIDLString cert1SubjectName;
     nsXPIDLString cert1IssuerName;
+    nsXPIDLString certn_2IssuerName;
+    nsXPIDLString certn_1SubjectName;
 
-    cert0->GetIssuerName(cert0IssuerName);
     cert0->GetSubjectName(cert0SubjectName);
-
     cert1->GetIssuerName(cert1IssuerName);
-    cert1->GetSubjectName(cert1SubjectName);
+    certn_2->GetIssuerName(certn_2IssuerName);
+    certn_1->GetSubjectName(certn_1SubjectName);
 
     if (cert1IssuerName.Equals(cert0SubjectName)) {
       // In this case, the first cert in the list signed the second,
-      // so the first cert is the root.  Let's display the last cert 
-      // in the list.
-      selCertIndex = numCerts-1;
-      certToShow = do_QueryElementAt(x509Certs, selCertIndex);
-    } else 
-    if (cert0IssuerName.Equals(cert1SubjectName)) { 
-      // In this case the second cert has signed the first cert.  The 
-      // first cert is the leaf, so let's display it.
+      // so the first cert is the root.  Let's display it. 
       selCertIndex = 0;
       certToShow = cert0;
+    } else 
+    if (certn_2IssuerName.Equals(certn_1SubjectName)) { 
+      // In this case the last cert has signed the second to last cert.
+      // The last cert is the root, so let's display it.
+      selCertIndex = numCerts-1;
+      certToShow = certn_1;
     } else {
       // It's not a chain, so let's just show the first one in the 
       // downloaded list.
@@ -356,6 +358,11 @@ nsNSSCertificateDB::handleCACertDownload(nsIArray *x509Certs,
 
   CERTCertificateCleaner tmpCertCleaner(tmpCert);
 
+  if (!CERT_IsCACert(tmpCert, NULL)) {
+    // Should pop up an error dialog.
+    return NS_ERROR_FAILURE;
+  }
+
   if (tmpCert->isperm) {
     nsPSMUITracker tracker;
     if (!tracker.isUIForbidden()) {