Bug 1034856 - Remove CryptoBuffer::ToSECItem() using moz_malloc(). r=rbarnes, a=lsblakk
☠☠ backed out by 86f3ce4cf56a ☠ ☠
authorTim Taubert <ttaubert@mozilla.com>
Wed, 03 Sep 2014 09:40:21 -0400
changeset 233534 29cf6a01532fdac997990b0e3cc9abd994b19f34
parent 233533 f6adbda3e224a2bc1c7a4c448bc307ea99483d35
child 233535 2cfa6e659ae62535c0676df74ce3ed833466b7ae
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrbarnes, lsblakk
bugs1034856
milestone35.0a2
Bug 1034856 - Remove CryptoBuffer::ToSECItem() using moz_malloc(). r=rbarnes, a=lsblakk
dom/crypto/CryptoBuffer.cpp
dom/crypto/CryptoBuffer.h
dom/crypto/CryptoKey.cpp
dom/crypto/WebCryptoTask.cpp
--- a/dom/crypto/CryptoBuffer.cpp
+++ b/dom/crypto/CryptoBuffer.cpp
@@ -138,34 +138,16 @@ CryptoBuffer::ToJwkBase64(nsString& aBas
   if (base64.FindCharInSet("+/", 0) != kNotFound) {
     return NS_ERROR_FAILURE;
   }
 
   CopyASCIItoUTF16(base64, aBase64);
   return NS_OK;
 }
 
