Attachment 368748 Details for Bug 197481 – Patch for landing

[patch] Patch for landing

bug-197481-20190501184045.patch (text/plain), 10.48 KB, created by Jiewen Tan on 2019-05-01 18:40:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jiewen Tan

Created: 2019-05-01 18:40:46 PDT

Size: 10.48 KB

patch

obsolete

>Subversion Revision: 244853
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 08fd31b57ea31fbba7a91d615ac63e2a59b03a9c..282df24c4b5c6065677a0b9133a46af6186cb0c3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-05-01  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        [WebAuthN] Adopt SecurityOrigin::isMatchingRegistrableDomainSuffix()
>+        https://bugs.webkit.org/show_bug.cgi?id=197481
>+
>+        Reviewed by Brent Fulgham.
>+
>+        This patch implements Step 6-7 from:
>+        https://www.w3.org/TR/webauthn/#createCredential,
>+        https://www.w3.org/TR/webauthn/#discover-from-external-source.
>+
>+        Test: http/wpt/webauthn/public-key-credential-ip-address.html
>+
>+        * Modules/webauthn/AuthenticatorCoordinator.cpp:
>+        (WebCore::AuthenticatorCoordinator::create const):
>+        (WebCore::AuthenticatorCoordinator::discoverFromExternalSource const):
>+
> 2019-05-01  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         Move Document::domainIsRegisterable to SecurityOrigin::isMatchingRegistrableDomainSuffix
>diff --git a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp
>index d263c1b81374c4166e50531ff521278264d399b5..9573927f304bbd092a74f87f7dfcb30bd26f325f 100644
>--- a/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp
>+++ b/Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp
>@@ -126,16 +126,21 @@ void AuthenticatorCoordinator::create(const SecurityOrigin& callerOrigin, const
>         return;
>     }
> 
>-    // Step 5-7.
>-    // FIXME(181950): We lack fundamental support from SecurityOrigin to determine if a host is a valid domain or not.
>-    // Step 6 is therefore skipped. Also, we lack the support to determine whether a domain is a registrable
>-    // domain suffix of another domain. Hence restrict the comparison to equal in Step 7.
>-    if (!options.rp.id.isEmpty() && callerOrigin.host() != options.rp.id) {
>-        promise.reject(Exception { SecurityError, "The origin of the document is not a registrable domain suffix of the provided RP ID."_s });
>+    // Step 5. Skipped since SecurityOrigin doesn't have the concept of "opaque origin".
>+    // Step 6. The effective domain may be represented in various manners, such as a domain or an ip address.
>+    // Only the domain format of host is permitted in WebAuthN.
>+    if (URL::hostIsIPAddress(callerOrigin.domain())) {
>+        promise.reject(Exception { SecurityError, "The effective domain of the document is not a valid domain."_s });
>+        return;
>+    }
>+
>+    // Step 7.
>+    if (!options.rp.id.isEmpty() && !callerOrigin.isMatchingRegistrableDomainSuffix(options.rp.id)) {
>+        promise.reject(Exception { SecurityError, "The provided RP ID is not a registrable domain suffix of the effective domain of the document."_s });
>         return;
>     }
>     if (options.rp.id.isEmpty())
>-        options.rp.id = callerOrigin.host();
>+        options.rp.id = callerOrigin.domain();
> 
>     // Step 8-10.
>     // Most of the jobs are done by bindings. However, we can't know if the JSValue of options.pubKeyCredParams
>@@ -188,16 +193,21 @@ void AuthenticatorCoordinator::discoverFromExternalSource(const SecurityOrigin&
>         return;
>     }
> 
>-    // Step 5-7.
>-    // FIXME(181950): We lack fundamental support from SecurityOrigin to determine if a host is a valid domain or not.
>-    // Step 6 is therefore skipped. Also, we lack the support to determine whether a domain is a registrable
>-    // domain suffix of another domain. Hence restrict the comparison to equal in Step 7.
>-    if (!options.rpId.isEmpty() && callerOrigin.host() != options.rpId) {
>-        promise.reject(Exception { SecurityError, "The origin of the document is not a registrable domain suffix of the provided RP ID."_s });
>+    // Step 5. Skipped since SecurityOrigin doesn't have the concept of "opaque origin".
>+    // Step 6. The effective domain may be represented in various manners, such as a domain or an ip address.
>+    // Only the domain format of host is permitted in WebAuthN.
>+    if (URL::hostIsIPAddress(callerOrigin.domain())) {
>+        promise.reject(Exception { SecurityError, "The effective domain of the document is not a valid domain."_s });
>+        return;
>+    }
>+
>+    // Step 7.
>+    if (!options.rpId.isEmpty() && !callerOrigin.isMatchingRegistrableDomainSuffix(options.rpId)) {
>+        promise.reject(Exception { SecurityError, "The provided RP ID is not a registrable domain suffix of the effective domain of the document."_s });
>         return;
>     }
>     if (options.rpId.isEmpty())
>-        options.rpId = callerOrigin.host();
>+        options.rpId = callerOrigin.domain();
> 
>     // Step 8-9.
>     // Only FIDO AppID Extension is supported.
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 26f94716a91169813da1ddff636ab83a8ef78344..855eba4db682b9acb098731bed70a05d0741d821 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-05-01  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        [WebAuthN] Adopt SecurityOrigin::isMatchingRegistrableDomainSuffix()
>+        https://bugs.webkit.org/show_bug.cgi?id=197481
>+
>+        Reviewed by Brent Fulgham.
>+
>+        * http/wpt/webauthn/public-key-credential-create-failure.https.html:
>+        * http/wpt/webauthn/public-key-credential-get-failure.https.html:
>+        * http/wpt/webauthn/public-key-credential-ip-address-expected.txt: Added.
>+        * http/wpt/webauthn/public-key-credential-ip-address.html: Added.
>+        * http/wpt/webauthn/resources/public-key-credential-ip-address.https.html: Added.
>+
> 2019-05-01  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         [iOS] Element::focus and Element::scrollIntoView do not clamp scroll positions
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>index 9ea3428bb447e5601b228a8baf0165df17cb24d4..4e131cbe4e2cbb4d71951309352fcd25d5580750 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
>@@ -47,7 +47,7 @@
>         };
> 
>         return promiseRejects(t, "SecurityError",
>-            navigator.credentials.create(options), "The origin of the document is not a registrable domain suffix of the provided RP ID.");
>+            navigator.credentials.create(options), "The provided RP ID is not a registrable domain suffix of the effective domain of the document.");
>     }, "PublicKeyCredential's [[create]] with a mismatched RP ID");
> 
>     promise_test(function(t) {
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>index a245690a22dae4c2c949502051aaf689138e7dca..8547fd8e040fbe22d01ebe6f82f58b27986edb89 100644
>--- a/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
>@@ -29,7 +29,7 @@
>         };
> 
>         return promiseRejects(t, "SecurityError",
>-            navigator.credentials.get(options), "The origin of the document is not a registrable domain suffix of the provided RP ID.");
>+            navigator.credentials.get(options), "The provided RP ID is not a registrable domain suffix of the effective domain of the document.");
>     }, "PublicKeyCredential's [[get]] with a mismatched RP ID");
> 
>     promise_test(t => {
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address-expected.txt b/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b14be5c95f767bfca4e237884c7eff26dd2312f5
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address-expected.txt
>@@ -0,0 +1,4 @@
>+
>+PASS PublicKeyCredential's [[create]] with ip addresses. 
>+PASS PublicKeyCredential's [[get]] with ip addresses. 
>+
>diff --git a/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address.html b/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..760286c4664a899a239294ed3d40a3169b5532e9
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webauthn/public-key-credential-ip-address.html
>@@ -0,0 +1,8 @@
>+<!DOCTYPE html>
>+<script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+    location="https://127.0.0.1:9443/WebKit/webauthn/resources/public-key-credential-ip-address.https.html"
>+</script>
>diff --git a/LayoutTests/http/wpt/webauthn/resources/public-key-credential-ip-address.https.html b/LayoutTests/http/wpt/webauthn/resources/public-key-credential-ip-address.https.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..6de3a943504bf4ff9923f7846add87543468d0cd
>--- /dev/null
>+++ b/LayoutTests/http/wpt/webauthn/resources/public-key-credential-ip-address.https.html
>@@ -0,0 +1,41 @@
>+<!DOCTYPE html>
>+<title>Web Authentication API: Invoke PublicKeyCredential in ip addresses.</title>
>+<script src="/resources/testharness.js"></script>
>+<script src="/resources/testharnessreport.js"></script>
>+<script src="util.js"></script>
>+<script>
>+    // Default mock configuration. Tests need to override if they need different configuration.
>+    if (window.testRunner)
>+        testRunner.setWebAuthenticationMockConfiguration({ });
>+
>+    promise_test(function(t) {
>+        const options = {
>+            publicKey: {
>+                rp: {
>+                    name: "example.com",
>+                },
>+                user: {
>+                    name: "John Appleseed",
>+                    id: asciiToUint8Array("123456"),
>+                    displayName: "John",
>+                },
>+                challenge: asciiToUint8Array("123456"),
>+                pubKeyCredParams: [{ type: "public-key", alg: -7 }],
>+            }
>+        };
>+
>+        return promiseRejects(t, "SecurityError",
>+            navigator.credentials.create(options), "The effective domain of the document is not a valid domain.");
>+    }, "PublicKeyCredential's [[create]] with ip addresses.");
>+
>+    promise_test(t => {
>+        const options = {
>+            publicKey: {
>+                challenge: asciiToUint8Array("123456")
>+            }
>+        };
>+
>+        return promiseRejects(t, "SecurityError",
>+            navigator.credentials.get(options), "The effective domain of the document is not a valid domain.");
>+    }, "PublicKeyCredential's [[get]] with ip addresses.");
>+</script>

Actions: View | Formatted Diff | Diff

Attachments on bug 197481: 368722 | 368748 | 368749