bug 1329360 - avoid some NSS functions that internally use PK11_GetInternalKeySlot r?Cykesiopka draft
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 06 Jan 2017 16:29:12 -0800
changeset 463345 e15adeacb579e9ae65eb833dfd01abdcb069d08f
parent 458610 e68cbc3b5b3d3fba4fe3e17e234713020f44e4a0
child 542649 6b54fa5bce21fc293419abfc218b8157d03d8f42
push id42036
push userdkeeler@mozilla.com
push dateWed, 18 Jan 2017 23:07:25 +0000
reviewersCykesiopka
bugs1329360
milestone53.0a1
bug 1329360 - avoid some NSS functions that internally use PK11_GetInternalKeySlot r?Cykesiopka CERT_AddTempCertToPerm and CERT_ImportCerts (when called with keepCerts=true) internally use PK11_GetInternalKeySlot. The current plan for making NSS always available involves initializing it in memory-only mode and later opening the user's certificate and key databases. Doing so means that PK11_GetInternalKeySlot will not return the right token, so we can't rely on functions that make use of it internally. For now we'll simply use equivalent functions that take an explicit PK11SlotInfo argument and pass in the current internal token. A later patch will change all places where PSM and Gecko use the internal token to use the correct token. MozReview-Commit-ID: CpSo5dIkyVW
security/manager/ssl/moz.build
security/manager/ssl/nsNSSCertificateDB.cpp
security/nss.symbols
--- a/security/manager/ssl/moz.build
+++ b/security/manager/ssl/moz.build
@@ -175,18 +175,16 @@ LOCAL_INCLUDES += [
 if CONFIG['NSS_DISABLE_DBM']:
     DEFINES['NSS_DISABLE_DBM'] = '1'
 
 DEFINES['SSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES'] = 'True'
 DEFINES['NSS_ENABLE_ECC'] = 'True'
 for var in ('DLL_PREFIX', 'DLL_SUFFIX'):
     DEFINES[var] = '"%s"' % CONFIG[var]
 
-DEFINES['CERT_AddTempCertToPerm'] = '__CERT_AddTempCertToPerm'
-
 if not CONFIG['MOZ_SYSTEM_NSS']:
     USE_LIBS += [
         'crmf',
     ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 if CONFIG['GNU_CXX']:
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -340,19 +340,27 @@ nsNSSCertificateDB::handleCACertDownload
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Created nick \"%s\"\n", nickname.get()));
 
   nsNSSCertTrust trust;
   trust.SetValidCA();
   trust.AddCATrust(!!(trustBits & nsIX509CertDB::TRUSTED_SSL),
                    !!(trustBits & nsIX509CertDB::TRUSTED_EMAIL),
                    !!(trustBits & nsIX509CertDB::TRUSTED_OBJSIGN));
 
