bug 1329360 - avoid some NSS functions that internally use PK11_GetInternalKeySlot r=Cykesiopka
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 06 Jan 2017 16:29:12 -0800
changeset 374989 e6d89beea4279a7ee0c8e3629f696dc08a03825d
parent 374988 fbd02ad448ebecb3b6ad03708b52878c86c95812
child 374990 951b80d391e6b2046ff7e71346b51a8b8b88f9a4
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCykesiopka
bugs1329360
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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