Bug 1626570 - Improve handling of copying arrays in security/manager/ssl/. r=keeler

Differential Revision: https://phabricator.services.mozilla.com/D72343

security/manager/ssl/PublicKeyPinningService.cpp

@@ -209,7 +209,7 @@ static nsresult FindPinningInformation(
     if (found && (evalHost == hostname || includeSubdomains)) {
       MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
               ("pkpin: Found dyn match for host: '%s'\n", evalHost));
-      dynamicFingerprints = pinArray;
+      dynamicFingerprints = std::move(pinArray);
       return NS_OK;
     }
 

security/manager/ssl/SSLServerCertVerification.cpp

@@ -1099,16 +1099,18 @@ Result AuthCertificate(
   nsTArray<nsTArray<uint8_t>> peerCertsBytes;
   // Don't include the end-entity certificate.
   if (!peerCertChain.IsEmpty()) {
-    peerCertsBytes.AppendElements(peerCertChain.Elements() + 1,
-                                  peerCertChain.Length() - 1);
+    std::transform(
+        peerCertChain.cbegin() + 1, peerCertChain.cend(),
+        MakeBackInserter(peerCertsBytes),
+        [](const auto& elementArray) { return elementArray.Clone(); });
   }
 
   Result rv = certVerifier.VerifySSLServerCert(
       cert, time, aPinArg, aHostName, builtCertChain, certVerifierFlags,
-      Some(peerCertsBytes), stapledOCSPResponse, sctsFromTLSExtension, dcInfo,
-      aOriginAttributes, saveIntermediates, &evOidPolicy, &ocspStaplingStatus,
-      &keySizeStatus, &sha1ModeResult, &pinningTelemetryInfo,
-      &certificateTransparencyInfo, &crliteTelemetryInfo,
+      Some(std::move(peerCertsBytes)), stapledOCSPResponse,
+      sctsFromTLSExtension, dcInfo, aOriginAttributes, saveIntermediates,
+      &evOidPolicy, &ocspStaplingStatus, &keySizeStatus, &sha1ModeResult,
+      &pinningTelemetryInfo, &certificateTransparencyInfo, &crliteTelemetryInfo,
       &aIsCertChainRootBuiltInRoot);
 
   CollectCertTelemetry(rv, evOidPolicy, ocspStaplingStatus, keySizeStatus,
@@ -1528,7 +1530,7 @@ SECStatus AuthCertificateHookWithInfo(
   // we currently only support single stapled responses
   Maybe<nsTArray<uint8_t>> stapledOCSPResponse;
   if (stapledOCSPResponses && (stapledOCSPResponses->Length() == 1)) {
-    stapledOCSPResponse.emplace(stapledOCSPResponses->ElementAt(0));
+    stapledOCSPResponse.emplace(stapledOCSPResponses->ElementAt(0).Clone());
   }
 
   uint32_t certVerifierFlags = 0;

security/manager/ssl/SSLServerCertVerification.h

@@ -93,6 +93,8 @@ class SSLServerCertVerificationResult final
 
 class SSLServerCertVerificationJob : public Runnable {
  public:
+  SSLServerCertVerificationJob(const SSLServerCertVerificationJob&) = delete;
+
   // Must be called only on the socket transport thread
   static SECStatus Dispatch(uint64_t addrForLogging, void* aPinArg,
                             const UniqueCERTCertificate& serverCert,

security/manager/ssl/SecretDecoderRing.cpp

@@ -206,7 +206,8 @@ SecretDecoderRing::AsyncEncryptStrings(const nsTArray<nsCString>& plaintexts,
 
   // plaintexts are already expected to be UTF-8.
   nsCOMPtr<nsIRunnable> runnable(NS_NewRunnableFunction(
-      "BackgroundSdrEncryptStrings", [promise, plaintexts]() mutable {
+      "BackgroundSdrEncryptStrings",
+      [promise, plaintexts = plaintexts.Clone()]() mutable {
         BackgroundSdrEncryptStrings(plaintexts, promise);
       }));
 
@@ -263,7 +264,8 @@ SecretDecoderRing::AsyncDecryptStrings(
 
   // encryptedStrings are expected to be base64.
   nsCOMPtr<nsIRunnable> runnable(NS_NewRunnableFunction(
-      "BackgroundSdrDecryptStrings", [promise, encryptedStrings]() mutable {
+      "BackgroundSdrDecryptStrings",
+      [promise, encryptedStrings = encryptedStrings.Clone()]() mutable {
         BackgroundSdrDecryptStrings(encryptedStrings, promise);
       }));
 

security/manager/ssl/VerifySSLServerCertChild.cpp

@@ -76,8 +76,8 @@ SECStatus RemoteProcessCertVerification(
     return SECFailure;
   }
 
-  nsTArray<uint8_t> serverCertSerialized;
-  serverCertSerialized.AppendElements(aCert->derCert.data, aCert->derCert.len);
+  const ByteArray serverCertSerialized =
+      CopyableTArray<uint8_t>{aCert->derCert.data, aCert->derCert.len};
 
   nsTArray<ByteArray> peerCertBytes;
   for (auto& certBytes : aPeerCertChain) {

security/manager/ssl/VerifySSLServerCertParent.cpp

@@ -141,12 +141,12 @@ bool VerifySSLServerCertParent::Dispatch(
 
   Maybe<nsTArray<uint8_t>> stapledOCSPResponse;
   if (aStapledOCSPResponse) {
-    stapledOCSPResponse.emplace(aStapledOCSPResponse->data());
+    stapledOCSPResponse.emplace(aStapledOCSPResponse->data().Clone());
   }
 
   Maybe<nsTArray<uint8_t>> sctsFromTLSExtension;
   if (aSctsFromTLSExtension) {
-    sctsFromTLSExtension.emplace(aSctsFromTLSExtension->data());
+    sctsFromTLSExtension.emplace(aSctsFromTLSExtension->data().Clone());
   }
 
   Maybe<DelegatedCredentialInfo> dcInfo;

security/manager/ssl/nsNSSIOLayer.cpp

@@ -1815,7 +1815,7 @@ class RemoteClientAuthDataRunnable : public ClientAuthDataRunnable {
  protected:
   virtual void RunOnTargetThread() override;
 
-  nsTArray<ByteArray> mBuiltChain;
+  CopyableTArray<ByteArray> mBuiltChain;
 };
 
 nsTArray<nsTArray<uint8_t>> CollectCANames(CERTDistNames* caNames) {
@@ -2250,7 +2250,7 @@ void ClientAuthDataRunnable::RunOnTargetThread() {
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
-  mEnterpriseCertificates.AppendElements(enterpriseRoots);
+  mEnterpriseCertificates.AppendElements(std::move(enterpriseRoots));
 
   if (NS_WARN_IF(NS_FAILED(CheckForSmartCardChanges()))) {
     return;
@@ -2475,9 +2475,8 @@ mozilla::pkix::Result RemoteClientAuthDataRunnable::BuildChainForCertificate(
 void RemoteClientAuthDataRunnable::RunOnTargetThread() {
   MOZ_ASSERT(NS_IsMainThread());
 
-  nsTArray<uint8_t> serverCertSerialized;
-  serverCertSerialized.AppendElements(mServerCert->derCert.data,
-                                      mServerCert->derCert.len);
+  const ByteArray serverCertSerialized = CopyableTArray<uint8_t>{
+      mServerCert->derCert.data, mServerCert->derCert.len};
 
   // Note that client cert is NULL in socket process until bug 1632809 is done.
   Maybe<ByteArray> clientCertSerialized;

security/manager/ssl/nsSiteSecurityService.cpp

@@ -379,7 +379,7 @@ SiteHPKPState::SiteHPKPState(const nsCString& aHost,
       mExpireTime(aExpireTime),
       mState(aState),
       mIncludeSubdomains(aIncludeSubdomains),
-      mSHA256keys(aSHA256keys) {}
+      mSHA256keys(aSHA256keys.Clone()) {}
 
 NS_IMETHODIMP
 SiteHPKPState::GetHostname(nsACString& aHostname) {
@@ -1662,7 +1662,7 @@ nsSiteSecurityService::GetKeyPinsForHostname(
       foundEntry = privateEntry;
     }
   }
-  pinArray = foundEntry->mSHA256keys;
+  pinArray = foundEntry->mSHA256keys.Clone();
   *aIncludeSubdomains = foundEntry->mIncludeSubdomains;
   *aFound = true;
   return NS_OK;