-  if (CERT_AddTempCertToPerm(tmpCert.get(), nickname.get(),
-                             trust.GetTrust()) != SECSuccess) {
-    return NS_ERROR_FAILURE;
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  SECStatus srv = PK11_ImportCert(slot.get(), tmpCert.get(), CK_INVALID_HANDLE,
+                                  nickname.get(),
+                                  false); // this parameter is ignored by NSS
+  if (srv != SECSuccess) {
+    return MapSECStatus(srv);
+  }
+  // NSS ignores the first argument to CERT_ChangeCertTrust
+  srv = CERT_ChangeCertTrust(nullptr, tmpCert.get(), trust.GetTrust());
+  if (srv != SECSuccess) {
+    return MapSECStatus(srv);
   }
 
   // Import additional delivered certificates that can be verified.
 
   // build a CertList for filtering
   UniqueCERTCertList certList(CERT_NewCertList());
   if (!certList) {
     return NS_ERROR_FAILURE;
@@ -502,44 +510,40 @@ ImportCertsIntoTempStorage(int numcerts,
   if (CERT_FilterCertListByUsage(filteredCerts.get(), usage, caOnly)
         != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
-static SECStatus
-ImportCertsIntoPermanentStorage(const UniqueCERTCertList& certChain,
-                                const SECCertUsage usage, const bool caOnly)
+static nsresult
+ImportCertsIntoPermanentStorage(const UniqueCERTCertList& certChain)
 {
-  int chainLen = 0;
-  for (CERTCertListNode *chainNode = CERT_LIST_HEAD(certChain);
+  bool encounteredFailure = false;
+  PRErrorCode savedErrorCode = 0;
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  for (CERTCertListNode* chainNode = CERT_LIST_HEAD(certChain);
        !CERT_LIST_END(chainNode, certChain);
        chainNode = CERT_LIST_NEXT(chainNode)) {
-    chainLen++;
+    UniquePORTString nickname(CERT_MakeCANickname(chainNode->cert));
+    SECStatus srv = PK11_ImportCert(slot.get(), chainNode->cert,
+                                    CK_INVALID_HANDLE, nickname.get(),
+                                    false); // this parameter is ignored by NSS
+    if (srv != SECSuccess) {
+      encounteredFailure = true;
+      savedErrorCode = PR_GetError();
+    }
   }
 
-  SECItem **rawArray;
-  rawArray = (SECItem **) PORT_Alloc(chainLen * sizeof(SECItem *));
-  if (!rawArray) {
-    return SECFailure;
+  if (encounteredFailure) {
+    return GetXPCOMFromNSSError(savedErrorCode);
   }
 
-  int i = 0;
-  for (CERTCertListNode *chainNode = CERT_LIST_HEAD(certChain);
-       !CERT_LIST_END(chainNode, certChain);
-       chainNode = CERT_LIST_NEXT(chainNode), i++) {
-    rawArray[i] = &chainNode->cert->derCert;
-  }
-  SECStatus srv = CERT_ImportCerts(CERT_GetDefaultCertDB(), usage, chainLen,
-                                   rawArray, nullptr, true, caOnly, nullptr);
-
-  PORT_Free(rawArray);
-  return srv;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::ImportEmailCertificate(uint8_t* data, uint32_t length,
                                            nsIInterfaceRequestor* ctx)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
@@ -588,21 +592,19 @@ nsNSSCertificateDB::ImportEmailCertifica
     mozilla::pkix::Result result =
       certVerifier->VerifyCert(node->cert, certificateUsageEmailRecipient,
                                mozilla::pkix::Now(), ctx, nullptr, certChain);
     if (result != mozilla::pkix::Success) {
       nsCOMPtr<nsIX509Cert> certToShow = nsNSSCertificate::Create(node->cert);
       DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow, locker);
       continue;
     }
-    SECStatus srv = ImportCertsIntoPermanentStorage(certChain,
-                                                    certUsageEmailRecipient,
-                                                    false);
-    if (srv != SECSuccess) {
-      return NS_ERROR_FAILURE;
+    rv = ImportCertsIntoPermanentStorage(certChain);
+    if (NS_FAILED(rv)) {
+      return rv;
     }
     CERT_SaveSMimeProfile(node->cert, nullptr, nullptr);
   }
 
   return NS_OK;
 }
 
 nsresult
@@ -645,20 +647,19 @@ nsNSSCertificateDB::ImportValidCACertsIn
       certVerifier->VerifyCert(node->cert, certificateUsageVerifyCA,
                                mozilla::pkix::Now(), ctx, nullptr, certChain);
     if (result != mozilla::pkix::Success) {
       nsCOMPtr<nsIX509Cert> certToShow = nsNSSCertificate::Create(node->cert);
       DisplayCertificateAlert(ctx, "NotImportingUnverifiedCert", certToShow, proofOfLock);
       continue;
     }
 
-    SECStatus srv = ImportCertsIntoPermanentStorage(certChain, certUsageAnyCA,
-                                                    true);
-    if (srv != SECSuccess) {
-      return NS_ERROR_FAILURE;
+    nsresult rv = ImportCertsIntoPermanentStorage(certChain);
+    if (NS_FAILED(rv)) {
+      return rv;
     }
   }
 
   return NS_OK;
 }
 
 void nsNSSCertificateDB::DisplayCertificateAlert(nsIInterfaceRequestor *ctx, 
                                                  const char *stringID, 
@@ -1274,18 +1275,25 @@ nsNSSCertificateDB::AddCertFromBase64(co
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Created nick \"%s\"\n", nickname.get()));
 
   rv = attemptToLogInWithDefaultPassword();
   if (NS_WARN_IF(rv != NS_OK)) {
     return rv;
   }
 
-  SECStatus srv = CERT_AddTempCertToPerm(tmpCert.get(), nickname.get(),
-                                         trust.GetTrust());
+  UniquePK11SlotInfo slot(PK11_GetInternalKeySlot());
+  SECStatus srv = PK11_ImportCert(slot.get(), tmpCert.get(), CK_INVALID_HANDLE,
+                                  nickname.get(),
+                                  false); // this parameter is ignored by NSS
+  if (srv != SECSuccess) {
+    return MapSECStatus(srv);
+  }
+  // NSS ignores the first argument to CERT_ChangeCertTrust
+  srv = CERT_ChangeCertTrust(nullptr, tmpCert.get(), trust.GetTrust());
   if (srv != SECSuccess) {
     return MapSECStatus(srv);
   }
   newCert.forget(addedCertificate);
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/security/nss.symbols
+++ b/security/nss.symbols
@@ -20,17 +20,16 @@ ATOB_ConvertAsciiToItem
 ATOB_ConvertAsciiToItem_Util
 BTOA_ConvertItemToAscii_Util
 BTOA_DataToAscii
 BTOA_DataToAscii_Util
 CERT_AddCertToListHead
 CERT_AddCertToListTail
 CERT_AddExtension
 CERT_AddExtensionByOID
-__CERT_AddTempCertToPerm
 CERT_AsciiToName
 CERT_CacheOCSPResponseFromSideChannel
 CERT_CertChainFromCert
 CERT_CertificateRequestTemplate @DATA@
 CERT_CertificateTemplate @DATA@
 CERT_CertListFromCert
 CERT_ChangeCertTrust
 CERT_CheckCertUsage