Bug 1578534 - Change nsIX509CertDB.constructX509 to take Array<uint8_t> r=keeler
authorSean Feng <sefeng@mozilla.com>
Tue, 12 Nov 2019 20:59:02 +0000
changeset 502063 4eb85afc10db43b9c64e0a19adefd78afd616418
parent 502062 78cd6dd843caca297127b2c190295501142cb3bd
child 502064 55c9bc5fb1fe59bb21afcde021cbb3553d17fd9e
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1578534
milestone72.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 1578534 - Change nsIX509CertDB.constructX509 to take Array<uint8_t> r=keeler Differential Revision: https://phabricator.services.mozilla.com/D44730
browser/components/enterprisepolicies/Policies.jsm
netwerk/base/BackgroundFileSaver.cpp
netwerk/base/TLSServerSocket.cpp
security/manager/ssl/nsIX509CertDB.idl
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSCertificateDB.h
security/manager/ssl/tests/unit/head_psm.js
security/manager/ssl/tests/unit/test_cert_utf8.js
security/manager/ssl/tests/unit/test_enterprise_roots.js
toolkit/components/reputationservice/ApplicationReputation.cpp
--- a/browser/components/enterprisepolicies/Policies.jsm
+++ b/browser/components/enterprisepolicies/Policies.jsm
@@ -246,19 +246,23 @@ var Policies = {
             }
             let reader = new FileReader();
             reader.onloadend = function() {
               if (reader.readyState != reader.DONE) {
                 log.error(`Unable to read certificate - ${certfile.path}`);
                 return;
               }
               let certFile = reader.result;
+              let certFileArray = [];
+              for (let i = 0; i < certFile.length; i++) {
+                certFileArray.push(certFile.charCodeAt(i));
+              }
               let cert;
               try {
-                cert = gCertDB.constructX509(certFile);
+                cert = gCertDB.constructX509(certFileArray);
               } catch (e) {
                 try {
                   // It might be PEM instead of DER.
                   cert = gCertDB.constructX509FromBase64(pemToBase64(certFile));
                 } catch (ex) {
                   log.error(`Unable to add certificate - ${certfile.path}`);
                 }
               }
--- a/netwerk/base/BackgroundFileSaver.cpp
+++ b/netwerk/base/BackgroundFileSaver.cpp
@@ -828,21 +828,22 @@ nsresult BackgroundFileSaver::ExtractSig
         bool extractionSuccess = true;
         for (DWORD k = 0; k < certSimpleChain->cElement; ++k) {
           CERT_CHAIN_ELEMENT* certChainElement = certSimpleChain->rgpElement[k];
           if (certChainElement->pCertContext->dwCertEncodingType !=
               X509_ASN_ENCODING) {
             continue;
           }
           nsCOMPtr<nsIX509Cert> nssCert = nullptr;
-          nsDependentCSubstring certDER(
-              reinterpret_cast<char*>(
-                  certChainElement->pCertContext->pbCertEncoded),
+          nsTArray<uint8_t> certByte;
+          certByte.AppendElements(
+              certChainElement->pCertContext->pbCertEncoded,
               certChainElement->pCertContext->cbCertEncoded);
-          rv = certDB->ConstructX509(certDER, getter_AddRefs(nssCert));
+
+          rv = certDB->ConstructX509(certByte, getter_AddRefs(nssCert));
           if (!nssCert) {
             extractionSuccess = false;
             LOG(("Couldn't create NSS cert [this = %p]", this));
             break;
           }
           rv = nssCertList->AddCert(nssCert);
           if (NS_FAILED(rv)) {
             extractionSuccess = false;
--- a/netwerk/base/TLSServerSocket.cpp
+++ b/netwerk/base/TLSServerSocket.cpp
@@ -389,20 +389,20 @@ nsresult TLSServerConnectionInfo::Handsh
   if (clientCert) {
     nsCOMPtr<nsIX509CertDB> certDB =
         do_GetService(NS_X509CERTDB_CONTRACTID, &rv);
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     nsCOMPtr<nsIX509Cert> clientCertPSM;
-    nsDependentCSubstring certDER(
-        reinterpret_cast<char*>(clientCert->derCert.data),
-        clientCert->derCert.len);
-    rv = certDB->ConstructX509(certDER, getter_AddRefs(clientCertPSM));
+    nsTArray<uint8_t> clientCertBytes;
+    clientCertBytes.AppendElements(clientCert->derCert.data,
+                                   clientCert->derCert.len);
+    rv = certDB->ConstructX509(clientCertBytes, getter_AddRefs(clientCertPSM));
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     mPeerCert = clientCertPSM;
   }
 
   SSLChannelInfo channelInfo;
--- a/security/manager/ssl/nsIX509CertDB.idl
+++ b/security/manager/ssl/nsIX509CertDB.idl
@@ -218,17 +218,17 @@ interface nsIX509CertDB : nsISupports {
   /*
    *  Decode a raw data presentation and instantiate an object in memory.
    *
    *  @param certDER The raw representation of a certificate,
    *                 encoded as raw DER.
    *  @return The new certificate object.
    */
   [must_use]
-  nsIX509Cert constructX509(in ACString certDER);
+  nsIX509Cert constructX509(in Array<uint8_t> certDER);
 
   /**
    *  Verifies the signature on the given JAR file to verify that it has a
    *  valid signature.  To be considered valid, there must be exactly one
    *  signature on the JAR file and that signature must have signed every
    *  entry. Further, the signature must come from a certificate that
    *  is trusted for code signing.
    *
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -837,31 +837,41 @@ nsNSSCertificateDB::ConstructX509FromBas
   }
 
   nsAutoCString certDER;
   nsresult rv = Base64Decode(base64, certDER);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  return ConstructX509(certDER, _retval);
+  return ConstructX509FromSpan(AsBytes(MakeSpan(certDER)), _retval);
 }
 
 NS_IMETHODIMP
-nsNSSCertificateDB::ConstructX509(const nsACString& certDER,
+nsNSSCertificateDB::ConstructX509(const nsTArray<uint8_t>& certDER,
                                   nsIX509Cert** _retval) {
+  return ConstructX509FromSpan(MakeSpan(certDER.Elements(), certDER.Length()),
+                               _retval);
+}
+
+nsresult nsNSSCertificateDB::ConstructX509FromSpan(
+    Span<const uint8_t> aInputSpan, nsIX509Cert** _retval) {
   if (NS_WARN_IF(!_retval)) {
     return NS_ERROR_INVALID_POINTER;
   }
 
+  if (aInputSpan.Length() > std::numeric_limits<unsigned int>::max()) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
+
   SECItem certData;
   certData.type = siDERCertBuffer;
-  certData.data =
-      BitwiseCast<unsigned char*, const char*>(certDER.BeginReading());
-  certData.len = certDER.Length();
+  certData.data = const_cast<unsigned char*>(
+      reinterpret_cast<const unsigned char*>(aInputSpan.Elements()));
+  certData.len = aInputSpan.Length();
 
   UniqueCERTCertificate cert(CERT_NewTempCertificate(
       CERT_GetDefaultCertDB(), &certData, nullptr, false, true));
   if (!cert)
     return (PORT_GetError() == SEC_ERROR_NO_MEMORY) ? NS_ERROR_OUT_OF_MEMORY
                                                     : NS_ERROR_FAILURE;
 
   nsCOMPtr<nsIX509Cert> nssCert = nsNSSCertificate::Create(cert.get());
@@ -997,17 +1007,18 @@ nsNSSCertificateDB::AddCert(const nsACSt
 
   nsNSSCertTrust trust;
   if (CERT_DecodeTrustString(&trust.GetTrust(),
                              PromiseFlatCString(aTrust).get()) != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIX509Cert> newCert;
-  nsresult rv = ConstructX509(aCertDER, getter_AddRefs(newCert));
+  nsresult rv = ConstructX509FromSpan(AsBytes(MakeSpan(aCertDER)),
+                                      getter_AddRefs(newCert));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   UniqueCERTCertificate tmpCert(newCert->GetCert());
   if (!tmpCert) {
     return NS_ERROR_FAILURE;
   }
--- a/security/manager/ssl/nsNSSCertificateDB.h
+++ b/security/manager/ssl/nsNSSCertificateDB.h
@@ -48,16 +48,18 @@ class nsNSSCertificateDB final : public 
   static void DisplayCertificateAlert(nsIInterfaceRequestor* ctx,
                                       const char* stringID,
                                       nsIX509Cert* certToShow);
 
   nsresult getCertsFromPackage(nsTArray<nsTArray<uint8_t>>& collectArgs,
                                uint8_t* data, uint32_t length);
   nsresult handleCACertDownload(mozilla::NotNull<nsIArray*> x509Certs,
                                 nsIInterfaceRequestor* ctx);
+  nsresult ConstructX509FromSpan(const mozilla::Span<const uint8_t> aInputSpan,
+                                 nsIX509Cert** _retval);
 };
 
 #define NS_X509CERTDB_CID                            \
   { /* fb0bbc5c-452e-4783-b32c-80124693d871 */       \
     0xfb0bbc5c, 0x452e, 0x4783, {                    \
       0xb3, 0x2c, 0x80, 0x12, 0x46, 0x93, 0xd8, 0x71 \
     }                                                \
   }
--- a/security/manager/ssl/tests/unit/head_psm.js
+++ b/security/manager/ssl/tests/unit/head_psm.js
@@ -201,17 +201,17 @@ function addCertFromFile(certdb, filenam
 
 function constructCertFromFile(filename) {
   let certFile = do_get_file(filename, false);
   let certBytes = readFile(certFile);
   let certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(
     Ci.nsIX509CertDB
   );
   try {
-    return certdb.constructX509(certBytes);
+    return certdb.constructX509(stringToArray(certBytes));
   } catch (e) {}
   // It might be PEM instead of DER.
   return certdb.constructX509FromBase64(pemToBase64(certBytes));
 }
 
 function setCertTrust(cert, trustString) {
   let certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(
     Ci.nsIX509CertDB
--- a/security/manager/ssl/tests/unit/test_cert_utf8.js
+++ b/security/manager/ssl/tests/unit/test_cert_utf8.js
@@ -113,17 +113,17 @@ function testUTF8InField(field, replacem
   let uniqueIssuerReplacement =
     "ALWAYS MAKE ME UNIQU" + String.fromCharCode(gUniqueIssuerCounter);
   bytes = bytes.replace("ALWAYS MAKE ME UNIQUE", uniqueIssuerReplacement);
   ok(
     gUniqueIssuerCounter < 127,
     "should have enough ASCII replacements to make a unique issuer DN"
   );
   gUniqueIssuerCounter++;
-  let cert = gCertDB.constructX509(bytes);
+  let cert = gCertDB.constructX509(stringToArray(bytes));
   notEqual(cert[field], null, `accessing nsIX509Cert.${field} shouldn't fail`);
   notEqual(
     cert.ASN1Structure,
     null,
     "accessing nsIX509Cert.ASN1Structure shouldn't assert"
   );
   notEqual(
     cert.getEmailAddresses(),
--- a/security/manager/ssl/tests/unit/test_enterprise_roots.js
+++ b/security/manager/ssl/tests/unit/test_enterprise_roots.js
@@ -33,36 +33,28 @@ async function check_no_enterprise_roots
     // imported enterprise root certificates. If so, they shouldn't be trusted
     // to issue TLS server auth certificates.
     if (cert) {
       await asyncTestCertificateUsages(certDB, cert, []);
     }
   }
 }
 
-function der_array_to_string(derArray) {
-  let derString = "";
-  for (let b of derArray) {
-    derString += String.fromCharCode(b);
-  }
-  return derString;
-}
-
 async function check_some_enterprise_roots_imported(nssComponent, certDB) {
   let enterpriseRoots = nssComponent.getEnterpriseRoots();
   notEqual(enterpriseRoots, null, "enterprise roots list should not be null");
   notEqual(
     enterpriseRoots.length,
     0,
     "should have imported some enterprise roots"
   );
   let foundNonBuiltIn = false;
   let savedDBKey = null;
   for (let certDer of enterpriseRoots) {
-    let cert = certDB.constructX509(der_array_to_string(certDer));
+    let cert = certDB.constructX509(certDer);
     notEqual(cert, null, "should be able to decode cert from DER");
     if (!cert.isBuiltInRoot && !savedDBKey) {
       foundNonBuiltIn = true;
       savedDBKey = cert.dbKey;
       info("saving dbKey from " + cert.commonName);
       await asyncTestCertificateUsages(certDB, cert, [certificateUsageSSLCA]);
       break;
     }
--- a/toolkit/components/reputationservice/ApplicationReputation.cpp
+++ b/toolkit/components/reputationservice/ApplicationReputation.cpp
@@ -1186,29 +1186,29 @@ nsresult PendingLookup::GenerateWhitelis
   }
 
   // Get the signer.
   nsresult rv;
   nsCOMPtr<nsIX509CertDB> certDB = do_GetService(NS_X509CERTDB_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIX509Cert> signer;
-  nsDependentCSubstring signerDER(
-      const_cast<char*>(aChain.element(0).certificate().data()),
-      aChain.element(0).certificate().size());
-  rv = certDB->ConstructX509(signerDER, getter_AddRefs(signer));
+  nsTArray<uint8_t> signerBytes;
+  signerBytes.AppendElements(aChain.element(0).certificate().data(),
+                             aChain.element(0).certificate().size());
+  rv = certDB->ConstructX509(signerBytes, getter_AddRefs(signer));
   NS_ENSURE_SUCCESS(rv, rv);
 
   for (int i = 1; i < aChain.element_size(); ++i) {
     // Get the issuer.
     nsCOMPtr<nsIX509Cert> issuer;
-    nsDependentCSubstring issuerDER(
-        const_cast<char*>(aChain.element(i).certificate().data()),
-        aChain.element(i).certificate().size());
-    rv = certDB->ConstructX509(issuerDER, getter_AddRefs(issuer));
+    nsTArray<uint8_t> issuerBytes;
+    issuerBytes.AppendElements(aChain.element(i).certificate().data(),
+                               aChain.element(i).certificate().size());
+    rv = certDB->ConstructX509(issuerBytes, getter_AddRefs(issuer));
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = GenerateWhitelistStringsForPair(signer, issuer);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }