fido: disable attestation consent prompt for cryptotoken requests

The ContentBrowserClient in WebAuthn requests originating from the cryptotoken
extension is not associated with any tab and therefore trying to show dialogs,
such as the one for collecting attestation consent, will fail. cryptotoken
has its own attestation consent dialog already, so we can safely disable
attestation consent prompts if the caller origin is cryptotoken.

Bug: 898718
Change-Id: Ic2d530665494cfa10ac4c3a7812e8284091e50fb
Reviewed-on: https://chromium-review.googlesource.com/c/1340586
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609947}

content/browser/webauth/authenticator_impl.cc

@@ -598,10 +598,10 @@ void AuthenticatorImpl::MakeCredential(
     return;
   }
 
-  url::Origin caller_origin = render_frame_host_->GetLastCommittedOrigin();
+  caller_origin_ = render_frame_host_->GetLastCommittedOrigin();
   relying_party_id_ = options->relying_party->id;
 
-  if (!HasValidEffectiveDomain(caller_origin)) {
+  if (!HasValidEffectiveDomain(caller_origin_)) {
     bad_message::ReceivedBadMessage(render_frame_host_->GetProcess(),
                                     bad_message::AUTH_INVALID_EFFECTIVE_DOMAIN);
     InvokeCallbackAndCleanup(std::move(callback),
@@ -610,7 +610,7 @@ void AuthenticatorImpl::MakeCredential(
     return;
   }
 
-  if (!IsRelyingPartyIdValid(relying_party_id_, caller_origin)) {
+  if (!IsRelyingPartyIdValid(relying_party_id_, caller_origin_)) {
     bad_message::ReceivedBadMessage(render_frame_host_->GetProcess(),
                                     bad_message::AUTH_INVALID_RELYING_PARTY);
     InvokeCallbackAndCleanup(std::move(callback),
@@ -641,7 +641,7 @@ void AuthenticatorImpl::MakeCredential(
   // Save client data to return with the authenticator response.
   // TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key
   // used to communicate with the origin.
-  if (OriginIsCryptoTokenExtension(caller_origin)) {
+  if (OriginIsCryptoTokenExtension(caller_origin_)) {
     // As Cryptotoken validates the origin, accept the relying party id as the
     // origin from requests originating from Cryptotoken.
     client_data_json_ = SerializeCollectedClientDataToJson(
@@ -649,7 +649,7 @@ void AuthenticatorImpl::MakeCredential(
         std::move(options->challenge), true /* use_legacy_u2f_type_key */);
   } else {
     client_data_json_ = SerializeCollectedClientDataToJson(
-        client_data::kCreateType, caller_origin.Serialize(),
+        client_data::kCreateType, caller_origin_.Serialize(),
         std::move(options->challenge));
   }
 
@@ -668,7 +668,7 @@ void AuthenticatorImpl::MakeCredential(
 
   auto ctap_request = CreateCtapMakeCredentialRequest(
       client_data_json_, options, individual_attestation);
-  ctap_request.set_is_u2f_only(OriginIsCryptoTokenExtension(caller_origin));
+  ctap_request.set_is_u2f_only(OriginIsCryptoTokenExtension(caller_origin_));
 
   request_ = std::make_unique<device::MakeCredentialRequestHandler>(
       connector_, transports_, std::move(ctap_request),
@@ -712,12 +712,12 @@ void AuthenticatorImpl::GetAssertion(
     return;
   }
 
-  url::Origin caller_origin = render_frame_host_->GetLastCommittedOrigin();
+  caller_origin_ = render_frame_host_->GetLastCommittedOrigin();
 
   // Save client data to return with the authenticator response.
   // TODO(kpaulhamus): Fetch and add the Channel ID/Token Binding ID public key
   // used to communicate with the origin.
-  if (OriginIsCryptoTokenExtension(caller_origin)) {
+  if (OriginIsCryptoTokenExtension(caller_origin_)) {
     // As Cryptotoken validates the origin, accept the relying party id as the
     // origin from requests originating from Cryptotoken.
     client_data_json_ = SerializeCollectedClientDataToJson(
@@ -725,11 +725,11 @@ void AuthenticatorImpl::GetAssertion(
         std::move(options->challenge), true /* use_legacy_u2f_type_key */);
   } else {
     client_data_json_ = SerializeCollectedClientDataToJson(
-        client_data::kGetType, caller_origin.Serialize(),
+        client_data::kGetType, caller_origin_.Serialize(),
         std::move(options->challenge));
   }
 
-  if (!HasValidEffectiveDomain(caller_origin)) {
+  if (!HasValidEffectiveDomain(caller_origin_)) {
     bad_message::ReceivedBadMessage(render_frame_host_->GetProcess(),
                                     bad_message::AUTH_INVALID_EFFECTIVE_DOMAIN);
     InvokeCallbackAndCleanup(std::move(callback),
@@ -738,7 +738,7 @@ void AuthenticatorImpl::GetAssertion(
     return;
   }
 
-  if (!IsRelyingPartyIdValid(options->relying_party_id, caller_origin)) {
+  if (!IsRelyingPartyIdValid(options->relying_party_id, caller_origin_)) {
     bad_message::ReceivedBadMessage(render_frame_host_->GetProcess(),
                                     bad_message::AUTH_INVALID_RELYING_PARTY);
     InvokeCallbackAndCleanup(std::move(callback),
@@ -758,7 +758,7 @@ void AuthenticatorImpl::GetAssertion(
 
   if (options->appid) {
     alternative_application_parameter_ =
-        ProcessAppIdExtension(*options->appid, caller_origin);
+        ProcessAppIdExtension(*options->appid, caller_origin_);
     if (!alternative_application_parameter_) {
       std::move(callback).Run(blink::mojom::AuthenticatorStatus::INVALID_DOMAIN,
                               nullptr);
@@ -892,12 +892,32 @@ void AuthenticatorImpl::OnRegisterResponse(
       return;
     case device::FidoReturnCode::kSuccess:
       DCHECK(response_data.has_value());
+
       if (transport_used) {
         request_delegate_->UpdateLastTransportUsed(*transport_used);
       }
 
       if (attestation_preference_ !=
           blink::mojom::AttestationConveyancePreference::NONE) {
+        // Cryptotoken requests may bypass the attestation prompt because the
+        // extension implements its own. Invoking the attestation prompt code
+        // here would not work anyway, because the WebContents associated with
+        // the extension is not associated with any tab and therefore cannot
+        // draw modal dialogs for the UI.
+        //
+        // Note that for AttestationConveyancePreference::NONE, attestation
+        // erasure is still performed as usual.
+        if (OriginIsCryptoTokenExtension(caller_origin_)) {
+          InvokeCallbackAndCleanup(
+              std::move(make_credential_response_callback_),
+              blink::mojom::AuthenticatorStatus::SUCCESS,
+              CreateMakeCredentialResponse(
+                  std::move(client_data_json_), std::move(*response_data),
+                  AttestationErasureOption::kIncludeAttestation),
+              Focus::kDoCheck);
+          return;
+        }
+
         UMA_HISTOGRAM_ENUMERATION("WebAuthentication.AttestationPromptResult",
                                   AttestationPromptResult::kQueried);
         awaiting_attestation_response_ = true;

content/browser/webauth/authenticator_impl.h

@@ -183,6 +183,7 @@ class CONTENT_EXPORT AuthenticatorImpl : public blink::mojom::Authenticator,
   GetAssertionCallback get_assertion_response_callback_;
   std::string client_data_json_;
   blink::mojom::AttestationConveyancePreference attestation_preference_;
+  url::Origin caller_origin_;
   std::string relying_party_id_;
   std::unique_ptr<base::OneShotTimer> timer_;
   // If the "appid" extension is in use then this is the SHA-256 hash of a U2F

content/browser/webauth/authenticator_impl_unittest.cc

@@ -772,8 +772,44 @@ TEST_F(AuthenticatorImplTest, CryptotokenBypass) {
 // Requests originating from cryptotoken should only target U2F devices.
 TEST_F(AuthenticatorImplTest, CryptoTokenU2fOnly) {
   EnableFeature(device::kWebAuthProxyCryptotoken);
-  constexpr char kCryptotokenOrigin[] =
-      "chrome-extension://kmendfapggjehodndflmmgagdbamhnfd";
+  TestServiceManagerContext smc;
+  SimulateNavigation(GURL(kTestOrigin1));
+  auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
+      base::Time::Now(), base::TimeTicks::Now());
+  auto authenticator = ConstructAuthenticatorWithTimer(task_runner);
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+
+  // TODO(martinkr): ScopedVirtualFidoDevice does not offer devices that
+  // support both U2F and CTAP yet; we should test those.
+  for (const bool u2f_authenticator : {true, false}) {
+    SCOPED_TRACE(u2f_authenticator ? "U2F" : "CTAP");
+    OverrideLastCommittedOrigin(main_rfh(),
+                                url::Origin::Create(GURL(kCryptotokenOrigin)));
+
+    device::test::ScopedVirtualFidoDevice scoped_virtual_device;
+    scoped_virtual_device.SetSupportedProtocol(
+        u2f_authenticator ? device::ProtocolVersion::kU2f
+                          : device::ProtocolVersion::kCtap);
+
+    PublicKeyCredentialCreationOptionsPtr options =
+        GetTestPublicKeyCredentialCreationOptions();
+    TestMakeCredentialCallback callback_receiver;
+    authenticator->MakeCredential(std::move(options),
+                                  callback_receiver.callback());
+
+    base::RunLoop().RunUntilIdle();
+    task_runner->FastForwardBy(base::TimeDelta::FromMinutes(1));
+    callback_receiver.WaitForCallback();
+
+    EXPECT_EQ((u2f_authenticator ? AuthenticatorStatus::SUCCESS
+                                 : AuthenticatorStatus::NOT_ALLOWED_ERROR),
+              callback_receiver.status());
+  }
+}
+
+// Requests originating from cryptotoken should only target U2F devices.
+TEST_F(AuthenticatorImplTest, AttestationPermitted) {
+  EnableFeature(device::kWebAuthProxyCryptotoken);
   TestServiceManagerContext smc;
   SimulateNavigation(GURL(kTestOrigin1));
   auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
@@ -1924,6 +1960,36 @@ TEST_F(AuthenticatorContentBrowserClientTest, IsUVPAAFalse) {
 }
 #endif  // !defined(OS_MACOSX)
 
+TEST_F(AuthenticatorContentBrowserClientTest,
+       CryptotokenBypassesAttestationConsentPrompt) {
+  EnableFeature(device::kWebAuthProxyCryptotoken);
+  TestServiceManagerContext smc;
+  SimulateNavigation(GURL(kTestOrigin1));
+  auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
+      base::Time::Now(), base::TimeTicks::Now());
+  auto authenticator = ConstructAuthenticatorWithTimer(task_runner);
+  url::AddStandardScheme("chrome-extension", url::SCHEME_WITH_HOST);
+  OverrideLastCommittedOrigin(main_rfh(),
+                              url::Origin::Create(GURL(kCryptotokenOrigin)));
+
+  virtual_device_.SetSupportedProtocol(device::ProtocolVersion::kU2f);
+  PublicKeyCredentialCreationOptionsPtr options =
+      GetTestPublicKeyCredentialCreationOptions();
+  // Despite the direct attestation conveyance preference, the request delegate
+  // is not asked for attestation consent. Hence the request will succeed,
+  // despite the handler denying all attestation consent prompts.
+  options->attestation = AttestationConveyancePreference::DIRECT;
+  test_client_.attestation_consent = AttestationConsent::DENIED;
+
+  TestMakeCredentialCallback callback_receiver;
+  authenticator->MakeCredential(std::move(options),
+                                callback_receiver.callback());
+
+  callback_receiver.WaitForCallback();
+
+  EXPECT_EQ(AuthenticatorStatus::SUCCESS, callback_receiver.status());
+}
+
 class MockAuthenticatorRequestDelegateObserver
     : public TestAuthenticatorRequestDelegate {
  public: