Title: [285617] trunk
Revision
285617
Author
j_pas...@apple.com
Date
2021-11-10 18:52:01 -0800 (Wed, 10 Nov 2021)

Log Message

[WebAuthn] Unify _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator's ClientDataJson generation
https://bugs.webkit.org/show_bug.cgi?id=232965
<rdar://problem/85268216>

Reviewed by Brent Fulgham.

Source/WebCore:

The _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator use different methods of generating
clientDataJson, which results in strings with the keys in a different order. This change abstracts
the clientDataJson generation out of AuthenticatorCoordinator and into WebAuthenticationUtils.

* Modules/webauthn/AuthenticatorCoordinator.cpp:
(WebCore::AuthenticatorCoordinator::create const):
(WebCore::AuthenticatorCoordinator::discoverFromExternalSource const):
(WebCore::AuthenticatorCoordinatorInternal::produceClientDataJson): Deleted.
(WebCore::AuthenticatorCoordinatorInternal::produceClientDataJsonHash): Deleted.
* Modules/webauthn/WebAuthenticationUtils.cpp:
(WebCore::buildClientDataJson):
(WebCore::buildClientDataJsonHash):
* Modules/webauthn/WebAuthenticationUtils.h:

Source/WebKit:

The _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator use different methods of generating
clientDataJson, which results in strings with the keys in a different order. This causes problems
because when generating asserts via ASC ui, the hash signed and the client data json used to generate
that hash are different from the client data json returned to js.

* UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:
(produceClientDataJson):

Tools:

Update api tests to reflect different clientDataJson format from WebAuthenticationUtils

* TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (285616 => 285617)


--- trunk/Source/WebCore/ChangeLog	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebCore/ChangeLog	2021-11-11 02:52:01 UTC (rev 285617)
@@ -1,3 +1,25 @@
+2021-11-10  J Pascoe  <j_pas...@apple.com>
+
+        [WebAuthn] Unify _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator's ClientDataJson generation 
+        https://bugs.webkit.org/show_bug.cgi?id=232965
+        <rdar://problem/85268216>
+
+        Reviewed by Brent Fulgham.
+
+        The _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator use different methods of generating
+        clientDataJson, which results in strings with the keys in a different order. This change abstracts
+        the clientDataJson generation out of AuthenticatorCoordinator and into WebAuthenticationUtils.
+
+        * Modules/webauthn/AuthenticatorCoordinator.cpp:
+        (WebCore::AuthenticatorCoordinator::create const):
+        (WebCore::AuthenticatorCoordinator::discoverFromExternalSource const):
+        (WebCore::AuthenticatorCoordinatorInternal::produceClientDataJson): Deleted.
+        (WebCore::AuthenticatorCoordinatorInternal::produceClientDataJsonHash): Deleted.
+        * Modules/webauthn/WebAuthenticationUtils.cpp:
+        (WebCore::buildClientDataJson):
+        (WebCore::buildClientDataJsonHash):
+        * Modules/webauthn/WebAuthenticationUtils.h:
+
 2021-11-10  Tim Nguyen  <n...@apple.com>
 
         Remove non-standard -webkit-border-fit CSS property

Modified: trunk/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp (285616 => 285617)


--- trunk/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp	2021-11-11 02:52:01 UTC (rev 285617)
@@ -41,43 +41,15 @@
 #include "PublicKeyCredentialRequestOptions.h"
 #include "RegistrableDomain.h"
 #include "LegacySchemeRegistry.h"
-#include "SecurityOrigin.h"
 #include "WebAuthenticationConstants.h"
+#include "WebAuthenticationUtils.h"
 #include <pal/crypto/CryptoDigest.h>
-#include <wtf/JSONValues.h>
 #include <wtf/NeverDestroyed.h>
