Bug 1329238 - Make public CryptoKey.h methods return UniqueX NSS types instead of raw pointers. r?ttaubert draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Wed, 25 Jan 2017 00:27:39 +0800
changeset 465661 d785472ebb2d450eb555f5f0e458e00282d12385
parent 465660 34b8529501d038d82583bf53ce78fa9cd2ac307c
child 543207 7616874aa897e239352866971f5ba53aaebcb636
push id42663
push usercykesiopka.bmo@gmail.com
push dateTue, 24 Jan 2017 16:29:25 +0000
reviewersttaubert
bugs1329238
milestone54.0a1
Bug 1329238 - Make public CryptoKey.h methods return UniqueX NSS types instead of raw pointers. r?ttaubert The std::unique_ptr based UniqueX types provide better safety over managing raw pointers. MozReview-Commit-ID: EwwOfs6RHqy
dom/crypto/CryptoKey.cpp
dom/crypto/CryptoKey.h
dom/crypto/WebCryptoTask.cpp
dom/media/webrtc/RTCCertificate.cpp
--- a/dom/crypto/CryptoKey.cpp
+++ b/dom/crypto/CryptoKey.cpp
@@ -74,17 +74,17 @@ DestroyPrivateKeyWithoutDestroyingPKCS11
   PK11_FreeSlot(key->pkcs11Slot);
   PORT_FreeArena(key->arena, PR_TRUE);
 }
 
 // To protect against key ID collisions, PrivateKeyFromPrivateKeyTemplate
 // generates a random ID for each key. The given template must contain an
 // attribute slot for a key ID, but it must consist of a null pointer and have a
 // length of 0.
