Attachment 358413 Details for Bug 193154 – Patch v4

[patch] Patch v4

bug-193154-20190104182604.patch (text/plain), 26.16 KB, created by David Kilzer (:ddkilzer) on 2019-01-04 18:26:05 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2019-01-04 18:26:05 PST

Size: 26.16 KB

patch

obsolete

>Subversion Revision: 239633
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7b30489104eecef630bacfe1e9d6f40d35208b38..00af4aec2bdcd2a945bf84ae890bd12493fca21c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,45 @@
>+2019-01-04  David Kilzer  <ddkilzer@apple.com>
>+
>+        Leak of two CCRSACryptorRef (4.0 Kbytes/1 page each) in com.apple.WebKit.WebContent running WebKit layout tests
>+        <https://webkit.org/b/193154>
>+        <rdar://problem/47052993>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Broadly:
>+        - Fix leaks by switching from using raw pointers to using
>+          std::unique_ptr<>.
>+        - Introduce PlatformRSAKeyContainer type to handle different
>+          std::unique_ptr<> on each platform.
>+        - Get rid of custom CryptoKeyRSA destructors since the
>+          std::unique_ptr<> handles that with a Deleter.
>+        - Initialize stack variables to nullptr.
>+
>+        * crypto/gcrypt/CryptoKeyRSAGCrypt.cpp:
>+        (WebCore::CryptoKeyRSA::create):
>+        (WebCore::CryptoKeyRSA::CryptoKeyRSA):
>+        (WebCore::CryptoKeyRSA::keySizeInBits const):
>+        (WebCore::CryptoKeyRSA::generatePair):
>+        (WebCore::CryptoKeyRSA::importSpki):
>+        (WebCore::CryptoKeyRSA::importPkcs8):
>+        (WebCore::CryptoKeyRSA::exportSpki const):
>+        (WebCore::CryptoKeyRSA::exportPkcs8 const):
>+        (WebCore::CryptoKeyRSA::algorithm const):
>+        (WebCore::CryptoKeyRSA::exportData const):
>+        (WebCore::CryptoKeyRSA::~CryptoKeyRSA): Deleted.
>+        * crypto/keys/CryptoKeyRSA.h:
>+        (WKCCRSACryptorRefDeleter::operator() const):
>+        * crypto/mac/CryptoKeyRSAMac.cpp:
>+        (WebCore::getPublicKeyComponents):
>+        (WebCore::getPrivateKeyComponents):
>+        (WebCore::CryptoKeyRSA::CryptoKeyRSA):
>+        (WebCore::CryptoKeyRSA::create):
>+        (WebCore::CryptoKeyRSA::exportData const):
>+        (WebCore::CryptoKeyRSA::generatePair):
>+        (WebCore::CryptoKeyRSA::importSpki):
>+        (WebCore::CryptoKeyRSA::importPkcs8):
>+        (WebCore::CryptoKeyRSA::~CryptoKeyRSA): Deleted.
>+
> 2019-01-04  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Cocoa] Merge WebEditCommandProxy::nameForEditAction and undoNameForEditAction into a single function
>diff --git a/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp b/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp
>index dfc5ba4d9be0dfe9da2254fe285583351d925400..5b897275076c98a11facc831f275b006f115773a 100644
>--- a/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp
>+++ b/Source/WebCore/crypto/gcrypt/CryptoKeyRSAGCrypt.cpp
>@@ -36,7 +36,6 @@
> #include <JavaScriptCore/GenericTypedArrayViewInlines.h>
> #include <JavaScriptCore/HeapInlines.h>
> #include <JavaScriptCore/JSGenericTypedArrayViewInlines.h>
>-#include <pal/crypto/gcrypt/Handle.h>
> #include <pal/crypto/gcrypt/Utilities.h>
> #include <pal/crypto/tasn1/Utilities.h>
> 
>@@ -145,23 +144,17 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::create(CryptoAlgorithmIdentifier identifier,
>         }
>     }
> 
>-    return adoptRef(new CryptoKeyRSA(identifier, hash, hasHash, keyType, keySexp.release(), extractable, usages));
>+    return adoptRef(new CryptoKeyRSA(identifier, hash, hasHash, keyType, PlatformRSAKeyContainer(keySexp.release()), extractable, usages));
> }
> 
>-CryptoKeyRSA::CryptoKeyRSA(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKey platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>+CryptoKeyRSA::CryptoKeyRSA(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKeyContainer&& platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>     : CryptoKey(identifier, type, extractable, usage)
>-    , m_platformKey(platformKey)
>+    , m_platformKey(WTFMove(platformKey))
>     , m_restrictedToSpecificHash(hasHash)
>     , m_hash(hash)
> {
> }
> 
>-CryptoKeyRSA::~CryptoKeyRSA()
>-{
>-    if (m_platformKey)
>-        PAL::GCrypt::HandleDeleter<gcry_sexp_t>()(m_platformKey);
>-}
>-
> bool CryptoKeyRSA::isRestrictedToHash(CryptoAlgorithmIdentifier& identifier) const
> {
>     if (!m_restrictedToSpecificHash)
>@@ -173,7 +166,7 @@ bool CryptoKeyRSA::isRestrictedToHash(CryptoAlgorithmIdentifier& identifier) con
> 
> size_t CryptoKeyRSA::keySizeInBits() const
> {
>-    return getRSAModulusLength(m_platformKey);
>+    return getRSAModulusLength(m_platformKey.get());
> }
> 
> // Convert the exponent vector to a 32-bit value, if possible.
>@@ -232,9 +225,9 @@ void CryptoKeyRSA::generatePair(CryptoAlgorithmIdentifier algorithm, CryptoAlgor
>     }
> 
>     context->postTask(
>-        [algorithm, hash, hasHash, extractable, usage, publicKeySexp = publicKeySexp.release(), privateKeySexp = privateKeySexp.release(), callback = WTFMove(callback)](auto&) mutable {
>-            auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, publicKeySexp, true, usage);
>-            auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, privateKeySexp, extractable, usage);
>+        [algorithm, hash, hasHash, extractable, usage, publicKeySexp = PlatformRSAKeyContainer(publicKeySexp.release()), privateKeySexp = PlatformRSAKeyContainer(privateKeySexp.release()), callback = WTFMove(callback)](auto&) mutable {
>+            auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, WTFMove(publicKeySexp), true, usage);
>+            auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, WTFMove(privateKeySexp), extractable, usage);
> 
>             callback(CryptoKeyPair { WTFMove(publicKey), WTFMove(privateKey) });
>         });
>@@ -313,7 +306,7 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::importSpki(CryptoAlgorithmIdentifier identifi
>         }
>     }
> 
>-    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Public, platformKey.release(), extractable, usages));
>+    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Public, PlatformRSAKeyContainer(platformKey.release()), extractable, usages));
> }
> 
> RefPtr<CryptoKeyRSA> CryptoKeyRSA::importPkcs8(CryptoAlgorithmIdentifier identifier, Optional<CryptoAlgorithmIdentifier> hash, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap usages)
>@@ -408,7 +401,7 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::importPkcs8(CryptoAlgorithmIdentifier identif
>         }
>     }
> 
>-    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Private, platformKey.release(), extractable, usages));
>+    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Private, PlatformRSAKeyContainer(platformKey.release()), extractable, usages));
> }
> 
> ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportSpki() const
>@@ -423,8 +416,8 @@ ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportSpki() const
>             return Exception { OperationError };
> 
>         // Retrieve the modulus and public exponent s-expressions.
>-        PAL::GCrypt::Handle<gcry_sexp_t> modulusSexp(gcry_sexp_find_token(m_platformKey, "n", 0));
>-        PAL::GCrypt::Handle<gcry_sexp_t> publicExponentSexp(gcry_sexp_find_token(m_platformKey, "e", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> modulusSexp(gcry_sexp_find_token(m_platformKey.get(), "n", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> publicExponentSexp(gcry_sexp_find_token(m_platformKey.get(), "e", 0));
>         if (!modulusSexp || !publicExponentSexp)
>             return Exception { OperationError };
> 
>@@ -498,11 +491,11 @@ ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportPkcs8() const
> 
>         // Retrieve the `n`, `e`, `d`, `q` and `p` s-expression tokens. libgcrypt swaps the usage of
>         // the p and q primes internally, so we adjust the lookup accordingly.
>-        PAL::GCrypt::Handle<gcry_sexp_t> nSexp(gcry_sexp_find_token(m_platformKey, "n", 0));
>-        PAL::GCrypt::Handle<gcry_sexp_t> eSexp(gcry_sexp_find_token(m_platformKey, "e", 0));
>-        PAL::GCrypt::Handle<gcry_sexp_t> dSexp(gcry_sexp_find_token(m_platformKey, "d", 0));
>-        PAL::GCrypt::Handle<gcry_sexp_t> pSexp(gcry_sexp_find_token(m_platformKey, "q", 0));
>-        PAL::GCrypt::Handle<gcry_sexp_t> qSexp(gcry_sexp_find_token(m_platformKey, "p", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> nSexp(gcry_sexp_find_token(m_platformKey.get(), "n", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> eSexp(gcry_sexp_find_token(m_platformKey.get(), "e", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> dSexp(gcry_sexp_find_token(m_platformKey.get(), "d", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> pSexp(gcry_sexp_find_token(m_platformKey.get(), "q", 0));
>+        PAL::GCrypt::Handle<gcry_sexp_t> qSexp(gcry_sexp_find_token(m_platformKey.get(), "p", 0));
>         if (!nSexp || !eSexp || !dSexp || !pSexp || !qSexp)
>             return Exception { OperationError };
> 
>@@ -622,8 +615,8 @@ ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportPkcs8() const
> 
> auto CryptoKeyRSA::algorithm() const -> KeyAlgorithm
> {
>-    auto modulusLength = getRSAModulusLength(m_platformKey);
>-    auto publicExponent = getRSAKeyParameter(m_platformKey, "e");
>+    auto modulusLength = getRSAModulusLength(m_platformKey.get());
>+    auto publicExponent = getRSAKeyParameter(m_platformKey.get(), "e");
> 
>     if (m_restrictedToSpecificHash) {
>         CryptoRsaHashedKeyAlgorithm result;
>@@ -645,7 +638,7 @@ std::unique_ptr<CryptoKeyRSAComponents> CryptoKeyRSA::exportData() const
> {
>     switch (type()) {
>     case CryptoKeyType::Public:
>-        return CryptoKeyRSAComponents::createPublic(getRSAKeyParameter(m_platformKey, "n"), getRSAKeyParameter(m_platformKey, "e"));
>+        return CryptoKeyRSAComponents::createPublic(getRSAKeyParameter(m_platformKey.get(), "n"), getRSAKeyParameter(m_platformKey.get(), "e"));
>     case CryptoKeyType::Private: {
>         auto parameterMPI =
>             [](gcry_sexp_t sexp, const char* name) -> gcry_mpi_t {
>@@ -655,11 +648,11 @@ std::unique_ptr<CryptoKeyRSAComponents> CryptoKeyRSA::exportData() const
>                 return gcry_sexp_nth_mpi(paramSexp, 1, GCRYMPI_FMT_USG);
>             };
> 
>-        PAL::GCrypt::Handle<gcry_mpi_t> dMPI(parameterMPI(m_platformKey, "d"));
>+        PAL::GCrypt::Handle<gcry_mpi_t> dMPI(parameterMPI(m_platformKey.get(), "d"));
>         // libgcrypt internally uses p and q such that p < q, while usually it's q < p.
>         // Switch the two primes here and continue with assuming the latter.
>-        PAL::GCrypt::Handle<gcry_mpi_t> pMPI(parameterMPI(m_platformKey, "q"));
>-        PAL::GCrypt::Handle<gcry_mpi_t> qMPI(parameterMPI(m_platformKey, "p"));
>+        PAL::GCrypt::Handle<gcry_mpi_t> pMPI(parameterMPI(m_platformKey.get(), "q"));
>+        PAL::GCrypt::Handle<gcry_mpi_t> qMPI(parameterMPI(m_platformKey.get(), "p"));
>         if (!dMPI || !pMPI || !qMPI)
>             return nullptr;
> 
>@@ -707,7 +700,7 @@ std::unique_ptr<CryptoKeyRSAComponents> CryptoKeyRSA::exportData() const
>             privateExponent = WTFMove(data.value());
> 
>         return CryptoKeyRSAComponents::createPrivateWithAdditionalData(
>-            getRSAKeyParameter(m_platformKey, "n"), getRSAKeyParameter(m_platformKey, "e"), WTFMove(privateExponent),
>+            getRSAKeyParameter(m_platformKey.get(), "n"), getRSAKeyParameter(m_platformKey.get(), "e"), WTFMove(privateExponent),
>             WTFMove(firstPrimeInfo), WTFMove(secondPrimeInfo), Vector<CryptoKeyRSAComponents::PrimeInfo> { });
>     }
>     default:
>diff --git a/Source/WebCore/crypto/keys/CryptoKeyRSA.h b/Source/WebCore/crypto/keys/CryptoKeyRSA.h
>index d24d67394c47580dc9967f025f22c5e59108a0c8..a0b8f3bf867cbf2e0bf4aa748ce08c7219d7346c 100644
>--- a/Source/WebCore/crypto/keys/CryptoKeyRSA.h
>+++ b/Source/WebCore/crypto/keys/CryptoKeyRSA.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -32,14 +32,18 @@
> #if ENABLE(WEB_CRYPTO)
> 
> #if OS(DARWIN) && !PLATFORM(GTK)
>-typedef struct _CCRSACryptor *CCRSACryptorRef;
>+#include "CommonCryptoUtilities.h"
> typedef CCRSACryptorRef PlatformRSAKey;
>+struct WKCCRSACryptorRefDeleter {
>+    void operator()(CCRSACryptorRef key) const { CCRSACryptorRelease(key); }
>+};
>+typedef std::unique_ptr<typename std::remove_pointer<CCRSACryptorRef>::type, WKCCRSACryptorRefDeleter> PlatformRSAKeyContainer;
> #endif
> 
> #if PLATFORM(GTK) || PLATFORM(WPE)
>-// gcry_sexp* equates gcry_sexp_t.
>-struct gcry_sexp;
>-typedef gcry_sexp* PlatformRSAKey;
>+#include <pal/crypto/gcrypt/Handle.h>
>+typedef gcry_sexp_t PlatformRSAKey;
>+typedef std::unique_ptr<typename std::remove_pointer<gcry_sexp_t>::type, PAL::GCrypt::HandleDeleter<gcry_sexp_t>> PlatformRSAKeyContainer;
> #endif
> 
> namespace WebCore {
>@@ -53,12 +57,12 @@ struct JsonWebKey;
> 
> class CryptoKeyRSA final : public CryptoKey {
> public:
>-    static Ref<CryptoKeyRSA> create(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKey platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>+    static Ref<CryptoKeyRSA> create(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKeyContainer&& platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>     {
>-        return adoptRef(*new CryptoKeyRSA(identifier, hash, hasHash, type, platformKey, extractable, usage));
>+        return adoptRef(*new CryptoKeyRSA(identifier, hash, hasHash, type, WTFMove(platformKey), extractable, usage));
>     }
>     static RefPtr<CryptoKeyRSA> create(CryptoAlgorithmIdentifier, CryptoAlgorithmIdentifier hash, bool hasHash, const CryptoKeyRSAComponents&, bool extractable, CryptoKeyUsageBitmap);
>-    virtual ~CryptoKeyRSA();
>+    ~CryptoKeyRSA() = default;
> 
>     bool isRestrictedToHash(CryptoAlgorithmIdentifier&) const;
> 
>@@ -71,7 +75,7 @@ public:
>     static RefPtr<CryptoKeyRSA> importSpki(CryptoAlgorithmIdentifier, Optional<CryptoAlgorithmIdentifier> hash, Vector<uint8_t>&&, bool extractable, CryptoKeyUsageBitmap);
>     static RefPtr<CryptoKeyRSA> importPkcs8(CryptoAlgorithmIdentifier, Optional<CryptoAlgorithmIdentifier> hash, Vector<uint8_t>&&, bool extractable, CryptoKeyUsageBitmap);
> 
>-    PlatformRSAKey platformKey() const { return m_platformKey; }
>+    PlatformRSAKey platformKey() const { return m_platformKey.get(); }
>     JsonWebKey exportJwk() const;
>     ExceptionOr<Vector<uint8_t>> exportSpki() const;
>     ExceptionOr<Vector<uint8_t>> exportPkcs8() const;
>@@ -81,13 +85,13 @@ public:
>     CryptoAlgorithmIdentifier hashAlgorithmIdentifier() const { return m_hash; }
> 
> private:
>-    CryptoKeyRSA(CryptoAlgorithmIdentifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType, PlatformRSAKey, bool extractable, CryptoKeyUsageBitmap);
>+    CryptoKeyRSA(CryptoAlgorithmIdentifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType, PlatformRSAKeyContainer&&, bool extractable, CryptoKeyUsageBitmap);
> 
>     CryptoKeyClass keyClass() const final { return CryptoKeyClass::RSA; }
> 
>     KeyAlgorithm algorithm() const final;
> 
>-    PlatformRSAKey m_platformKey;
>+    PlatformRSAKeyContainer m_platformKey;
> 
>     bool m_restrictedToSpecificHash;
>     CryptoAlgorithmIdentifier m_hash;
>diff --git a/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp b/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp
>index 11a7c916b612fd6339bde37b22cce259ab6a9704..214e5c09fc4b9f48adc16f5d998498de1d0d135b 100644
>--- a/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp
>+++ b/Source/WebCore/crypto/mac/CryptoKeyRSAMac.cpp
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -58,23 +58,19 @@ static const unsigned char RSAOIDHeader[] = {0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86,
> 
> // FIXME: We should get rid of magic number 16384. It assumes that the length of provided key will not exceed 16KB.
> // https://bugs.webkit.org/show_bug.cgi?id=164942
>-static CCCryptorStatus getPublicKeyComponents(CCRSACryptorRef rsaKey, Vector<uint8_t>& modulus, Vector<uint8_t>& publicExponent)
>+static CCCryptorStatus getPublicKeyComponents(const PlatformRSAKeyContainer& rsaKey, Vector<uint8_t>& modulus, Vector<uint8_t>& publicExponent)
> {
>-    ASSERT(CCRSAGetKeyType(rsaKey) == ccRSAKeyPublic || CCRSAGetKeyType(rsaKey) == ccRSAKeyPrivate);
>-    bool keyIsPublic = CCRSAGetKeyType(rsaKey) == ccRSAKeyPublic;
>+    ASSERT(CCRSAGetKeyType(rsaKey.get()) == ccRSAKeyPublic || CCRSAGetKeyType(rsaKey.get()) == ccRSAKeyPrivate);
>+    bool keyIsPublic = CCRSAGetKeyType(rsaKey.get()) == ccRSAKeyPublic;
>     ALLOW_DEPRECATED_DECLARATIONS_BEGIN
>-    CCRSACryptorRef publicKey = keyIsPublic ? rsaKey : CCRSACryptorGetPublicKeyFromPrivateKey(rsaKey);
>+    PlatformRSAKeyContainer publicKeyFromPrivateKey(keyIsPublic ? nullptr : CCRSACryptorGetPublicKeyFromPrivateKey(rsaKey.get()));
>     ALLOW_DEPRECATED_DECLARATIONS_END
> 
>     modulus.resize(16384);
>     size_t modulusLength = modulus.size();
>     publicExponent.resize(16384);
>     size_t exponentLength = publicExponent.size();
>-    CCCryptorStatus status = CCRSAGetKeyComponents(publicKey, modulus.data(), &modulusLength, publicExponent.data(), &exponentLength, 0, 0, 0, 0);
>-    if (!keyIsPublic) {
>-        // CCRSACryptorGetPublicKeyFromPrivateKey has "Get" in the name, but its result needs to be released (see <rdar://problem/15449697>).
>-        CCRSACryptorRelease(publicKey);
>-    }
>+    CCCryptorStatus status = CCRSAGetKeyComponents(keyIsPublic ? rsaKey.get() : publicKeyFromPrivateKey.get(), modulus.data(), &modulusLength, publicExponent.data(), &exponentLength, 0, 0, 0, 0);
>     if (status)
>         return status;
> 
>@@ -83,9 +79,9 @@ static CCCryptorStatus getPublicKeyComponents(CCRSACryptorRef rsaKey, Vector<uin
>     return status;
> }
> 
>-static CCCryptorStatus getPrivateKeyComponents(CCRSACryptorRef rsaKey, Vector<uint8_t>& privateExponent, CryptoKeyRSAComponents::PrimeInfo& firstPrimeInfo, CryptoKeyRSAComponents::PrimeInfo& secondPrimeInfo)
>+static CCCryptorStatus getPrivateKeyComponents(const PlatformRSAKeyContainer& rsaKey, Vector<uint8_t>& privateExponent, CryptoKeyRSAComponents::PrimeInfo& firstPrimeInfo, CryptoKeyRSAComponents::PrimeInfo& secondPrimeInfo)
> {
>-    ASSERT(CCRSAGetKeyType(rsaKey) == ccRSAKeyPrivate);
>+    ASSERT(CCRSAGetKeyType(rsaKey.get()) == ccRSAKeyPrivate);
> 
>     Vector<uint8_t> unusedModulus(16384);
>     size_t modulusLength = unusedModulus.size();
>@@ -96,7 +92,7 @@ static CCCryptorStatus getPrivateKeyComponents(CCRSACryptorRef rsaKey, Vector<ui
>     secondPrimeInfo.primeFactor.resize(16384);
>     size_t qLength = secondPrimeInfo.primeFactor.size();
> 
>-    CCCryptorStatus status = CCRSAGetKeyComponents(rsaKey, unusedModulus.data(), &modulusLength, privateExponent.data(), &exponentLength, firstPrimeInfo.primeFactor.data(), &pLength, secondPrimeInfo.primeFactor.data(), &qLength);
>+    CCCryptorStatus status = CCRSAGetKeyComponents(rsaKey.get(), unusedModulus.data(), &modulusLength, privateExponent.data(), &exponentLength, firstPrimeInfo.primeFactor.data(), &pLength, secondPrimeInfo.primeFactor.data(), &qLength);
>     if (status)
>         return status;
> 
>@@ -108,13 +104,13 @@ static CCCryptorStatus getPrivateKeyComponents(CCRSACryptorRef rsaKey, Vector<ui
>     size_t dpSize;
>     size_t dqSize;
>     size_t qinvSize;
>-    if (auto status = CCRSAGetCRTComponentsSizes(rsaKey, &dpSize, &dqSize, &qinvSize))
>+    if (auto status = CCRSAGetCRTComponentsSizes(rsaKey.get(), &dpSize, &dqSize, &qinvSize))
>         return status;
> 
>     Vector<uint8_t> dp(dpSize);
>     Vector<uint8_t> dq(dqSize);
>     Vector<uint8_t> qinv(qinvSize);
>-    if (auto status = CCRSAGetCRTComponents(rsaKey, dp.data(), dpSize, dq.data(), dqSize, qinv.data(), qinvSize))
>+    if (auto status = CCRSAGetCRTComponents(rsaKey.get(), dp.data(), dpSize, dq.data(), dqSize, qinv.data(), qinvSize))
>         return status;
> 
>     firstPrimeInfo.factorCRTExponent = WTFMove(dp);
>@@ -137,9 +133,9 @@ static CCCryptorStatus getPrivateKeyComponents(CCRSACryptorRef rsaKey, Vector<ui
>     return status;
> }
> 
>-CryptoKeyRSA::CryptoKeyRSA(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKey platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>+CryptoKeyRSA::CryptoKeyRSA(CryptoAlgorithmIdentifier identifier, CryptoAlgorithmIdentifier hash, bool hasHash, CryptoKeyType type, PlatformRSAKeyContainer&& platformKey, bool extractable, CryptoKeyUsageBitmap usage)
>     : CryptoKey(identifier, type, extractable, usage)
>-    , m_platformKey(platformKey)
>+    , m_platformKey(WTFMove(platformKey))
>     , m_restrictedToSpecificHash(hasHash)
>     , m_hash(hash)
> {
>@@ -162,7 +158,7 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::create(CryptoAlgorithmIdentifier identifier,
>     if (keyData.type() == CryptoKeyRSAComponents::Type::Private && keyData.firstPrimeInfo().primeFactor.isEmpty())
>         return nullptr;
> 
>-    CCRSACryptorRef cryptor;
>+    CCRSACryptorRef cryptor = nullptr;
>     // FIXME: It is so weired that we recaculate the private exponent from first prime factor and second prime factor,
>     // given the fact that we have already had it. Also, the re-caculated private exponent may not match the given one.
>     // See <rdar://problem/15452324>.
>@@ -179,12 +175,7 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::create(CryptoAlgorithmIdentifier identifier,
>         return nullptr;
>     }
> 
>-    return adoptRef(new CryptoKeyRSA(identifier, hash, hasHash, keyData.type() == CryptoKeyRSAComponents::Type::Public ? CryptoKeyType::Public : CryptoKeyType::Private, cryptor, extractable, usage));
>-}
>-
>-CryptoKeyRSA::~CryptoKeyRSA()
>-{
>-    CCRSACryptorRelease(m_platformKey);
>+    return adoptRef(new CryptoKeyRSA(identifier, hash, hasHash, keyData.type() == CryptoKeyRSAComponents::Type::Public ? CryptoKeyType::Public : CryptoKeyType::Private, PlatformRSAKeyContainer(cryptor), extractable, usage));
> }
> 
> bool CryptoKeyRSA::isRestrictedToHash(CryptoAlgorithmIdentifier& identifier) const
>@@ -248,7 +239,7 @@ auto CryptoKeyRSA::algorithm() const -> KeyAlgorithm
> 
> std::unique_ptr<CryptoKeyRSAComponents> CryptoKeyRSA::exportData() const
> {
>-    switch (CCRSAGetKeyType(m_platformKey)) {
>+    switch (CCRSAGetKeyType(m_platformKey.get())) {
>     case ccRSAKeyPublic: {
>         Vector<uint8_t> modulus;
>         Vector<uint8_t> publicExponent;
>@@ -318,8 +309,8 @@ void CryptoKeyRSA::generatePair(CryptoAlgorithmIdentifier algorithm, CryptoAlgor
>     // FIXME: There is a risk that localCallback and localFailureCallback are never freed.
>     // Fix this by using unique pointers and move them from one lambda to the other.
>     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
>-        CCRSACryptorRef ccPublicKey;
>-        CCRSACryptorRef ccPrivateKey;
>+        CCRSACryptorRef ccPublicKey = nullptr;
>+        CCRSACryptorRef ccPrivateKey = nullptr;
>         CCCryptorStatus status = CCRSACryptorGeneratePair(modulusLength, e, &ccPublicKey, &ccPrivateKey);
>         if (status) {
>             WTFLogAlways("Could not generate a key pair, status %d", status);
>@@ -330,9 +321,9 @@ void CryptoKeyRSA::generatePair(CryptoAlgorithmIdentifier algorithm, CryptoAlgor
>             });
>             return;
>         }
>-        ScriptExecutionContext::postTaskTo(contextIdentifier, [algorithm, hash, hasHash, extractable, usage, localCallback, localFailureCallback, ccPublicKey, ccPrivateKey](auto&) {
>-            auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, ccPublicKey, true, usage);
>-            auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, ccPrivateKey, extractable, usage);
>+        ScriptExecutionContext::postTaskTo(contextIdentifier, [algorithm, hash, hasHash, extractable, usage, localCallback, localFailureCallback, ccPublicKey = PlatformRSAKeyContainer(ccPublicKey), ccPrivateKey = PlatformRSAKeyContainer(ccPrivateKey)](auto&) mutable {
>+            auto publicKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Public, WTFMove(ccPublicKey), true, usage);
>+            auto privateKey = CryptoKeyRSA::create(algorithm, hash, hasHash, CryptoKeyType::Private, WTFMove(ccPrivateKey), extractable, usage);
> 
>             (*localCallback)(CryptoKeyPair { WTFMove(publicKey), WTFMove(privateKey) });
> 
>@@ -358,12 +349,12 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::importSpki(CryptoAlgorithmIdentifier identifi
>         return nullptr;
>     headerSize += bytesUsedToEncodedLength(keyData[headerSize]) + sizeof(InitialOctet);
> 
>-    CCRSACryptorRef ccPublicKey;
>+    CCRSACryptorRef ccPublicKey = nullptr;
>     if (CCRSACryptorImport(keyData.data() + headerSize, keyData.size() - headerSize, &ccPublicKey))
>         return nullptr;
> 
>     // Notice: CryptoAlgorithmIdentifier::SHA_1 is just a placeholder. It should not have any effect if hash is WTF::nullopt.
>-    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Public, ccPublicKey, extractable, usages));
>+    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Public, PlatformRSAKeyContainer(ccPublicKey), extractable, usages));
> }
> 
> ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportSpki() const
>@@ -415,12 +406,12 @@ RefPtr<CryptoKeyRSA> CryptoKeyRSA::importPkcs8(CryptoAlgorithmIdentifier identif
>         return nullptr;
>     headerSize += bytesUsedToEncodedLength(keyData[headerSize]);
> 
>-    CCRSACryptorRef ccPrivateKey;
>+    CCRSACryptorRef ccPrivateKey = nullptr;
>     if (CCRSACryptorImport(keyData.data() + headerSize, keyData.size() - headerSize, &ccPrivateKey))
>         return nullptr;
> 
>     // Notice: CryptoAlgorithmIdentifier::SHA_1 is just a placeholder. It should not have any effect if hash is WTF::nullopt.
>-    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Private, ccPrivateKey, extractable, usages));
>+    return adoptRef(new CryptoKeyRSA(identifier, hash.valueOr(CryptoAlgorithmIdentifier::SHA_1), !!hash, CryptoKeyType::Private, PlatformRSAKeyContainer(ccPrivateKey), extractable, usages));
> }
> 
> ExceptionOr<Vector<uint8_t>> CryptoKeyRSA::exportPkcs8() const

Actions: View | Formatted Diff | Diff

Attachments on bug 193154: 358350 | 358356 | 358408 | 358411 | 358413 | 358426 | 358434 | 358435