Title: [220933] trunk
Revision
220933
Author
[email protected]
Date
2017-08-18 14:30:45 -0700 (Fri, 18 Aug 2017)

Log Message

[WebCrypto][Mac] Replace CCECCryptorGetKeyComponents with CCECCryptorExportKey for exporting ECC JWKs
https://bugs.webkit.org/show_bug.cgi?id=175657
<rdar://problem/33797150>

Reviewed by Brent Fulgham.

Source/WebCore:

CCECCryptorGetKeyComponents returns components in unpadded formats. In some minor cases, the ECC components
do need padding. Therefore, we occasionally see some corrupted outputs in JWKs. To overcome that, this patch
replaces CCECCryptorGetKeyComponents with CCECCryptorExportKey which does padding all the time.

In the meantime, this patch also makes export* methods return OperationError if any error occur in the
underlying operations though very unlikely.

Test: crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html

* crypto/algorithms/CryptoAlgorithmECDH.cpp:
(WebCore::CryptoAlgorithmECDH::exportKey):
* crypto/algorithms/CryptoAlgorithmECDSA.cpp:
(WebCore::CryptoAlgorithmECDSA::exportKey):
* crypto/gcrypt/CryptoKeyECGCrypt.cpp:
(WebCore::CryptoKeyEC::platformAddFieldElements const):
* crypto/keys/CryptoKeyEC.cpp:
(WebCore::CryptoKeyEC::exportRaw const):
(WebCore::CryptoKeyEC::exportJwk const):
(WebCore::CryptoKeyEC::exportSpki const):
(WebCore::CryptoKeyEC::exportPkcs8 const):
* crypto/keys/CryptoKeyEC.h:
* crypto/mac/CryptoKeyECMac.cpp:
(WebCore::CryptoKeyEC::platformExportRaw const):
(WebCore::CryptoKeyEC::platformAddFieldElements const):
(WebCore::CryptoKeyEC::platformExportSpki const):
(WebCore::CryptoKeyEC::platformExportPkcs8 const):

LayoutTests:

* crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify-expected.txt: Added.
* crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (220932 => 220933)


--- trunk/LayoutTests/ChangeLog	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/LayoutTests/ChangeLog	2017-08-18 21:30:45 UTC (rev 220933)
@@ -1,3 +1,14 @@
+2017-08-18  Jiewen Tan  <[email protected]>
+
+        [WebCrypto][Mac] Replace CCECCryptorGetKeyComponents with CCECCryptorExportKey for exporting ECC JWKs
+        https://bugs.webkit.org/show_bug.cgi?id=175657
+        <rdar://problem/33797150>
+
+        Reviewed by Brent Fulgham.
+
+        * crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify-expected.txt: Added.
+        * crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html: Added.
+
 2017-08-18  Matt Lewis  <[email protected]>
 
         Marked imported/w3c/web-platform-tests/IndexedDB/open-request-queue.html as flaky timeout.

Added: trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify-expected.txt (0 => 220933)


--- trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify-expected.txt	2017-08-18 21:30:45 UTC (rev 220933)
@@ -0,0 +1,23 @@
+This test tries to test whether the JWK format exporting/importing methods work as expected.To do so, it first generates an EC Key Pair, then exports them into JWKs, then imports them back, and finally uses them to do signature verification.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Generating a key pair...
+Exporting the private key in Jwk format...
+PASS jwkPrivateKey.x is defined.
+PASS jwkPrivateKey.y is defined.
+PASS jwkPrivateKey.d is defined.
+Importing the JWK private key...
+Signing the data...
+PASS signature.byteLength is 64
+Exporting the public key in Jwk format...
+PASS jwkPublicKey.x is defined.
+PASS jwkPublicKey.y is defined.
+Importing the JWK public key...
+Verifying the signature...
+PASS verified is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html (0 => 220933)


--- trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html	                        (rev 0)
+++ trunk/LayoutTests/crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html	2017-08-18 21:30:45 UTC (rev 220933)
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src=""
+    <script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+description("This test tries to test whether the JWK format exporting/importing methods work as expected." +
+    "To do so, it first generates an EC Key Pair, then exports them into JWKs, then imports them back, and finally uses them to do signature verification.");
+
+jsTestIsAsync = true;
+
+var extractable = true;
+var ecKeyParams = {
+    name: "ECDSA",
+    namedCurve: "P-256",
+}
+var ecdsaParams = {
+    name: "ECDSA",
+    hash: "SHA-256",
+}
+var data = "" World!");
+
+debug("Generating a key pair...");
+crypto.subtle.generateKey(ecKeyParams, extractable, ["verify", "sign"]).then(function(result) {
+    keyPair = result;
+
+    debug("Exporting the private key in Jwk format...");
+    return crypto.subtle.exportKey("jwk", keyPair.privateKey);
+}, failAndFinishJSTest).then(function(result) {
+    jwkPrivateKey = result;
+    shouldBeDefined("jwkPrivateKey.x");
+    shouldBeDefined("jwkPrivateKey.y");
+    shouldBeDefined("jwkPrivateKey.d");
+
+    debug("Importing the JWK private key...");
+    return crypto.subtle.importKey("jwk", jwkPrivateKey, ecKeyParams, extractable, ["sign"]);
+}, failAndFinishJSTest).then(function(privateKey) {
+    debug("Signing the data...");
+    return crypto.subtle.sign(ecdsaParams, privateKey, data);
+}, failAndFinishJSTest).then(function(result) {
+    signature = result;
+    shouldBe("signature.byteLength", "64");
+
+    debug("Exporting the public key in Jwk format...");
+    return crypto.subtle.exportKey("jwk", keyPair.publicKey);
+}, failAndFinishJSTest).then(function(result) {
+    jwkPublicKey = result;
+    shouldBeDefined("jwkPublicKey.x");
+    shouldBeDefined("jwkPublicKey.y");
+
+    debug("Importing the JWK public key...");
+    return crypto.subtle.importKey("jwk", jwkPublicKey, ecKeyParams, extractable, ["verify"]);
+}, failAndFinishJSTest).then(function(publicKey) {
+    debug("Verifying the signature...");
+    return crypto.subtle.verify(ecdsaParams, publicKey, signature, data);
+}, failAndFinishJSTest).then(function(result) {
+    verified = result;
+
+    shouldBeTrue("verified");
+
+    finishJSTest();
+}, failAndFinishJSTest);
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (220932 => 220933)


--- trunk/Source/WebCore/ChangeLog	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/ChangeLog	2017-08-18 21:30:45 UTC (rev 220933)
@@ -1,3 +1,38 @@
+2017-08-18  Jiewen Tan  <[email protected]>
+
+        [WebCrypto][Mac] Replace CCECCryptorGetKeyComponents with CCECCryptorExportKey for exporting ECC JWKs
+        https://bugs.webkit.org/show_bug.cgi?id=175657
+        <rdar://problem/33797150>
+
+        Reviewed by Brent Fulgham.
+
+        CCECCryptorGetKeyComponents returns components in unpadded formats. In some minor cases, the ECC components
+        do need padding. Therefore, we occasionally see some corrupted outputs in JWKs. To overcome that, this patch
+        replaces CCECCryptorGetKeyComponents with CCECCryptorExportKey which does padding all the time.
+
+        In the meantime, this patch also makes export* methods return OperationError if any error occur in the
+        underlying operations though very unlikely.
+
+        Test: crypto/subtle/ecdsa-generate-export-import-jwk-sign-verify.html
+
+        * crypto/algorithms/CryptoAlgorithmECDH.cpp:
+        (WebCore::CryptoAlgorithmECDH::exportKey):
+        * crypto/algorithms/CryptoAlgorithmECDSA.cpp:
+        (WebCore::CryptoAlgorithmECDSA::exportKey):
+        * crypto/gcrypt/CryptoKeyECGCrypt.cpp:
+        (WebCore::CryptoKeyEC::platformAddFieldElements const):
+        * crypto/keys/CryptoKeyEC.cpp:
+        (WebCore::CryptoKeyEC::exportRaw const):
+        (WebCore::CryptoKeyEC::exportJwk const):
+        (WebCore::CryptoKeyEC::exportSpki const):
+        (WebCore::CryptoKeyEC::exportPkcs8 const):
+        * crypto/keys/CryptoKeyEC.h:
+        * crypto/mac/CryptoKeyECMac.cpp:
+        (WebCore::CryptoKeyEC::platformExportRaw const):
+        (WebCore::CryptoKeyEC::platformAddFieldElements const):
+        (WebCore::CryptoKeyEC::platformExportSpki const):
+        (WebCore::CryptoKeyEC::platformExportPkcs8 const):
+
 2017-08-18  Per Arne Vollan  <[email protected]>
 
         [Win] accessibility/heading-crash-after-hidden.html is a flaky crash.

Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp (220932 => 220933)


--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDH.cpp	2017-08-18 21:30:45 UTC (rev 220933)
@@ -186,9 +186,15 @@
 
     KeyData result;
     switch (format) {
-    case CryptoKeyFormat::Jwk:
-        result = ecKey.exportJwk();
+    case CryptoKeyFormat::Jwk: {
+        auto jwk = ecKey.exportJwk();
+        if (jwk.hasException()) {
+            exceptionCallback(jwk.releaseException().code());
+            return;
+        }
+        result = jwk.releaseReturnValue();
         break;
+    }
     case CryptoKeyFormat::Raw: {
         auto raw = ecKey.exportRaw();
         if (raw.hasException()) {

Modified: trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDSA.cpp (220932 => 220933)


--- trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDSA.cpp	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/algorithms/CryptoAlgorithmECDSA.cpp	2017-08-18 21:30:45 UTC (rev 220933)
@@ -160,9 +160,15 @@
 
     KeyData result;
     switch (format) {
-    case CryptoKeyFormat::Jwk:
-        result = ecKey.exportJwk();
+    case CryptoKeyFormat::Jwk: {
+        auto jwk = ecKey.exportJwk();
+        if (jwk.hasException()) {
+            exceptionCallback(jwk.releaseException().code());
+            return;
+        }
+        result = jwk.releaseReturnValue();
         break;
+    }
     case CryptoKeyFormat::Raw: {
         auto raw = ecKey.exportRaw();
         if (raw.hasException()) {

Modified: trunk/Source/WebCore/crypto/gcrypt/CryptoKeyECGCrypt.cpp (220932 => 220933)


--- trunk/Source/WebCore/crypto/gcrypt/CryptoKeyECGCrypt.cpp	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/gcrypt/CryptoKeyECGCrypt.cpp	2017-08-18 21:30:45 UTC (rev 220933)
@@ -500,13 +500,13 @@
     return WTFMove(q.value());
 }
 
-void CryptoKeyEC::platformAddFieldElements(JsonWebKey& jwk) const
+bool CryptoKeyEC::platformAddFieldElements(JsonWebKey& jwk) const
 {
     PAL::GCrypt::Handle<gcry_ctx_t> context;
     gcry_error_t error = gcry_mpi_ec_new(&context, m_platformKey, nullptr);
     if (error != GPG_ERR_NO_ERROR) {
         PAL::GCrypt::logError(error);
-        return;
+        return false;
     }
 
     unsigned uncompressedFieldElementSize = curveUncompressedFieldElementSize(m_curve);
@@ -533,6 +533,8 @@
                 jwk.d = base64URLEncode(*d);
         }
     }
+
+    return true;
 }
 
 Vector<uint8_t> CryptoKeyEC::platformExportSpki() const

Modified: trunk/Source/WebCore/crypto/keys/CryptoKeyEC.cpp (220932 => 220933)


--- trunk/Source/WebCore/crypto/keys/CryptoKeyEC.cpp	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/keys/CryptoKeyEC.cpp	2017-08-18 21:30:45 UTC (rev 220933)
@@ -143,10 +143,13 @@
     if (type() != CryptoKey::Type::Public)
         return Exception { InvalidAccessError };
 
-    return platformExportRaw();
+    auto&& result = platformExportRaw();
+    if (result.isEmpty())
+        return Exception { OperationError };
+    return WTFMove(result);
 }
 
-JsonWebKey CryptoKeyEC::exportJwk() const
+ExceptionOr<JsonWebKey> CryptoKeyEC::exportJwk() const
 {
     JsonWebKey result;
     result.kty = "EC";
@@ -160,8 +163,9 @@
     }
     result.key_ops = usages();
     result.ext = extractable();
-    platformAddFieldElements(result);
-    return result;
+    if (!platformAddFieldElements(result))
+        return Exception { OperationError };
+    return WTFMove(result);
 }
 
 ExceptionOr<Vector<uint8_t>> CryptoKeyEC::exportSpki() const
@@ -169,7 +173,10 @@
     if (type() != CryptoKey::Type::Public)
         return Exception { InvalidAccessError };
 
-    return platformExportSpki();
+    auto&& result = platformExportSpki();
+    if (result.isEmpty())
+        return Exception { OperationError };
+    return WTFMove(result);
 }
 
 ExceptionOr<Vector<uint8_t>> CryptoKeyEC::exportPkcs8() const
