Skip to content

Commit

Permalink
chrome/browser/ui/webauthn: double-UI resident creds on Windows API v1.
Browse files Browse the repository at this point in the history
This change adds a confirmation step when creating resident credentials
with Windows Webauthn DLL version one (currently in 19H1). This version
requires the use of native APIs for interaction with security keys, but
doesn't show a notice informing the user that creating a resident
credential will store information on their security key.

However, such a notice is a privacy requirement and therefore a
confirmation step is added prior to dispatching the API call.

Bug: 941120
Change-Id: Ia39aefe7e2b294a604346a66851dbda67372bd63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1601299
Reviewed-by: Martin Kreichgauer <martinkr@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659267}
  • Loading branch information
Martin Kreichgauer authored and Commit Bot committed May 13, 2019
1 parent 4e8749b commit e95b9a3
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 3 deletions.
6 changes: 6 additions & 0 deletions chrome/browser/ui/views/webauthn/sheet_view_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ std::unique_ptr<AuthenticatorRequestSheetView> CreateSheetViewForCurrentStepOf(
AuthenticatorGenericErrorSheetModel::
ForClientPinErrorAuthenticatorRemoved(dialog_model));
break;
case Step::kResidentCredentialConfirmation:
sheet_view = std::make_unique<AuthenticatorRequestSheetView>(
std::make_unique<
AuthenticatorResidentCredentialConfirmationSheetView>(
dialog_model));
break;
case Step::kSelectAccount:
sheet_view = std::make_unique<AuthenticatorSelectAccountSheetView>(
std::make_unique<AuthenticatorSelectAccountSheetModel>(dialog_model));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ IN_PROC_BROWSER_TEST_F(AuthenticatorDialogTest, InvokeUi_storage_full) {
ShowAndVerifyUi();
}

IN_PROC_BROWSER_TEST_F(AuthenticatorDialogTest,
InvokeUi_resident_credential_confirm) {
ShowAndVerifyUi();
}

IN_PROC_BROWSER_TEST_F(AuthenticatorDialogTest, InvokeUi_account_select) {
ShowAndVerifyUi();
}
Expand Down
57 changes: 55 additions & 2 deletions chrome/browser/ui/webauthn/sheet_models.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ base::string16 GetRelyingPartyIdString(
base::Optional<base::string16> PossibleResidentKeyWarning(
AuthenticatorRequestDialogModel* dialog_model) {
if (dialog_model->might_create_resident_credential()) {
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_RESIDENT_KEY_PRIVACY,
GetRelyingPartyIdString(dialog_model));
return l10n_util::GetStringUTF16(IDS_WEBAUTHN_RESIDENT_KEY_PRIVACY);
}
return base::nullopt;
}
Expand Down Expand Up @@ -987,6 +986,60 @@ base::string16 AuthenticatorGenericErrorSheetModel::GetStepDescription() const {
return description_;
}

// AuthenticatorResidentCredentialConfirmationSheetView -----------------------

AuthenticatorResidentCredentialConfirmationSheetView::
AuthenticatorResidentCredentialConfirmationSheetView(
AuthenticatorRequestDialogModel* dialog_model)
: AuthenticatorSheetModelBase(dialog_model) {}

AuthenticatorResidentCredentialConfirmationSheetView::
~AuthenticatorResidentCredentialConfirmationSheetView() = default;

const gfx::VectorIcon&
AuthenticatorResidentCredentialConfirmationSheetView::GetStepIllustration(
ImageColorScheme color_scheme) const {
return color_scheme == ImageColorScheme::kDark ? kWebauthnPermissionDarkIcon
: kWebauthnPermissionIcon;
}

bool AuthenticatorResidentCredentialConfirmationSheetView::IsBackButtonVisible()
const {
return false;
}

bool AuthenticatorResidentCredentialConfirmationSheetView::
IsAcceptButtonVisible() const {
return true;
}

bool AuthenticatorResidentCredentialConfirmationSheetView::
IsAcceptButtonEnabled() const {
return true;
}

base::string16
AuthenticatorResidentCredentialConfirmationSheetView::GetAcceptButtonLabel()
const {
return l10n_util::GetStringUTF16(IDS_WEBAUTHN_WELCOME_SCREEN_NEXT);
}

base::string16
AuthenticatorResidentCredentialConfirmationSheetView::GetStepTitle() const {
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_GENERIC_TITLE,
GetRelyingPartyIdString(dialog_model()));
}

base::string16
AuthenticatorResidentCredentialConfirmationSheetView::GetStepDescription()
const {
return l10n_util::GetStringUTF16(IDS_WEBAUTHN_RESIDENT_KEY_PRIVACY);
}

void AuthenticatorResidentCredentialConfirmationSheetView::OnAccept() {
dialog_model()->OnResidentCredentialConfirmed();
}

// AuthenticatorSelectAccountSheetModel ---------------------------------------

