Bug 1260644 - Use UniquePLArenaPool to manage PLArenaPools in PSM. draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Mon, 04 Apr 2016 17:35:24 -0700
changeset 347493 d28fba342a3229c43789652f4916fc17888ba7bb
parent 347474 76134d6ad6365377ae629a783a8446e22280d073
child 517645 2bd41ad2d661c269287a1e429c9c9c2c7d3e898e
push id14597
push usercykesiopka.bmo@gmail.com
push dateTue, 05 Apr 2016 01:20:34 +0000
bugs1260644
milestone48.0a1
Bug 1260644 - Use UniquePLArenaPool to manage PLArenaPools in PSM. MozReview-Commit-ID: HyLXbWoHMGz
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/OCSPRequestor.cpp
security/certverifier/OCSPRequestor.h
security/manager/ssl/SSLServerCertVerification.cpp
security/manager/ssl/TransportSecurityInfo.cpp
security/manager/ssl/nsDataSignatureVerifier.cpp
security/manager/ssl/nsKeygenHandler.cpp
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/tests/unit/tlsserver/cmd/GenerateOCSPResponse.cpp
security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.h
security/manager/ssl/tests/unit/tlsserver/lib/TLSServer.cpp
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -279,29 +279,34 @@ OCSPFetchingTypeToTimeoutTime(NSSCertDBT
       break;
   }
 
   PR_NOT_REACHED("we're not handling every OCSPFetching type");
   return PR_SecondsToInterval(2);
 }
 
 // Copied and modified from CERT_GetOCSPAuthorityInfoAccessLocation and
-// CERT_GetGeneralNameByType. Returns SECFailure on error, SECSuccess
-// with url == nullptr when an OCSP URI was not found, and SECSuccess with
+// CERT_GetGeneralNameByType. Returns a non-Result::Success result on error,
+// Success with url == nullptr when an OCSP URI was not found, and Success with
 // url != nullptr when an OCSP URI was found. The output url will be owned
 // by the arena.
 static Result
