[WebAuthn] Provide a _WKWebAuthenticationPanelUpdatePINInvalid update to UI clients...
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jun 2020 23:40:04 +0000 (23:40 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jun 2020 23:40:04 +0000 (23:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213404
<rdar://problem/64543894>

Reviewed by Brent Fulgham.

Source/WebKit:

Provide a _WKWebAuthenticationPanelUpdatePINInvalid update to UI clients if the returned PIN from the client is not valid such that clients can
reuse the same logic to handle invalid pin from the authenticator. This change makes their life easier.

Covered by API tests.

* UIProcess/API/APIWebAuthenticationPanelClient.h:
(API::WebAuthenticationPanelClient::requestPin const):
* UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:
(WebKit::WebAuthenticationPanelClient::requestPin const):
Now, only null strings are intepreted as cancels.

* UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp:
(WebKit::CtapAuthenticator::continueMakeCredentialAfterResponseReceived):
(WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived):
(WebKit::CtapAuthenticator::continueGetPinTokenAfterRequestPin):
(WebKit::CtapAuthenticator::continueRequestAfterGetPinToken):
This patch also removes potential null pointer dereferences.

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@263296 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIWebAuthenticationPanelClient.h
Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm
Source/WebKit/UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html [new file with mode: 0644]

index 2e5209d..bf26ba9 100644 (file)
@@ -1,3 +1,29 @@
+2020-06-19  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthn] Provide a _WKWebAuthenticationPanelUpdatePINInvalid update to UI clients if the returned PIN from the client is not valid
+        https://bugs.webkit.org/show_bug.cgi?id=213404
+        <rdar://problem/64543894>
+
+        Reviewed by Brent Fulgham.
+
+        Provide a _WKWebAuthenticationPanelUpdatePINInvalid update to UI clients if the returned PIN from the client is not valid such that clients can
+        reuse the same logic to handle invalid pin from the authenticator. This change makes their life easier.
+
+        Covered by API tests.
+
+        * UIProcess/API/APIWebAuthenticationPanelClient.h:
+        (API::WebAuthenticationPanelClient::requestPin const):
+        * UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:
+        (WebKit::WebAuthenticationPanelClient::requestPin const):
+        Now, only null strings are intepreted as cancels.
+
+        * UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp:
+        (WebKit::CtapAuthenticator::continueMakeCredentialAfterResponseReceived):
+        (WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived):
+        (WebKit::CtapAuthenticator::continueGetPinTokenAfterRequestPin):
+        (WebKit::CtapAuthenticator::continueRequestAfterGetPinToken):
+        This patch also removes potential null pointer dereferences.
+
 2020-06-19  Per Arne Vollan  <pvollan@apple.com>
 
         [macOS] Connections to the preference daemon are established before entering the sandbox
index 7afdfd2..a368c37 100644 (file)
@@ -49,7 +49,7 @@ public:
 
     virtual void updatePanel(WebKit::WebAuthenticationStatus) const { }
     virtual void dismissPanel(WebKit::WebAuthenticationResult) const { }
-    virtual void requestPin(uint64_t, CompletionHandler<void(const WTF::String&)>&& completionHandler) const { completionHandler(emptyString()); }
+    virtual void requestPin(uint64_t, CompletionHandler<void(const WTF::String&)>&& completionHandler) const { completionHandler(WTF::String()); }
     virtual void selectAssertionResponse(Vector<Ref<WebCore::AuthenticatorAssertionResponse>>&&, WebKit::WebAuthenticationSource, CompletionHandler<void(WebCore::AuthenticatorAssertionResponse*)>&& completionHandler) const { completionHandler(nullptr); }
     virtual void decidePolicyForLocalAuthenticator(CompletionHandler<void(WebKit::LocalAuthenticatorPolicy)>&& completionHandler) const { completionHandler(WebKit::LocalAuthenticatorPolicy::Disallow); }
 };
