[WebAuthN] A new request should always suppress the pending request if any
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 23:43:18 +0000 (23:43 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 May 2019 23:43:18 +0000 (23:43 +0000)
commit42e8a1c9c27f64d52b41577c4e74960518f70de7
treef5263e782ecdd57621151962e79904b7e90d197d
parent8474cdfc9082ddb41820464b2ee8a13093a1b92b
[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: https://svn.webkit.org/repository/webkit/trunk@245043 268f45cc-cd09-0410-ab3c-d52691b4dbfc
21 files changed:
LayoutTests/ChangeLog
LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt
LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html
LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https-expected.txt
LayoutTests/http/wpt/webauthn/public-key-credential-create-success-hid.https.html
LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https-expected.txt
LayoutTests/http/wpt/webauthn/public-key-credential-get-failure.https.html
LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt
LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html
LayoutTests/http/wpt/webauthn/resources/new-page.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.cpp
Source/WebCore/Modules/webauthn/AuthenticatorCoordinatorClient.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp
Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.cpp
Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h
Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.messages.in
Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.messages.in