AuthenticatorSelectAccountSheetModel::AuthenticatorSelectAccountSheetModel(
Expand Down
20 changes: 20 additions & 0 deletions chrome/browser/ui/webauthn/sheet_models.h
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,26 @@ class AuthenticatorGenericErrorSheetModel : public AuthenticatorSheetModelBase {
base::string16 description_;
};

class AuthenticatorResidentCredentialConfirmationSheetView
: public AuthenticatorSheetModelBase {
public:
AuthenticatorResidentCredentialConfirmationSheetView(
AuthenticatorRequestDialogModel* dialog_model);
~AuthenticatorResidentCredentialConfirmationSheetView() override;

private:
// AuthenticatorSheetModelBase:
const gfx::VectorIcon& GetStepIllustration(
ImageColorScheme color_scheme) const override;
bool IsBackButtonVisible() const override;
bool IsAcceptButtonVisible() const override;
bool IsAcceptButtonEnabled() const override;
base::string16 GetAcceptButtonLabel() const override;
base::string16 GetStepTitle() const override;
base::string16 GetStepDescription() const override;
void OnAccept() override;
};

// The sheet shown when the user needs to select an account.
class AuthenticatorSelectAccountSheetModel
: public AuthenticatorSheetModelBase {
Expand Down
13 changes: 12 additions & 1 deletion chrome/browser/webauthn/authenticator_request_dialog_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,13 @@ void AuthenticatorRequestDialogModel::
// Windows UI.
if (transport_availability_.has_win_native_api_authenticator &&
transport_availability_.available_transports.empty()) {
HideDialogAndDispatchToNativeWindowsApi();
if (might_create_resident_credential_ &&
!transport_availability_
.win_native_ui_shows_resident_credential_notice) {
SetCurrentStep(Step::kResidentCredentialConfirmation);
} else {
HideDialogAndDispatchToNativeWindowsApi();
}
return;
}

Expand Down Expand Up @@ -476,6 +482,11 @@ void AuthenticatorRequestDialogModel::OnHavePIN(const std::string& pin) {
has_attempted_pin_entry_ = true;
}

void AuthenticatorRequestDialogModel::OnResidentCredentialConfirmed() {
DCHECK_EQ(current_step(), Step::kResidentCredentialConfirmation);
HideDialogAndDispatchToNativeWindowsApi();
}

void AuthenticatorRequestDialogModel::OnAttestationPermissionResponse(
bool attestation_permission_granted) {
if (!attestation_callback_) {
Expand Down
9 changes: 9 additions & 0 deletions chrome/browser/webauthn/authenticator_request_dialog_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ class AuthenticatorRequestDialogModel {
kClientPinErrorHardBlock,
kClientPinErrorAuthenticatorRemoved,

// Confirm user consent to create a resident credential. Used prior to
// triggering Windows-native APIs when Windows itself won't show any
// notice about resident credentials.
kResidentCredentialConfirmation,

// Account selection,
kSelectAccount,

Expand Down Expand Up @@ -334,6 +339,10 @@ class AuthenticatorRequestDialogModel {
// OnHavePIN is called when the user enters a PIN in the UI.
void OnHavePIN(const std::string& pin);

// OnResidentCredentialConfirmed is called when a user accepts a dialog
// confirming that they're happy to create a resident credential.
void OnResidentCredentialConfirmed();

// OnAttestationPermissionResponse is called when the user either allows or
// disallows an attestation permission request.
void OnAttestationPermissionResponse(bool attestation_permission_granted);
Expand Down
6 changes: 6 additions & 0 deletions device/fido/fido_request_handler_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include "device/fido/fido_discovery_factory.h"
#include "services/service_manager/public/cpp/connector.h"

#if defined(OS_WIN)
#include "device/fido/win/authenticator.h"
#endif

namespace device {

// PlatformAuthenticatorInfo --------------------------
Expand Down Expand Up @@ -154,6 +158,8 @@ void FidoRequestHandlerBase::InitDiscoveriesWin(
// responsible for dispatch of the authenticator and whether they
// display any UI in addition to the one provided by the OS.
transport_availability_info_.has_win_native_api_authenticator = true;
transport_availability_info_.win_native_ui_shows_resident_credential_notice =
WinWebAuthnApiAuthenticator::ShowsResidentCredentialPrivacyNotice();

// Allow caBLE as a potential additional transport if requested by
// the implementing class because it is not subject to the OS'
Expand Down
4 changes: 4 additions & 0 deletions device/fido/fido_request_handler_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
// - should dispatch immediately if no other transport is available
bool has_win_native_api_authenticator = false;

// Indicates whether the Windows native UI will include a privacy notice
// when creating a resident credential.
bool win_native_ui_shows_resident_credential_notice = false;

// Contains the authenticator ID of the native Windows
// authenticator if |has_win_native_api_authenticator| is true.
// This allows the observer to distinguish it from other
Expand Down
7 changes: 7 additions & 0 deletions device/fido/win/authenticator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ bool WinWebAuthnApiAuthenticator::
result == TRUE;
}

// static
bool WinWebAuthnApiAuthenticator::ShowsResidentCredentialPrivacyNotice() {
// TODO: Once Windows shows a resident credential privacy notice in their UI,
// this should check for the respective API version.
return false;
}

WinWebAuthnApiAuthenticator::WinWebAuthnApiAuthenticator(HWND current_window)
: FidoAuthenticator(),
current_window_(current_window),
Expand Down
4 changes: 4 additions & 0 deletions device/fido/win/authenticator.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) WinWebAuthnApiAuthenticator
// WinWebAuthnApi::IsAvailable().
static bool IsUserVerifyingPlatformAuthenticatorAvailable();

// ShowsResidentCredentialPrivacyNotice returns true if the Windows native UI
// will show a privacy notice when creating a resident credential.
static bool ShowsResidentCredentialPrivacyNotice();

// Instantiates an authenticator that uses the default WinWebAuthnApi.
//
// Callers must ensure that WinWebAuthnApi::IsAvailable() returns true
Expand Down

0 comments on commit e95b9a3

Please sign in to comment.