index c5aa007..8376972 100644 (file)
@@ -118,13 +118,13 @@ void WebAuthenticationPanelClient::dismissPanel(WebAuthenticationResult result)
 void WebAuthenticationPanelClient::requestPin(uint64_t retries, CompletionHandler<void(const WTF::String&)>&& completionHandler) const
 {
     if (!m_delegateMethods.panelRequestPinWithRemainingRetriesCompletionHandler) {
-        completionHandler(emptyString());
+        completionHandler(String());
         return;
     }
 
     auto delegate = m_delegate.get();
     if (!delegate) {
-        completionHandler(emptyString());
+        completionHandler(String());
         return;
     }
 
index 7f46cfb..ef85a31 100644 (file)
@@ -115,7 +115,7 @@ void CtapAuthenticator::continueMakeCredentialAfterResponseReceived(Vector<uint8
         }
 
         if (isPinError(error)) {
-            if (!m_pinAuth.isEmpty()) // Skip the very first command that acts like wink.
+            if (!m_pinAuth.isEmpty() && observer()) // Skip the very first command that acts like wink.
                 observer()->authenticatorStatusUpdated(toStatus(error));
             if (tryRestartPin(error))
                 return;
@@ -154,7 +154,7 @@ void CtapAuthenticator::continueGetAssertionAfterResponseReceived(Vector<uint8_t
             return;
 
         if (isPinError(error)) {
-            if (!m_pinAuth.isEmpty()) // Skip the very first command that acts like wink.
+            if (!m_pinAuth.isEmpty() && observer()) // Skip the very first command that acts like wink.
                 observer()->authenticatorStatusUpdated(toStatus(error));
             if (tryRestartPin(error))
                 return;
@@ -274,9 +274,17 @@ void CtapAuthenticator::continueRequestPinAfterGetKeyAgreement(Vector<uint8_t>&&
 
 void CtapAuthenticator::continueGetPinTokenAfterRequestPin(const String& pin, const CryptoKeyEC& peerKey)
 {
+    if (pin.isNull()) {
+        receiveRespond(ExceptionData { UnknownError, "Pin is null."_s });
+        return;
+    }
+
     auto pinUtf8 = pin::validateAndConvertToUTF8(pin);
     if (!pinUtf8) {
-        receiveRespond(ExceptionData { UnknownError, makeString("Pin is not valid: ", pin) });
+        // Fake a pin invalid response from the authenticator such that clients could show some error to the user.
+        if (auto* observer = this->observer())
+            observer->authenticatorStatusUpdated(WebAuthenticationStatus::PinInvalid);
+        tryRestartPin(CtapDeviceResponseCode::kCtap2ErrPinInvalid);
         return;
     }
     auto tokenRequest = pin::TokenRequest::tryCreate(*pinUtf8, peerKey);
@@ -301,7 +309,8 @@ void CtapAuthenticator::continueRequestAfterGetPinToken(Vector<uint8_t>&& data,
         auto error = getResponseCode(data);
 
         if (isPinError(error)) {
-            observer()->authenticatorStatusUpdated(toStatus(error));
+            if (auto* observer = this->observer())
+                observer->authenticatorStatusUpdated(toStatus(error));
             if (tryRestartPin(error))
                 return;
         }
index 4e629e1..3a8d6fe 100644 (file)
@@ -1,3 +1,16 @@
+2020-06-19  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthn] Provide a _WKWebAuthenticationPanelUpdatePINInvalid update to UI clients if the returned PIN from the client is not valid
+        https://bugs.webkit.org/show_bug.cgi?id=213404
+        <rdar://problem/64543894>
+
+        Reviewed by Brent Fulgham.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html: Added.
+
 2020-06-19  Jonathan Bedard  <jbedard@apple.com>
 
         Bring up watchOS/tvOS on build.webkit.org
index 78b541c..9e88112 100644 (file)
                5742178A2400AED8002B303D /* web-authentication-make-credential-la-duplicate-credential.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 574217892400AED0002B303D /* web-authentication-make-credential-la-duplicate-credential.html */; };
                5742178E2400D2DF002B303D /* web-authentication-make-credential-la.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5742178D2400D26C002B303D /* web-authentication-make-credential-la.html */; };
                574F55D2204D47F0002948C6 /* Security.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 574F55D0204D471C002948C6 /* Security.framework */; };
+               5751B28A249D5BC500664C2A /* web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5751B289249D5B9900664C2A /* web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html */; };
                5758597F23A2527A00C74572 /* CtapPinTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5758597E23A2527A00C74572 /* CtapPinTest.cpp */; };
                5758598423C3C3A400C74572 /* web-authentication-make-credential-hid-pin-get-retries-error.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5758598323C3C36200C74572 /* web-authentication-make-credential-hid-pin-get-retries-error.html */; };
                57599E211F07191900A3FB8C /* IndexedDBStructuredCloneBackwardCompatibility.mm in Sources */ = {isa = PBXBuildFile; fileRef = 57599E201F07191700A3FB8C /* IndexedDBStructuredCloneBackwardCompatibility.mm */; };
                        dstPath = TestWebKitAPI.resources;
                        dstSubfolderSpec = 7;
                        files = (
+                               5751B28A249D5BC500664C2A /* web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html in Copy Resources */,
                                55A817FF2181021A0004A39A /* 100x100-red.tga in Copy Resources */,
                                1A9E52C913E65EF4006917F5 /* 18-characters.html in Copy Resources */,
                                55A81800218102210004A39A /* 400x400-green.png in Copy Resources */,
                574217892400AED0002B303D /* web-authentication-make-credential-la-duplicate-credential.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "web-authentication-make-credential-la-duplicate-credential.html"; sourceTree = "<group>"; };
                5742178D2400D26C002B303D /* web-authentication-make-credential-la.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "web-authentication-make-credential-la.html"; sourceTree = "<group>"; };
                574F55D0204D471C002948C6 /* Security.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Security.framework; path = System/Library/Frameworks/Security.framework; sourceTree = SDKROOT; };
+               5751B289249D5B9900664C2A /* web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html"; sourceTree = "<group>"; };
                5758597D23A2527A00C74572 /* CtapPinTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CtapPinTest.h; sourceTree = "<group>"; };
                5758597E23A2527A00C74572 /* CtapPinTest.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = CtapPinTest.cpp; sourceTree = "<group>"; };
                5758598323C3C36200C74572 /* web-authentication-make-credential-hid-pin-get-retries-error.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "web-authentication-make-credential-hid-pin-get-retries-error.html"; sourceTree = "<group>"; };
                                57C6244F2346C1EC00383FE7 /* web-authentication-get-assertion.html */,
                                578DA44723ECD01300246010 /* web-authentication-make-credential-hid-pin-auth-blocked-error.html */,
                                570D26F323C3CA5500D5CF67 /* web-authentication-make-credential-hid-pin-get-key-agreement-error.html */,
+                               5751B289249D5B9900664C2A /* web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html */,
                                578DA44123ECC76B00246010 /* web-authentication-make-credential-hid-pin-get-pin-token-pin-auth-blocked-error.html */,
                                578DA44523ECCBD000246010 /* web-authentication-make-credential-hid-pin-get-pin-token-pin-auth-invalid-error-retry.html */,
                                570D26F923C3F24500D5CF67 /* web-authentication-make-credential-hid-pin-get-pin-token-pin-blocked-error.html */,
index 259289b..d72c889 100644 (file)
@@ -937,7 +937,7 @@ TEST(WebAuthenticationPanel, PinRequestPinErrorNoDelegate)
 
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSZeroRect configuration:configuration]);
     [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
-    [webView waitForMessage:@"Pin is not valid: "];
+    [webView waitForMessage:@"Pin is null."];
 }
 
 TEST(WebAuthenticationPanel, PinRequestPinErrorNullDelegate)