-#include <wtf/text/Base64.h>
 
 namespace WebCore {
 
 namespace AuthenticatorCoordinatorInternal {
 
-// FIXME(181948): Add token binding ID.
-static Ref<ArrayBuffer> produceClientDataJson(ClientDataType type, const BufferSource& challenge, const SecurityOrigin& origin)
-{
-    auto object = JSON::Object::create();
-    switch (type) {
-    case ClientDataType::Create:
-        object->setString("type"_s, "webauthn.create"_s);
-        break;
-    case ClientDataType::Get:
-        object->setString("type"_s, "webauthn.get"_s);
-        break;
-    }
-    object->setString("challenge"_s, base64URLEncodeToString(challenge.data(), challenge.length()));
-    object->setString("origin"_s, origin.toRawString());
-
-    auto utf8JSONString = object->toJSONString().utf8();
-    return ArrayBuffer::create(utf8JSONString.data(), utf8JSONString.length());
-}
-
-static Vector<uint8_t> produceClientDataJsonHash(const ArrayBuffer& clientDataJson)
-{
-    auto crypto = PAL::CryptoDigest::create(PAL::CryptoDigest::Algorithm::SHA_256);
-    crypto->addBytes(clientDataJson.data(), clientDataJson.byteLength());
-    return crypto->computeHash();
-}
-
 static bool needsAppIdQuirks(const String& host, const String& appId)
 {
     // FIXME(197524): Remove this quirk in 2023. As an early adopter of U2F features, Google has a large number of
@@ -176,8 +148,8 @@
     options.extensions = AuthenticationExtensionsClientInputs { String(), processGoogleLegacyAppIdSupportExtension(options.extensions, options.rp.id) };
 
     // Step 13-15.
-    auto clientDataJson = produceClientDataJson(ClientDataType::Create, options.challenge, callerOrigin);
-    auto clientDataJsonHash = produceClientDataJsonHash(clientDataJson);
+    auto clientDataJson = buildClientDataJson(ClientDataType::Create, options.challenge, callerOrigin);
+    auto clientDataJsonHash = buildClientDataJsonHash(clientDataJson);
 
     // Step 4, 17-21.
     if (!m_client) {
@@ -247,8 +219,8 @@
     }
 
     // Step 10-12.
-    auto clientDataJson = produceClientDataJson(ClientDataType::Get, options.challenge, callerOrigin);
-    auto clientDataJsonHash = produceClientDataJsonHash(clientDataJson);
+    auto clientDataJson = buildClientDataJson(ClientDataType::Get, options.challenge, callerOrigin);
+    auto clientDataJsonHash = buildClientDataJsonHash(clientDataJson);
 
     // Step 4, 14-19.
     if (!m_client) {

Modified: trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.cpp (285616 => 285617)


--- trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.cpp	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.cpp	2021-11-11 02:52:01 UTC (rev 285617)
@@ -31,6 +31,8 @@
 #include "CBORWriter.h"
 #include "WebAuthenticationConstants.h"
 #include <pal/crypto/CryptoDigest.h>
+#include <wtf/JSONValues.h>
+#include <wtf/text/Base64.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -131,7 +133,33 @@
     return *attestationObject;
 }
 
+// FIXME(181948): Add token binding ID.
+Ref<ArrayBuffer> buildClientDataJson(ClientDataType type, const BufferSource& challenge, const SecurityOrigin& origin)
+{
+    auto object = JSON::Object::create();
+    switch (type) {
+    case ClientDataType::Create:
+        object->setString("type"_s, "webauthn.create"_s);
+        break;
+    case ClientDataType::Get:
+        object->setString("type"_s, "webauthn.get"_s);
+        break;
+    }
+    object->setString("challenge"_s, base64URLEncodeToString(challenge.data(), challenge.length()));
+    object->setString("origin"_s, origin.toRawString());
 
+    auto utf8JSONString = object->toJSONString().utf8();
+
+    return ArrayBuffer::create(utf8JSONString.data(), utf8JSONString.length());
+}
+
+Vector<uint8_t> buildClientDataJsonHash(const ArrayBuffer& clientDataJson)
+{
+    auto crypto = PAL::CryptoDigest::create(PAL::CryptoDigest::Algorithm::SHA_256);
+    crypto->addBytes(clientDataJson.data(), clientDataJson.byteLength());
+    return crypto->computeHash();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(WEB_AUTHN)

Modified: trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.h (285616 => 285617)


--- trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.h	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebCore/Modules/webauthn/WebAuthenticationUtils.h	2021-11-11 02:52:01 UTC (rev 285617)
@@ -28,7 +28,10 @@
 #if ENABLE(WEB_AUTHN)
 
 #include "AttestationConveyancePreference.h"
+#include "BufferSource.h"
 #include "CBORValue.h"
+#include "SecurityOrigin.h"
+#include "WebAuthenticationConstants.h"
 #include <wtf/Forward.h>
 
 namespace WebCore {
@@ -49,6 +52,10 @@
 // https://www.w3.org/TR/webauthn/#attestation-object
 WEBCORE_EXPORT Vector<uint8_t> buildAttestationObject(Vector<uint8_t>&& authData, String&& format, cbor::CBORValue::MapValue&& statementMap, const AttestationConveyancePreference&);
 
+WEBCORE_EXPORT Ref<ArrayBuffer> buildClientDataJson(ClientDataType /*type*/, const BufferSource& challenge, const SecurityOrigin& /*origin*/);
+
+WEBCORE_EXPORT Vector<uint8_t> buildClientDataJsonHash(const ArrayBuffer& clientDataJson);
+
 } // namespace WebCore
 
 #endif // ENABLE(WEB_AUTHN)

Modified: trunk/Source/WebKit/ChangeLog (285616 => 285617)


--- trunk/Source/WebKit/ChangeLog	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebKit/ChangeLog	2021-11-11 02:52:01 UTC (rev 285617)
@@ -1,3 +1,19 @@
+2021-11-10  J Pascoe  <j_pas...@apple.com>
+
+        [WebAuthn] Unify _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator's ClientDataJson generation 
+        https://bugs.webkit.org/show_bug.cgi?id=232965
+        <rdar://problem/85268216>
+
+        Reviewed by Brent Fulgham.
+
+        The _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator use different methods of generating 
+        clientDataJson, which results in strings with the keys in a different order. This causes problems
+        because when generating asserts via ASC ui, the hash signed and the client data json used to generate
+        that hash are different from the client data json returned to js. 
+
+        * UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:
+        (produceClientDataJson):
+
 2021-11-10  Per Arne Vollan <pvol...@apple.com>
 
         Block sandbox access to consume mach extensions

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm (285616 => 285617)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm	2021-11-11 02:52:01 UTC (rev 285617)
@@ -45,6 +45,7 @@
 #import <WebCore/AuthenticatorAttachment.h>
 #import <WebCore/AuthenticatorResponse.h>
 #import <WebCore/AuthenticatorResponseData.h>
+#import <WebCore/BufferSource.h>
 #import <WebCore/CBORReader.h>
 #import <WebCore/CBORWriter.h>
 #import <WebCore/FidoConstants.h>
@@ -52,7 +53,7 @@
 #import <WebCore/PublicKeyCredentialCreationOptions.h>
 #import <WebCore/PublicKeyCredentialRequestOptions.h>
 #import <WebCore/WebAuthenticationConstants.h>
-#import <WebCore/WebCoreObjCExtras.h>
+#import <WebCore/WebAuthenticationUtils.h>
 #import <objc/runtime.h>
 #import <pal/crypto/CryptoDigest.h>
 #import <wtf/BlockPtr.h>
@@ -73,19 +74,20 @@
 
 static RetainPtr<NSData> produceClientDataJson(_WKWebAuthenticationType type, NSData *challenge, NSString *origin)
 {
-    auto dictionary = adoptNS([[NSMutableDictionary alloc] init]);
+    WebCore::ClientDataType clientDataType;
     switch (type) {
     case _WKWebAuthenticationTypeCreate:
-        [dictionary setObject:@"webauthn.create" forKey:@"type"];
+        clientDataType = WebCore::ClientDataType::Create;
         break;
     case _WKWebAuthenticationTypeGet:
-        [dictionary setObject:@"webauthn.get" forKey:@"type"];
+        clientDataType = WebCore::ClientDataType::Get;
         break;
     }
-    [dictionary setObject:base64URLEncodeToString(challenge.bytes, challenge.length) forKey:@"challenge"];
-    [dictionary setObject:origin forKey:@"origin"];
+    auto challengeBuffer = ArrayBuffer::tryCreate(reinterpret_cast<const uint8_t*>(challenge.bytes), challenge.length);
+    auto securityOrigin = WebCore::SecurityOrigin::createFromString(origin);
 
-    return [NSJSONSerialization dataWithJSONObject:dictionary.get() options:(NSJSONWritingSortedKeys | NSJSONWritingWithoutEscapingSlashes) error:nil];
+    auto clientDataJson = buildClientDataJson(clientDataType, WebCore::BufferSource(challengeBuffer), securityOrigin);
+    return adoptNS([[NSData alloc] initWithBytes:clientDataJson->data() length:clientDataJson->byteLength()]);
 }
 
 static Vector<uint8_t> produceClientDataJsonHash(NSData *clientDataJson)

Modified: trunk/Tools/ChangeLog (285616 => 285617)


--- trunk/Tools/ChangeLog	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Tools/ChangeLog	2021-11-11 02:52:01 UTC (rev 285617)
@@ -1,3 +1,16 @@
+2021-11-10  J Pascoe  <j_pas...@apple.com>
+
+        [WebAuthn] Unify _WKWebAuthenticationPanel SPI and AuthenticatorCoordinator's ClientDataJson generation 
+        https://bugs.webkit.org/show_bug.cgi?id=232965
+        <rdar://problem/85268216>
+
+        Reviewed by Brent Fulgham.
+
+        Update api tests to reflect different clientDataJson format from WebAuthenticationUtils
+
+        * TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
+        (TestWebKitAPI::TEST):
+
 2021-11-10  Tim Nguyen  <n...@apple.com>
 
         Remove non-standard -webkit-border-fit CSS property

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm (285616 => 285617)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm	2021-11-11 02:20:02 UTC (rev 285616)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm	2021-11-11 02:52:01 UTC (rev 285617)
@@ -1891,7 +1891,7 @@
         EXPECT_NULL(error);
 
         EXPECT_NOT_NULL(response);
-        EXPECT_WK_STREQ([[NSString alloc] initWithData:response.clientDataJSON encoding:NSUTF8StringEncoding], "{\"challenge\":\"AQIDBAECAwQBAgMEAQIDBAECAwQBAgMEAQIDBAECAwQ\",\"origin\":\"https://example.com\",\"type\":\"webauthn.create\"}");
+        EXPECT_WK_STREQ([[NSString alloc] initWithData:response.clientDataJSON encoding:NSUTF8StringEncoding], "{\"type\":\"webauthn.create\",\"challenge\":\"AQIDBAECAwQBAgMEAQIDBAECAwQBAgMEAQIDBAECAwQ\",\"origin\":\"https://example.com\"}");
         EXPECT_WK_STREQ([response.rawId base64EncodedStringWithOptions:0], "SMSXHngF7hEOsElA73C3RY+8bR4=");
         EXPECT_NULL(response.extensions);
         EXPECT_WK_STREQ([response.attestationObject base64EncodedStringWithOptions:0], "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YViYo3mm9u6vuaVeN4wRgDTidR5oL6ufLTCrE9ISVYbOGUdFAAAAAAAAAAAAAAAAAAAAAAAAAAAAFEjElx54Be4RDrBJQO9wt0WPvG0epQECAyYgASFYIDj/zxSkzKgaBuS3cdWDF558of8AaIpgFpsjF/Qm1749IlggVBJPgqUIwfhWHJ91nb7UPH76c0+WFOzZKslPyyFse4g=");
@@ -2024,7 +2024,7 @@
         EXPECT_NULL(error);
 
         EXPECT_NOT_NULL(response);
-        EXPECT_WK_STREQ([[NSString alloc] initWithData:response.clientDataJSON encoding:NSUTF8StringEncoding], "{\"challenge\":\"AQIDBAECAwQBAgMEAQIDBAECAwQBAgMEAQIDBAECAwQ\",\"origin\":\"https://example.com\",\"type\":\"webauthn.get\"}");
+        EXPECT_WK_STREQ([[NSString alloc] initWithData:response.clientDataJSON encoding:NSUTF8StringEncoding], "{\"type\":\"webauthn.get\",\"challenge\":\"AQIDBAECAwQBAgMEAQIDBAECAwQBAgMEAQIDBAECAwQ\",\"origin\":\"https://example.com\"}");
         EXPECT_WK_STREQ([response.rawId base64EncodedStringWithOptions:0], "SMSXHngF7hEOsElA73C3RY+8bR4=");
         EXPECT_NULL(response.extensions);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to