From 933f62123863c6c8da3ef4d59656c4d030bdb321 Mon Sep 17 00:00:00 2001 From: "John M. Schanck" Date: Wed, 1 Oct 2025 00:23:34 +0000 Subject: [PATCH] Bug 1981587 - avoid showing multiple attestation consent prompts. a=RyanVM Original Revision: https://phabricator.services.mozilla.com/D265363 Differential Revision: https://phabricator.services.mozilla.com/D266839 --- dom/webauthn/WebAuthnResult.cpp | 7 +++++++ dom/webauthn/WebAuthnResult.h | 6 +++++- dom/webauthn/WebAuthnService.cpp | 17 +++++++++++++---- dom/webauthn/authrs_bridge/src/lib.rs | 5 +++++ dom/webauthn/nsIWebAuthnResult.idl | 2 ++ 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/dom/webauthn/WebAuthnResult.cpp b/dom/webauthn/WebAuthnResult.cpp index a1964a3f44a8..a2bfef930637 100644 --- a/dom/webauthn/WebAuthnResult.cpp +++ b/dom/webauthn/WebAuthnResult.cpp @@ -54,6 +54,13 @@ WebAuthnRegisterResult::GetAttestationObject( return NS_OK; } +NS_IMETHODIMP +WebAuthnRegisterResult::GetAttestationConsentPromptShown( + bool* aAttestationConsentPromptShown) { + *aAttestationConsentPromptShown = mAttestationConsentPromptShown; + return NS_OK; +} + NS_IMETHODIMP WebAuthnRegisterResult::GetCredentialId(nsTArray& aCredentialId) { aCredentialId.Assign(mCredentialId); diff --git a/dom/webauthn/WebAuthnResult.h b/dom/webauthn/WebAuthnResult.h index 6b9a8a08af7b..9836fdf8bf03 100644 --- a/dom/webauthn/WebAuthnResult.h +++ b/dom/webauthn/WebAuthnResult.h @@ -38,7 +38,8 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { const Maybe& aPrfSupported, const Maybe>& aPrfFirst, const Maybe>& aPrfSecond) - : mClientDataJSON(aClientDataJSON), + : mAttestationConsentPromptShown(false), + mClientDataJSON(aClientDataJSON), mCredPropsRk(Nothing()), mAuthenticatorAttachment(aAuthenticatorAttachment), mLargeBlobSupported(aLargeBlobSupported), @@ -63,6 +64,7 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { reinterpret_cast( aResponse->AttestationObject()->GetElements().Elements()), aResponse->AttestationObject()->Length()); + mAttestationConsentPromptShown = false; if (aResponse->ClientDataJson()) { mClientDataJSON = Some(nsAutoCString( reinterpret_cast( @@ -92,6 +94,7 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { mAttestationObject.AppendElements(aResponse->pbAttestationObject, aResponse->cbAttestationObject); + mAttestationConsentPromptShown = true; nsTArray extensions; if (aResponse->dwVersion >= WEBAUTHN_CREDENTIAL_ATTESTATION_VERSION_2) { @@ -166,6 +169,7 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { ~WebAuthnRegisterResult() = default; nsTArray mAttestationObject; + bool mAttestationConsentPromptShown; nsTArray mCredentialId; nsTArray mTransports; Maybe mClientDataJSON; diff --git a/dom/webauthn/WebAuthnService.cpp b/dom/webauthn/WebAuthnService.cpp index 8405e527630b..708c4f4bd743 100644 --- a/dom/webauthn/WebAuthnService.cpp +++ b/dom/webauthn/WebAuthnService.cpp @@ -115,10 +115,18 @@ WebAuthnService::MakeCredential(uint64_t aTransactionId, } nsIWebAuthnRegisterResult* result = aValue.ResolveValue(); - // If the RP requested attestation, we need to show a consent prompt - // before returning any identifying information. The platform may - // have already done this for us, so we need to inspect the - // attestation object at this point. + // We can return whatever result we have if the authenticator + // handled attestation consent for us. + bool attestationConsentPromptShown = false; + Unused << result->GetAttestationConsentPromptShown( + &attestationConsentPromptShown); + if (attestationConsentPromptShown) { + guard->ref().parentRegisterPromise.ref()->Resolve(result); + guard->reset(); + return; + } + // If the RP requested attestation and the response contains + // identifying information, then we need to show a consent prompt. bool resultIsIdentifying = true; Unused << result->HasIdentifyingAttestation(&resultIsIdentifying); if (attestationRequested && resultIsIdentifying) { @@ -127,6 +135,7 @@ WebAuthnService::MakeCredential(uint64_t aTransactionId, aBrowsingContextId); return; } + // In all other cases we strip out identifying information. result->Anonymize(); guard->ref().parentRegisterPromise.ref()->Resolve(result); guard->reset(); diff --git a/dom/webauthn/authrs_bridge/src/lib.rs b/dom/webauthn/authrs_bridge/src/lib.rs index 32349f4afe28..e76a4c44a2bf 100644 --- a/dom/webauthn/authrs_bridge/src/lib.rs +++ b/dom/webauthn/authrs_bridge/src/lib.rs @@ -185,6 +185,11 @@ impl WebAuthnRegisterResult { Ok(out) } + xpcom_method!(get_attestation_consent_prompt_shown => GetAttestationConsentPromptShown() -> bool); + fn get_attestation_consent_prompt_shown(&self) -> Result { + Ok(false) + } + xpcom_method!(get_credential_id => GetCredentialId() -> ThinVec); fn get_credential_id(&self) -> Result, nsresult> { let Some(credential_data) = &self.result.borrow().att_obj.auth_data.credential_data else { diff --git a/dom/webauthn/nsIWebAuthnResult.idl b/dom/webauthn/nsIWebAuthnResult.idl index 087f6b5bcf6d..84e7e97e52e6 100644 --- a/dom/webauthn/nsIWebAuthnResult.idl +++ b/dom/webauthn/nsIWebAuthnResult.idl @@ -15,6 +15,8 @@ interface nsIWebAuthnRegisterResult : nsISupports { // the authenticator data. readonly attribute Array attestationObject; + readonly attribute boolean attestationConsentPromptShown; + // The Credential ID field of the Attestation Object's Attested // Credential Data. This is used to construct the rawID field of a // WebAuthn PublicKeyCredential without having to parse the