webauthn: require that requests come from a frame in a focused window.

The spec suggests aborting an operation when focus[1] is lost, but
that's advice for site developers. I can't find anything in the spec
about preventing background tabs from triggering operations.

The cryptotoken extension refused to start a registration request, or to
send a registration response to, anything but the active tab in the
focused window. But background tabs could complete an authentication
request.

This change does something similar: it rejects both authentication and
registration requests unless the requesting frame is in a focused
window. It also performs that check before returning responses.

This is slightly different from the cryptotoken behaviour because
cryptotoken could only use what the extensions API exposed. For example,
if the omnibox was focused, cryptotoken would complete a registration
from the foreground tab but this code will reject it. I think this
behaviour is better, and it's certainly far more inline with the content
/ browser separation.

This change has been split from its tests, which will come in a future
CL.

[1] https://w3c.github.io/webauthn/#abortoperation

Bug: 827266
Change-Id: If6e97dd6526e175f40718724eda21e3efd434f7f
Reviewed-on: https://chromium-review.googlesource.com/991073
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550195}

chrome/browser/chrome_content_browser_client.cc

@@ -4261,6 +4261,10 @@ bool ChromeContentBrowserClient::
   return IsWebauthnRPIDListedInEnterprisePolicy(browser_context, rp_id);
 }
 
+bool ChromeContentBrowserClient::ShouldEnforceFocusChecksForWebauthn() {
+  return true;
+}
+
 void ChromeContentBrowserClient::ShouldReturnAttestationForWebauthnRPID(
     content::RenderFrameHost* rfh,
     const std::string& rp_id,
@@ -4290,8 +4294,17 @@ void ChromeContentBrowserClient::ShouldReturnAttestationForWebauthnRPID(
   }
 
   // The created AttestationPermissionRequest deletes itself once complete.
-  permission_request_manager->AddRequest(
-      NewAttestationPermissionRequest(origin, std::move(callback)));
+  //
+  // |callback| is called via the |MessageLoop| because otherwise the
+  // permissions bubble will have focus and |AuthenticatorImpl| checks that the
+  // frame still has focus before returning any results.
+  permission_request_manager->AddRequest(NewAttestationPermissionRequest(
+      origin, base::BindOnce(
+                  [](base::OnceCallback<void(bool)> callback, bool result) {
+                    base::ThreadTaskRunnerHandle::Get()->PostTask(
+                        FROM_HERE, base::BindOnce(std::move(callback), result));
+                  },
+                  std::move(callback))));
 #endif
 }
 

chrome/browser/chrome_content_browser_client.h

@@ -403,6 +403,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       content::BrowserContext* browser_context,
       const GURL& url,
       base::OnceCallback<void(bool, int, int)> callback) override;
+
   bool ShouldPermitIndividualAttestationForWebauthnRPID(
       content::BrowserContext* browser_context,
       const std::string& rp_id) override;
@@ -411,6 +412,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       const std::string& rp_id,
       const url::Origin& origin,
       base::OnceCallback<void(bool)> callback) override;
