Title: [284489] trunk/Source/WebKit
Revision
284489
Author
commit-qu...@webkit.org
Date
2021-10-19 13:32:40 -0700 (Tue, 19 Oct 2021)

Log Message

WebAuthn Platform UI callbacks are not guaranteed to happen on the main thread
https://bugs.webkit.org/show_bug.cgi?id=231963
<rdar://84420452>

Patch by Garrett Davidson <garrett_david...@apple.com> on 2021-10-19
Reviewed by Brent Fulgham.

- The clearanceHandler is not guaranteed to happen on the main thread, but
weakThis.get() needs to happen there.

- `proxy` needs to stay alive for the whole transaction, otherwise it will tear
down the connection and cancel the request, so explicitly capture it everywhere.

- Start logging if we get an error from clearanceHandler.

Covered by existing tests.

* UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:
(WebKit::WebAuthenticatorCoordinatorProxy::performRequest):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (284488 => 284489)


--- trunk/Source/WebKit/ChangeLog	2021-10-19 20:15:32 UTC (rev 284488)
+++ trunk/Source/WebKit/ChangeLog	2021-10-19 20:32:40 UTC (rev 284489)
@@ -1,3 +1,24 @@
+2021-10-19  Garrett Davidson  <garrett_david...@apple.com>
+
+        WebAuthn Platform UI callbacks are not guaranteed to happen on the main thread
+        https://bugs.webkit.org/show_bug.cgi?id=231963
+        <rdar://84420452>
+
+        Reviewed by Brent Fulgham.
+
+        - The clearanceHandler is not guaranteed to happen on the main thread, but
+        weakThis.get() needs to happen there.
+
+        - `proxy` needs to stay alive for the whole transaction, otherwise it will tear
+        down the connection and cancel the request, so explicitly capture it everywhere.
+
+        - Start logging if we get an error from clearanceHandler.
+
+        Covered by existing tests.
+
+        * UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:
+        (WebKit::WebAuthenticatorCoordinatorProxy::performRequest):
+
 2021-10-19  Wenson Hsieh  <wenson_hs...@apple.com>
 
         REGRESSION (r284079): fast/canvas/gradient-with-clip.html and fast/canvas/gradient-text-with-shadow.html are flaky failures

Modified: trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm (284488 => 284489)


--- trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm	2021-10-19 20:15:32 UTC (rev 284488)
+++ trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm	2021-10-19 20:32:40 UTC (rev 284489)
@@ -264,75 +264,74 @@
     auto proxy = adoptNS([allocASCAgentProxyInstance() init]);
 
     RetainPtr<NSWindow> window = m_webPageProxy.platformWindow();