-SECKEYPrivateKey*
+UniqueSECKEYPrivateKey
 PrivateKeyFromPrivateKeyTemplate(CK_ATTRIBUTE* aTemplate,
                                  CK_ULONG aTemplateSize)
 {
   // Create a generic object with the contents of the key
   UniquePK11SlotInfo slot(PK11_GetInternalSlot());
   if (!slot) {
     return nullptr;
   }
@@ -142,17 +142,18 @@ PrivateKeyFromPrivateKeyTemplate(CK_ATTR
   // lives for the scope of this function doesn't escape.
   idAttributeSlot->pValue = nullptr;
   idAttributeSlot->ulValueLen = 0;
   if (!obj) {
     return nullptr;
   }
 
   // Have NSS translate the object to a private key.
-  return PK11_FindKeyByKeyID(slot.get(), objID.get(), nullptr);
+  return UniqueSECKEYPrivateKey(
+    PK11_FindKeyByKeyID(slot.get(), objID.get(), nullptr));
 }
 
 CryptoKey::CryptoKey(nsIGlobalObject* aGlobal)
   : mGlobal(aGlobal)
   , mAttributes(0)
   , mSymKey()
   , mPrivateKey(nullptr)
   , mPublicKey(nullptr)
@@ -359,18 +360,18 @@ CryptoKey::AddPublicKeyData(SECKEYPublic
     { CKA_PRIVATE,          &falseValue,          sizeof(falseValue) },
     // PrivateKeyFromPrivateKeyTemplate sets the ID.
     { CKA_ID,               nullptr,              0 },
     { CKA_EC_PARAMS,        params.data,          params.len },
     { CKA_EC_POINT,         point->data,          point->len },
     { CKA_VALUE,            value.data,           value.len },
   };
 
-  mPrivateKey = UniqueSECKEYPrivateKey(
-    PrivateKeyFromPrivateKeyTemplate(keyTemplate, ArrayLength(keyTemplate)));
+  mPrivateKey = PrivateKeyFromPrivateKeyTemplate(keyTemplate,
+                                                 ArrayLength(keyTemplate));
   NS_ENSURE_TRUE(mPrivateKey, NS_ERROR_DOM_OPERATION_ERR);
 
   return NS_OK;
 }
 
 void
 CryptoKey::ClearUsages()
 {
@@ -480,55 +481,54 @@ CryptoKey::SetPublicKey(SECKEYPublicKey*
 }
 
 const CryptoBuffer&
 CryptoKey::GetSymKey() const
 {
   return mSymKey;
 }
 
-SECKEYPrivateKey*
+UniqueSECKEYPrivateKey
 CryptoKey::GetPrivateKey() const
 {
   nsNSSShutDownPreventionLock locker;
   if (!mPrivateKey || isAlreadyShutDown()) {
     return nullptr;
   }
-  return SECKEY_CopyPrivateKey(mPrivateKey.get());
+  return UniqueSECKEYPrivateKey(SECKEY_CopyPrivateKey(mPrivateKey.get()));
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CryptoKey::GetPublicKey() const
 {
   nsNSSShutDownPreventionLock locker;
   if (!mPublicKey || isAlreadyShutDown()) {
     return nullptr;
   }
-  return SECKEY_CopyPublicKey(mPublicKey.get());
+  return UniqueSECKEYPublicKey(SECKEY_CopyPublicKey(mPublicKey.get()));
 }
 
 void CryptoKey::virtualDestroyNSSReference()
 {
   destructorSafeDestroyNSSReference();
 }
 
 void CryptoKey::destructorSafeDestroyNSSReference()
 {
   mPrivateKey = nullptr;
   mPublicKey = nullptr;
 }
 
 
 // Serialization and deserialization convenience methods
 
-SECKEYPrivateKey*
+UniqueSECKEYPrivateKey
 CryptoKey::PrivateKeyFromPkcs8(CryptoBuffer& aKeyData,
                          const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
-  SECKEYPrivateKey* privKey;
   UniquePK11SlotInfo slot(PK11_GetInternalSlot());
   if (!slot) {
     return nullptr;
   }
 
   UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
@@ -537,27 +537,29 @@ CryptoKey::PrivateKeyFromPkcs8(CryptoBuf
   SECItem pkcs8Item = { siBuffer, nullptr, 0 };
   if (!aKeyData.ToSECItem(arena.get(), &pkcs8Item)) {
     return nullptr;
   }
 
   // Allow everything, we enforce usage ourselves
   unsigned int usage = KU_ALL;
 
+  SECKEYPrivateKey* privKey;
   SECStatus rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
                  slot.get(), &pkcs8Item, nullptr, nullptr, false, false,
                  usage, &privKey, nullptr);
 
   if (rv == SECFailure) {
     return nullptr;
   }
-  return privKey;
+
+  return UniqueSECKEYPrivateKey(privKey);
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CryptoKey::PublicKeyFromSpki(CryptoBuffer& aKeyData,
                        const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
   }
 
@@ -602,17 +604,17 @@ CryptoKey::PublicKeyFromSpki(CryptoBuffe
     }
   }
 
   UniqueSECKEYPublicKey tmp(SECKEY_ExtractPublicKey(spki.get()));
   if (!tmp.get() || !PublicKeyValid(tmp.get())) {
     return nullptr;
   }
 
-  return SECKEY_CopyPublicKey(tmp.get());
+  return UniqueSECKEYPublicKey(SECKEY_CopyPublicKey(tmp.get()));
 }
 
 nsresult
 CryptoKey::PrivateKeyToPkcs8(SECKEYPrivateKey* aPrivKey,
                        CryptoBuffer& aRetVal,
                        const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   UniqueSECItem pkcs8Item(PK11_ExportDERPrivateKeyInfo(aPrivKey, nullptr));
@@ -743,17 +745,17 @@ CreateECPointForCoordinates(const Crypto
   // Set point data.
   point->data[0] = EC_POINT_FORM_UNCOMPRESSED;
   memcpy(point->data + 1, aX.Elements(), aX.Length());
   memcpy(point->data + 1 + aX.Length(), aY.Elements(), aY.Length());
 
   return point;
 }
 
-SECKEYPrivateKey*
+UniqueSECKEYPrivateKey
 CryptoKey::PrivateKeyFromJwk(const JsonWebKey& aJwk,
                              const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   CK_OBJECT_CLASS privateKeyValue = CKO_PRIVATE_KEY;
   CK_BBOOL falseValue = CK_FALSE;
 
   if (aJwk.mKty.EqualsLiteral(JWK_TYPE_EC)) {
     // Verify that all of the required parameters are present
@@ -995,17 +997,17 @@ CryptoKey::PrivateKeyToJwk(SECKEYPrivate
 
       return NS_OK;
     }
     default:
       return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
   }
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CreateECPublicKey(const SECItem* aKeyData, const nsString& aNamedCurve)
 {
   UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
   }
 
   // It's important that this be a UniqueSECKEYPublicKey, as this ensures that
@@ -1032,20 +1034,20 @@ CreateECPublicKey(const SECItem* aKeyDat
   // Set public point.
   key->u.ec.publicValue = *aKeyData;
 
   // Ensure the given point is on the curve.
   if (!CryptoKey::PublicKeyValid(key.get())) {
     return nullptr;
   }
 
-  return SECKEY_CopyPublicKey(key.get());
+  return UniqueSECKEYPublicKey(SECKEY_CopyPublicKey(key.get()));
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CryptoKey::PublicKeyFromJwk(const JsonWebKey& aJwk,
                             const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   if (aJwk.mKty.EqualsLiteral(JWK_TYPE_RSA)) {
     // Verify that all of the required parameters are present
     CryptoBuffer n, e;
     if (!aJwk.mN.WasPassed() || NS_FAILED(n.FromJwkBase64(aJwk.mN.Value())) ||
         !aJwk.mE.WasPassed() || NS_FAILED(e.FromJwkBase64(aJwk.mE.Value()))) {
@@ -1069,17 +1071,17 @@ CryptoKey::PublicKeyFromJwk(const JsonWe
     };
 
     UniqueSECItem pkDer(SEC_ASN1EncodeItem(nullptr, nullptr, &input,
                                            rsaPublicKeyTemplate));
     if (!pkDer.get()) {
       return nullptr;
     }
 
-    return SECKEY_ImportDERPublicKey(pkDer.get(), CKK_RSA);
+    return UniqueSECKEYPublicKey(SECKEY_ImportDERPublicKey(pkDer.get(), CKK_RSA));
   }
 
   if (aJwk.mKty.EqualsLiteral(JWK_TYPE_EC)) {
     // Verify that all of the required parameters are present
     CryptoBuffer x, y;
     if (!aJwk.mCrv.WasPassed() ||
         !aJwk.mX.WasPassed() || NS_FAILED(x.FromJwkBase64(aJwk.mX.Value())) ||
         !aJwk.mY.WasPassed() || NS_FAILED(y.FromJwkBase64(aJwk.mY.Value()))) {
@@ -1135,17 +1137,17 @@ CryptoKey::PublicKeyToJwk(SECKEYPublicKe
         return NS_ERROR_DOM_OPERATION_ERR;
       }
       return NS_OK;
     default:
       return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
   }
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CryptoKey::PublicDhKeyFromRaw(CryptoBuffer& aKeyData,
                               const CryptoBuffer& aPrime,
                               const CryptoBuffer& aGenerator,
                               const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
@@ -1166,31 +1168,31 @@ CryptoKey::PublicDhKeyFromRaw(CryptoBuff
       !aKeyData.ToSECItem(arena.get(), &key->u.dh.publicValue)) {
     return nullptr;
   }
 
   key->u.dh.prime.type = siUnsignedInteger;
   key->u.dh.base.type = siUnsignedInteger;
   key->u.dh.publicValue.type = siUnsignedInteger;
 
-  return SECKEY_CopyPublicKey(key);
+  return UniqueSECKEYPublicKey(SECKEY_CopyPublicKey(key));
 }
 
 nsresult
 CryptoKey::PublicDhKeyToRaw(SECKEYPublicKey* aPubKey,
                             CryptoBuffer& aRetVal,
                             const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   if (!aRetVal.Assign(&aPubKey->u.dh.publicValue)) {
     return NS_ERROR_DOM_OPERATION_ERR;
   }
   return NS_OK;
 }
 
-SECKEYPublicKey*
+UniqueSECKEYPublicKey
 CryptoKey::PublicECKeyFromRaw(CryptoBuffer& aKeyData,
                               const nsString& aNamedCurve,
                               const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return nullptr;
   }
@@ -1317,22 +1319,20 @@ CryptoKey::ReadStructuredClone(JSStructu
   if (!read) {
     return false;
   }
 
   if (sym.Length() > 0 && !mSymKey.Assign(sym))  {
     return false;
   }
   if (priv.Length() > 0) {
-    mPrivateKey = UniqueSECKEYPrivateKey(
-      CryptoKey::PrivateKeyFromPkcs8(priv, locker));
+    mPrivateKey = CryptoKey::PrivateKeyFromPkcs8(priv, locker);
   }
   if (pub.Length() > 0)  {
-    mPublicKey = UniqueSECKEYPublicKey(
-      CryptoKey::PublicKeyFromSpki(pub, locker));
+    mPublicKey = CryptoKey::PublicKeyFromSpki(pub, locker);
   }
 
   // Ensure that what we've read is consistent
   // If the attributes indicate a key type, should have a key of that type
   if (!((GetKeyType() == SECRET  && mSymKey.Length() > 0) ||
         (GetKeyType() == PRIVATE && mPrivateKey) ||
         (GetKeyType() == PUBLIC  && mPublicKey))) {
     return false;
--- a/dom/crypto/CryptoKey.h
+++ b/dom/crypto/CryptoKey.h
@@ -127,67 +127,70 @@ public:
   static bool IsRecognizedUsage(const nsString& aUsage);
   static bool AllUsagesRecognized(const Sequence<nsString>& aUsages);
 
   nsresult SetSymKey(const CryptoBuffer& aSymKey);
   nsresult SetPrivateKey(SECKEYPrivateKey* aPrivateKey);
   nsresult SetPublicKey(SECKEYPublicKey* aPublicKey);
 
   // Accessors for the keys themselves
-  // Note: GetPrivateKey and GetPublicKey return copies of the internal
-  // key handles, which the caller must free with SECKEY_DestroyPrivateKey
-  // or SECKEY_DestroyPublicKey.
   const CryptoBuffer& GetSymKey() const;
-  SECKEYPrivateKey* GetPrivateKey() const;
-  SECKEYPublicKey* GetPublicKey() const;
+  UniqueSECKEYPrivateKey GetPrivateKey() const;
+  UniqueSECKEYPublicKey GetPublicKey() const;
 
   // For nsNSSShutDownObject
   virtual void virtualDestroyNSSReference() override;
   void destructorSafeDestroyNSSReference();
 
   // Serialization and deserialization convenience methods
   // Note:
   // 1. The inputs aKeyData are non-const only because the NSS import
   //    functions lack the const modifier.  They should not be modified.
   // 2. All of the NSS key objects returned need to be freed by the caller.
-  static SECKEYPrivateKey* PrivateKeyFromPkcs8(CryptoBuffer& aKeyData,
-                                               const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPrivateKey PrivateKeyFromPkcs8(
+    CryptoBuffer& aKeyData,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PrivateKeyToPkcs8(SECKEYPrivateKey* aPrivKey,
                                     CryptoBuffer& aRetVal,
                                     const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
-  static SECKEYPublicKey* PublicKeyFromSpki(CryptoBuffer& aKeyData,
-                                            const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPublicKey PublicKeyFromSpki(
+    CryptoBuffer& aKeyData,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PublicKeyToSpki(SECKEYPublicKey* aPubKey,
                                   CryptoBuffer& aRetVal,
                                   const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
-  static SECKEYPrivateKey* PrivateKeyFromJwk(const JsonWebKey& aJwk,
-                                             const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPrivateKey PrivateKeyFromJwk(
+    const JsonWebKey& aJwk,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PrivateKeyToJwk(SECKEYPrivateKey* aPrivKey,
                                   JsonWebKey& aRetVal,
                                   const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
-  static SECKEYPublicKey* PublicKeyFromJwk(const JsonWebKey& aKeyData,
-                                           const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPublicKey PublicKeyFromJwk(
+    const JsonWebKey& aKeyData,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PublicKeyToJwk(SECKEYPublicKey* aPubKey,
                                  JsonWebKey& aRetVal,
                                  const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
-  static SECKEYPublicKey* PublicDhKeyFromRaw(CryptoBuffer& aKeyData,
-                                             const CryptoBuffer& aPrime,
-                                             const CryptoBuffer& aGenerator,
-                                             const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPublicKey PublicDhKeyFromRaw(
+    CryptoBuffer& aKeyData,
+    const CryptoBuffer& aPrime,
+    const CryptoBuffer& aGenerator,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PublicDhKeyToRaw(SECKEYPublicKey* aPubKey,
                                    CryptoBuffer& aRetVal,
                                    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
-  static SECKEYPublicKey* PublicECKeyFromRaw(CryptoBuffer& aKeyData,
-                                             const nsString& aNamedCurve,
-                                             const nsNSSShutDownPreventionLock& /*proofOfLock*/);
+  static UniqueSECKEYPublicKey PublicECKeyFromRaw(
+    CryptoBuffer& aKeyData,
+    const nsString& aNamedCurve,
+    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
   static nsresult PublicECKeyToRaw(SECKEYPublicKey* aPubKey,
                                    CryptoBuffer& aRetVal,
                                    const nsNSSShutDownPreventionLock& /*proofOfLock*/);
 
   static bool PublicKeyValid(SECKEYPublicKey* aPubKey);
 
   // Structured clone methods use these to clone keys
   bool WriteStructuredClone(JSStructuredCloneWriter* aWriter) const;
--- a/dom/crypto/WebCryptoTask.cpp
+++ b/dom/crypto/WebCryptoTask.cpp
@@ -1775,42 +1775,38 @@ private:
     // Import the key data itself
     UniqueSECKEYPublicKey pubKey;
     UniqueSECKEYPrivateKey privKey;
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI) ||
         (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) &&
          !mJwk.mD.WasPassed())) {
       // Public key import
       if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicKeyFromSpki(mKeyData, locker));
+        pubKey = CryptoKey::PublicKeyFromSpki(mKeyData, locker);
       } else {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicKeyFromJwk(mJwk, locker));
+        pubKey = CryptoKey::PublicKeyFromJwk(mJwk, locker);
       }
 
       if (!pubKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
       if (NS_FAILED(mKey->SetPublicKey(pubKey.get()))) {
         return NS_ERROR_DOM_OPERATION_ERR;
       }
 
       mKey->SetType(CryptoKey::PUBLIC);
     } else if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_PKCS8) ||
         (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) &&
          mJwk.mD.WasPassed())) {
       // Private key import
       if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_PKCS8)) {
-        privKey = UniqueSECKEYPrivateKey(
-          CryptoKey::PrivateKeyFromPkcs8(mKeyData, locker));
+        privKey = CryptoKey::PrivateKeyFromPkcs8(mKeyData, locker);
       } else {
-        privKey = UniqueSECKEYPrivateKey(
-          CryptoKey::PrivateKeyFromJwk(mJwk, locker));
+        privKey = CryptoKey::PrivateKeyFromJwk(mJwk, locker);
       }
 
       if (!privKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
       if (NS_FAILED(mKey->SetPrivateKey(privKey.get()))) {
         return NS_ERROR_DOM_OPERATION_ERR;
@@ -1924,41 +1920,37 @@ private:
   {
     // Import the key data itself
     UniqueSECKEYPublicKey pubKey;
     UniqueSECKEYPrivateKey privKey;
 
     nsNSSShutDownPreventionLock locker;
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) && mJwk.mD.WasPassed()) {
       // Private key import
-      privKey = UniqueSECKEYPrivateKey(
-        CryptoKey::PrivateKeyFromJwk(mJwk, locker));
+      privKey = CryptoKey::PrivateKeyFromJwk(mJwk, locker);
       if (!privKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
       if (NS_FAILED(mKey->SetPrivateKey(privKey.get()))) {
         return NS_ERROR_DOM_OPERATION_ERR;
       }
 
       mKey->SetType(CryptoKey::PRIVATE);
     } else if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW) ||
                mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI) ||
                (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) &&
                 !mJwk.mD.WasPassed())) {
       // Public key import
       if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicECKeyFromRaw(mKeyData, mNamedCurve, locker));
+        pubKey = CryptoKey::PublicECKeyFromRaw(mKeyData, mNamedCurve, locker);
       } else if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicKeyFromSpki(mKeyData, locker));
+        pubKey = CryptoKey::PublicKeyFromSpki(mKeyData, locker);
       } else if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicKeyFromJwk(mJwk, locker));
+        pubKey = CryptoKey::PublicKeyFromJwk(mJwk, locker);
       } else {
         MOZ_ASSERT(false);
       }
 
       if (!pubKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
@@ -2082,21 +2074,20 @@ private:
     // Import the key data itself
     UniqueSECKEYPublicKey pubKey;
 
     nsNSSShutDownPreventionLock locker;
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW) ||
         mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
       // Public key import
       if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicDhKeyFromRaw(mKeyData, mPrime, mGenerator, locker));
+        pubKey = CryptoKey::PublicDhKeyFromRaw(mKeyData, mPrime, mGenerator,
+                                               locker);
       } else if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
-        pubKey = UniqueSECKEYPublicKey(
-          CryptoKey::PublicKeyFromSpki(mKeyData, locker));
+        pubKey = CryptoKey::PublicKeyFromSpki(mKeyData, locker);
       } else {
         MOZ_ASSERT(false);
       }
 
       if (!pubKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
@@ -3020,17 +3011,17 @@ public:
     RootedDictionary<EcdhKeyDeriveParams> params(aCx);
     mEarlyRv = Coerce(aCx, params, aAlgorithm);
     if (NS_FAILED(mEarlyRv)) {
       mEarlyRv = NS_ERROR_DOM_SYNTAX_ERR;
       return;
     }
 
     CryptoKey* publicKey = params.mPublic;
-    mPubKey = UniqueSECKEYPublicKey(publicKey->GetPublicKey());
+    mPubKey = publicKey->GetPublicKey();
     if (!mPubKey) {
       mEarlyRv = NS_ERROR_DOM_INVALID_ACCESS_ERR;
       return;
     }
 
     CHECK_KEY_ALGORITHM(publicKey->Algorithm(), WEBCRYPTO_ALG_ECDH);
 
     // Both keys must use the same named curve.
@@ -3120,17 +3111,17 @@ public:
     RootedDictionary<DhKeyDeriveParams> params(aCx);
     mEarlyRv = Coerce(aCx, params, aAlgorithm);
     if (NS_FAILED(mEarlyRv)) {
       mEarlyRv = NS_ERROR_DOM_SYNTAX_ERR;
       return;
     }
 
     CryptoKey* publicKey = params.mPublic;
-    mPubKey = UniqueSECKEYPublicKey(publicKey->GetPublicKey());
+    mPubKey = publicKey->GetPublicKey();
     if (!mPubKey) {
       mEarlyRv = NS_ERROR_DOM_INVALID_ACCESS_ERR;
       return;
     }
 
     KeyAlgorithmProxy alg1 = publicKey->Algorithm();
     CHECK_KEY_ALGORITHM(alg1, WEBCRYPTO_ALG_DH);
 
--- a/dom/media/webrtc/RTCCertificate.cpp
+++ b/dom/media/webrtc/RTCCertificate.cpp
@@ -215,21 +215,21 @@ private:
 
     return NS_OK;
   }
 
   virtual void Resolve() override
   {
     // Make copies of the private key and certificate, otherwise, when this
     // object is deleted, the structures they reference will be deleted too.
-    SECKEYPrivateKey* key = mKeyPair->mPrivateKey.get()->GetPrivateKey();
+    UniqueSECKEYPrivateKey key = mKeyPair->mPrivateKey.get()->GetPrivateKey();
     CERTCertificate* cert = CERT_DupCertificate(mCertificate.get());
     RefPtr<RTCCertificate> result =
         new RTCCertificate(mResultPromise->GetParentObject(),
-                           key, cert, mAuthType, mExpires);
+                           key.release(), cert, mAuthType, mExpires);
     mResultPromise->MaybeResolve(result);
   }
 };
 
 static PRTime
 ReadExpires(JSContext* aCx, const ObjectOrString& aOptions,
             ErrorResult& aRv)
 {
@@ -411,17 +411,17 @@ RTCCertificate::ReadPrivateKey(JSStructu
   nsString json;
   if (!ReadString(aReader, json)) {
     return false;
   }
   JsonWebKey jwk;
   if (!jwk.Init(json)) {
     return false;
   }
-  mPrivateKey.reset(CryptoKey::PrivateKeyFromJwk(jwk, aLockProof));
+  mPrivateKey = CryptoKey::PrivateKeyFromJwk(jwk, aLockProof);
   return !!mPrivateKey;
 }
 
 bool
 RTCCertificate::ReadCertificate(JSStructuredCloneReader* aReader,
                                 const nsNSSShutDownPreventionLock& /*proof*/)
 {
   CryptoBuffer cert;