+  bool ShouldEnforceFocusChecksForWebauthn() override;
+
   std::unique_ptr<net::ClientCertStore> CreateClientCertStore(
       content::ResourceContext* resource_context) override;
   scoped_refptr<content::LoginDelegate> CreateLoginDelegate(

components/password_manager/content/common/credential_manager_mojom_traits.cc

@@ -92,6 +92,7 @@ bool EnumTraits<password_manager::mojom::CredentialManagerError,
     case password_manager::mojom::CredentialManagerError::INVALID_DOMAIN:
     case password_manager::mojom::CredentialManagerError::INVALID_STATE:
     case password_manager::mojom::CredentialManagerError::NOT_IMPLEMENTED:
+    case password_manager::mojom::CredentialManagerError::NOT_FOCUSED:
     case password_manager::mojom::CredentialManagerError::UNKNOWN:
       *output = password_manager::CredentialManagerError::UNKNOWN;
       return true;

content/browser/webauth/authenticator_impl.cc

@@ -19,6 +19,7 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -426,6 +427,13 @@ void AuthenticatorImpl::MakeCredential(
     return;
   }
 
+  if (GetContentClient()->browser()->ShouldEnforceFocusChecksForWebauthn() &&
+      !render_frame_host_->GetView()->HasFocus()) {
+    std::move(callback).Run(webauth::mojom::AuthenticatorStatus::NOT_FOCUSED,
+                            nullptr);
+    return;
+  }
+
   url::Origin caller_origin = render_frame_host_->GetLastCommittedOrigin();
   relying_party_id_ = options->relying_party->id;
 
@@ -539,6 +547,13 @@ void AuthenticatorImpl::GetAssertion(
     return;
   }
 
+  if (GetContentClient()->browser()->ShouldEnforceFocusChecksForWebauthn() &&
+      !render_frame_host_->GetView()->HasFocus()) {
+    std::move(callback).Run(webauth::mojom::AuthenticatorStatus::NOT_FOCUSED,
+                            nullptr);
+    return;
+  }
+
   url::Origin caller_origin = render_frame_host_->GetLastCommittedOrigin();
 
   if (!HasValidEffectiveDomain(caller_origin)) {
@@ -788,7 +803,14 @@ void AuthenticatorImpl::InvokeCallbackAndCleanup(
     MakeCredentialCallback callback,
     webauth::mojom::AuthenticatorStatus status,
     webauth::mojom::MakeCredentialAuthenticatorResponsePtr response) {
-  std::move(callback).Run(status, std::move(response));
+  if (GetContentClient()->browser()->ShouldEnforceFocusChecksForWebauthn() &&
+      !render_frame_host_->GetView()->HasFocus()) {
+    std::move(callback).Run(webauth::mojom::AuthenticatorStatus::NOT_FOCUSED,
+                            nullptr);
+  } else {
+    std::move(callback).Run(status, std::move(response));
+  }
+
   Cleanup();
 }
 
@@ -796,7 +818,14 @@ void AuthenticatorImpl::InvokeCallbackAndCleanup(
     GetAssertionCallback callback,
     webauth::mojom::AuthenticatorStatus status,
     webauth::mojom::GetAssertionAuthenticatorResponsePtr response) {
-  std::move(callback).Run(status, std::move(response));
+  if (GetContentClient()->browser()->ShouldEnforceFocusChecksForWebauthn() &&
+      !render_frame_host_->GetView()->HasFocus()) {
+    std::move(callback).Run(webauth::mojom::AuthenticatorStatus::NOT_FOCUSED,
+                            nullptr);
+  } else {
+    std::move(callback).Run(status, std::move(response));
+  }
+
   Cleanup();
 }
 

content/public/browser/content_browser_client.cc

@@ -683,6 +683,10 @@ void ContentBrowserClient::ShouldReturnAttestationForWebauthnRPID(
   std::move(callback).Run(true);
 }
 
+bool ContentBrowserClient::ShouldEnforceFocusChecksForWebauthn() {
+  return false;
+}
+
 std::unique_ptr<net::ClientCertStore>
 ContentBrowserClient::CreateClientCertStore(ResourceContext* resource_context) {
   return nullptr;

content/public/browser/content_browser_client.h

@@ -1120,6 +1120,11 @@ class CONTENT_EXPORT ContentBrowserClient {
       const url::Origin& origin,
       base::OnceCallback<void(bool)> callback);
 
+  // Returns true if the Webauthn implementation should require that the
+  // |RenderFrameHost|'s View has focus before processing requests, and before
+  // sending replies.
+  virtual bool ShouldEnforceFocusChecksForWebauthn();
+
   // Get platform ClientCertStore. May return nullptr.
   virtual std::unique_ptr<net::ClientCertStore> CreateClientCertStore(
       ResourceContext* resource_context);

third_party/blink/public/platform/modules/credentialmanager/credential_manager.mojom

@@ -29,6 +29,7 @@ enum CredentialManagerError {
   INVALID_DOMAIN,
   INVALID_STATE,
   NOT_IMPLEMENTED,
+  NOT_FOCUSED,
   UNKNOWN
 };
 

third_party/blink/public/platform/modules/webauth/authenticator.mojom

@@ -20,6 +20,7 @@ enum AuthenticatorStatus {
   INVALID_DOMAIN,
   INVALID_STATE,
   NOT_IMPLEMENTED,
+  NOT_FOCUSED,
   UNKNOWN_ERROR,
 };
 

third_party/blink/renderer/modules/credentialmanager/credential_manager_type_converters.cc

@@ -122,6 +122,8 @@ TypeConverter<CredentialManagerError, AuthenticatorStatus>::Convert(
       return CredentialManagerError::INVALID_STATE;
     case webauth::mojom::blink::AuthenticatorStatus::NOT_IMPLEMENTED:
       return CredentialManagerError::NOT_IMPLEMENTED;
+    case webauth::mojom::blink::AuthenticatorStatus::NOT_FOCUSED:
+      return CredentialManagerError::NOT_FOCUSED;
     case webauth::mojom::blink::AuthenticatorStatus::SUCCESS:
       NOTREACHED();
       break;

third_party/blink/renderer/modules/credentialmanager/credentials_container.cc

@@ -259,6 +259,10 @@ DOMException* CredentialManagerErrorToDOMException(
           "Attempting to register an already-registered key.");
     case CredentialManagerError::NOT_IMPLEMENTED:
       return DOMException::Create(kNotSupportedError, "Not implemented");
+    case CredentialManagerError::NOT_FOCUSED:
+      return DOMException::Create(kNotAllowedError,
+                                  "The operation is not allowed at this time "
+                                  "because the page does not have focus.");
     case CredentialManagerError::UNKNOWN:
       return DOMException::Create(kNotReadableError,
                                   "An unknown error occurred while talking "