@@ -177,7 +184,10 @@
     if (type() != CryptoKey::Type::Private)
         return Exception { InvalidAccessError };
 
-    return platformExportPkcs8();
+    auto&& result = platformExportPkcs8();
+    if (result.isEmpty())
+        return Exception { OperationError };
+    return WTFMove(result);
 }
 
 String CryptoKeyEC::namedCurveString() const

Modified: trunk/Source/WebCore/crypto/keys/CryptoKeyEC.h (220932 => 220933)


--- trunk/Source/WebCore/crypto/keys/CryptoKeyEC.h	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/keys/CryptoKeyEC.h	2017-08-18 21:30:45 UTC (rev 220933)
@@ -86,7 +86,7 @@
     static RefPtr<CryptoKeyEC> importPkcs8(CryptoAlgorithmIdentifier, const String& curve, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap);
 
     ExceptionOr<Vector<uint8_t>> exportRaw() const;
-    JsonWebKey exportJwk() const;
+    ExceptionOr<JsonWebKey> exportJwk() const;
     ExceptionOr<Vector<uint8_t>> exportSpki() const;
     ExceptionOr<Vector<uint8_t>> exportPkcs8() const;
 
@@ -111,7 +111,7 @@
     static RefPtr<CryptoKeyEC> platformImportSpki(CryptoAlgorithmIdentifier, NamedCurve, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap);
     static RefPtr<CryptoKeyEC> platformImportPkcs8(CryptoAlgorithmIdentifier, NamedCurve, Vector<uint8_t>&& keyData, bool extractable, CryptoKeyUsageBitmap);
     Vector<uint8_t> platformExportRaw() const;
-    void platformAddFieldElements(JsonWebKey&) const;
+    bool platformAddFieldElements(JsonWebKey&) const;
     Vector<uint8_t> platformExportSpki() const;
     Vector<uint8_t> platformExportPkcs8() const;
 

Modified: trunk/Source/WebCore/crypto/mac/CryptoKeyECMac.cpp (220932 => 220933)


--- trunk/Source/WebCore/crypto/mac/CryptoKeyECMac.cpp	2017-08-18 21:07:51 UTC (rev 220932)
+++ trunk/Source/WebCore/crypto/mac/CryptoKeyECMac.cpp	2017-08-18 21:30:45 UTC (rev 220933)
@@ -100,14 +100,6 @@
     return result ? result : 0;
 }
 
-Vector<uint8_t> CryptoKeyEC::platformExportRaw() const
-{
-    Vector<uint8_t> result(keySizeInBits() / 4 + 1); // Per Section 2.3.4 of http://www.secg.org/sec1-v2.pdf
-    size_t size = result.size();
-    CCECCryptorExportKey(kCCImportKeyBinary, result.data(), &size, ccECKeyPublic, m_platformKey);
-    return result;
-}
-
 std::optional<CryptoKeyPair> CryptoKeyEC::platformGeneratePair(CryptoAlgorithmIdentifier identifier, NamedCurve curve, bool extractable, CryptoKeyUsageBitmap usages)
 {
     size_t size = getKeySizeFromNamedCurve(curve);
@@ -133,6 +125,16 @@
     return create(identifier, curve, CryptoKeyType::Public, ccPublicKey, extractable, usages);
 }
 
+Vector<uint8_t> CryptoKeyEC::platformExportRaw() const
+{
+    size_t expectedSize = keySizeInBits() / 4 + 1; // Per Section 2.3.4 of http://www.secg.org/sec1-v2.pdf
+    Vector<uint8_t> result(expectedSize);
+    size_t size = result.size();
+    if (UNLIKELY(CCECCryptorExportKey(kCCImportKeyBinary, result.data(), &size, ccECKeyPublic, m_platformKey) || size != expectedSize))
+        return { };
+    return result;
+}
+
 RefPtr<CryptoKeyEC> CryptoKeyEC::platformImportJWKPublic(CryptoAlgorithmIdentifier identifier, NamedCurve curve, Vector<uint8_t>&& x, Vector<uint8_t>&& y, bool extractable, CryptoKeyUsageBitmap usages)
 {
     if (!doesFieldElementMatchNamedCurve(curve, x.size()) || !doesFieldElementMatchNamedCurve(curve, y.size()))
@@ -166,22 +168,35 @@
     return create(identifier, curve, CryptoKeyType::Private, ccPrivateKey, extractable, usages);
 }
 
