Bug 1260644 - Use UniquePLArenaPool to manage PLArenaPools in PSM. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Mon, 04 Apr 2016 17:35:24 -0700
changeset 317121 63c6be19398d5654cd577fc009c3874742f028e7
parent 317120 153c28889bfc6dac38739d31f2e7fe3e0406f8bd
child 317122 946e39f5d97dd31ed852ae19150143181d945844
push idunknown
push userunknown
push dateunknown
reviewerskeeler
bugs1260644
milestone48.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 1260644 - Use UniquePLArenaPool to manage PLArenaPools in PSM. r=keeler 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;