-    [proxy performAuthorizationRequestsForContext:requestContext.get() withClearanceHandler:makeBlockPtr([weakThis = WeakPtr { *this }, handler = WTFMove(handler), window = WTFMove(window)](NSXPCListenerEndpoint *daemonEnpoint, NSError *error) mutable {
-        auto protectedThis = weakThis.get();
-
-        if (!protectedThis || !daemonEnpoint) {
-            RunLoop::main().dispatch([handler = WTFMove(handler)]() mutable {
+    [proxy performAuthorizationRequestsForContext:requestContext.get() withClearanceHandler:makeBlockPtr([weakThis = WeakPtr { *this }, handler = WTFMove(handler), window = WTFMove(window), proxy = WTFMove(proxy)](NSXPCListenerEndpoint *daemonEndpoint, NSError *error) mutable {
+        callOnMainRunLoop([weakThis, handler = WTFMove(handler), window = WTFMove(window), proxy = WTFMove(proxy), daemonEndpoint = retainPtr(daemonEndpoint), error = retainPtr(error)] () mutable {
+            if (!weakThis || !daemonEndpoint) {
+                LOG_ERROR("Could not connect to authorization daemon: %@\n", error.get());
                 handler({ }, (AuthenticatorAttachment)0, ExceptionData { NotAllowedError, "Operation failed." });
-            });
-            return;
-        }
+                return;
+            }
 
-        protectedThis->m_presenter = adoptNS([allocASCAuthorizationRemotePresenterInstance() init]);
-        [protectedThis->m_presenter presentWithWindow:window.get() daemonEndpoint:daemonEnpoint completionHandler:makeBlockPtr([handler = WTFMove(handler)](id <ASCCredentialProtocol> credential, NSError *error) mutable {
-            AuthenticatorResponseData response = { };
-            AuthenticatorAttachment attachment;
-            ExceptionData exceptionData = { };
+            weakThis->m_presenter = adoptNS([allocASCAuthorizationRemotePresenterInstance() init]);
+            [weakThis->m_presenter presentWithWindow:window.get() daemonEndpoint:daemonEndpoint.get() completionHandler:makeBlockPtr([handler = WTFMove(handler), proxy = WTFMove(proxy)](id <ASCCredentialProtocol> credential, NSError *error) mutable {
+                AuthenticatorResponseData response = { };
+                AuthenticatorAttachment attachment;
+                ExceptionData exceptionData = { };
 
-            if ([credential isKindOfClass:getASCPlatformPublicKeyCredentialRegistrationClass()]) {
-                attachment = AuthenticatorAttachment::Platform;
-                response.isAuthenticatorAttestationResponse = true;
+                if ([credential isKindOfClass:getASCPlatformPublicKeyCredentialRegistrationClass()]) {
+                    attachment = AuthenticatorAttachment::Platform;
+                    response.isAuthenticatorAttestationResponse = true;
 
-                ASCPlatformPublicKeyCredentialRegistration *registrationCredential = credential;
-                response.rawId = toArrayBuffer(registrationCredential.credentialID);
-                response.attestationObject = toArrayBuffer(registrationCredential.attestationObject);
-            } else if ([credential isKindOfClass:getASCSecurityKeyPublicKeyCredentialRegistrationClass()]) {
-                attachment = AuthenticatorAttachment::CrossPlatform;
-                response.isAuthenticatorAttestationResponse = true;
+                    ASCPlatformPublicKeyCredentialRegistration *registrationCredential = credential;
+                    response.rawId = toArrayBuffer(registrationCredential.credentialID);
+                    response.attestationObject = toArrayBuffer(registrationCredential.attestationObject);
+                } else if ([credential isKindOfClass:getASCSecurityKeyPublicKeyCredentialRegistrationClass()]) {
+                    attachment = AuthenticatorAttachment::CrossPlatform;
+                    response.isAuthenticatorAttestationResponse = true;
 
-                ASCSecurityKeyPublicKeyCredentialRegistration *registrationCredential = credential;
-                response.rawId = toArrayBuffer(registrationCredential.credentialID);
-                response.attestationObject = toArrayBuffer(registrationCredential.attestationObject);
-            } else if ([credential isKindOfClass:getASCPlatformPublicKeyCredentialAssertionClass()]) {
-                attachment = AuthenticatorAttachment::Platform;
-                response.isAuthenticatorAttestationResponse = false;
+                    ASCSecurityKeyPublicKeyCredentialRegistration *registrationCredential = credential;
+                    response.rawId = toArrayBuffer(registrationCredential.credentialID);
+                    response.attestationObject = toArrayBuffer(registrationCredential.attestationObject);
+                } else if ([credential isKindOfClass:getASCPlatformPublicKeyCredentialAssertionClass()]) {
+                    attachment = AuthenticatorAttachment::Platform;
+                    response.isAuthenticatorAttestationResponse = false;
 
-                ASCPlatformPublicKeyCredentialAssertion *assertionCredential = credential;
-                response.rawId = toArrayBuffer(assertionCredential.credentialID);
-                response.authenticatorData = toArrayBuffer(assertionCredential.authenticatorData);
-                response.signature = toArrayBuffer(assertionCredential.signature);
-                response.userHandle = toArrayBuffer(assertionCredential.userHandle);
-            } else if ([credential isKindOfClass:getASCSecurityKeyPublicKeyCredentialAssertionClass()]) {
-                attachment = AuthenticatorAttachment::CrossPlatform;
-                response.isAuthenticatorAttestationResponse = false;
+                    ASCPlatformPublicKeyCredentialAssertion *assertionCredential = credential;
+                    response.rawId = toArrayBuffer(assertionCredential.credentialID);
+                    response.authenticatorData = toArrayBuffer(assertionCredential.authenticatorData);
+                    response.signature = toArrayBuffer(assertionCredential.signature);
+                    response.userHandle = toArrayBuffer(assertionCredential.userHandle);
+                } else if ([credential isKindOfClass:getASCSecurityKeyPublicKeyCredentialAssertionClass()]) {
+                    attachment = AuthenticatorAttachment::CrossPlatform;
+                    response.isAuthenticatorAttestationResponse = false;
 
-                ASCSecurityKeyPublicKeyCredentialAssertion *assertionCredential = credential;
-                response.rawId = toArrayBuffer(assertionCredential.credentialID);
-                response.authenticatorData = toArrayBuffer(assertionCredential.authenticatorData);
-                response.signature = toArrayBuffer(assertionCredential.signature);
-                response.userHandle = toArrayBuffer(assertionCredential.userHandle);
-            } else {
-                attachment = (AuthenticatorAttachment) 0;
-                ExceptionCode exceptionCode;
-                NSString *errorMessage = nil;
-                if ([error.domain isEqualToString:WKErrorDomain]) {
-                    exceptionCode = toExceptionCode(error.code);
-                    errorMessage = error.userInfo[NSLocalizedDescriptionKey];
+                    ASCSecurityKeyPublicKeyCredentialAssertion *assertionCredential = credential;
+                    response.rawId = toArrayBuffer(assertionCredential.credentialID);
+                    response.authenticatorData = toArrayBuffer(assertionCredential.authenticatorData);
+                    response.signature = toArrayBuffer(assertionCredential.signature);
+                    response.userHandle = toArrayBuffer(assertionCredential.userHandle);
                 } else {
-                    exceptionCode = NotAllowedError;
+                    attachment = (AuthenticatorAttachment) 0;
+                    ExceptionCode exceptionCode;
+                    NSString *errorMessage = nil;
+                    if ([error.domain isEqualToString:WKErrorDomain]) {
+                        exceptionCode = toExceptionCode(error.code);
+                        errorMessage = error.userInfo[NSLocalizedDescriptionKey];
+                    } else {
+                        exceptionCode = NotAllowedError;
 
-                    if ([error.domain isEqualToString:ASCAuthorizationErrorDomain] && error.code == ASCAuthorizationErrorUserCanceled)
-                        errorMessage = @"This request has been cancelled by the user.";
-                    else
-                        errorMessage = @"Operation failed.";
+                        if ([error.domain isEqualToString:ASCAuthorizationErrorDomain] && error.code == ASCAuthorizationErrorUserCanceled)
+                            errorMessage = @"This request has been cancelled by the user.";
+                        else
+                            errorMessage = @"Operation failed.";
+                    }
+
+                    exceptionData = { exceptionCode, errorMessage };
                 }
 
-                exceptionData = { exceptionCode, errorMessage };
-            }
-
-            handler(response, attachment, exceptionData);
-        }).get()];
+                handler(response, attachment, exceptionData);
+            }).get()];
+        });
     }).get()];
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to