commit ae50adea10492cb562f96593a297b68c07a6b75e
author jiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Tue May 07 23:43:18 2019 +0000
committer jiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Tue May 07 23:43:18 2019 +0000
tree f5263e782ecdd57621151962e79904b7e90d197d
parent a54a25ceb5acfb5715a23414b30b6d499be99d76

[WebAuthN] A new request should always suppress the pending request if any
https://bugs.webkit.org/show_bug.cgi?id=191517
<rdar://problem/46888222>

Reviewed by Brent Fulgham.

Source/WebCore:

Blocking new requests from the same page when there is a pending request could DoS the
WebAuthN API in the period between [the page is refreshed, the pending request is
hanedled/timeout]. Therefore, the policy will be to always cancel any pending requests
whenever a new request is made. This will enforce the policy of handling only one
request at a time.

Covered by new tests in existing files.

* Modules/webauthn/AuthenticatorCoordinatorClient.cpp:
(WebCore::AuthenticatorCoordinatorClient::requestReply):
(WebCore::AuthenticatorCoordinatorClient::setRequestCompletionHandler):
(WebCore::AuthenticatorCoordinatorClient::addQueryCompletionHandler):
* Modules/webauthn/AuthenticatorCoordinatorClient.h:

Source/WebKit:

Previously we blocked new WebAuthN requests whenever a pending request was in progress
to prevent background tabs from DoS foreground tabs. However, in r244938, the WebAuthN
API was changed to restrict request handling to the focused document. Therefore, we no
longer have a risk of DoS.

Apart from the vanished benefit, this behavoir actually blocks new pages to use
WebAuthN API in the period between [the previous initating page is closed, the pending
request is hanedled/timeout].

Also, it makes sense to have the current focused document preempt the pending request.
Therefore, the policy will be to always cancel any pending requests whenever a new
request is made. This will enforce the policy of handling only one request at a time.

Note that the current implementation doesn't explicitly cancel pending requests in the
Authenticators, which means that we could receive responses from the Authenticator that
were meant for a previous (now cancelled) request. A follow-up patch (see Bug 191523)
will implement an Authenticator feature to support immediate cancellation.

In the meantime, to protect the atomicity of the request/response pair, i.e., preventing an old
response being used for a new request, there are two safeguards:
1) In web process, each request to UI process is paired with an incremental ID, and therefore an old
response from UI process would have a different ID than the current request, which will then be ignored.
2) In UI process, all responses from authenticators will be piped to the main run loop for processing.
Therefore, when the new request comes in, the old response is either processed or waiting in the pipe.
To prevent the latter being processed, the new request will immediately destroy any authenticators bound
to the old response in the current run loop. Hence, in the next run loop when dealing the old response,
the lambda will have no where to hand the response over.

* UIProcess/WebAuthentication/AuthenticatorManager.cpp:
(WebKit::AuthenticatorManager::makeCredential):
(WebKit::AuthenticatorManager::getAssertion):
(WebKit::AuthenticatorManager::clearStateAsync):
(WebKit::AuthenticatorManager::clearState):
(WebKit::AuthenticatorManager::timeOutTimerFired):
* UIProcess/WebAuthentication/AuthenticatorManager.h:
* UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp:
(WebKit::WebAuthenticatorCoordinatorProxy::makeCredential):
(WebKit::WebAuthenticatorCoordinatorProxy::getAssertion):
(WebKit::WebAuthenticatorCoordinatorProxy::requestReply):
* UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h:
* UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in:
* WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp:
(WebKit::WebAuthenticatorCoordinator::makeCredential):
(WebKit::WebAuthenticatorCoordinator::getAssertion):
* WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in:

LayoutTests:

* http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt:
* http/wpt/webauthn/public-key-credential-create-failure.https.html:
* http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt:
* http/wpt/webauthn/public-key-credential-create-success-hid.https.html:
* http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt:
* http/wpt/webauthn/public-key-credential-get-failure.https.html:
* http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt:
* http/wpt/webauthn/public-key-credential-get-success-hid.https.html:
* http/wpt/webauthn/resources/new-page.html: Added.


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