@@ -954,13 +954,13 @@ TEST(WebAuthenticationPanel, PinRequestPinErrorNullDelegate)
     [webView setUIDelegate:delegate.get()];
 
     [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
-    [webView waitForMessage:@"Pin is not valid: "];
+    [webView waitForMessage:@"Pin is null."];
 }
 
 TEST(WebAuthenticationPanel, PinRequestPinError)
 {
     reset();
-    RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-make-credential-hid-pin" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
 
     auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
     [[configuration preferences] _setEnabled:YES forExperimentalFeature:webAuthenticationExperimentalFeature()];
@@ -971,7 +971,9 @@ TEST(WebAuthenticationPanel, PinRequestPinError)
 
     webAuthenticationPanelPin = "123";
     [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
-    [webView waitForMessage:@"Pin is not valid: 123"];
+    Util::run(&webAuthenticationPanelUpdatePINInvalid);
+    webAuthenticationPanelPin = "1234";
+    [webView waitForMessage:@"Succeeded!"];
 }
 
 TEST(WebAuthenticationPanel, PinGetPinTokenPinBlockedError)
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html b/Tools/TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid-pin-get-pin-token-fake-pin-invalid-error-retry.html
new file mode 100644 (file)
index 0000000..cb7586f
--- /dev/null
@@ -0,0 +1,56 @@
+<input type="text" id="input">
+<script>
+    const testCtapPinInvalidErrorBase64 = "MQ==";
+    const testPinGetRetriesResponseBase64 = "AKEDCA==";
+    const testPinGetKeyAgreementResponseBase64 = "AKEBpQECAzgYIAEhWCDodiWJbuTkbcAydm6Ah5YvNt+d/otWfzdjAVsZkKYOFCJYICfeYS1mQYvaGVBYHrxcjB2tcQyxTCL4yXBF9GEvsgyR";
+    const testPinGetPinTokenResponseBase64 = "AKECUBOk7rcOyRrqAB6TFvYeQfc=";
+    const testCreationMessageBase64 =
+        "AKMBZnBhY2tlZAJYxEbMf7lnnVWy25CS4cjZ5eHQK3WA8LSBLHcJYuHkj1rYQQAA" +
+        "AE74oBHzjApNFYAGFxEfntx9AEAoCK3O6P5OyXN6V/f+9nAga0NA2Cgp4V3mgSJ5" +
+        "jOHLMDrmxp/S0rbD+aihru1C0aAN3BkiM6GNy5nSlDVqOgTgpQECAyYgASFYIEFb" +
+        "he3RkNud6sgyraBGjlh1pzTlCZehQlL/b18HZ6WGIlggJgfUd/en9p5AIqMQbUni" +
+        "nEeXdFLkvW0/zV5BpEjjNxADo2NhbGcmY3NpZ1hHMEUCIQDKg+ZBmEBtf0lWq4Re" +
+        "dH4/i/LOYqOR4uR2NAj2zQmw9QIgbTXb4hvFbj4T27bv/rGrc+y+0puoYOBkBk9P" +
+        "mCewWlNjeDVjgVkCwjCCAr4wggGmoAMCAQICBHSG/cIwDQYJKoZIhvcNAQELBQAw" +
+        "LjEsMCoGA1UEAxMjWXViaWNvIFUyRiBSb290IENBIFNlcmlhbCA0NTcyMDA2MzEw" +
+        "IBcNMTQwODAxMDAwMDAwWhgPMjA1MDA5MDQwMDAwMDBaMG8xCzAJBgNVBAYTAlNF" +
+        "MRIwEAYDVQQKDAlZdWJpY28gQUIxIjAgBgNVBAsMGUF1dGhlbnRpY2F0b3IgQXR0" +
+        "ZXN0YXRpb24xKDAmBgNVBAMMH1l1YmljbyBVMkYgRUUgU2VyaWFsIDE5NTUwMDM4" +
+        "NDIwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASVXfOt9yR9MXXv/ZzE8xpOh466" +
+        "4YEJVmFQ+ziLLl9lJ79XQJqlgaUNCsUvGERcChNUihNTyKTlmnBOUjvATevto2ww" +
+        "ajAiBgkrBgEEAYLECgIEFTEuMy42LjEuNC4xLjQxNDgyLjEuMTATBgsrBgEEAYLl" +
+        "HAIBAQQEAwIFIDAhBgsrBgEEAYLlHAEBBAQSBBD4oBHzjApNFYAGFxEfntx9MAwG" +
+        "A1UdEwEB/wQCMAAwDQYJKoZIhvcNAQELBQADggEBADFcSIDmmlJ+OGaJvWn9Cqhv" +
+        "SeueToVFQVVvqtALOgCKHdwB+Wx29mg2GpHiMsgQp5xjB0ybbnpG6x212FxESJ+G" +
+        "inZD0ipchi7APwPlhIvjgH16zVX44a4e4hOsc6tLIOP71SaMsHuHgCcdH0vg5d2s" +
+        "c006WJe9TXO6fzV+ogjJnYpNKQLmCXoAXE3JBNwKGBIOCvfQDPyWmiiG5bGxYfPt" +
+        "y8Z3pnjX+1MDnM2hhr40ulMxlSNDnX/ZSnDyMGIbk8TOQmjTF02UO8auP8k3wt5D" +
+        "1rROIRU9+FCSX5WQYi68RuDrGMZB8P5+byoJqbKQdxn2LmE1oZAyohPAmLcoPO4=";
+    if (window.internals) {
+        internals.setMockWebAuthenticationConfiguration({ hid: { supportClientPin: true, payloadBase64: [testCtapPinInvalidErrorBase64, testPinGetRetriesResponseBase64, testPinGetKeyAgreementResponseBase64, testPinGetRetriesResponseBase64, testPinGetKeyAgreementResponseBase64, testPinGetPinTokenResponseBase64, testCreationMessageBase64] } });
+        internals.withUserGesture(() => { input.focus(); });
+    }
+
+    const options = {
+        publicKey: {
+            rp: {
+                name: "localhost",
+            },
+            user: {
+                name: "John Appleseed",
+                id: new Uint8Array(16),
+                displayName: "Appleseed",
+            },
+            challenge: new Uint8Array(16),
+            pubKeyCredParams: [{ type: "public-key", alg: -7 }]
+        }
+    };
+
+    navigator.credentials.create(options).then(credential => {
+        // console.log("Succeeded!");
+        window.webkit.messageHandlers.testHandler.postMessage("Succeeded!");
+    }, error => {
+        // console.log(error.message);
+        window.webkit.messageHandlers.testHandler.postMessage(error.message);
+    });
+</script>