[WebAuthn] Allow retry on failed enclave PIN attempts for UV
With this change, entering an incorrect GPM PIN for a UV passkey request
will cause the PIN entry dialog to be displayed once again, in order for
the user to retry. The number of consecutive failed PIN attempts is
tracked in a profile pref.
The UI will later be updated to reflect that a PIN re-entry is required.
Also in a coming CL, exceeding the PIN failure maximum will trigger UI
prompting the user to change their PIN.
Bug: 40274370
Change-Id: I9316a17e1ac577ee8bdc0c6cd9e7193da9c358ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466051
Commit-Queue: Ken Buchanan <[email protected]>
Reviewed-by: Dominic Battre <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1289882}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index f324c89..5b82694 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -2174,6 +2174,9 @@
registry->RegisterStringPref(
webauthn::pref_names::kLastUsedPairingFromSyncPublicKey, "");
+ registry->RegisterIntegerPref(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount, 0);
+
side_panel_prefs::RegisterProfilePrefs(registry);
#endif
diff --git a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
index a3b7364d..ef75b8e7 100644
--- a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
+++ b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
@@ -33,10 +33,12 @@
#include "chrome/browser/webauthn/gpm_enclave_controller.h"
#include "chrome/browser/webauthn/passkey_model_factory.h"
#include "chrome/browser/webauthn/test_util.h"
+#include "chrome/browser/webauthn/webauthn_pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "components/os_crypt/sync/os_crypt_mocker.h"
+#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "components/sync/service/sync_service.h"
@@ -142,6 +144,16 @@
e => window.domAutomationController.send('error ' + e));
})())";
+static constexpr char kGetAssertionUvRequired[] = R"((() => {
+ return navigator.credentials.get({ publicKey: {
+ challenge: new Uint8Array([0]),
+ timeout: 10000,
+ userVerification: 'required',
+ allowCredentials: [],
+ }}).then(c => window.domAutomationController.send('webauthn: OK'),
+ e => window.domAutomationController.send('error ' + e));
+})())";
+
struct TempDir {
public:
TempDir() { CHECK(dir_.CreateUniqueTempDir()); }
@@ -215,7 +227,10 @@
}
virtual ~DelegateObserver() = default;
- void WaitForUI() { run_loop_->Run(); }
+ void WaitForUI() {
+ run_loop_->Run();
+ run_loop_ = std::make_unique<base::RunLoop>();
+ }
void SetPendingTrustedVaultConnection(
std::unique_ptr<trusted_vault::TrustedVaultConnection> connection) {
@@ -802,4 +817,87 @@
EXPECT_EQ(script_result, "\"webauthn: OK\"");
}
+IN_PROC_BROWSER_TEST_F(EnclaveAuthenticatorBrowserTest,
+ RegisterDeviceWithGpmPin_UVRequestsWithWrongPIN) {
+ /* Test script:
+ * - Prerequisites:
+ * Enclave not registered
+ * No existing security domain secrets
+ * Existing user account with password sync enabled
+ * Platform UV unavailable
+ * 1. Modal MakeCredential request invoked by RP, requires UV.
+ * 2. Mechanism selection appears; test chooses enclave.
+ * 3. UI for onboarding appears; test accepts it
+ * 4. Test selects a GPM PIN
+ * 5. Device registration with enclave succeeds
+ * 6. MakeCredential succeeds
+ * 7. Modal GetAssertion request invoked by RP, requires UV.
+ * 9. Test enters wrong PIN
+ * 10. PIN entry dialog appears again, test enters correct PIN.
+ *
+ * Notably, user verification is asserted without a second GPM PIN prompt.
+ */
+ trusted_vault::DownloadAuthenticationFactorsRegistrationStateResult
+ registration_state_result;
+ registration_state_result.state = trusted_vault::
+ DownloadAuthenticationFactorsRegistrationStateResult::State::kEmpty;
+ SetMockVaultConnectionOnRequestDelegate(std::move(registration_state_result));
+
+ content::WebContents* web_contents =
+ browser()->tab_strip_model()->GetActiveWebContents();
+ content::DOMMessageQueue message_queue(web_contents);
+ content::ExecuteScriptAsync(web_contents, kMakeCredentialUvRequired);
+ delegate_observer()->WaitForUI();
+
+ EXPECT_EQ(dialog_model()->step(),
+ AuthenticatorRequestDialogModel::Step::kMechanismSelection);
+ EXPECT_EQ(request_delegate()
+ ->enclave_controller_for_testing()
+ ->account_state_for_testing(),
+ GPMEnclaveController::AccountState::kEmpty);
+ model_observer()->SetStepToObserve(
+ AuthenticatorRequestDialogController::Step::kGPMOnboarding);
+ SimulateEnclaveMechanismSelection();
+ model_observer()->WaitForStep();
+
+ dialog_model()->OnGPMOnboardingAccepted();
+ EXPECT_EQ(dialog_model()->step(),
+ AuthenticatorRequestDialogModel::Step::kGPMCreatePin);
+ dialog_model()->OnGPMPinEntered(u"123456");
+
+ std::string script_result;
+ ASSERT_TRUE(message_queue.WaitForMessage(&script_result));
+ EXPECT_EQ(script_result, "\"webauthn: uv=true\"");
+
+ content::ExecuteScriptAsync(web_contents, kGetAssertionUvRequired);
+ delegate_observer()->WaitForUI();
+
+ EXPECT_EQ(dialog_model()->step(),
+ AuthenticatorRequestDialogModel::Step::kSelectPriorityMechanism);
+ model_observer()->SetStepToObserve(
+ AuthenticatorRequestDialogController::Step::kGPMEnterPin);
+ dialog_model()->OnUserConfirmedPriorityMechanism();
+ model_observer()->WaitForStep();
+
+ EXPECT_EQ(browser()->profile()->GetPrefs()->GetInteger(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount),
+ 0);
+ model_observer()->SetStepToObserve(
+ AuthenticatorRequestDialogController::Step::kGPMEnterPin);
+ dialog_model()->OnGPMPinEntered(u"111111");
+ model_observer()->WaitForStep();
+
+ EXPECT_EQ(browser()->profile()->GetPrefs()->GetInteger(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount),
+ 1);
+ dialog_model()->OnGPMPinEntered(u"123456");
+
+ ASSERT_TRUE(message_queue.WaitForMessage(&script_result));
+ EXPECT_EQ(script_result, "\"webauthn: OK\"");
+
+ EXPECT_EQ(browser()->profile()->GetPrefs()->GetInteger(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount),
+ 0);
+}
+
#endif // !defined(MEMORY_SANITIZER)
diff --git a/chrome/browser/webauthn/gpm_enclave_controller.cc b/chrome/browser/webauthn/gpm_enclave_controller.cc
index 6e8fec7..9629b09 100644
--- a/chrome/browser/webauthn/gpm_enclave_controller.cc
+++ b/chrome/browser/webauthn/gpm_enclave_controller.cc
@@ -13,7 +13,9 @@
#include "chrome/browser/webauthn/enclave_manager_factory.h"
#include "chrome/browser/webauthn/passkey_model_factory.h"
#include "chrome/browser/webauthn/proto/enclave_local_state.pb.h"
+#include "chrome/browser/webauthn/webauthn_pref_names.h"
#include "components/device_event_log/device_event_log.h"
+#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
#include "components/trusted_vault/frontend_trusted_vault_connection.h"
@@ -426,6 +428,7 @@
have_added_device_ = true;
SetAccountStateReady();
+ SetFailedPINAttemptCount(0);
switch (account_state_) {
case AccountState::kReady:
@@ -570,6 +573,8 @@
}
void GPMEnclaveController::PromptForPin() {
+ // TODO(enclave): Check for failed PIN attempts having been exhausted, and
+ // trigger a PIN reset flow in that case.
model_->SetStep(pin_is_arbitrary_ ? Step::kGPMEnterArbitraryPin
: Step::kGPMEnterPin);
}
@@ -709,6 +714,9 @@
enclave_manager_->HardwareKeySigningCallback();
CHECK(claimed_pin);
request->claimed_pin = std::move(claimed_pin);
+ request->pin_result_callback =
+ base::BindOnce(&GPMEnclaveController::HandlePINValidationResult,
+ weak_ptr_factory_.GetWeakPtr());
break;
case EnclaveUserVerificationMethod::kUVKeyWithChromeUI:
@@ -786,3 +794,34 @@
PasskeyModelFactory::GetInstance()->GetForProfile(GetProfile());
passkey_model->CreatePasskey(passkey);
}
+
+bool GPMEnclaveController::GetFailedPINAttemptCount() {
+ return GetProfile()->GetPrefs()->GetInteger(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount);
+}
+
+void GPMEnclaveController::SetFailedPINAttemptCount(int count) {
+ GetProfile()->GetPrefs()->SetInteger(
+ webauthn::pref_names::kEnclaveFailedPINAttemptsCount, count);
+}
+
+void GPMEnclaveController::HandlePINValidationResult(
+ device::enclave::PINValidationResult result) {
+ switch (result) {
+ case device::enclave::PINValidationResult::kSuccess:
+ SetFailedPINAttemptCount(0);
+ break;
+ case device::enclave::PINValidationResult::kIncorrect: {
+ int count = GetFailedPINAttemptCount();
+ // TODO(enclave): If the new count is 5, go to the locked PIN UI.
+ SetFailedPINAttemptCount(++count);
+ // TODO(enclave): This needs better UI, but for now just pops up PIN
+ // entry again.
+ model_->SetStep(Step::kGPMEnterPin);
+ break;
+ }
+ case device::enclave::PINValidationResult::kLocked:
+ // TODO(enclave): Handle locked PIN.
+ break;
+ }
+}
diff --git a/chrome/browser/webauthn/gpm_enclave_controller.h b/chrome/browser/webauthn/gpm_enclave_controller.h
index c45627a..03b1b867 100644
--- a/chrome/browser/webauthn/gpm_enclave_controller.h
+++ b/chrome/browser/webauthn/gpm_enclave_controller.h
@@ -154,6 +154,15 @@
// Invoked when a new GPM passkey is created, to save it to sync data.
void OnPasskeyCreated(sync_pb::WebauthnCredentialSpecifics passkey);
+ // Accessors for the profile pref that counts the number of consecutive failed
+ // PIN attempts to know when a lockout will happen.
+ bool GetFailedPINAttemptCount();
+ void SetFailedPINAttemptCount(int count);
+
+ // Invoked when a passkey request has been sent to the enclave service with
+ // PIN UV, and the request succeeded or a PIN validation error occurred.
+ void HandlePINValidationResult(device::enclave::PINValidationResult type);
+
const content::GlobalRenderFrameHostId render_frame_host_id_;
const std::string rp_id_;
const device::FidoRequestType request_type_;
diff --git a/chrome/browser/webauthn/webauthn_pref_names.cc b/chrome/browser/webauthn/webauthn_pref_names.cc
index dbc604cf..a9fb03c 100644
--- a/chrome/browser/webauthn/webauthn_pref_names.cc
+++ b/chrome/browser/webauthn/webauthn_pref_names.cc
@@ -6,12 +6,15 @@
namespace webauthn::pref_names {
-const char kRemoteProxiedRequestsAllowed[] =
- "webauthn.remote_proxied_requests_allowed";
-
const char kAllowWithBrokenCerts[] = "webauthn.allow_with_broken_certs";
+const char kEnclaveFailedPINAttemptsCount[] =
+ "webauthn.enclave_failed_pin_attempts_count";
+
extern const char kLastUsedPairingFromSyncPublicKey[] =
"webauthn.last_used_pairing_from_sync_public_key";
+const char kRemoteProxiedRequestsAllowed[] =
+ "webauthn.remote_proxied_requests_allowed";
+
} // namespace webauthn::pref_names
diff --git a/chrome/browser/webauthn/webauthn_pref_names.h b/chrome/browser/webauthn/webauthn_pref_names.h
index 11d3acee..7a144e7 100644
--- a/chrome/browser/webauthn/webauthn_pref_names.h
+++ b/chrome/browser/webauthn/webauthn_pref_names.h
@@ -7,18 +7,22 @@
namespace webauthn::pref_names {
-// Maps to the WebAuthenticationRemoteProxiedRequestsAllowed enterprise
-// policy.
-extern const char kRemoteProxiedRequestsAllowed[];
-
// Maps to the AllowWebAuthnWithBrokenCerts enterprise policy.
extern const char kAllowWithBrokenCerts[];
+// Tracks how many consecutive failed GPM PIN attempts have been made to the
+// enclave service from this device and profile.
+extern const char kEnclaveFailedPINAttemptsCount[];
+
// The most recently used phone pairing from sync, identified by its public key
// encoded in base64. If there is no last recently used phone, the preference
// will be an empty string.
extern const char kLastUsedPairingFromSyncPublicKey[];
+// Maps to the WebAuthenticationRemoteProxiedRequestsAllowed enterprise
+// policy.
+extern const char kRemoteProxiedRequestsAllowed[];
+
} // namespace webauthn::pref_names
#endif // CHROME_BROWSER_WEBAUTHN_WEBAUTHN_PREF_NAMES_H_