-SECItem*
-CryptoBuffer::ToSECItem() const
-{
-  uint8_t* data = (uint8_t*) moz_malloc(Length());
-  if (!data) {
-    return nullptr;
-  }
-
-  SECItem* item = ::SECITEM_AllocItem(nullptr, nullptr, 0);
-  item->type = siBuffer;
-  item->data = data;
-  item->len = Length();
-
-  memcpy(item->data, Elements(), Length());
-
-  return item;
-}
-
 bool
 CryptoBuffer::ToSECItem(PLArenaPool *aArena, SECItem* aItem) const
 {
   aItem->type = siBuffer;
   aItem->data = nullptr;
 
   if (!::SECITEM_AllocItem(aArena, aItem, Length())) {
     return false;
--- a/dom/crypto/CryptoBuffer.h
+++ b/dom/crypto/CryptoBuffer.h
@@ -34,17 +34,16 @@ public:
   uint8_t* Assign(const TypedArray_base<T, UnwrapArray, GetLengthAndData>& aArray)
   {
     aArray.ComputeLengthAndData();
     return Assign(aArray.Data(), aArray.Length());
   }
 
   nsresult FromJwkBase64(const nsString& aBase64);
   nsresult ToJwkBase64(nsString& aBase64);
-  SECItem* ToSECItem() const;
   bool ToSECItem(PLArenaPool* aArena, SECItem* aItem) const;
   JSObject* ToUint8Array(JSContext* aCx) const;
 
   bool GetBigIntValue(unsigned long& aRetVal);
 };
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/crypto/CryptoKey.cpp
+++ b/dom/crypto/CryptoKey.cpp
@@ -362,44 +362,55 @@ void CryptoKey::destructorSafeDestroyNSS
 // Serialization and deserialization convenience methods
 
 SECKEYPrivateKey*
 CryptoKey::PrivateKeyFromPkcs8(CryptoBuffer& aKeyData,
                          const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
   SECKEYPrivateKey* privKey;
   ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
-  ScopedSECItem pkcs8Item(aKeyData.ToSECItem());
-  if (!pkcs8Item) {
+
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    return nullptr;
+  }
+
+  SECItem pkcs8Item = { siBuffer, nullptr, 0 };
+  if (!aKeyData.ToSECItem(arena, &pkcs8Item)) {
     return nullptr;
   }
 
   // Allow everything, we enforce usage ourselves
   unsigned int usage = KU_ALL;
 
   SECStatus rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
-                 slot.get(), pkcs8Item.get(), nullptr, nullptr, false, false,
+                 slot.get(), &pkcs8Item, nullptr, nullptr, false, false,
                  usage, &privKey, nullptr);
 
   if (rv == SECFailure) {
     return nullptr;
   }
   return privKey;
 }
 
 SECKEYPublicKey*
 CryptoKey::PublicKeyFromSpki(CryptoBuffer& aKeyData,
                        const nsNSSShutDownPreventionLock& /*proofOfLock*/)
 {
-  ScopedSECItem spkiItem(aKeyData.ToSECItem());
-  if (!spkiItem) {
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
     return nullptr;
   }
 
-  ScopedCERTSubjectPublicKeyInfo spki(SECKEY_DecodeDERSubjectPublicKeyInfo(spkiItem.get()));
+  SECItem spkiItem = { siBuffer, nullptr, 0 };
+  if (!aKeyData.ToSECItem(arena, &spkiItem)) {
+    return nullptr;
+  }
+
+  ScopedCERTSubjectPublicKeyInfo spki(SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
   if (!spki) {
     return nullptr;
   }
 
   bool isECDHAlgorithm = SECITEM_ItemsAreEqual(&SEC_OID_DATA_EC_DH,
                                                &spki->algorithm.algorithm);
   bool isDHAlgorithm = SECITEM_ItemsAreEqual(&SEC_OID_DATA_DH_KEY_AGREEMENT,
                                              &spki->algorithm.algorithm);
@@ -671,24 +682,29 @@ CryptoKey::PrivateKeyFromJwk(const JsonW
         !aJwk.mP.WasPassed() || NS_FAILED(p.FromJwkBase64(aJwk.mP.Value())) ||
         !aJwk.mQ.WasPassed() || NS_FAILED(q.FromJwkBase64(aJwk.mQ.Value())) ||
         !aJwk.mDp.WasPassed() || NS_FAILED(dp.FromJwkBase64(aJwk.mDp.Value())) ||
         !aJwk.mDq.WasPassed() || NS_FAILED(dq.FromJwkBase64(aJwk.mDq.Value())) ||
         !aJwk.mQi.WasPassed() || NS_FAILED(qi.FromJwkBase64(aJwk.mQi.Value()))) {
       return nullptr;
     }
 
-    // Compute the ID for this key
-    // This is generated with a SHA-1 hash, so unlikely to collide
-    ScopedSECItem nItem(n.ToSECItem());
-    if (!nItem.get()) {
+    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
       return nullptr;
     }
 
-    ScopedSECItem objID(PK11_MakeIDFromPubKey(nItem.get()));
+    // Compute the ID for this key
+    // This is generated with a SHA-1 hash, so unlikely to collide
+    SECItem nItem = { siBuffer, nullptr, 0 };
+    if (!n.ToSECItem(arena, &nItem)) {
+      return nullptr;
+    }
+
+    ScopedSECItem objID(PK11_MakeIDFromPubKey(&nItem));
     if (!objID.get()) {
       return nullptr;
     }
 
     // Populate template from parameters
     CK_KEY_TYPE rsaValue = CKK_RSA;
     CK_ATTRIBUTE keyTemplate[14] = {
       { CKA_CLASS,            &privateKeyValue,      sizeof(privateKeyValue) },
--- a/dom/crypto/WebCryptoTask.cpp
+++ b/dom/crypto/WebCryptoTask.cpp
@@ -75,19 +75,18 @@ enum TelemetryAlgorithm {
 // OOM-safe CryptoBuffer initialization, suitable for constructors
 #define ATTEMPT_BUFFER_INIT(dst, src) \
   if (!dst.Assign(src)) { \
     mEarlyRv = NS_ERROR_DOM_UNKNOWN_ERR; \
     return; \
   }
 
 // OOM-safe CryptoBuffer-to-SECItem copy, suitable for DoCrypto
-#define ATTEMPT_BUFFER_TO_SECITEM(dst, src) \
-  dst = src.ToSECItem(); \
-  if (!dst) { \
+#define ATTEMPT_BUFFER_TO_SECITEM(arena, dst, src) \
+  if (!src.ToSECItem(arena, dst)) { \
     return NS_ERROR_DOM_UNKNOWN_ERR; \
   }
 
 // OOM-safe CryptoBuffer copy, suitable for DoCrypto
 #define ATTEMPT_BUFFER_ASSIGN(dst, src) \
   if (!dst.Assign(src)) { \
     return NS_ERROR_DOM_UNKNOWN_ERR; \
   }
@@ -506,25 +505,28 @@ private:
   virtual nsresult DoCrypto() MOZ_OVERRIDE
   {
     nsresult rv;
 
     if (!mDataIsSet) {
       return NS_ERROR_DOM_OPERATION_ERR;
     }
 
+    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
+      return NS_ERROR_DOM_OPERATION_ERR;
+    }
+
     // Construct the parameters object depending on algorithm
-    SECItem param;
-    ScopedSECItem cbcParam;
+    SECItem param = { siBuffer, nullptr, 0 };
     CK_AES_CTR_PARAMS ctrParams;
     CK_GCM_PARAMS gcmParams;
     switch (mMechanism) {
       case CKM_AES_CBC_PAD:
-        ATTEMPT_BUFFER_TO_SECITEM(cbcParam, mIv);
-        param = *cbcParam;
+        ATTEMPT_BUFFER_TO_SECITEM(arena, &param, mIv);
         break;
       case CKM_AES_CTR:
         ctrParams.ulCounterBits = mCounterLength;
         MOZ_ASSERT(mIv.Length() == 16);
         memcpy(&ctrParams.cb, mIv.Elements(), 16);
         param.type = siBuffer;
         param.data = (unsigned char*) &ctrParams;
         param.len  = sizeof(ctrParams);
@@ -539,22 +541,22 @@ private:
         param.data = (unsigned char*) &gcmParams;
         param.len  = sizeof(gcmParams);
         break;
       default:
         return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
     }
 
     // Import the key
-    ScopedSECItem keyItem;
-    ATTEMPT_BUFFER_TO_SECITEM(keyItem, mSymKey);
+    SECItem keyItem = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &keyItem, mSymKey);
     ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
     MOZ_ASSERT(slot.get());
     ScopedPK11SymKey symKey(PK11_ImportSymKey(slot, mMechanism, PK11_OriginUnwrap,
-                                              CKA_ENCRYPT, keyItem.get(), nullptr));
+                                              CKA_ENCRYPT, &keyItem, nullptr));
     if (!symKey) {
       return NS_ERROR_DOM_INVALID_ACCESS_ERR;
     }
 
     // Initialize the output buffer (enough space for padding / a full tag)
     uint32_t dataLen = mData.Length();
     uint32_t maxLen = dataLen + 16;
     if (!mResult.SetLength(maxLen)) {
@@ -641,40 +643,45 @@ private:
       return NS_ERROR_DOM_OPERATION_ERR;
     }
 
     // Check that the input is a multiple of 64 bits long
     if (mData.Length() == 0 || mData.Length() % 8 != 0) {
       return NS_ERROR_DOM_DATA_ERR;
     }
 
+    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
+      return NS_ERROR_DOM_OPERATION_ERR;
+    }
+
     // Import the key
-    ScopedSECItem keyItem;
-    ATTEMPT_BUFFER_TO_SECITEM(keyItem, mSymKey);
+    SECItem keyItem = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &keyItem, mSymKey);
     ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
     MOZ_ASSERT(slot.get());
     ScopedPK11SymKey symKey(PK11_ImportSymKey(slot, mMechanism, PK11_OriginUnwrap,
-                                              CKA_WRAP, keyItem.get(), nullptr));
+                                              CKA_WRAP, &keyItem, nullptr));
     if (!symKey) {
       return NS_ERROR_DOM_INVALID_ACCESS_ERR;
     }
 
     // Import the data to a SECItem
