From b129ae569a92f457b78f20011c9d728c86f953ef Mon Sep 17 00:00:00 2001 From: Martin Kreichgauer Date: Wed, 14 Nov 2018 21:26:33 +0000 Subject: [PATCH] fido: invoke Windows API authenticator for all transports Rather than only forwarding USB requests to the Windows WebAuthn API, we are going to forward all requests. This consequently renames the FidoAuthenticator subclass to WinWebAuthnApiAuthenticator. The corresponding FidoDiscovery is instantiated for all requests where the WebAuthn API is available (i.e., the feature is flag enabled, and the DLL can be loaded and is at least version 1). When it is indeed available, all other discoveries are disabled and the UI is suppressed via a newly introduced option in the TransportAvailabilityInfo struct relayed to embedders via the TransportAvailabilityObserver interface. Note this temporarily breaks caBLE whenever the Windows API is flag-enabled and available. (But the OS would presumably block the device communication in that case anyway.) Bug: 898718 Change-Id: If2bddac4bac519d1aa5aa9cb5d8fdc326297de73 Reviewed-on: https://chromium-review.googlesource.com/c/1330897 Commit-Queue: Martin Kreichgauer Reviewed-by: Jun Choi Reviewed-by: Adam Langley Cr-Commit-Position: refs/heads/master@{#608127} --- .../authenticator_request_dialog_model.cc | 1 + .../chrome_authenticator_request_delegate.cc | 23 +++++-- .../chrome_authenticator_request_delegate.h | 6 ++ device/fido/fido_discovery_factory.cc | 37 ++++++---- device/fido/fido_discovery_factory.h | 7 ++ device/fido/fido_request_handler_base.cc | 62 ++++++++++++++++- device/fido/fido_request_handler_base.h | 15 ++++ device/fido/get_assertion_handler_unittest.cc | 13 ++-- device/fido/win/authenticator.cc | 69 ++++++++++--------- device/fido/win/authenticator.h | 16 +++-- device/fido/win/discovery.cc | 15 ++-- device/fido/win/discovery.h | 11 ++- 12 files changed, 192 insertions(+), 83 deletions(-) diff --git a/chrome/browser/webauthn/authenticator_request_dialog_model.cc b/chrome/browser/webauthn/authenticator_request_dialog_model.cc index 30b65e3837a69d..0c782a168939f5 100644 --- a/chrome/browser/webauthn/authenticator_request_dialog_model.cc +++ b/chrome/browser/webauthn/authenticator_request_dialog_model.cc @@ -88,6 +88,7 @@ void AuthenticatorRequestDialogModel::StartFlow( TransportAvailabilityInfo transport_availability, base::Optional last_used_transport) { DCHECK_EQ(current_step(), Step::kNotStarted); + DCHECK(!transport_availability.disable_embedder_ui); transport_availability_ = std::move(transport_availability); last_used_transport_ = last_used_transport; diff --git a/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc b/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc index a2ec68b91e4156..bd6610708e70a3 100644 --- a/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc +++ b/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc @@ -62,10 +62,6 @@ bool IsWebauthnRPIDListedInEnterprisePolicy( #endif } -bool IsWebAuthnUiEnabled() { - return base::FeatureList::IsEnabled(features::kWebAuthenticationUI); -} - } // namespace #if defined(OS_MACOSX) @@ -267,6 +263,11 @@ void ChromeAuthenticatorRequestDelegate::UpdateLastTransportUsed( void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated( device::FidoRequestHandlerBase::TransportAvailabilityInfo data) { #if !defined(OS_ANDROID) + if (data.disable_embedder_ui) { + disable_ui_ = true; + return; + } + if (!IsWebAuthnUiEnabled()) return; @@ -282,11 +283,14 @@ void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated( bool ChromeAuthenticatorRequestDelegate::EmbedderControlsAuthenticatorDispatch( const device::FidoAuthenticator& authenticator) { - // TODO(hongjunchoi): Change this so that requests for BLE authenticators are - // not dispatched immediately if WebAuthN UI is enabled. + // TODO(hongjunchoi): Change this so that requests for BLE authenticators + // are not dispatched immediately if WebAuthN UI is enabled. if (!IsWebAuthnUiEnabled()) return false; + // On macOS, a native dialog is shown for the Touch ID authenticator + // immediately after dispatch to that authenticator. This dialog must not + // be triggered before Chrome's WebAuthn UI has advanced accordingly. return authenticator.AuthenticatorTransport() && *authenticator.AuthenticatorTransport() == device::FidoTransportProtocol::kInternal; @@ -384,3 +388,10 @@ ChromeAuthenticatorRequestDelegate::GetPreviouslyPairedFidoBleDeviceAddresses() Profile::FromBrowserContext(browser_context())->GetPrefs(); return prefs->GetList(kWebAuthnBlePairedMacAddressesPrefName); } + +bool ChromeAuthenticatorRequestDelegate::IsWebAuthnUiEnabled() const { + // UI can be disabled via flag or by the request handler for certain + // requests (e.g. on Windows, where the native API renders its own UI). + return base::FeatureList::IsEnabled(features::kWebAuthenticationUI) && + !disable_ui_; +} diff --git a/chrome/browser/webauthn/chrome_authenticator_request_delegate.h b/chrome/browser/webauthn/chrome_authenticator_request_delegate.h index 6fd5c9d475558e..680c8be2b0c66b 100644 --- a/chrome/browser/webauthn/chrome_authenticator_request_delegate.h +++ b/chrome/browser/webauthn/chrome_authenticator_request_delegate.h @@ -106,6 +106,7 @@ class ChromeAuthenticatorRequestDelegate void AddFidoBleDeviceToPairedList(std::string device_address); base::Optional GetLastTransportUsed() const; const base::ListValue* GetPreviouslyPairedFidoBleDeviceAddresses() const; + bool IsWebAuthnUiEnabled() const; content::RenderFrameHost* const render_frame_host_; AuthenticatorRequestDialogModel* weak_dialog_model_ = nullptr; @@ -119,6 +120,11 @@ class ChromeAuthenticatorRequestDelegate base::OnceClosure cancel_callback_; device::FidoRequestHandlerBase::RequestCallback request_callback_; + // If in the TransportAvailabilityInfo reported by the request handler, + // disable_embedder_ui is set, this will be set to true. No UI must be + // rendered and all request handler callbacks will be ignored. + bool disable_ui_ = false; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(ChromeAuthenticatorRequestDelegate); diff --git a/device/fido/fido_discovery_factory.cc b/device/fido/fido_discovery_factory.cc index 5386d6af0ad421..bcf26c59af4889 100644 --- a/device/fido/fido_discovery_factory.cc +++ b/device/fido/fido_discovery_factory.cc @@ -33,19 +33,6 @@ std::unique_ptr CreateUsbFidoDiscovery( return nullptr; #else -#if defined(OS_WIN) - // On platforms where the Windows webauthn.dll is present, access to USB - // devices is blocked and we use a special authenticator that forwards - // requests to the Windows WebAuthn API instead. - if (base::FeatureList::IsEnabled(device::kWebAuthUseNativeWinApi) && - WinWebAuthnApi::GetDefault()->IsAvailable()) { - return std::make_unique( - WinWebAuthnApi::GetDefault(), - // TODO(martinkr): Inject the window from which the request originated. - GetForegroundWindow()); - } -#endif // defined(OS_WIN) - DCHECK(connector); return std::make_unique(connector); #endif // !defined(OS_ANDROID) @@ -74,13 +61,13 @@ std::unique_ptr CreateFidoDiscoveryImpl( return nullptr; } -} // namespace - std::unique_ptr CreateCableDiscoveryImpl( std::vector cable_data) { return std::make_unique(std::move(cable_data)); } +} // namespace + // static FidoDiscoveryFactory::FactoryFuncPtr FidoDiscoveryFactory::g_factory_func_ = &CreateFidoDiscoveryImpl; @@ -102,6 +89,26 @@ std::unique_ptr FidoDiscoveryFactory::CreateCable( return (*g_cable_factory_func_)(std::move(cable_data)); } +// static +#if defined(OS_WIN) +std::unique_ptr +FidoDiscoveryFactory::MaybeCreateWinWebAuthnApiDiscovery() { + if (!base::FeatureList::IsEnabled(device::kWebAuthUseNativeWinApi) || + !WinWebAuthnApi::GetDefault()->IsAvailable()) { + return nullptr; + } + return std::make_unique( + WinWebAuthnApi::GetDefault(), + // TODO(martinkr): Inject the window from which the request + // originated. Windows uses this parameter to center the + // dialog over the parent. The dialog should be centered + // over the originating Chrome Window; the foreground window + // may have changed to something else since the request was + // issued. + GetForegroundWindow()); +} +#endif // defined(OS_WIN) + // ScopedFidoDiscoveryFactory ------------------------------------------------- namespace internal { diff --git a/device/fido/fido_discovery_factory.h b/device/fido/fido_discovery_factory.h index 573d8b1cc62045..cc21790622c83a 100644 --- a/device/fido/fido_discovery_factory.h +++ b/device/fido/fido_discovery_factory.h @@ -9,6 +9,7 @@ #include #include "base/component_export.h" +#include "build/build_config.h" #include "device/fido/cable/cable_discovery_data.h" #include "device/fido/fido_device_discovery.h" #include "device/fido/fido_discovery_base.h" @@ -39,6 +40,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory { // Instantiates a FidoDiscovery for caBLE. static std::unique_ptr CreateCable( std::vector cable_data); +#if defined(OS_WIN) + // Instantiates a FidoDiscovery for the native Windows WebAuthn + // API where available. Returns nullptr otherwise. + static std::unique_ptr + MaybeCreateWinWebAuthnApiDiscovery(); +#endif // defined(OS_WIN) private: friend class internal::ScopedFidoDiscoveryFactory; diff --git a/device/fido/fido_request_handler_base.cc b/device/fido/fido_request_handler_base.cc index 2143e19a299fa6..d07ddc2f2018d4 100644 --- a/device/fido/fido_request_handler_base.cc +++ b/device/fido/fido_request_handler_base.cc @@ -58,7 +58,16 @@ FidoRequestHandlerBase::TransportAvailabilityObserver:: FidoRequestHandlerBase::FidoRequestHandlerBase( service_manager::Connector* connector, const base::flat_set& available_transports) - : weak_factory_(this) { + : connector_(connector), weak_factory_(this) { +#if defined(OS_WIN) + InitDiscoveriesWin(available_transports); +#else + InitDiscoveries(available_transports); +#endif // !defined(OS_WIN) +} + +void FidoRequestHandlerBase::InitDiscoveries( + const base::flat_set& available_transports) { // The number of times |notify_observer_callback_| needs to be invoked before // Observer::OnTransportAvailabilityEnumerated is dispatched. Essentially this // is used to wait until all the parts of |transport_availability_info_| are @@ -85,7 +94,7 @@ FidoRequestHandlerBase::FidoRequestHandlerBase( continue; } - auto discovery = FidoDiscoveryFactory::Create(transport, connector); + auto discovery = FidoDiscoveryFactory::Create(transport, connector_); if (discovery == nullptr) { // This can occur in tests when a ScopedVirtualU2fDevice is in effect and // HID transports are not configured. @@ -116,6 +125,55 @@ FidoRequestHandlerBase::FidoRequestHandlerBase( weak_factory_.GetWeakPtr())); } +#if defined(OS_WIN) +void FidoRequestHandlerBase::InitDiscoveriesWin( + const base::flat_set& available_transports) { + // Try to instantiate the discovery for proxying requests to the native + // Windows WebAuthn API; or fall back to using the regular device transport + // discoveries if the API is unavailable. + auto discovery = FidoDiscoveryFactory::MaybeCreateWinWebAuthnApiDiscovery(); + if (!discovery) { + InitDiscoveries(available_transports); + return; + } + + // The Windows WebAuthn API is available. On this platform, communicating + // with authenticator devices directly is blocked by the OS, so we need to go + // through the native API instead. No device discoveries may be instantiated. + // + // The Windows API supports USB, NFC, BLE and platform authenticators, but + // not caBLE. Communicating with caBLE devices directly is subject to the + // same block by the OS, so this platform is without caBLE support for now. + // + // TODO(martinkr): Re-enable the caBLE discovery once caBLE has moved to a + // different UUID. See crbug.com/905111. + + discovery->set_observer(this); + discoveries_.push_back(std::move(discovery)); + + // Tell the embedder to not render a UI and ignore all future callbacks. Also + // don't report any available transports; the embedder is not supposed to use + // this information anyway. + transport_availability_info_.disable_embedder_ui = true; + transport_availability_info_.available_transports = {}; + + // The number of times |notify_observer_callback_| needs to be invoked before + // Observer::OnTransportAvailabilityEnumerated is dispatched. Essentially this + // is used to wait until all the parts of |transport_availability_info_| are + // filled out. In the case of Windows, there are no transport discoveries to + // wait for, so the |notify_observer_callback_| is only invoked in: + // 1) SetPlatformAuthenticatorOrMarkUnavailable(). + // 2) set_observer(). + constexpr size_t transport_info_callback_count = 2u; + + notify_observer_callback_ = base::BarrierClosure( + transport_info_callback_count, + base::BindOnce( + &FidoRequestHandlerBase::NotifyObserverTransportAvailability, + weak_factory_.GetWeakPtr())); +} +#endif // defined(OS_WIN) + FidoRequestHandlerBase::~FidoRequestHandlerBase() = default; void FidoRequestHandlerBase::StartAuthenticatorRequest( diff --git a/device/fido/fido_request_handler_base.h b/device/fido/fido_request_handler_base.h index 575aebb7d5aa5d..125984e494187a 100644 --- a/device/fido/fido_request_handler_base.h +++ b/device/fido/fido_request_handler_base.h @@ -19,6 +19,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/strings/string_piece_forward.h" +#include "build/build_config.h" #include "device/fido/fido_device_authenticator.h" #include "device/fido/fido_discovery_base.h" #include "device/fido/fido_transport_protocol.h" @@ -87,6 +88,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase bool has_recognized_mac_touch_id_credential = false; bool is_ble_powered = false; bool can_power_on_ble_adapter = false; + + // If true, dispatch of the request cannot be controlled by + // the embedder. The embedder must not display a UI for this + // request and must ignore all subsequent invocations of the + // TransportAvailabilityObserver interface methods. + bool disable_embedder_ui = false; }; class COMPONENT_EXPORT(DEVICE_FIDO) TransportAvailabilityObserver { @@ -201,6 +208,13 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase private: friend class FidoRequestHandlerTest; + void InitDiscoveries( + const base::flat_set& available_transports); +#if defined(OS_WIN) + void InitDiscoveriesWin( + const base::flat_set& available_transports); +#endif + // FidoDiscoveryBase::Observer void AuthenticatorAdded(FidoDiscoveryBase* discovery, FidoAuthenticator* authenticator) final; @@ -232,6 +246,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase // TODO(martinkr): Inject platform authenticators through a new // FidoDiscoveryBase specialization and hold ownership there. std::unique_ptr platform_authenticator_; + service_manager::Connector* const connector_; base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(FidoRequestHandlerBase); diff --git a/device/fido/get_assertion_handler_unittest.cc b/device/fido/get_assertion_handler_unittest.cc index a33f76d8056e1d..30391c3d161f13 100644 --- a/device/fido/get_assertion_handler_unittest.cc +++ b/device/fido/get_assertion_handler_unittest.cc @@ -31,6 +31,7 @@ #include "testing/gtest/include/gtest/gtest.h" #if defined(OS_WIN) +#include "device/fido/win/authenticator.h" #include "device/fido/win/fake_webauthn_api.h" #endif // defined(OS_WIN) @@ -753,10 +754,13 @@ TEST_F(GetAssertionRequestHandlerWinTest, TestWinUsbDiscovery) { if (test.enable_feature_flag) scoped_feature_list.InitAndEnableFeature(kWebAuthUseNativeWinApi); + // Simulate a connected HID device. + ScopedFakeHidManager fake_hid_manager; + fake_hid_manager.AddFidoHidDevice("guid"); + TestGetAssertionRequestCallback cb; - ScopedFakeHidManager fake_hid_manager_; auto handler = std::make_unique( - fake_hid_manager_.service_manager_connector(), + fake_hid_manager.service_manager_connector(), base::flat_set( {FidoTransportProtocol::kUsbHumanInterfaceDevice}), CtapGetAssertionRequest(test_data::kRelyingPartyId, @@ -765,14 +769,11 @@ TEST_F(GetAssertionRequestHandlerWinTest, TestWinUsbDiscovery) { cb.callback()); scoped_task_environment_.RunUntilIdle(); - fake_hid_manager_.AddFidoHidDevice("guid"); - scoped_task_environment_.RunUntilIdle(); - EXPECT_EQ(1u, handler->AuthenticatorsForTesting().size()); // Crudely distinguish authenticator type by FidoAuthenticator::GetId. EXPECT_EQ(test.expect_device_type == DeviceType::kHid ? "hid:guid" - : "WinNativeCrossPlatformAuthenticator", + : WinWebAuthnApiAuthenticator::kAuthenticatorId, handler->AuthenticatorsForTesting().begin()->second->GetId()); } } diff --git a/device/fido/win/authenticator.cc b/device/fido/win/authenticator.cc index 9c4f4b8d7074ef..3dc9d3c02a820d 100644 --- a/device/fido/win/authenticator.cc +++ b/device/fido/win/authenticator.cc @@ -39,7 +39,11 @@ base::string16 OptionalGURLToUTF16(const base::Optional& in) { } // namespace -WinNativeCrossPlatformAuthenticator::WinNativeCrossPlatformAuthenticator( +// static +const char WinWebAuthnApiAuthenticator::kAuthenticatorId[] = + "WinWebAuthnApiAuthenticator"; + +WinWebAuthnApiAuthenticator::WinWebAuthnApiAuthenticator( WinWebAuthnApi* win_api, HWND current_window) : FidoAuthenticator(), @@ -50,18 +54,18 @@ WinNativeCrossPlatformAuthenticator::WinNativeCrossPlatformAuthenticator( CoCreateGuid(&cancellation_id_); } -WinNativeCrossPlatformAuthenticator::~WinNativeCrossPlatformAuthenticator() { +WinWebAuthnApiAuthenticator::~WinWebAuthnApiAuthenticator() { // Cancel in order to dismiss any pending API request and UI dialog and shut // down |thread_|. Cancel(); } -void WinNativeCrossPlatformAuthenticator::InitializeAuthenticator( +void WinWebAuthnApiAuthenticator::InitializeAuthenticator( base::OnceClosure callback) { std::move(callback).Run(); } -void WinNativeCrossPlatformAuthenticator::MakeCredential( +void WinWebAuthnApiAuthenticator::MakeCredential( CtapMakeCredentialRequest request, MakeCredentialCallback callback) { DCHECK(!thread_); @@ -78,22 +82,21 @@ void WinNativeCrossPlatformAuthenticator::MakeCredential( thread_->task_runner()->PostTask( FROM_HERE, base::BindOnce( - &WinNativeCrossPlatformAuthenticator::MakeCredentialBlocking, + &WinWebAuthnApiAuthenticator::MakeCredentialBlocking, // Because |thread_| and its task runner are owned by this // authenticator instance, binding to Unretained(this) here is // fine. If the instance got destroyed before invocation of the // task, so would the task. Once the task is running, destruction // of the authenticator instance blocks on the thread exiting. base::Unretained(this), std::move(request), - base::BindOnce(&WinNativeCrossPlatformAuthenticator:: - InvokeMakeCredentialCallback, - weak_factory_.GetWeakPtr(), std::move(callback)), + base::BindOnce( + &WinWebAuthnApiAuthenticator::InvokeMakeCredentialCallback, + weak_factory_.GetWeakPtr(), std::move(callback)), base::SequencedTaskRunnerHandle::Get())); } -void WinNativeCrossPlatformAuthenticator::GetAssertion( - CtapGetAssertionRequest request, - GetAssertionCallback callback) { +void WinWebAuthnApiAuthenticator::GetAssertion(CtapGetAssertionRequest request, + GetAssertionCallback callback) { DCHECK(!thread_); if (thread_) { return; @@ -104,7 +107,7 @@ void WinNativeCrossPlatformAuthenticator::GetAssertion( thread_->task_runner()->PostTask( FROM_HERE, base::BindOnce( - &WinNativeCrossPlatformAuthenticator::GetAssertionBlocking, + &WinWebAuthnApiAuthenticator::GetAssertionBlocking, // Because |thread_| and its task runner are owned by this // authenticator instance, binding to Unretained(this) here is // fine. If the instance got destroyed before invocation of the @@ -112,7 +115,7 @@ void WinNativeCrossPlatformAuthenticator::GetAssertion( // of the authenticator instance blocks on the thread exiting. base::Unretained(this), std::move(request), base::BindOnce( - &WinNativeCrossPlatformAuthenticator::InvokeGetAssertionCallback, + &WinWebAuthnApiAuthenticator::InvokeGetAssertionCallback, weak_factory_.GetWeakPtr(), std::move(callback)), base::SequencedTaskRunnerHandle::Get())); } @@ -120,7 +123,7 @@ void WinNativeCrossPlatformAuthenticator::GetAssertion( // Invokes the blocking WEBAUTHN_AUTHENTICATOR_MAKE_CREDENTIAL API call. This // method is run on |thread_|. Note that the destructor for this class blocks // on |thread_| shutdown. -void WinNativeCrossPlatformAuthenticator::MakeCredentialBlocking( +void WinWebAuthnApiAuthenticator::MakeCredentialBlocking( CtapMakeCredentialRequest request, MakeCredentialCallback callback, scoped_refptr callback_runner) { @@ -190,11 +193,10 @@ void WinNativeCrossPlatformAuthenticator::MakeCredentialBlocking( kWinWebAuthnTimeoutMilliseconds, WEBAUTHN_CREDENTIALS{exclude_list.size(), exclude_list.data()}, WEBAUTHN_EXTENSIONS{extensions.size(), extensions.data()}, - // Forcibly set authenticator attachment to cross-platform in order to - // avoid triggering the platform authenticator option, which is - // generally displayed first in the Windows UI. + // TODO(martinkr): Plumb authenticator attachment into + // CtapMakeCredentialRequest and set here. use_u2f_only_ ? WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM_U2F_V2 - : WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM, + : WEBAUTHN_AUTHENTICATOR_ATTACHMENT_ANY, request.resident_key_required(), ToWinUserVerificationRequirement(request.user_verification()), WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_DIRECT, 0 /* flags */, @@ -270,7 +272,7 @@ void WinNativeCrossPlatformAuthenticator::MakeCredentialBlocking( // Invokes the blocking WEBAUTHN_AUTHENTICATOR_GET_ASSERTION API call. This // method is run on |thread_|. Note that the destructor for this class blocks // on |thread_| shutdown. -void WinNativeCrossPlatformAuthenticator::GetAssertionBlocking( +void WinWebAuthnApiAuthenticator::GetAssertionBlocking( CtapGetAssertionRequest request, GetAssertionCallback callback, scoped_refptr callback_runner) { @@ -298,8 +300,10 @@ void WinNativeCrossPlatformAuthenticator::GetAssertionBlocking( kWinWebAuthnTimeoutMilliseconds, WEBAUTHN_CREDENTIALS{allow_list.size(), allow_list.data()}, WEBAUTHN_EXTENSIONS{0, nullptr}, + // TODO(martinkr): Plumb authenticator attachment into + // CtapMakeCredentialRequest and set here. use_u2f_only_ ? WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM_U2F_V2 - : WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM, + : WEBAUTHN_AUTHENTICATOR_ATTACHMENT_ANY, ToWinUserVerificationRequirement(request.user_verification()), 0, // flags use_u2f_only_ ? rp_id16.c_str() : nullptr, // pwszU2fAppId @@ -333,7 +337,7 @@ void WinNativeCrossPlatformAuthenticator::GetAssertionBlocking( base::BindOnce(std::move(callback), status, std::move(response))); } -void WinNativeCrossPlatformAuthenticator::Cancel() { +void WinWebAuthnApiAuthenticator::Cancel() { if (!thread_ || operation_cancelled_.IsSet()) { return; } @@ -345,31 +349,31 @@ void WinNativeCrossPlatformAuthenticator::Cancel() { thread_->Stop(); } -std::string WinNativeCrossPlatformAuthenticator::GetId() const { - return "WinNativeCrossPlatformAuthenticator"; +std::string WinWebAuthnApiAuthenticator::GetId() const { + return kAuthenticatorId; } -base::string16 WinNativeCrossPlatformAuthenticator::GetDisplayName() const { - return L"WinNativeCrossPlatformAuthenticator"; +base::string16 WinWebAuthnApiAuthenticator::GetDisplayName() const { + return base::UTF8ToUTF16(GetId()); } -bool WinNativeCrossPlatformAuthenticator::IsInPairingMode() const { +bool WinWebAuthnApiAuthenticator::IsInPairingMode() const { return false; } -bool WinNativeCrossPlatformAuthenticator::IsPaired() const { +bool WinWebAuthnApiAuthenticator::IsPaired() const { return false; } base::Optional -WinNativeCrossPlatformAuthenticator::AuthenticatorTransport() const { +WinWebAuthnApiAuthenticator::AuthenticatorTransport() const { // The Windows API could potentially use any external or // platform authenticator. return base::nullopt; } const base::Optional& -WinNativeCrossPlatformAuthenticator::Options() const { +WinWebAuthnApiAuthenticator::Options() const { // The request can potentially be fulfilled by any device that Windows // communicates with, so returning AuthenticatorSupportedOptions really // doesn't make much sense. @@ -378,12 +382,11 @@ WinNativeCrossPlatformAuthenticator::Options() const { return no_options; } -base::WeakPtr -WinNativeCrossPlatformAuthenticator::GetWeakPtr() { +base::WeakPtr WinWebAuthnApiAuthenticator::GetWeakPtr() { return weak_factory_.GetWeakPtr(); } -void WinNativeCrossPlatformAuthenticator::InvokeMakeCredentialCallback( +void WinWebAuthnApiAuthenticator::InvokeMakeCredentialCallback( MakeCredentialCallback cb, CtapDeviceResponseCode status, base::Optional response) { @@ -394,7 +397,7 @@ void WinNativeCrossPlatformAuthenticator::InvokeMakeCredentialCallback( } std::move(cb).Run(status, std::move(response)); } -void WinNativeCrossPlatformAuthenticator::InvokeGetAssertionCallback( +void WinWebAuthnApiAuthenticator::InvokeGetAssertionCallback( GetAssertionCallback cb, CtapDeviceResponseCode status, base::Optional response) { diff --git a/device/fido/win/authenticator.h b/device/fido/win/authenticator.h index 79ad8781dbd23f..899ed90c27b12f 100644 --- a/device/fido/win/authenticator.h +++ b/device/fido/win/authenticator.h @@ -20,15 +20,17 @@ namespace device { -// WinNativeCrossPlatformAuthenticator forwards WebAuthn requests to external +// WinWebAuthnApiAuthenticator forwards WebAuthn requests to external // authenticators via the native Windows WebAuthentication API // (webauthn.dll). -class COMPONENT_EXPORT(DEVICE_FIDO) WinNativeCrossPlatformAuthenticator +class COMPONENT_EXPORT(DEVICE_FIDO) WinWebAuthnApiAuthenticator : public FidoAuthenticator { public: - WinNativeCrossPlatformAuthenticator(WinWebAuthnApi* win_api, - HWND current_window); - ~WinNativeCrossPlatformAuthenticator() override; + // The return value of |GetId|. + static const char kAuthenticatorId[]; + + WinWebAuthnApiAuthenticator(WinWebAuthnApi* win_api, HWND current_window); + ~WinWebAuthnApiAuthenticator() override; // Forces the Windows WebAuthn API not to communicate with CTAP2 devices for // this request. Dual-protocol devices will use U2F. @@ -84,8 +86,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) WinNativeCrossPlatformAuthenticator GUID cancellation_id_ = {}; base::AtomicFlag operation_cancelled_; - base::WeakPtrFactory weak_factory_; - DISALLOW_COPY_AND_ASSIGN(WinNativeCrossPlatformAuthenticator); + base::WeakPtrFactory weak_factory_; + DISALLOW_COPY_AND_ASSIGN(WinWebAuthnApiAuthenticator); }; } // namespace device diff --git a/device/fido/win/discovery.cc b/device/fido/win/discovery.cc index 7970f6edf64a66..0e72b97ff120fd 100644 --- a/device/fido/win/discovery.cc +++ b/device/fido/win/discovery.cc @@ -6,18 +6,17 @@ namespace device { -WinNativeCrossPlatformAuthenticatorDiscovery:: - WinNativeCrossPlatformAuthenticatorDiscovery( - WinWebAuthnApi* const win_webauthn_api, - HWND parent_window) +WinWebAuthnApiAuthenticatorDiscovery::WinWebAuthnApiAuthenticatorDiscovery( + WinWebAuthnApi* const win_webauthn_api, + HWND parent_window) : FidoDiscoveryBase(FidoTransportProtocol::kUsbHumanInterfaceDevice), win_webauthn_api_(win_webauthn_api), parent_window_(parent_window) {} -WinNativeCrossPlatformAuthenticatorDiscovery:: - ~WinNativeCrossPlatformAuthenticatorDiscovery() = default; +WinWebAuthnApiAuthenticatorDiscovery::~WinWebAuthnApiAuthenticatorDiscovery() = + default; -void WinNativeCrossPlatformAuthenticatorDiscovery::Start() { +void WinWebAuthnApiAuthenticatorDiscovery::Start() { DCHECK(!authenticator_); if (!observer()) { return; @@ -29,7 +28,7 @@ void WinNativeCrossPlatformAuthenticatorDiscovery::Start() { } observer()->DiscoveryStarted(this, true /* success */); - authenticator_ = std::make_unique( + authenticator_ = std::make_unique( WinWebAuthnApi::GetDefault(), parent_window_); observer()->AuthenticatorAdded(this, authenticator_.get()); } diff --git a/device/fido/win/discovery.h b/device/fido/win/discovery.h index b7e3d17e38721a..ba7a803af8d2d3 100644 --- a/device/fido/win/discovery.h +++ b/device/fido/win/discovery.h @@ -16,19 +16,18 @@ namespace device { // Instantiates the authenticator subclass for forwarding requests to external // authenticators via the Windows WebAuthn API. -class COMPONENT_EXPORT(DEVICE_FIDO) WinNativeCrossPlatformAuthenticatorDiscovery +class COMPONENT_EXPORT(DEVICE_FIDO) WinWebAuthnApiAuthenticatorDiscovery : public FidoDiscoveryBase { public: - WinNativeCrossPlatformAuthenticatorDiscovery( - WinWebAuthnApi* const win_webauthn_api, - HWND parent_window); - ~WinNativeCrossPlatformAuthenticatorDiscovery() override; + WinWebAuthnApiAuthenticatorDiscovery(WinWebAuthnApi* const win_webauthn_api, + HWND parent_window); + ~WinWebAuthnApiAuthenticatorDiscovery() override; // FidoDiscoveryBase: void Start() override; private: - std::unique_ptr authenticator_; + std::unique_ptr authenticator_; WinWebAuthnApi* const win_webauthn_api_; const HWND parent_window_; };