Bug 1204155 - Check if we're OOM when calling SECKEY_CopyPrivateKey() and SECKEY_CopyPublicKey() r=mt
authorTim Taubert <ttaubert@mozilla.com>
Mon, 14 Sep 2015 11:53:02 +0200
changeset 300983 a4bf09cda682d02ef212a8c4fbc27f00898bebef
parent 300982 48dfe8c77b2f6f3cf8a81ea22d47c3f095d38b35
child 300984 6fa9cdf4002b6726daedcf81b3e1e47702ca585f
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt
bugs1204155
milestone44.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 1204155 - Check if we're OOM when calling SECKEY_CopyPrivateKey() and SECKEY_CopyPublicKey() r=mt
dom/crypto/CryptoKey.cpp
dom/crypto/CryptoKey.h
dom/crypto/WebCryptoTask.cpp
--- a/dom/crypto/CryptoKey.cpp
+++ b/dom/crypto/CryptoKey.cpp
@@ -389,36 +389,42 @@ nsresult CryptoKey::SetSymKey(const Cryp
 {
   if (!mSymKey.Assign(aSymKey)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   return NS_OK;
 }
 
-void
+nsresult
 CryptoKey::SetPrivateKey(SECKEYPrivateKey* aPrivateKey)
 {
   nsNSSShutDownPreventionLock locker;
+
   if (!aPrivateKey || isAlreadyShutDown()) {
     mPrivateKey = nullptr;
-    return;
+    return NS_OK;
   }
+
   mPrivateKey = SECKEY_CopyPrivateKey(aPrivateKey);
+  return mPrivateKey ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
 }
 
-void
+nsresult
 CryptoKey::SetPublicKey(SECKEYPublicKey* aPublicKey)
 {
   nsNSSShutDownPreventionLock locker;
+
   if (!aPublicKey || isAlreadyShutDown()) {
     mPublicKey = nullptr;
-    return;
+    return NS_OK;
   }
+
   mPublicKey = SECKEY_CopyPublicKey(aPublicKey);
+  return mPublicKey ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
 }
 
 const CryptoBuffer&
 CryptoKey::GetSymKey() const
 {
   return mSymKey;
 }
 
--- a/dom/crypto/CryptoKey.h
+++ b/dom/crypto/CryptoKey.h
@@ -123,18 +123,18 @@ public:
   void AddUsage(KeyUsage aUsage);
   bool HasAnyUsage();
   bool HasUsage(KeyUsage aUsage);
   bool HasUsageOtherThan(uint32_t aUsages);
   static bool IsRecognizedUsage(const nsString& aUsage);
   static bool AllUsagesRecognized(const Sequence<nsString>& aUsages);
 
   nsresult SetSymKey(const CryptoBuffer& aSymKey);
-  void SetPrivateKey(SECKEYPrivateKey* aPrivateKey);
-  void SetPublicKey(SECKEYPublicKey* aPublicKey);
+  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;
--- a/dom/crypto/WebCryptoTask.cpp
+++ b/dom/crypto/WebCryptoTask.cpp
@@ -1576,33 +1576,39 @@ private:
       } else {
         pubKey = CryptoKey::PublicKeyFromJwk(mJwk, locker);
       }
 
       if (!pubKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
-      mKey->SetPublicKey(pubKey.get());
+      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 = CryptoKey::PrivateKeyFromPkcs8(mKeyData, locker);
       } else {
         privKey = CryptoKey::PrivateKeyFromJwk(mJwk, locker);
       }
 
       if (!privKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
-      mKey->SetPrivateKey(privKey.get());
+      if (NS_FAILED(mKey->SetPrivateKey(privKey.get()))) {
+        return NS_ERROR_DOM_OPERATION_ERR;
+      }
+
       mKey->SetType(CryptoKey::PRIVATE);
       pubKey = SECKEY_ConvertToPublicKey(privKey.get());
       if (!pubKey) {
         return NS_ERROR_DOM_UNKNOWN_ERR;
       }
     } else {
       // Invalid key format
       return NS_ERROR_DOM_SYNTAX_ERR;
@@ -1710,17 +1716,20 @@ private:
     nsNSSShutDownPreventionLock locker;
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) && mJwk.mD.WasPassed()) {
       // Private key import
       privKey = CryptoKey::PrivateKeyFromJwk(mJwk, locker);
       if (!privKey) {
         return NS_ERROR_DOM_DATA_ERR;
       }
 
-      mKey->SetPrivateKey(privKey.get());
+      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 = CryptoKey::PublicECKeyFromRaw(mKeyData, mNamedCurve, locker);
@@ -1747,17 +1756,20 @@ private:
         oid.data = pubKey->u.ec.DEREncodedParams.data + 2;
 
         // Find a matching and supported named curve.
         if (!MapOIDTagToNamedCurve(SECOID_FindOIDTag(&oid), mNamedCurve)) {
           return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
         }
       }
 
-      mKey->SetPublicKey(pubKey.get());
+      if (NS_FAILED(mKey->SetPublicKey(pubKey.get()))) {
+        return NS_ERROR_DOM_OPERATION_ERR;
+      }
+
       mKey->SetType(CryptoKey::PUBLIC);
     } else {
       return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
     }
 
     // Extract 'crv' parameter from JWKs.
     if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK)) {
       if (!NormalizeToken(mJwk.mCrv.Value(), mNamedCurve)) {
@@ -1869,17 +1881,20 @@ private:
         return NS_ERROR_DOM_DATA_ERR;
       }
 
       if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
         ATTEMPT_BUFFER_ASSIGN(mPrime, &pubKey->u.dh.prime);
         ATTEMPT_BUFFER_ASSIGN(mGenerator, &pubKey->u.dh.base);
       }
 
-      mKey->SetPublicKey(pubKey.get());
+      if (NS_FAILED(mKey->SetPublicKey(pubKey.get()))) {
+        return NS_ERROR_DOM_OPERATION_ERR;
+      }
+
       mKey->SetType(CryptoKey::PUBLIC);
     } else {
       return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
     }
 
     return NS_OK;
   }
 
@@ -2383,23 +2398,25 @@ GenerateAsymmetricKeyTask::DoCrypto()
   SECKEYPublicKey* pubKey = nullptr;
   mPrivateKey = PK11_GenerateKeyPair(slot.get(), mMechanism, param, &pubKey,
                                      PR_FALSE, PR_FALSE, nullptr);
   mPublicKey = pubKey;
   if (!mPrivateKey.get() || !mPublicKey.get()) {
     return NS_ERROR_DOM_UNKNOWN_ERR;
   }
 
-  mKeyPair.mPrivateKey.get()->SetPrivateKey(mPrivateKey);
-  mKeyPair.mPublicKey.get()->SetPublicKey(mPublicKey);
+  nsresult rv = mKeyPair.mPrivateKey.get()->SetPrivateKey(mPrivateKey);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_OPERATION_ERR);
+  rv = mKeyPair.mPublicKey.get()->SetPublicKey(mPublicKey);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_OPERATION_ERR);
 
   // PK11_GenerateKeyPair() does not set a CKA_EC_POINT attribute on the
   // private key, we need this later when exporting to PKCS8 and JWK though.
   if (mMechanism == CKM_EC_KEY_PAIR_GEN) {
-    nsresult rv = mKeyPair.mPrivateKey->AddPublicKeyData(mPublicKey);
+    rv = mKeyPair.mPrivateKey->AddPublicKeyData(mPublicKey);
     NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_OPERATION_ERR);
   }
 
   return NS_OK;
 }
 
 void
 GenerateAsymmetricKeyTask::Resolve()