-    ScopedSECItem dataItem;
-    ATTEMPT_BUFFER_TO_SECITEM(dataItem, mData);
+    SECItem dataItem = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &dataItem, mData);
 
     // Parameters for the fake keys
     CK_MECHANISM_TYPE fakeMechanism = CKM_SHA_1_HMAC;
     CK_ATTRIBUTE_TYPE fakeOperation = CKA_SIGN;
 
     if (mEncrypt) {
       // Import the data into a fake PK11SymKey structure
       ScopedPK11SymKey keyToWrap(PK11_ImportSymKey(slot, fakeMechanism,
                                                    PK11_OriginUnwrap, fakeOperation,
-                                                   dataItem.get(), nullptr));
+                                                   &dataItem, nullptr));
       if (!keyToWrap) {
         return NS_ERROR_DOM_OPERATION_ERR;
       }
 
       // Encrypt and return the wrapped key
       // AES-KW encryption results in a wrapped key 64 bits longer
       if (!mResult.SetLength(mData.Length() + 8)) {
         return NS_ERROR_DOM_OPERATION_ERR;
@@ -684,17 +691,17 @@ private:
       rv = MapSECStatus(PK11_WrapSymKey(mMechanism, nullptr, symKey.get(),
                                         keyToWrap.get(), &resultItem));
       NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_OPERATION_ERR);
     } else {
       // Decrypt the ciphertext into a temporary PK11SymKey
       // Unwrapped key should be 64 bits shorter
       int keySize = mData.Length() - 8;
       ScopedPK11SymKey unwrappedKey(PK11_UnwrapSymKey(symKey, mMechanism, nullptr,
-                                                 dataItem.get(), fakeMechanism,
+                                                 &dataItem, fakeMechanism,
                                                  fakeOperation, keySize));
       if (!unwrappedKey) {
         return NS_ERROR_DOM_OPERATION_ERR;
       }
 
       // Export the key to get the cleartext
       rv = MapSECStatus(PK11_ExtractKeyValue(unwrappedKey));
       if (NS_FAILED(rv)) {
@@ -896,25 +903,30 @@ private:
   bool mSign;
 
   virtual nsresult DoCrypto() MOZ_OVERRIDE
   {
     // Initialize the output buffer
     if (!mResult.SetLength(HASH_LENGTH_MAX)) {
       return NS_ERROR_DOM_UNKNOWN_ERR;
     }
-    uint32_t outLen;
+
+    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
+      return NS_ERROR_DOM_OPERATION_ERR;
+    }
 
     // Import the key
-    ScopedSECItem keyItem;
-    ATTEMPT_BUFFER_TO_SECITEM(keyItem, mSymKey);
+    uint32_t outLen;
+    SECItem keyItem = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &keyItem, mSymKey);
     ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
     MOZ_ASSERT(slot.get());
     ScopedPK11SymKey symKey(PK11_ImportSymKey(slot, mMechanism, PK11_OriginUnwrap,
-                                              CKA_SIGN, keyItem.get(), nullptr));
+                                              CKA_SIGN, &keyItem, nullptr));
     if (!symKey) {
       return NS_ERROR_DOM_INVALID_ACCESS_ERR;
     }
 
     // Compute the MAC
     SECItem param = { siBuffer, nullptr, 0 };
     ScopedPK11Context ctx(PK11_CreateContextBySymKey(mMechanism, CKA_SIGN,
                                                      symKey.get(), &param));
