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 460692 3700e5761da4fe96986e5b42f165048f72bdedc9
parent 458610 e68cbc3b5b3d3fba4fe3e17e234713020f44e4a0
child 542123 1b45dec8895345c978fbf4a3a0628adb86ff5b09
push id41472
push userdkeeler@mozilla.com
push dateFri, 13 Jan 2017 17:45:16 +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;
@@ -503,43 +511,40 @@ ImportCertsIntoTempStorage(int numcerts,
         != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 static SECStatus
-ImportCertsIntoPermanentStorage(const UniqueCERTCertList& certChain,
-                                const SECCertUsage usage, const bool caOnly)
+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) {
+  if (encounteredFailure) {
+    PR_SetError(savedErrorCode, 0);
     return SECFailure;
   }
 
-  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 SECSuccess;
 }
 
 NS_IMETHODIMP
 nsNSSCertificateDB::ImportEmailCertificate(uint8_t* data, uint32_t length,
                                            nsIInterfaceRequestor* ctx)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown()) {
@@ -588,19 +593,17 @@ 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);
+    SECStatus srv = ImportCertsIntoPermanentStorage(certChain);
     if (srv != SECSuccess) {
       return NS_ERROR_FAILURE;
     }
     CERT_SaveSMimeProfile(node->cert, nullptr, nullptr);
   }
 
   return NS_OK;
 }
@@ -645,18 +648,17 @@ 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);
+    SECStatus srv = ImportCertsIntoPermanentStorage(certChain);
     if (srv != SECSuccess) {
       return NS_ERROR_FAILURE;
     }
   }
 
   return NS_OK;
 }
 
@@ -1274,18 +1276,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