-void CryptoKeyEC::platformAddFieldElements(JsonWebKey& jwk) const
+bool CryptoKeyEC::platformAddFieldElements(JsonWebKey& jwk) const
 {
-    size_t size = getKeySizeFromNamedCurve(m_curve);
-    size_t sizeInBytes = size / 8;
-    Vector<uint8_t> x(sizeInBytes);
-    size_t xSize = x.size();
-    Vector<uint8_t> y(sizeInBytes);
-    size_t ySize = y.size();
-    Vector<uint8_t> d(sizeInBytes);
-    size_t dSize = d.size();
+    size_t keySizeInBytes = keySizeInBits() / 8;
+    size_t publicKeySize = keySizeInBytes * 2 + 1; // 04 + X + Y per Section 2.3.4 of http://www.secg.org/sec1-v2.pdf
+    size_t privateKeySize = keySizeInBytes * 3 + 1; // 04 + X + Y + D
 
-    CCECCryptorGetKeyComponents(m_platformKey, &size, x.data(), &xSize, y.data(), &ySize, d.data(), &dSize);
-    jwk.x = base64URLEncode(x);
-    jwk.y = base64URLEncode(y);
-    if (type() == Type::Private)
-        jwk.d = base64URLEncode(d);
+    Vector<uint8_t> result(privateKeySize);
+    size_t size = result.size();
+    switch (type()) {
+    case CryptoKeyType::Public:
+        if (UNLIKELY(CCECCryptorExportKey(kCCImportKeyBinary, result.data(), &size, ccECKeyPublic, m_platformKey)))
+            return false;
+        break;
+    case CryptoKeyType::Private:
+        if (UNLIKELY(CCECCryptorExportKey(kCCImportKeyBinary, result.data(), &size, ccECKeyPrivate, m_platformKey)))
+            return false;
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+        return false;
+    }
+
+    if (UNLIKELY((size != publicKeySize) && (size != privateKeySize)))
+        return false;
+    jwk.x = WTF::base64URLEncode(result.data() + 1, keySizeInBytes);
+    jwk.y = WTF::base64URLEncode(result.data() + keySizeInBytes + 1, keySizeInBytes);
+    if (size > publicKeySize)
+        jwk.d = WTF::base64URLEncode(result.data() + publicKeySize, keySizeInBytes);
+    return true;
 }
 
 static size_t getOID(CryptoKeyEC::NamedCurve curve, const uint8_t*& oid)
@@ -246,9 +261,11 @@
 
 Vector<uint8_t> CryptoKeyEC::platformExportSpki() const
 {
-    Vector<uint8_t> keyBytes(keySizeInBits() / 4 + 1); // Per Section 2.3.4 of http://www.secg.org/sec1-v2.pdf
+    size_t expectedKeySize = keySizeInBits() / 4 + 1; // Per Section 2.3.4 of http://www.secg.org/sec1-v2.pdf
+    Vector<uint8_t> keyBytes(expectedKeySize);
     size_t keySize = keyBytes.size();
-    CCECCryptorExportKey(kCCImportKeyBinary, keyBytes.data(), &keySize, ccECKeyPublic, m_platformKey);
+    if (UNLIKELY(CCECCryptorExportKey(kCCImportKeyBinary, keyBytes.data(), &keySize, ccECKeyPublic, m_platformKey) || keySize != expectedKeySize))
+        return { };
 
     // The following addes SPKI header to a raw EC public key.
     // Once the underlying crypto library is updated to output SPKI EC Key, we should remove this hack.
@@ -342,9 +359,11 @@
 Vector<uint8_t> CryptoKeyEC::platformExportPkcs8() const
 {
     size_t keySizeInBytes = keySizeInBits() / 8;
-    Vector<uint8_t> keyBytes(keySizeInBytes * 3 + 1); // 04 + X + Y + private key
+    size_t expectedKeySize = keySizeInBytes * 3 + 1; // 04 + X + Y + D
+    Vector<uint8_t> keyBytes(expectedKeySize);
     size_t keySize = keyBytes.size();
-    CCECCryptorExportKey(kCCImportKeyBinary, keyBytes.data(), &keySize, ccECKeyPrivate, m_platformKey);
+    if (UNLIKELY(CCECCryptorExportKey(kCCImportKeyBinary, keyBytes.data(), &keySize, ccECKeyPrivate, m_platformKey) || keySize != expectedKeySize))
+        return { };
 
     // The following addes PKCS8 header to a raw EC private key.
     // Once the underlying crypto library is updated to output PKCS8 EC Key, we should remove this hack.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to