@@ -1064,17 +1076,17 @@ private:
   bool mSign;
   bool mVerified;
   bool mEcdsa;
 
   virtual nsresult DoCrypto() MOZ_OVERRIDE
   {
     nsresult rv;
     if (mSign) {
-      ScopedSECItem signature((SECItem*) PORT_Alloc(sizeof(SECItem)));
+      ScopedSECItem signature(::SECITEM_AllocItem(nullptr, nullptr, 0));
       ScopedSGNContext ctx(SGN_NewContext(mOidTag, mPrivKey));
       if (!signature.get() || !ctx.get()) {
         return NS_ERROR_DOM_OPERATION_ERR;
       }
 
       rv = MapSECStatus(SEC_SignData(signature, mData.Elements(),
                                      mData.Length(), mPrivKey, mOidTag));
 
@@ -1088,37 +1100,33 @@ private:
         }
 
         ATTEMPT_BUFFER_ASSIGN(mSignature, rawSignature);
       } else {
         ATTEMPT_BUFFER_ASSIGN(mSignature, signature);
       }
 
     } else {
-      ScopedSECItem signature;
+      ScopedSECItem signature(::SECITEM_AllocItem(nullptr, nullptr, 0));
+      if (!signature.get()) {
+        return NS_ERROR_DOM_UNKNOWN_ERR;
+      }
 
       if (mEcdsa) {
         // DER-encode the signature
-        ScopedSECItem rawSignature(mSignature.ToSECItem());
-        if (!rawSignature.get()) {
+        ScopedSECItem rawSignature(::SECITEM_AllocItem(nullptr, nullptr, 0));
+        if (!rawSignature || !mSignature.ToSECItem(nullptr, rawSignature)) {
           return NS_ERROR_DOM_UNKNOWN_ERR;
         }
 
-        signature = (SECItem*) PORT_Alloc(sizeof(SECItem));
-        if (!signature.get()) {
-          return NS_ERROR_DOM_UNKNOWN_ERR;
-        }
         rv = MapSECStatus(DSAU_EncodeDerSigWithLen(signature, rawSignature,
                                                    rawSignature->len));
         NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_OPERATION_ERR);
-      } else {
-        signature = mSignature.ToSECItem();
-        if (!signature) {
-          return NS_ERROR_DOM_UNKNOWN_ERR;
-        }
+      } else if (!mSignature.ToSECItem(nullptr, signature)) {
+        return NS_ERROR_DOM_UNKNOWN_ERR;
       }
 
       rv = MapSECStatus(VFY_VerifyData(mData.Elements(), mData.Length(),
                                        mPubKey, signature, mOidTag, nullptr));
       mVerified = NS_SUCCEEDED(rv);
     }
 
     return NS_OK;