-GetOCSPAuthorityInfoAccessLocation(PLArenaPool* arena,
+GetOCSPAuthorityInfoAccessLocation(const UniquePLArenaPool& arena,
                                    Input aiaExtension,
                                    /*out*/ char const*& url)
 {
+  MOZ_ASSERT(arena.get());
+  if (!arena.get()) {
+    return Result::FATAL_ERROR_INVALID_ARGS;
+  }
+
   url = nullptr;
   SECItem aiaExtensionSECItem = UnsafeMapInputToSECItem(aiaExtension);
   CERTAuthInfoAccess** aia =
-    CERT_DecodeAuthInfoAccessExtension(arena, &aiaExtensionSECItem);
+    CERT_DecodeAuthInfoAccessExtension(arena.get(), &aiaExtensionSECItem);
   if (!aia) {
     return Result::ERROR_CERT_BAD_ACCESS_LOCATION;
   }
   for (size_t i = 0; aia[i]; ++i) {
     if (SECOID_FindOIDTag(&aia[i]->method) == SEC_OID_PKIX_OCSP) {
       // NSS chooses the **last** OCSP URL; we choose the **first**
       CERTGeneralName* current = aia[i]->location;
       if (!current) {
@@ -312,18 +317,18 @@ GetOCSPAuthorityInfoAccessLocation(PLAre
           const SECItem& location = current->name.other;
           // (location.len + 1) must be small enough to fit into a uint32_t,
           // but we limit it to a smaller bound to reduce OOM risk.
           if (location.len > 1024 || memchr(location.data, 0, location.len)) {
             // Reject embedded nulls. (NSS doesn't do this)
             return Result::ERROR_CERT_BAD_ACCESS_LOCATION;
           }
           // Copy the non-null-terminated SECItem into a null-terminated string.
-          char* nullTerminatedURL(static_cast<char*>(
-                                    PORT_ArenaAlloc(arena, location.len + 1)));
+          char* nullTerminatedURL(
+            static_cast<char*>(PORT_ArenaAlloc(arena.get(), location.len + 1)));
           if (!nullTerminatedURL) {
             return Result::FATAL_ERROR_NO_MEMORY;
           }
           memcpy(nullTerminatedURL, location.data, location.len);
           nullTerminatedURL[location.len] = 0;
           url = nullTerminatedURL;
           return Success;
         }
@@ -490,26 +495,26 @@ NSSCertDBTrustDomain::CheckRevocation(En
 
   if (mOCSPFetching == LocalOnlyOCSPForEV) {
     if (cachedResponseResult != Success) {
       return cachedResponseResult;
     }
     return Result::ERROR_OCSP_UNKNOWN_CERT;
   }
 
-  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return Result::FATAL_ERROR_NO_MEMORY;
   }
 
   Result rv;
   const char* url = nullptr; // owned by the arena
 
   if (aiaExtension) {
-    rv = GetOCSPAuthorityInfoAccessLocation(arena.get(), *aiaExtension, url);
+    rv = GetOCSPAuthorityInfoAccessLocation(arena, *aiaExtension, url);
     if (rv != Success) {
       return rv;
     }
   }
 
   if (!url) {
     if (mOCSPFetching == FetchOCSPForEV ||
         cachedResponseResult == Result::ERROR_OCSP_UNKNOWN_CERT) {
@@ -546,17 +551,17 @@ NSSCertDBTrustDomain::CheckRevocation(En
     SECItem ocspRequestItem = {
       siBuffer,
       ocspRequest,
       static_cast<unsigned int>(ocspRequestLength)
     };
     // Owned by arena
     SECItem* responseSECItem = nullptr;
     Result tempRV =
-      DoOCSPRequest(arena.get(), url, &ocspRequestItem,
+      DoOCSPRequest(arena, url, &ocspRequestItem,
                     OCSPFetchingTypeToTimeoutTime(mOCSPFetching),
                     mOCSPGetConfig == CertVerifier::ocspGetEnabled,
                     responseSECItem);
     MOZ_ASSERT((tempRV != Success) || responseSECItem);
     if (tempRV != Success) {
       rv = tempRV;
     } else if (response.Init(responseSECItem->data, responseSECItem->len)
                  != Success) {
--- a/security/certverifier/OCSPRequestor.cpp
+++ b/security/certverifier/OCSPRequestor.cpp
@@ -66,22 +66,26 @@ AppendEscapedBase64Item(const SECItem* e
   base64Request.ReplaceSubstring("+", "%2B");
   base64Request.ReplaceSubstring("/", "%2F");
   base64Request.ReplaceSubstring("=", "%3D");
   path.Append(base64Request);
   return NS_OK;
 }
 
 Result
-DoOCSPRequest(PLArenaPool* arena, const char* url,
+DoOCSPRequest(const UniquePLArenaPool& arena, const char* url,
               const SECItem* encodedRequest, PRIntervalTime timeout,
               bool useGET,
       /*out*/ SECItem*& encodedResponse)
 {
-  if (!arena || !url || !encodedRequest || !encodedRequest->data) {
+  MOZ_ASSERT(arena.get());
+  MOZ_ASSERT(url);
+  MOZ_ASSERT(encodedRequest);
+  MOZ_ASSERT(encodedRequest->data);
+  if (!arena.get() || !url || !encodedRequest || !encodedRequest->data) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
   uint32_t urlLen = PL_strlen(url);
   if (urlLen > static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
 
   nsCOMPtr<nsIURLParser> urlParser = do_GetService(NS_STDURLPARSER_CONTRACTID);
@@ -195,17 +199,17 @@ DoOCSPRequest(PLArenaPool* arena, const 
   if (rv != Success) {
     return rv;
   }
 
   if (httpResponseCode != 200) {
     return Result::ERROR_OCSP_SERVER_ERROR;
   }
 
-  encodedResponse = SECITEM_AllocItem(arena, nullptr, httpResponseDataLen);
+  encodedResponse = SECITEM_AllocItem(arena.get(), nullptr, httpResponseDataLen);
   if (!encodedResponse) {
     return Result::FATAL_ERROR_NO_MEMORY;
   }
 
   memcpy(encodedResponse->data, httpResponseData, httpResponseDataLen);
   return Success;
 }
 
--- a/security/certverifier/OCSPRequestor.h
+++ b/security/certverifier/OCSPRequestor.h
@@ -1,23 +1,23 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef mozilla_psm_OCSPRequestor_h
-#define mozilla_psm_OCSPRequestor_h
+#ifndef OCSPRequestor_h
+#define OCSPRequestor_h
 
 #include "CertVerifier.h"
 #include "secmodt.h"
 
 namespace mozilla { namespace psm {
 
 // The memory returned via |encodedResponse| is owned by the given arena.
-Result DoOCSPRequest(PLArenaPool* arena, const char* url,
+Result DoOCSPRequest(const UniquePLArenaPool& arena, const char* url,
                      const SECItem* encodedRequest, PRIntervalTime timeout,
                      bool useGET,
              /*out*/ SECItem*& encodedResponse);
 
 } } // namespace mozilla::psm
 
-#endif // mozilla_psm_OCSPRequestor_h
+#endif // OCSPRequestor_h
--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -930,19 +930,19 @@ GatherBaselineRequirementsTelemetry(cons
            ("BR telemetry: no subject alt names extension for '%s'\n",
             commonName.get()));
     // 1 means there is no subject alt names extension
     Telemetry::Accumulate(Telemetry::BR_9_2_1_SUBJECT_ALT_NAMES, 1);
     AccumulateSubjectCommonNameTelemetry(commonName.get(), false);
     return;
   }
 
-  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   CERTGeneralName* subjectAltNames =
-    CERT_DecodeAltNameExtension(arena, &altNameExtension);
+    CERT_DecodeAltNameExtension(arena.get(), &altNameExtension);
   // CERT_FindCertExtension takes a pointer to a SECItem and allocates memory
   // in its data field. This is a bad way to do this because we can't use a
   // ScopedSECItem and neither is that memory tracked by an arena. We have to
   // manually reach in and free the memory.
   PORT_Free(altNameExtension.data);
   if (!subjectAltNames) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("BR telemetry: could not decode subject alt names for '%s'\n",
--- a/security/manager/ssl/TransportSecurityInfo.cpp
+++ b/security/manager/ssl/TransportSecurityInfo.cpp
@@ -635,17 +635,17 @@ GetSubjectAltNames(CERTCertificate *nssC
   CERTGeneralName *sanNameList = nullptr;
 
   SECStatus rv = CERT_FindCertExtension(nssCert, SEC_OID_X509_SUBJECT_ALT_NAME,
                                         &altNameExtension);
   if (rv != SECSuccess) {
     return false;
   }
 
-  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return false;
   }
 
   sanNameList = CERT_DecodeAltNameExtension(arena.get(), &altNameExtension);
   if (!sanNameList) {
     return false;
   }
--- a/security/manager/ssl/nsDataSignatureVerifier.cpp
+++ b/security/manager/ssl/nsDataSignatureVerifier.cpp
@@ -37,81 +37,75 @@ const SEC_ASN1Template CERT_SignatureDat
 
 NS_IMETHODIMP
 nsDataSignatureVerifier::VerifyData(const nsACString & aData,
                                     const nsACString & aSignature,
                                     const nsACString & aPublicKey,
                                     bool *_retval)
 {
     // Allocate an arena to handle the majority of the allocations
-    PLArenaPool *arena;
-    arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-    if (!arena)
+    UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
         return NS_ERROR_OUT_OF_MEMORY;
+    }
 
     // Base 64 decode the key
     SECItem keyItem;
     PORT_Memset(&keyItem, 0, sizeof(SECItem));
-    if (!NSSBase64_DecodeBuffer(arena, &keyItem,
+    if (!NSSBase64_DecodeBuffer(arena.get(), &keyItem,
                                 nsPromiseFlatCString(aPublicKey).get(),
                                 aPublicKey.Length())) {
-        PORT_FreeArena(arena, false);
         return NS_ERROR_FAILURE;
     }
-    
+
     // Extract the public key from the data
     CERTSubjectPublicKeyInfo *pki = SECKEY_DecodeDERSubjectPublicKeyInfo(&keyItem);
     if (!pki) {
-        PORT_FreeArena(arena, false);
         return NS_ERROR_FAILURE;
     }
     SECKEYPublicKey *publicKey = SECKEY_ExtractPublicKey(pki);
     SECKEY_DestroySubjectPublicKeyInfo(pki);
     pki = nullptr;
-    
+
     if (!publicKey) {
-        PORT_FreeArena(arena, false);
         return NS_ERROR_FAILURE;
     }
-    
+
     // Base 64 decode the signature
     SECItem signatureItem;
     PORT_Memset(&signatureItem, 0, sizeof(SECItem));
-    if (!NSSBase64_DecodeBuffer(arena, &signatureItem,
+    if (!NSSBase64_DecodeBuffer(arena.get(), &signatureItem,
                                 nsPromiseFlatCString(aSignature).get(),
                                 aSignature.Length())) {
         SECKEY_DestroyPublicKey(publicKey);
-        PORT_FreeArena(arena, false);
         return NS_ERROR_FAILURE;
     }
-    
+
     // Decode the signature and algorithm
     CERTSignedData sigData;
     PORT_Memset(&sigData, 0, sizeof(CERTSignedData));
-    SECStatus ss = SEC_QuickDERDecodeItem(arena, &sigData, 
+    SECStatus ss = SEC_QuickDERDecodeItem(arena.get(), &sigData,
                                           CERT_SignatureDataTemplate,
                                           &signatureItem);
     if (ss != SECSuccess) {
         SECKEY_DestroyPublicKey(publicKey);
-        PORT_FreeArena(arena, false);
         return NS_ERROR_FAILURE;
     }
-    
+
     // Perform the final verification
     DER_ConvertBitString(&(sigData.signature));
     ss = VFY_VerifyDataWithAlgorithmID((const unsigned char*)nsPromiseFlatCString(aData).get(),
                                        aData.Length(), publicKey,
                                        &(sigData.signature),
                                        &(sigData.signatureAlgorithm),
                                        nullptr, nullptr);
-    
+
     // Clean up remaining objects
     SECKEY_DestroyPublicKey(publicKey);
-    PORT_FreeArena(arena, false);
-    
+
     *_retval = (ss == SECSuccess);
 
     return NS_OK;
 }
 
 namespace mozilla {
 
 nsresult
--- a/security/manager/ssl/nsKeygenHandler.cpp
+++ b/security/manager/ssl/nsKeygenHandler.cpp
@@ -465,46 +465,45 @@ nsKeygenFormProcessor::GetPublicKey(cons
     PK11SlotInfo *slot = nullptr;
     PK11RSAGenParams rsaParams;
     SECOidTag algTag;
     int keysize = 0;
     void *params = nullptr;
     SECKEYPrivateKey *privateKey = nullptr;
     SECKEYPublicKey *publicKey = nullptr;
     CERTSubjectPublicKeyInfo *spkInfo = nullptr;
-    PLArenaPool *arena = nullptr;
-    SECStatus sec_rv = SECFailure;
+    SECStatus srv = SECFailure;
     SECItem spkiItem;
     SECItem pkacItem;
     SECItem signedItem;
     CERTPublicKeyAndChallenge pkac;
     pkac.challenge.data = nullptr;
     nsIGeneratingKeypairInfoDialogs * dialogs;
     nsKeygenThread *KeygenRunnable = 0;
     nsCOMPtr<nsIKeygenThread> runnable;
 
     // permanent and sensitive flags for keygen
     PK11AttrFlags attrFlags = PK11_ATTR_TOKEN | PK11_ATTR_SENSITIVE | PK11_ATTR_PRIVATE;
 
+    UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
+        goto loser;
+    }
+
     // Get the key size //
     for (size_t i = 0; i < number_of_key_size_choices; ++i) {
         if (aValue.Equals(mSECKeySizeChoiceList[i].name)) {
             keysize = mSECKeySizeChoiceList[i].size;
             break;
         }
     }
     if (!keysize) {
         goto loser;
     }
 
-    arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-    if (!arena) {
-        goto loser;
-    }
-
     // Set the keygen mechanism
     if (aKeyType.IsEmpty() || aKeyType.LowerCaseEqualsLiteral("rsa")) {
         keyGenMechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;
     } else if (aKeyType.LowerCaseEqualsLiteral("ec")) {
         keyparamsString = ToNewCString(aKeyParams);
         if (!keyparamsString) {
             rv = NS_ERROR_OUT_OF_MEMORY;
             goto loser;
@@ -574,18 +573,18 @@ nsKeygenFormProcessor::GetPublicKey(cons
           goto loser;
       }
 
     /* Make sure token is initialized. */
     rv = setPassword(slot, m_ctx, locker);
     if (NS_FAILED(rv))
         goto loser;
 
-    sec_rv = PK11_Authenticate(slot, true, m_ctx);
-    if (sec_rv != SECSuccess) {
+    srv = PK11_Authenticate(slot, true, m_ctx);
+    if (srv != SECSuccess) {
         goto loser;
     }
 
     rv = getNSSDialogs((void**)&dialogs,
                        NS_GET_IID(nsIGeneratingKeypairInfoDialogs),
                        NS_GENERATINGKEYPAIRINFODIALOGS_CONTRACTID);
 
     if (NS_SUCCEEDED(rv)) {
@@ -628,86 +627,85 @@ nsKeygenFormProcessor::GetPublicKey(cons
 
     /*
      * Create a subject public key info from the public key.
      */
     spkInfo = SECKEY_CreateSubjectPublicKeyInfo(publicKey);
     if ( !spkInfo ) {
         goto loser;
     }
-    
+
     /*
      * Now DER encode the whole subjectPublicKeyInfo.
      */
-    sec_rv=DER_Encode(arena, &spkiItem, CERTSubjectPublicKeyInfoTemplate, spkInfo);
-    if (sec_rv != SECSuccess) {
+    srv = DER_Encode(arena.get(), &spkiItem, CERTSubjectPublicKeyInfoTemplate,
+                     spkInfo);
+    if (srv != SECSuccess) {
         goto loser;
     }
 
     /*
      * set up the PublicKeyAndChallenge data structure, then DER encode it
      */
     pkac.spki = spkiItem;
     pkac.challenge.len = aChallenge.Length();
     pkac.challenge.data = (unsigned char *)ToNewCString(aChallenge);
     if (!pkac.challenge.data) {
         rv = NS_ERROR_OUT_OF_MEMORY;
         goto loser;
     }
-    
-    sec_rv = DER_Encode(arena, &pkacItem, CERTPublicKeyAndChallengeTemplate, &pkac);
-    if ( sec_rv != SECSuccess ) {
+
+    srv = DER_Encode(arena.get(), &pkacItem, CERTPublicKeyAndChallengeTemplate,
+                     &pkac);
+    if (srv != SECSuccess) {
         goto loser;
     }
 
     /*
      * now sign the DER encoded PublicKeyAndChallenge
      */
-    sec_rv = SEC_DerSignData(arena, &signedItem, pkacItem.data, pkacItem.len,
-			 privateKey, algTag);
-    if ( sec_rv != SECSuccess ) {
+    srv = SEC_DerSignData(arena.get(), &signedItem, pkacItem.data, pkacItem.len,
+                          privateKey, algTag);
+    if (srv != SECSuccess) {
         goto loser;
     }
-    
+
     /*
      * Convert the signed public key and challenge into base64/ascii.
      */
     keystring = BTOA_DataToAscii(signedItem.data, signedItem.len);
     if (!keystring) {
         rv = NS_ERROR_OUT_OF_MEMORY;
         goto loser;
     }
 
     CopyASCIItoUTF16(keystring, aOutPublicKey);
     free(keystring);
 
     rv = NS_OK;
 
     GatherKeygenTelemetry(keyGenMechanism, keysize, keyparamsString);
 loser:
-    if ( sec_rv != SECSuccess ) {
+    if (srv != SECSuccess) {
         if ( privateKey ) {
             PK11_DestroyTokenObject(privateKey->pkcs11Slot,privateKey->pkcs11ID);
         }
         if ( publicKey ) {
             PK11_DestroyTokenObject(publicKey->pkcs11Slot,publicKey->pkcs11ID);
         }
     }
     if ( spkInfo ) {
         SECKEY_DestroySubjectPublicKeyInfo(spkInfo);
     }
     if ( publicKey ) {
         SECKEY_DestroyPublicKey(publicKey);
     }
     if ( privateKey ) {
         SECKEY_DestroyPrivateKey(privateKey);
     }
-    if ( arena ) {
-        PORT_FreeArena(arena, true);
-    }
     if (slot) {
         PK11_FreeSlot(slot);
     }
     if (KeygenRunnable) {
         NS_RELEASE(KeygenRunnable);
     }
     if (keyparamsString) {
         free(keyparamsString);
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -1228,26 +1228,26 @@ nsNSSCertificate::ExportAsCMS(uint32_t c
     Unused << sigd.release();
   }
   else {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("nsNSSCertificate::ExportAsCMS - can't attach SignedData\n"));
     return NS_ERROR_FAILURE;
   }
 
-  ScopedPLArenaPool arena(PORT_NewArena(1024));
+  UniquePLArenaPool arena(PORT_NewArena(1024));
   if (!arena) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("nsNSSCertificate::ExportAsCMS - out of memory\n"));
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   SECItem certP7 = { siBuffer, nullptr, 0 };
   NSSCMSEncoderContext* ecx = NSS_CMSEncoder_Start(cmsg.get(), nullptr, nullptr,
-                                                   &certP7, arena, nullptr,
+                                                   &certP7, arena.get(), nullptr,
                                                    nullptr, nullptr, nullptr,
                                                    nullptr, nullptr);
   if (!ecx) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("nsNSSCertificate::ExportAsCMS - can't create encoder context\n"));
     return NS_ERROR_FAILURE;
   }
 
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1847,19 +1847,27 @@ nsSSLIOLayerNewSocket(int32_t family,
 //
 // - arena: arena to allocate strings on
 // - caNameStrings: filled with CA names strings on return
 // - caNames: CERTDistNames to extract strings from
 // - return: SECSuccess if successful; error code otherwise
 //
 // Note: copied in its entirety from Nova code
 static SECStatus
-nsConvertCANamesToStrings(PLArenaPool* arena, char** caNameStrings,
+nsConvertCANamesToStrings(const UniquePLArenaPool& arena, char** caNameStrings,
                           CERTDistNames* caNames)
 {
+    MOZ_ASSERT(arena.get());
+    MOZ_ASSERT(caNameStrings);
+    MOZ_ASSERT(caNames);
+    if (!arena.get() || !caNameStrings || !caNames) {
+        PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
+        return SECFailure;
+    }
+
     SECItem* dername;
     SECStatus rv;
     int headerlen;
     uint32_t contentlen;
     SECItem newitem;
     int n;
     char* namestring;
 
@@ -1909,17 +1917,17 @@ nsConvertCANamesToStrings(PLArenaPool* a
             dername = &newitem;
         }
 
         namestring = CERT_DerNameToAscii(dername);
         if (!namestring) {
             // XXX - keep going until we fail to convert the name
             caNameStrings[n] = const_cast<char*>("");
         } else {
-            caNameStrings[n] = PORT_ArenaStrdup(arena, namestring);
+            caNameStrings[n] = PORT_ArenaStrdup(arena.get(), namestring);
             PR_Free(namestring);
             if (!caNameStrings[n]) {
                 goto loser;
             }
         }
 
         if (newitem.data) {
             PR_Free(newitem.data);
@@ -2086,17 +2094,17 @@ nsNSS_SSLGetClientAuthData(void* arg, PR
   }
 
   return runnable->mRV;
 }
 
 void
 ClientAuthDataRunnable::RunOnTargetThread()
 {
-  PLArenaPool* arena = nullptr;
+  UniquePLArenaPool arena;
   char** caNameStrings;
   ScopedCERTCertificate cert;
   UniqueSECKEYPrivateKey privKey;
   ScopedCERTCertList certList;
   CERTCertListNode* node;
   UniqueCERTCertNicknames nicknames;
   int keyError = 0; // used for private key retrieval error
   SSM_UserCertChoice certChoice;
@@ -2123,23 +2131,23 @@ ClientAuthDataRunnable::RunOnTargetThrea
 
     *mPRetCert = cert.forget();
     *mPRetKey = privKey.release();
     mRV = SECSuccess;
     return;
   }
 
   // create caNameStrings
-  arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
+  arena.reset(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     goto loser;
   }
 
-  caNameStrings = (char**) PORT_ArenaAlloc(arena,
-                                           sizeof(char*) * (mCANames->nnames));
+  caNameStrings = static_cast<char**>(
+    PORT_ArenaAlloc(arena.get(), sizeof(char*) * mCANames->nnames));
   if (!caNameStrings) {
     goto loser;
   }
 
   mRV = nsConvertCANamesToStrings(arena, caNameStrings, mCANames);
   if (mRV != SECSuccess) {
     goto loser;
   }
@@ -2445,20 +2453,16 @@ ClientAuthDataRunnable::RunOnTargetThrea
 noCert:
 loser:
   if (mRV == SECSuccess) {
     mRV = SECFailure;
   }
 done:
   int error = PR_GetError();
 
-  if (arena) {
-    PORT_FreeArena(arena, false);
-  }
-
   *mPRetCert = cert.forget();
   *mPRetKey = privKey.release();
 
   if (mRV == SECFailure) {
     mErrorCodeToReport = error;
   }
 }
 
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/GenerateOCSPResponse.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/GenerateOCSPResponse.cpp
@@ -121,17 +121,17 @@ main(int argc, char* argv[])
                           argv[0]);
     exit(EXIT_FAILURE);
   }
   SECStatus rv = InitializeNSS(argv[1]);
   if (rv != SECSuccess) {
     PR_fprintf(PR_STDERR, "Failed to initialize NSS\n");
     exit(EXIT_FAILURE);
   }
-  PLArenaPool* arena = PORT_NewArena(256 * argc);
+  UniquePLArenaPool arena(PORT_NewArena(256 * argc));
   if (!arena) {
     PrintPRError("PORT_NewArena failed");
     exit(EXIT_FAILURE);
   }
 
   for (int i = 2; i + 3 < argc; i += 4) {
     const char* ocspTypeText  = argv[i];
     const char* certNick      = argv[i + 1];
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/OCSPStaplingServer.cpp
@@ -89,33 +89,31 @@ DoSNISocketConfig(PRFileDesc *aFd, const
     return SSL_SNI_SEND_ALERT;
   }
 
   // If the OCSP response type is "none", don't staple a response.
   if (host->mORT == ORTNone) {
     return 0;
   }
 
-  PLArenaPool *arena = PORT_NewArena(1024);
+  UniquePLArenaPool arena(PORT_NewArena(1024));
   if (!arena) {
     PrintPRError("PORT_NewArena failed");
     return SSL_SNI_SEND_ALERT;
   }
 
   // response is contained by the arena - freeing the arena will free it
   SECItemArray *response = GetOCSPResponseForType(host->mORT, cert, arena,
                                                   host->mAdditionalCertName);
   if (!response) {
-    PORT_FreeArena(arena, PR_FALSE);
     return SSL_SNI_SEND_ALERT;
   }
 
   // SSL_SetStapledOCSPResponses makes a deep copy of response
   SECStatus st = SSL_SetStapledOCSPResponses(aFd, response, certKEA);
-  PORT_FreeArena(arena, PR_FALSE);
   if (st != SECSuccess) {
     PrintPRError("SSL_SetStapledOCSPResponses failed");
     return SSL_SNI_SEND_ALERT;
   }
 
   return 0;
 }
 
--- a/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.cpp
@@ -35,30 +35,36 @@ CreateTestKeyPairFromCert(CERTCertificat
   }
   ScopedSECKEYPublicKey publicKey(CERT_ExtractPublicKey(&cert));
   if (!publicKey) {
     return nullptr;
   }
   return CreateTestKeyPair(RSA_PKCS1(), *publicKey, privateKey.release());
 }
 
-SECItemArray *
-GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate *aCert,
-                       PLArenaPool *aArena, const char *aAdditionalCertName)
+SECItemArray*
+GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate* aCert,
+                       const UniquePLArenaPool& aArena,
+                       const char* aAdditionalCertName)
 {
+  MOZ_ASSERT(aArena.get());
+  MOZ_ASSERT(aCert);
+  // Note: |aAdditionalCertName| may or may not need to be non-null depending
+  //       on the |aORT| value given.
+
   if (aORT == ORTNone) {
     if (gDebugLevel >= DEBUG_WARNINGS) {
       fprintf(stderr, "GetOCSPResponseForType called with type ORTNone, "
                       "which makes no sense.\n");
     }
     return nullptr;
   }
 
   if (aORT == ORTEmpty) {
-    SECItemArray* arr = SECITEM_AllocArray(aArena, nullptr, 1);
+    SECItemArray* arr = SECITEM_AllocArray(aArena.get(), nullptr, 1);
     arr->items[0].data = nullptr;
     arr->items[0].len = 0;
     return arr;
   }
 
   time_t now = time(nullptr);
   time_t oldNow = now - (8 * Time::ONE_DAY_IN_SECONDS);
 
@@ -201,10 +207,10 @@ GetOCSPResponseForType(OCSPResponseType 
   }
 
   SECItem item = {
     siBuffer,
     const_cast<uint8_t*>(response.data()),
     static_cast<unsigned int>(response.length())
   };
   SECItemArray arr = { &item, 1 };
-  return SECITEM_DupArray(aArena, &arr);
+  return SECITEM_DupArray(aArena.get(), &arr);
 }
--- a/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.h
+++ b/security/manager/ssl/tests/unit/tlsserver/lib/OCSPCommon.h
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // Implements generating OCSP responses of various types. Used by the
 // programs in tlsserver/cmd.
 
 #ifndef OCSPCommon_h
 #define OCSPCommon_h
 
+#include "ScopedNSSTypes.h"
 #include "certt.h"
 #include "seccomon.h"
 
 enum OCSPResponseType
 {
   ORTNull = 0,
   ORTGood,             // the certificate is good
   ORTRevoked,          // the certificate has been revoked
@@ -46,13 +47,14 @@ enum OCSPResponseType
 struct OCSPHost
 {
   const char *mHostName;
   OCSPResponseType mORT;
   const char *mAdditionalCertName; // useful for ORTGoodOtherCert, etc.
   const char *mServerCertName;
 };
 
-SECItemArray *
-GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate *aCert,
-                       PLArenaPool *aArena, const char *aAdditionalCertName);
+SECItemArray*
+GetOCSPResponseForType(OCSPResponseType aORT, CERTCertificate* aCert,
+                       const mozilla::UniquePLArenaPool& aArena,
+                       const char* aAdditionalCertName);
 
 #endif // OCSPCommon_h
--- a/security/manager/ssl/tests/unit/tlsserver/lib/TLSServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/lib/TLSServer.cpp
@@ -427,28 +427,28 @@ ConfigSecureServerWithNamedCert(PRFileDe
   ScopedCERTCertificate issuerCert(
     CERT_FindCertByName(CERT_GetDefaultCertDB(), &cert->derIssuer));
   // If we can't find the issuer cert, continue without it.
   if (issuerCert) {
     // Sadly, CERTCertificateList does not have a CERT_NewCertificateList
     // utility function, so we must create it ourselves. This consists
     // of creating an arena, allocating space for the CERTCertificateList,
     // and then transferring ownership of the arena to that list.
-    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
     if (!arena) {
       PrintPRError("PORT_NewArena failed");
       return SECFailure;
     }
     certList.reset(static_cast<CERTCertificateList*>(
       PORT_ArenaAlloc(arena.get(), sizeof(CERTCertificateList))));
     if (!certList) {
       PrintPRError("PORT_ArenaAlloc failed");
       return SECFailure;
     }
-    certList->arena = arena.forget();
+    certList->arena = arena.release();
     // We also have to manually copy the certificates we care about to the
     // list, because there aren't any utility functions for that either.
     certList->certs = reinterpret_cast<SECItem*>(
       PORT_ArenaAlloc(certList->arena, 2 * sizeof(SECItem)));
     if (SECITEM_CopyItem(certList->arena, certList->certs, &cert->derCert)
           != SECSuccess) {
       PrintPRError("SECITEM_CopyItem failed");
       return SECFailure;