@@ -2396,40 +2404,45 @@ private:
   size_t mLength;
   size_t mIterations;
   CryptoBuffer mSalt;
   CryptoBuffer mSymKey;
   SECOidTag mHashOidTag;
 
   virtual nsresult DoCrypto() MOZ_OVERRIDE
   {
-    ScopedSECItem salt;
-    ATTEMPT_BUFFER_TO_SECITEM(salt, mSalt);
+    ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+    if (!arena) {
+      return NS_ERROR_DOM_OPERATION_ERR;
+    }
+
+    SECItem salt = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &salt, mSalt);
 
     // Always pass in cipherAlg=SEC_OID_HMAC_SHA1 (i.e. PBMAC1) as this
     // parameter is unused for key generation. It is currently only used
     // for PBKDF2 authentication or key (un)wrapping when specifying an
     // encryption algorithm (PBES2).
     ScopedSECAlgorithmID alg_id(PK11_CreatePBEV2AlgorithmID(
       SEC_OID_PKCS5_PBKDF2, SEC_OID_HMAC_SHA1, mHashOidTag,
-      mLength, mIterations, salt));
+      mLength, mIterations, &salt));
 
     if (!alg_id.get()) {
       return NS_ERROR_DOM_OPERATION_ERR;
     }
 
     ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
     if (!slot.get()) {
       return NS_ERROR_DOM_OPERATION_ERR;
     }
 
-    ScopedSECItem keyItem;
-    ATTEMPT_BUFFER_TO_SECITEM(keyItem, mSymKey);
-
-    ScopedPK11SymKey symKey(PK11_PBEKeyGen(slot, alg_id, keyItem, false, nullptr));
+    SECItem keyItem = { siBuffer, nullptr, 0 };
+    ATTEMPT_BUFFER_TO_SECITEM(arena, &keyItem, mSymKey);
+
+    ScopedPK11SymKey symKey(PK11_PBEKeyGen(slot, alg_id, &keyItem, false, nullptr));
     if (!symKey.get()) {
       return NS_ERROR_DOM_OPERATION_ERR;
     }
 
     nsresult rv = MapSECStatus(PK11_ExtractKeyValue(symKey));
     if (NS_FAILED(rv)) {
       return NS_ERROR_DOM_OPERATION_ERR;
     }