From d8bbc7858622b6d9c278469aab701ca0b609cddf Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 15 May 2024 05:35:49 +0200 Subject: Merging upstream version 126.0. Signed-off-by: Daniel Baumann --- dom/webauthn/AndroidWebAuthnService.cpp | 10 +- dom/webauthn/MacOSWebAuthnService.mm | 73 +++++++-- dom/webauthn/WebAuthnArgs.cpp | 13 +- dom/webauthn/WebAuthnResult.cpp | 22 ++- dom/webauthn/WebAuthnResult.h | 10 +- dom/webauthn/WebAuthnService.cpp | 179 ++++++++++++++++++--- dom/webauthn/WebAuthnService.h | 22 ++- dom/webauthn/WinWebAuthnService.cpp | 13 +- .../authrs_bridge/src/about_webauthn_controller.rs | 4 +- dom/webauthn/authrs_bridge/src/lib.rs | 125 +++++++------- dom/webauthn/nsIWebAuthnArgs.idl | 10 +- dom/webauthn/nsIWebAuthnAttObj.idl | 2 + dom/webauthn/nsIWebAuthnResult.idl | 9 +- dom/webauthn/nsIWebAuthnService.idl | 20 +-- .../browser_webauthn_conditional_mediation.js | 6 +- .../tests/browser/browser_webauthn_prompts.js | 104 +++--------- 16 files changed, 385 insertions(+), 237 deletions(-) (limited to 'dom/webauthn') diff --git a/dom/webauthn/AndroidWebAuthnService.cpp b/dom/webauthn/AndroidWebAuthnService.cpp index 162cc52033..a4fa230455 100644 --- a/dom/webauthn/AndroidWebAuthnService.cpp +++ b/dom/webauthn/AndroidWebAuthnService.cpp @@ -186,12 +186,6 @@ AndroidWebAuthnService::MakeCredential(uint64_t aTransactionId, GetCurrentSerialEventTarget(), __func__, [aPromise, credPropsResponse = std::move(credPropsResponse)]( RefPtr&& aValue) { - // We don't have a way for the user to consent to attestation - // on Android, so always anonymize the result. - nsresult rv = aValue->Anonymize(); - if (NS_FAILED(rv)) { - aPromise->Reject(NS_ERROR_DOM_NOT_ALLOWED_ERR); - } if (credPropsResponse.isSome()) { Unused << aValue->SetCredPropsRk(credPropsResponse.ref()); } @@ -357,8 +351,8 @@ AndroidWebAuthnService::PinCallback(uint64_t aTransactionId, } NS_IMETHODIMP -AndroidWebAuthnService::ResumeMakeCredential(uint64_t aTransactionId, - bool aForceNoneAttestation) { +AndroidWebAuthnService::SetHasAttestationConsent(uint64_t aTransactionId, + bool aHasConsent) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/dom/webauthn/MacOSWebAuthnService.mm b/dom/webauthn/MacOSWebAuthnService.mm index cec9600e57..fc08ee1a48 100644 --- a/dom/webauthn/MacOSWebAuthnService.mm +++ b/dom/webauthn/MacOSWebAuthnService.mm @@ -338,6 +338,12 @@ nsTArray NSDataToArray(NSData* data) { } #endif } else { + // The platform didn't tell us what transport was used, but we know it + // wasn't the internal transport. The transport response is not signed by + // the authenticator. It represents the "transports that the authenticator + // is believed to support, or an empty sequence if the information is + // unavailable". We believe macOS supports usb, so we return usb. + transports.AppendElement(u"usb"_ns); authenticatorAttachment.emplace(u"cross-platform"_ns); } mCallback->FinishMakeCredential(rawAttestationObject, credentialId, @@ -605,6 +611,9 @@ MacOSWebAuthnService::MakeCredential(uint64_t aTransactionId, userVerificationPreference = Nothing(); nsAutoString userVerification; Unused << aArgs->GetUserVerification(userVerification); + // This mapping needs to be reviewed if values are added to the + // UserVerificationRequirement enum. + static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 3); if (userVerification.EqualsLiteral( MOZ_WEBAUTHN_USER_VERIFICATION_REQUIREMENT_REQUIRED)) { userVerificationPreference.emplace( @@ -620,12 +629,51 @@ MacOSWebAuthnService::MakeCredential(uint64_t aTransactionId, ASAuthorizationPublicKeyCredentialUserVerificationPreferenceDiscouraged); } - // The API doesn't support attestation for platform passkeys and shows - // no consent UI for non-none attestation for cross-platform devices, - // so this must always be none. - ASAuthorizationPublicKeyCredentialAttestationKind - attestationPreference = - ASAuthorizationPublicKeyCredentialAttestationKindNone; + // The API doesn't support attestation for platform passkeys, so this is + // only used for security keys. + ASAuthorizationPublicKeyCredentialAttestationKind attestationPreference; + nsAutoString mozAttestationPreference; + Unused << aArgs->GetAttestationConveyancePreference( + mozAttestationPreference); + if (mozAttestationPreference.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_INDIRECT)) { + attestationPreference = + ASAuthorizationPublicKeyCredentialAttestationKindIndirect; + } else if (mozAttestationPreference.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_DIRECT)) { + attestationPreference = + ASAuthorizationPublicKeyCredentialAttestationKindDirect; + } else if ( + mozAttestationPreference.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_ENTERPRISE)) { + attestationPreference = + ASAuthorizationPublicKeyCredentialAttestationKindEnterprise; + } else { + attestationPreference = + ASAuthorizationPublicKeyCredentialAttestationKindNone; + } + + ASAuthorizationPublicKeyCredentialResidentKeyPreference + residentKeyPreference; + nsAutoString mozResidentKey; + Unused << aArgs->GetResidentKey(mozResidentKey); + // This mapping needs to be reviewed if values are added to the + // ResidentKeyRequirement enum. + static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 3); + if (mozResidentKey.EqualsLiteral( + MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_REQUIRED)) { + residentKeyPreference = + ASAuthorizationPublicKeyCredentialResidentKeyPreferenceRequired; + } else if (mozResidentKey.EqualsLiteral( + MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_PREFERRED)) { + residentKeyPreference = + ASAuthorizationPublicKeyCredentialResidentKeyPreferencePreferred; + } else { + MOZ_ASSERT(mozResidentKey.EqualsLiteral( + MOZ_WEBAUTHN_RESIDENT_KEY_REQUIREMENT_DISCOURAGED)); + residentKeyPreference = + ASAuthorizationPublicKeyCredentialResidentKeyPreferenceDiscouraged; + } // Initialize the platform provider with the rpId. ASAuthorizationPlatformPublicKeyCredentialProvider* platformProvider = @@ -639,8 +687,10 @@ MacOSWebAuthnService::MakeCredential(uint64_t aTransactionId, name:userNameNS userID:userIdNS]; [platformProvider release]; + + // The API doesn't support attestation for platform passkeys platformRegistrationRequest.attestationPreference = - attestationPreference; + ASAuthorizationPublicKeyCredentialAttestationKindNone; if (userVerificationPreference.isSome()) { platformRegistrationRequest.userVerificationPreference = *userVerificationPreference; @@ -665,6 +715,8 @@ MacOSWebAuthnService::MakeCredential(uint64_t aTransactionId, attestationPreference; crossPlatformRegistrationRequest.credentialParameters = credentialParameters; + crossPlatformRegistrationRequest.residentKeyPreference = + residentKeyPreference; if (userVerificationPreference.isSome()) { crossPlatformRegistrationRequest.userVerificationPreference = *userVerificationPreference; @@ -914,6 +966,9 @@ void MacOSWebAuthnService::DoGetAssertion( userVerificationPreference = Nothing(); nsAutoString userVerification; Unused << aArgs->GetUserVerification(userVerification); + // This mapping needs to be reviewed if values are added to the + // UserVerificationRequirement enum. + static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 3); if (userVerification.EqualsLiteral( MOZ_WEBAUTHN_USER_VERIFICATION_REQUIREMENT_REQUIRED)) { userVerificationPreference.emplace( @@ -1115,8 +1170,8 @@ MacOSWebAuthnService::PinCallback(uint64_t aTransactionId, } NS_IMETHODIMP -MacOSWebAuthnService::ResumeMakeCredential(uint64_t aTransactionId, - bool aForceNoneAttestation) { +MacOSWebAuthnService::SetHasAttestationConsent(uint64_t aTransactionId, + bool aHasConsent) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/dom/webauthn/WebAuthnArgs.cpp b/dom/webauthn/WebAuthnArgs.cpp index 7c78d39bef..c094a71963 100644 --- a/dom/webauthn/WebAuthnArgs.cpp +++ b/dom/webauthn/WebAuthnArgs.cpp @@ -155,7 +155,18 @@ WebAuthnRegisterArgs::GetTimeoutMS(uint32_t* aTimeoutMS) { NS_IMETHODIMP WebAuthnRegisterArgs::GetAttestationConveyancePreference( nsAString& aAttestationConveyancePreference) { - aAttestationConveyancePreference = mInfo.attestationConveyancePreference(); + const nsString& attPref = mInfo.attestationConveyancePreference(); + if (attPref.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_INDIRECT) || + attPref.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_DIRECT) || + attPref.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_ENTERPRISE)) { + aAttestationConveyancePreference.Assign(attPref); + } else { + aAttestationConveyancePreference.AssignLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE); + } return NS_OK; } diff --git a/dom/webauthn/WebAuthnResult.cpp b/dom/webauthn/WebAuthnResult.cpp index 268dd62f20..6fd446ffa0 100644 --- a/dom/webauthn/WebAuthnResult.cpp +++ b/dom/webauthn/WebAuthnResult.cpp @@ -102,7 +102,27 @@ WebAuthnRegisterResult::GetAuthenticatorAttachment( return NS_ERROR_NOT_AVAILABLE; } -nsresult WebAuthnRegisterResult::Anonymize() { +NS_IMETHODIMP +WebAuthnRegisterResult::HasIdentifyingAttestation( + bool* aHasIdentifyingAttestation) { + // Assume the attestation statement is identifying in case the constructor or + // the getter below fail. + bool isIdentifying = true; + + nsCOMPtr attObj; + nsresult rv = authrs_webauthn_att_obj_constructor(mAttestationObject, + /* anonymize */ false, + getter_AddRefs(attObj)); + if (NS_SUCCEEDED(rv)) { + Unused << attObj->IsIdentifying(&isIdentifying); + } + + *aHasIdentifyingAttestation = isIdentifying; + return NS_OK; +} + +NS_IMETHODIMP +WebAuthnRegisterResult::Anonymize() { // The anonymize flag in the nsIWebAuthnAttObj constructor causes the // attestation statement to be removed during deserialization. It also // causes the AAGUID to be zeroed out. If we can't deserialize the diff --git a/dom/webauthn/WebAuthnResult.h b/dom/webauthn/WebAuthnResult.h index f7653fd4b0..13cea0e187 100644 --- a/dom/webauthn/WebAuthnResult.h +++ b/dom/webauthn/WebAuthnResult.h @@ -63,8 +63,8 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { mTransports.AppendElement( jni::String::LocalRef(transports->GetElement(i))->ToString()); } - // authenticator attachment is not available on Android - mAuthenticatorAttachment = Nothing(); + mAuthenticatorAttachment = + Some(aResponse->AuthenticatorAttachment()->ToString()); } #endif @@ -134,8 +134,6 @@ class WebAuthnRegisterResult final : public nsIWebAuthnRegisterResult { } #endif - nsresult Anonymize(); - private: ~WebAuthnRegisterResult() = default; @@ -191,8 +189,8 @@ class WebAuthnSignResult final : public nsIWebAuthnSignResult { reinterpret_cast( aResponse->UserHandle()->GetElements().Elements()), aResponse->UserHandle()->Length()); - // authenticator attachment is not available on Android - mAuthenticatorAttachment = Nothing(); + mAuthenticatorAttachment = + Some(aResponse->AuthenticatorAttachment()->ToString()); } #endif diff --git a/dom/webauthn/WebAuthnService.cpp b/dom/webauthn/WebAuthnService.cpp index 3e1557edbc..0c214ccd90 100644 --- a/dom/webauthn/WebAuthnService.cpp +++ b/dom/webauthn/WebAuthnService.cpp @@ -5,7 +5,9 @@ #include "mozilla/Services.h" #include "mozilla/StaticPrefs_security.h" #include "nsIObserverService.h" +#include "nsTextFormatter.h" #include "nsThreadUtils.h" +#include "WebAuthnEnumStrings.h" #include "WebAuthnService.h" #include "WebAuthnTransportIdentifiers.h" @@ -18,32 +20,139 @@ already_AddRefed NewWebAuthnService() { NS_IMPL_ISUPPORTS(WebAuthnService, nsIWebAuthnService) +void WebAuthnService::ShowAttestationConsentPrompt( + const nsString& aOrigin, uint64_t aTransactionId, + uint64_t aBrowsingContextId) { + RefPtr self = this; +#ifdef MOZ_WIDGET_ANDROID + // We don't have a way to prompt the user for consent on Android, so just + // assume consent not granted. + nsCOMPtr runnable( + NS_NewRunnableFunction(__func__, [self, aTransactionId]() { + self->SetHasAttestationConsent( + aTransactionId, + StaticPrefs:: + security_webauth_webauthn_testing_allow_direct_attestation()); + })); +#else + nsCOMPtr runnable(NS_NewRunnableFunction( + __func__, [self, aOrigin, aTransactionId, aBrowsingContextId]() { + if (StaticPrefs:: + security_webauth_webauthn_testing_allow_direct_attestation()) { + self->SetHasAttestationConsent(aTransactionId, true); + return; + } + nsCOMPtr os = services::GetObserverService(); + if (!os) { + return; + } + const nsLiteralString jsonFmt = + u"{\"prompt\": {\"type\":\"attestation-consent\"},"_ns + u"\"origin\": \"%S\","_ns + u"\"tid\": %llu, \"browsingContextId\": %llu}"_ns; + nsString json; + nsTextFormatter::ssprintf(json, jsonFmt.get(), aOrigin.get(), + aTransactionId, aBrowsingContextId); + MOZ_ALWAYS_SUCCEEDS( + os->NotifyObservers(nullptr, "webauthn-prompt", json.get())); + })); +#endif + NS_DispatchToMainThread(runnable.forget()); +} + NS_IMETHODIMP WebAuthnService::MakeCredential(uint64_t aTransactionId, - uint64_t browsingContextId, + uint64_t aBrowsingContextId, nsIWebAuthnRegisterArgs* aArgs, nsIWebAuthnRegisterPromise* aPromise) { + MOZ_ASSERT(aArgs); + MOZ_ASSERT(aPromise); + auto guard = mTransactionState.Lock(); - if (guard->isSome()) { - guard->ref().service->Reset(); - *guard = Nothing(); + ResetLocked(guard); + *guard = Some(TransactionState{.service = DefaultService(), + .transactionId = aTransactionId, + .parentRegisterPromise = Some(aPromise)}); + + // We may need to show an attestation consent prompt before we return a + // credential to WebAuthnTransactionParent, so we insert a new promise that + // chains to `aPromise` here. + + nsString attestation; + Unused << aArgs->GetAttestationConveyancePreference(attestation); + bool attestationRequested = !attestation.EqualsLiteral( + MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE); + + nsString origin; + Unused << aArgs->GetOrigin(origin); + + RefPtr promiseHolder = + new WebAuthnRegisterPromiseHolder(GetCurrentSerialEventTarget()); + + RefPtr self = this; + RefPtr promise = promiseHolder->Ensure(); + promise + ->Then( + GetCurrentSerialEventTarget(), __func__, + [self, origin, aTransactionId, aBrowsingContextId, + attestationRequested]( + const WebAuthnRegisterPromise::ResolveOrRejectValue& aValue) { + auto guard = self->mTransactionState.Lock(); + if (guard->isNothing()) { + return; + } + MOZ_ASSERT(guard->ref().parentRegisterPromise.isSome()); + MOZ_ASSERT(guard->ref().registerResult.isNothing()); + MOZ_ASSERT(guard->ref().childRegisterRequest.Exists()); + + guard->ref().childRegisterRequest.Complete(); + + if (aValue.IsReject()) { + guard->ref().parentRegisterPromise.ref()->Reject( + aValue.RejectValue()); + guard->reset(); + return; + } + + 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. + bool resultIsIdentifying = true; + Unused << result->HasIdentifyingAttestation(&resultIsIdentifying); + if (attestationRequested && resultIsIdentifying) { + guard->ref().registerResult = Some(result); + self->ShowAttestationConsentPrompt(origin, aTransactionId, + aBrowsingContextId); + return; + } + result->Anonymize(); + guard->ref().parentRegisterPromise.ref()->Resolve(result); + guard->reset(); + }) + ->Track(guard->ref().childRegisterRequest); + + nsresult rv = guard->ref().service->MakeCredential( + aTransactionId, aBrowsingContextId, aArgs, promiseHolder); + if (NS_FAILED(rv)) { + promiseHolder->Reject(NS_ERROR_DOM_NOT_ALLOWED_ERR); } - *guard = Some(TransactionState{DefaultService()}); - return guard->ref().service->MakeCredential(aTransactionId, browsingContextId, - aArgs, aPromise); + return NS_OK; } NS_IMETHODIMP WebAuthnService::GetAssertion(uint64_t aTransactionId, - uint64_t browsingContextId, + uint64_t aBrowsingContextId, nsIWebAuthnSignArgs* aArgs, nsIWebAuthnSignPromise* aPromise) { + MOZ_ASSERT(aArgs); + MOZ_ASSERT(aPromise); + auto guard = mTransactionState.Lock(); - if (guard->isSome()) { - guard->ref().service->Reset(); - *guard = Nothing(); - } - *guard = Some(TransactionState{DefaultService()}); + ResetLocked(guard); + *guard = Some(TransactionState{.service = DefaultService(), + .transactionId = aTransactionId}); nsresult rv; #if defined(XP_MACOSX) @@ -71,7 +180,7 @@ WebAuthnService::GetAssertion(uint64_t aTransactionId, } #endif - rv = guard->ref().service->GetAssertion(aTransactionId, browsingContextId, + rv = guard->ref().service->GetAssertion(aTransactionId, aBrowsingContextId, aArgs, aPromise); if (NS_FAILED(rv)) { return rv; @@ -125,13 +234,22 @@ WebAuthnService::ResumeConditionalGet(uint64_t aTransactionId) { return SelectedService()->ResumeConditionalGet(aTransactionId); } +void WebAuthnService::ResetLocked( + const TransactionStateMutex::AutoLock& aGuard) { + if (aGuard->isSome()) { + aGuard->ref().childRegisterRequest.DisconnectIfExists(); + if (aGuard->ref().parentRegisterPromise.isSome()) { + aGuard->ref().parentRegisterPromise.ref()->Reject(NS_ERROR_DOM_ABORT_ERR); + } + aGuard->ref().service->Reset(); + } + aGuard->reset(); +} + NS_IMETHODIMP WebAuthnService::Reset() { auto guard = mTransactionState.Lock(); - if (guard->isSome()) { - guard->ref().service->Reset(); - } - *guard = Nothing(); + ResetLocked(guard); return NS_OK; } @@ -146,10 +264,27 @@ WebAuthnService::PinCallback(uint64_t aTransactionId, const nsACString& aPin) { } NS_IMETHODIMP -WebAuthnService::ResumeMakeCredential(uint64_t aTransactionId, - bool aForceNoneAttestation) { - return SelectedService()->ResumeMakeCredential(aTransactionId, - aForceNoneAttestation); +WebAuthnService::SetHasAttestationConsent(uint64_t aTransactionId, + bool aHasConsent) { + auto guard = this->mTransactionState.Lock(); + if (guard->isNothing() || guard->ref().transactionId != aTransactionId) { + // This could happen if the transaction was reset just when the prompt was + // receiving user input. + return NS_OK; + } + + MOZ_ASSERT(guard->ref().parentRegisterPromise.isSome()); + MOZ_ASSERT(guard->ref().registerResult.isSome()); + MOZ_ASSERT(!guard->ref().childRegisterRequest.Exists()); + + if (!aHasConsent) { + guard->ref().registerResult.ref()->Anonymize(); + } + guard->ref().parentRegisterPromise.ref()->Resolve( + guard->ref().registerResult.ref()); + + guard->reset(); + return NS_OK; } NS_IMETHODIMP diff --git a/dom/webauthn/WebAuthnService.h b/dom/webauthn/WebAuthnService.h index 254b75d251..a3f4dab797 100644 --- a/dom/webauthn/WebAuthnService.h +++ b/dom/webauthn/WebAuthnService.h @@ -7,6 +7,7 @@ #include "nsIWebAuthnService.h" #include "AuthrsBridge_ffi.h" +#include "mozilla/dom/WebAuthnPromiseHolder.h" #ifdef MOZ_WIDGET_ANDROID # include "AndroidWebAuthnService.h" @@ -55,6 +56,21 @@ class WebAuthnService final : public nsIWebAuthnService { private: ~WebAuthnService() = default; + struct TransactionState { + nsCOMPtr service; + uint64_t transactionId; + Maybe> parentRegisterPromise; + Maybe> registerResult; + MozPromiseRequestHolder childRegisterRequest; + }; + using TransactionStateMutex = DataMutex>; + TransactionStateMutex mTransactionState; + + void ShowAttestationConsentPrompt(const nsString& aOrigin, + uint64_t aTransactionId, + uint64_t aBrowsingContextId); + void ResetLocked(const TransactionStateMutex::AutoLock& aGuard); + nsIWebAuthnService* DefaultService() { if (StaticPrefs::security_webauth_webauthn_enable_softtoken()) { return mAuthrsService; @@ -72,12 +88,6 @@ class WebAuthnService final : public nsIWebAuthnService { return DefaultService(); } - struct TransactionState { - nsCOMPtr service; - }; - using TransactionStateMutex = DataMutex>; - TransactionStateMutex mTransactionState; - nsCOMPtr mAuthrsService; nsCOMPtr mPlatformService; }; diff --git a/dom/webauthn/WinWebAuthnService.cpp b/dom/webauthn/WinWebAuthnService.cpp index 8667cf5615..a9f73a4cf7 100644 --- a/dom/webauthn/WinWebAuthnService.cpp +++ b/dom/webauthn/WinWebAuthnService.cpp @@ -411,14 +411,12 @@ WinWebAuthnService::MakeCredential(uint64_t aTransactionId, // AttestationConveyance nsString attestation; Unused << aArgs->GetAttestationConveyancePreference(attestation); - bool anonymize = false; // This mapping needs to be reviewed if values are added to the // AttestationConveyancePreference enum. static_assert(MOZ_WEBAUTHN_ENUM_STRINGS_VERSION == 3); if (attestation.EqualsLiteral( MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE)) { winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE; - anonymize = true; } else if ( attestation.EqualsLiteral( MOZ_WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_INDIRECT)) { @@ -579,13 +577,6 @@ WinWebAuthnService::MakeCredential(uint64_t aTransactionId, } gWinWebauthnFreeCredentialAttestation(pWebAuthNCredentialAttestation); - if (anonymize) { - nsresult rv = result->Anonymize(); - if (NS_FAILED(rv)) { - aPromise->Reject(NS_ERROR_DOM_NOT_ALLOWED_ERR); - return; - } - } aPromise->Resolve(result); } else { PCWSTR errorName = gWinWebauthnGetErrorName(hr); @@ -977,8 +968,8 @@ WinWebAuthnService::PinCallback(uint64_t aTransactionId, } NS_IMETHODIMP -WinWebAuthnService::ResumeMakeCredential(uint64_t aTransactionId, - bool aForceNoneAttestation) { +WinWebAuthnService::SetHasAttestationConsent(uint64_t aTransactionId, + bool aHasConsent) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/dom/webauthn/authrs_bridge/src/about_webauthn_controller.rs b/dom/webauthn/authrs_bridge/src/about_webauthn_controller.rs index 8d77a62df4..5c160b9333 100644 --- a/dom/webauthn/authrs_bridge/src/about_webauthn_controller.rs +++ b/dom/webauthn/authrs_bridge/src/about_webauthn_controller.rs @@ -4,9 +4,9 @@ use super::*; use authenticator::{ - ctap2::commands::{PinUvAuthResult, StatusCode}, + ctap2::commands::StatusCode, errors::{CommandError, HIDError}, - BioEnrollmentCmd, CredManagementCmd, InteractiveRequest, InteractiveUpdate, PinError, + BioEnrollmentCmd, CredManagementCmd, InteractiveUpdate, PinError, }; use serde::{Deserialize, Serialize}; diff --git a/dom/webauthn/authrs_bridge/src/lib.rs b/dom/webauthn/authrs_bridge/src/lib.rs index 353e1e89a4..b6d54a30b9 100644 --- a/dom/webauthn/authrs_bridge/src/lib.rs +++ b/dom/webauthn/authrs_bridge/src/lib.rs @@ -10,7 +10,7 @@ extern crate xpcom; use authenticator::{ authenticatorservice::{RegisterArgs, SignArgs}, - ctap2::attestation::AttestationObject, + ctap2::attestation::{AAGuid, AttestationObject, AttestationStatement}, ctap2::commands::{get_info::AuthenticatorVersion, PinUvAuthResult}, ctap2::server::{ AuthenticationExtensionsClientInputs, AuthenticatorAttachment, @@ -33,8 +33,8 @@ use nserror::{ }; use nsstring::{nsACString, nsAString, nsCString, nsString}; use serde::Serialize; -use serde_cbor; use serde_json::json; +use std::cell::RefCell; use std::fmt::Write; use std::sync::mpsc::{channel, Receiver, RecvError, Sender}; use std::sync::{Arc, Mutex, MutexGuard}; @@ -87,7 +87,6 @@ enum BrowserPromptType<'a> { }, PinIsTooLong, PinIsTooShort, - RegisterDirect, UvInvalid { retries: Option, }, @@ -167,7 +166,8 @@ fn cancel_prompts(tid: u64) -> Result<(), nsresult> { #[xpcom(implement(nsIWebAuthnRegisterResult), atomic)] pub struct WebAuthnRegisterResult { - result: RegisterResult, + // result is only borrowed mutably in `Anonymize`. + result: RefCell, } impl WebAuthnRegisterResult { @@ -179,13 +179,13 @@ impl WebAuthnRegisterResult { xpcom_method!(get_attestation_object => GetAttestationObject() -> ThinVec); fn get_attestation_object(&self) -> Result, nsresult> { let mut out = ThinVec::new(); - serde_cbor::to_writer(&mut out, &self.result.att_obj).or(Err(NS_ERROR_FAILURE))?; + serde_cbor::to_writer(&mut out, &self.result.borrow().att_obj).or(Err(NS_ERROR_FAILURE))?; Ok(out) } xpcom_method!(get_credential_id => GetCredentialId() -> ThinVec); fn get_credential_id(&self) -> Result, nsresult> { - let Some(credential_data) = &self.result.att_obj.auth_data.credential_data else { + let Some(credential_data) = &self.result.borrow().att_obj.auth_data.credential_data else { return Err(NS_ERROR_FAILURE); }; Ok(credential_data.credential_id.as_slice().into()) @@ -198,7 +198,7 @@ impl WebAuthnRegisterResult { // In tests, the result is not very important, but we can at least return "internal" if // we're simulating platform attachment. if static_prefs::pref!("security.webauth.webauthn_enable_softtoken") - && self.result.attachment == AuthenticatorAttachment::Platform + && self.result.borrow().attachment == AuthenticatorAttachment::Platform { Ok(thin_vec![nsString::from("internal")]) } else { @@ -208,7 +208,7 @@ impl WebAuthnRegisterResult { xpcom_method!(get_hmac_create_secret => GetHmacCreateSecret() -> bool); fn get_hmac_create_secret(&self) -> Result { - let Some(hmac_create_secret) = self.result.extensions.hmac_create_secret else { + let Some(hmac_create_secret) = self.result.borrow().extensions.hmac_create_secret else { return Err(NS_ERROR_NOT_AVAILABLE); }; Ok(hmac_create_secret) @@ -216,7 +216,7 @@ impl WebAuthnRegisterResult { xpcom_method!(get_cred_props_rk => GetCredPropsRk() -> bool); fn get_cred_props_rk(&self) -> Result { - let Some(cred_props) = &self.result.extensions.cred_props else { + let Some(cred_props) = &self.result.borrow().extensions.cred_props else { return Err(NS_ERROR_NOT_AVAILABLE); }; Ok(cred_props.rk) @@ -229,12 +229,29 @@ impl WebAuthnRegisterResult { xpcom_method!(get_authenticator_attachment => GetAuthenticatorAttachment() -> nsAString); fn get_authenticator_attachment(&self) -> Result { - match self.result.attachment { + match self.result.borrow().attachment { AuthenticatorAttachment::CrossPlatform => Ok(nsString::from("cross-platform")), AuthenticatorAttachment::Platform => Ok(nsString::from("platform")), AuthenticatorAttachment::Unknown => Err(NS_ERROR_NOT_AVAILABLE), } } + + xpcom_method!(has_identifying_attestation => HasIdentifyingAttestation() -> bool); + fn has_identifying_attestation(&self) -> Result { + if self.result.borrow().att_obj.att_stmt != AttestationStatement::None { + return Ok(true); + } + if let Some(data) = &self.result.borrow().att_obj.auth_data.credential_data { + return Ok(data.aaguid != AAGuid::default()); + } + Ok(false) + } + + xpcom_method!(anonymize => Anonymize()); + fn anonymize(&self) -> Result { + self.result.borrow_mut().att_obj.anonymize(); + Ok(NS_OK) + } } #[xpcom(implement(nsIWebAuthnAttObj), atomic)] @@ -276,6 +293,17 @@ impl WebAuthnAttObj { // safe to cast to i32 by inspection of defined values Ok(credential_data.credential_public_key.alg as i32) } + + xpcom_method!(is_identifying => IsIdentifying() -> bool); + fn is_identifying(&self) -> Result { + if self.att_obj.att_stmt != AttestationStatement::None { + return Ok(true); + } + if let Some(data) = &self.att_obj.auth_data.credential_data { + return Ok(data.aaguid != AAGuid::default()); + } + Ok(false) + } } #[xpcom(implement(nsIWebAuthnSignResult), atomic)] @@ -491,10 +519,11 @@ impl RegisterPromise { fn resolve_or_reject(&self, result: Result) -> Result<(), nsresult> { match result { Ok(result) => { - let wrapped_result = - WebAuthnRegisterResult::allocate(InitWebAuthnRegisterResult { result }) - .query_interface::() - .ok_or(NS_ERROR_FAILURE)?; + let wrapped_result = WebAuthnRegisterResult::allocate(InitWebAuthnRegisterResult { + result: RefCell::new(result), + }) + .query_interface::() + .ok_or(NS_ERROR_FAILURE)?; unsafe { self.0.Resolve(wrapped_result.coerce()) }; } Err(result) => { @@ -544,7 +573,6 @@ impl TransactionPromise { } enum TransactionArgs { - Register(/* timeout */ u64, RegisterArgs), Sign(/* timeout */ u64, SignArgs), } @@ -714,13 +742,6 @@ impl AuthrsService { } } - let mut attestation_conveyance_preference = nsString::new(); - unsafe { args.GetAttestationConveyancePreference(&mut *attestation_conveyance_preference) } - .to_result()?; - let none_attestation = !(attestation_conveyance_preference.eq("indirect") - || attestation_conveyance_preference.eq("direct") - || attestation_conveyance_preference.eq("enterprise")); - let mut cred_props = false; unsafe { args.GetCredProps(&mut cred_props) }.to_result()?; @@ -761,51 +782,19 @@ impl AuthrsService { use_ctap1_fallback: !static_prefs::pref!("security.webauthn.ctap2"), }; - *self.transaction.lock().unwrap() = Some(TransactionState { + let mut guard = self.transaction.lock().unwrap(); + *guard = Some(TransactionState { tid, browsing_context_id, - pending_args: Some(TransactionArgs::Register(timeout_ms as u64, info)), + pending_args: None, promise: TransactionPromise::Register(promise), pin_receiver: None, selection_receiver: None, interactive_receiver: None, puat_cache: None, }); - - if none_attestation - || static_prefs::pref!("security.webauth.webauthn_testing_allow_direct_attestation") - { - self.resume_make_credential(tid, none_attestation) - } else { - send_prompt( - BrowserPromptType::RegisterDirect, - tid, - Some(&origin), - Some(browsing_context_id), - ) - } - } - - xpcom_method!(resume_make_credential => ResumeMakeCredential(aTid: u64, aForceNoneAttestation: bool)); - fn resume_make_credential( - &self, - tid: u64, - force_none_attestation: bool, - ) -> Result<(), nsresult> { - let mut guard = self.transaction.lock().unwrap(); - let Some(state) = guard.as_mut() else { - return Err(NS_ERROR_FAILURE); - }; - if state.tid != tid { - return Err(NS_ERROR_FAILURE); - }; - let browsing_context_id = state.browsing_context_id; - let Some(TransactionArgs::Register(timeout_ms, info)) = state.pending_args.take() else { - return Err(NS_ERROR_FAILURE); - }; - // We have to drop the guard here, as there _may_ still be another operation - // ongoing and `register()` below will try to cancel it. This will call the state - // callback of that operation, which in turn may try to access `transaction`, deadlocking. + // drop the guard here to ensure we don't deadlock if the call to `register()` below + // hairpins the state callback. drop(guard); let (status_tx, status_rx) = channel::(); @@ -826,7 +815,7 @@ impl AuthrsService { let callback_transaction = self.transaction.clone(); let callback_origin = info.origin.clone(); let state_callback = StateCallback::>::new( - Box::new(move |mut result| { + Box::new(move |result| { let mut guard = callback_transaction.lock().unwrap(); let Some(state) = guard.as_mut() else { return; @@ -837,13 +826,6 @@ impl AuthrsService { let TransactionPromise::Register(ref promise) = state.promise else { return; }; - if let Ok(inner) = result.as_mut() { - // Tokens always provide attestation, but the user may have asked we not - // include the attestation statement in the response. - if force_none_attestation { - inner.att_obj.anonymize(); - } - } if let Err(AuthenticatorError::CredentialExcluded) = result { let _ = send_prompt( BrowserPromptType::AlreadyRegistered, @@ -875,14 +857,14 @@ impl AuthrsService { Some(browsing_context_id), )?; self.usb_token_manager.lock().unwrap().register( - timeout_ms, + timeout_ms.into(), info, status_tx, state_callback, ); } else if static_prefs::pref!("security.webauth.webauthn_enable_softtoken") { self.test_token_manager - .register(timeout_ms, info, status_tx, state_callback); + .register(timeout_ms.into(), info, status_tx, state_callback); } else { return Err(NS_ERROR_FAILURE); } @@ -890,6 +872,11 @@ impl AuthrsService { Ok(()) } + xpcom_method!(set_has_attestation_consent => SetHasAttestationConsent(aTid: u64, aHasConsent: bool)); + fn set_has_attestation_consent(&self, _tid: u64, _has_consent: bool) -> Result<(), nsresult> { + Err(NS_ERROR_NOT_IMPLEMENTED) + } + xpcom_method!(get_assertion => GetAssertion(aTid: u64, aBrowsingContextId: u64, aArgs: *const nsIWebAuthnSignArgs, aPromise: *const nsIWebAuthnSignPromise)); fn get_assertion( &self, diff --git a/dom/webauthn/nsIWebAuthnArgs.idl b/dom/webauthn/nsIWebAuthnArgs.idl index 72999092fa..06a6c5ec85 100644 --- a/dom/webauthn/nsIWebAuthnArgs.idl +++ b/dom/webauthn/nsIWebAuthnArgs.idl @@ -40,9 +40,9 @@ interface nsIWebAuthnRegisterArgs : nsISupports { // WebAuthn AuthenticationExtensionsClientInputs. That's not feasible here. // So we define a getter for each supported extension input and use the // return code to signal presence. - [must_use] readonly attribute bool credProps; - [must_use] readonly attribute bool hmacCreateSecret; - [must_use] readonly attribute bool minPinLength; + [must_use] readonly attribute boolean credProps; + [must_use] readonly attribute boolean hmacCreateSecret; + [must_use] readonly attribute boolean minPinLength; // Options. readonly attribute AString residentKey; @@ -83,7 +83,7 @@ interface nsIWebAuthnSignArgs : nsISupports { // WebAuthn AuthenticationExtensionsClientInputs. That's not feasible here. // So we define a getter for each supported extension input and use the // return code to signal presence. - [must_use] readonly attribute bool hmacCreateSecret; + [must_use] readonly attribute boolean hmacCreateSecret; [must_use] readonly attribute AString appId; // Options @@ -94,5 +94,5 @@ interface nsIWebAuthnSignArgs : nsISupports { // cancel transactions. readonly attribute unsigned long timeoutMS; - readonly attribute bool conditionallyMediated; + readonly attribute boolean conditionallyMediated; }; diff --git a/dom/webauthn/nsIWebAuthnAttObj.idl b/dom/webauthn/nsIWebAuthnAttObj.idl index 32d4f0aba3..101d4d1433 100644 --- a/dom/webauthn/nsIWebAuthnAttObj.idl +++ b/dom/webauthn/nsIWebAuthnAttObj.idl @@ -17,4 +17,6 @@ interface nsIWebAuthnAttObj : nsISupports { readonly attribute Array publicKey; readonly attribute COSEAlgorithmIdentifier publicKeyAlgorithm; + + boolean isIdentifying(); }; diff --git a/dom/webauthn/nsIWebAuthnResult.idl b/dom/webauthn/nsIWebAuthnResult.idl index aaf34cc5f9..32ce698277 100644 --- a/dom/webauthn/nsIWebAuthnResult.idl +++ b/dom/webauthn/nsIWebAuthnResult.idl @@ -23,11 +23,14 @@ interface nsIWebAuthnRegisterResult : nsISupports { readonly attribute Array transports; - readonly attribute bool hmacCreateSecret; + readonly attribute boolean hmacCreateSecret; - [must_use] attribute bool credPropsRk; + [must_use] attribute boolean credPropsRk; [must_use] readonly attribute AString authenticatorAttachment; + + boolean hasIdentifyingAttestation(); + void anonymize(); }; // The nsIWebAuthnSignResult interface is used to construct IPDL-defined @@ -56,7 +59,7 @@ interface nsIWebAuthnSignResult : nsISupports { [must_use] readonly attribute ACString userName; // appId field of AuthenticationExtensionsClientOutputs (Optional) - [must_use] attribute bool usedAppId; + [must_use] attribute boolean usedAppId; [must_use] readonly attribute AString authenticatorAttachment; }; diff --git a/dom/webauthn/nsIWebAuthnService.idl b/dom/webauthn/nsIWebAuthnService.idl index 6525508057..f2993a9e47 100644 --- a/dom/webauthn/nsIWebAuthnService.idl +++ b/dom/webauthn/nsIWebAuthnService.idl @@ -11,7 +11,7 @@ interface nsICredentialParameters : nsISupports { readonly attribute ACString credentialId; - readonly attribute bool isResidentCredential; + readonly attribute boolean isResidentCredential; readonly attribute ACString rpId; readonly attribute ACString privateKey; readonly attribute ACString userHandle; @@ -37,14 +37,16 @@ interface nsIWebAuthnAutoFillEntry: nsISupports interface nsIWebAuthnService : nsISupports { // IsUserVerifyingPlatformAuthenticatorAvailable - readonly attribute bool isUVPAA; + readonly attribute boolean isUVPAA; + [noscript] void makeCredential( in uint64_t aTransactionId, in uint64_t browsingContextId, in nsIWebAuthnRegisterArgs args, in nsIWebAuthnRegisterPromise promise); + [noscript] void getAssertion( in uint64_t aTransactionId, in uint64_t browsingContextId, @@ -83,7 +85,7 @@ interface nsIWebAuthnService : nsISupports void resumeConditionalGet(in uint64_t aTransactionId); void pinCallback(in uint64_t aTransactionId, in ACString aPin); - void resumeMakeCredential(in uint64_t aTransactionId, in bool aForceNoneAttestation); + void setHasAttestationConsent(in uint64_t aTransactionId, in boolean aHasConsent); void selectionCallback(in uint64_t aTransactionId, in uint64_t aIndex); // Adds a virtual (software) authenticator for use in tests (particularly @@ -92,10 +94,10 @@ interface nsIWebAuthnService : nsISupports uint64_t addVirtualAuthenticator( in ACString protocol, in ACString transport, - in bool hasResidentKey, - in bool hasUserVerification, - in bool isUserConsenting, - in bool isUserVerified); + in boolean hasResidentKey, + in boolean hasUserVerification, + in boolean isUserConsenting, + in boolean isUserVerified); // Removes a previously-added virtual authenticator, as identified by its // id. See @@ -107,7 +109,7 @@ interface nsIWebAuthnService : nsISupports void addCredential( in uint64_t authenticatorId, in ACString credentialId, - in bool isResidentCredential, + in boolean isResidentCredential, in ACString rpId, in ACString privateKey, in ACString userHandle, @@ -127,7 +129,7 @@ interface nsIWebAuthnService : nsISupports // Sets the "isUserVerified" bit on a virtual authenticator. See // https://w3c.github.io/webauthn/#sctn-automation-set-user-verified - void setUserVerified(in uint64_t authenticatorId, in bool isUserVerified); + void setUserVerified(in uint64_t authenticatorId, in boolean isUserVerified); // about:webauthn-specific functions void listen(); diff --git a/dom/webauthn/tests/browser/browser_webauthn_conditional_mediation.js b/dom/webauthn/tests/browser/browser_webauthn_conditional_mediation.js index fff1ec5dab..7bbce1ae58 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_conditional_mediation.js +++ b/dom/webauthn/tests/browser/browser_webauthn_conditional_mediation.js @@ -46,15 +46,13 @@ add_task(async function test_webauthn_modal_request_cancels_conditional_get() { ok(active, "conditional request should still be active"); let promptPromise = promiseNotification("webauthn-prompt-register-direct"); - let modalPromise = promiseWebAuthnMakeCredential(tab, "direct") - .then(arrivingHereIsBad) - .catch(gExpectNotAllowedError); + let modalPromise = promiseWebAuthnMakeCredential(tab, "direct"); await condPromise; ok(!active, "conditional request should not be active"); - // Cancel the modal request with the button. + // Proceed through the consent prompt await promptPromise; PopupNotifications.panel.firstElementChild.secondaryButton.click(); await modalPromise; diff --git a/dom/webauthn/tests/browser/browser_webauthn_prompts.js b/dom/webauthn/tests/browser/browser_webauthn_prompts.js index 05c77271d5..68f1bf81f4 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_prompts.js +++ b/dom/webauthn/tests/browser/browser_webauthn_prompts.js @@ -43,34 +43,26 @@ add_task(async function test_setup_usbtoken() { }); add_task(test_register); add_task(test_register_escape); -add_task(test_register_direct_cancel); -add_task(test_register_direct_presence); add_task(test_sign); add_task(test_sign_escape); add_task(test_tab_switching); add_task(test_window_switching); -add_task(async function test_setup_fullscreen() { +add_task(async function test_setup_softtoken() { + gAuthenticatorId = add_virtual_authenticator(); return SpecialPowers.pushPrefEnv({ set: [ ["browser.fullscreen.autohide", true], ["full-screen-api.enabled", true], ["full-screen-api.allow-trusted-requests-only", false], - ], - }); -}); -add_task(test_fullscreen_show_nav_toolbar); -add_task(test_no_fullscreen_dom); -add_task(async function test_setup_softtoken() { - gAuthenticatorId = add_virtual_authenticator(); - return SpecialPowers.pushPrefEnv({ - set: [ ["security.webauth.webauthn_enable_softtoken", true], ["security.webauth.webauthn_enable_usbtoken", false], ], }); }); -add_task(test_register_direct_proceed); -add_task(test_register_direct_proceed_anon); +add_task(test_fullscreen_show_nav_toolbar); +add_task(test_no_fullscreen_dom); +add_task(test_register_direct_with_consent); +add_task(test_register_direct_without_consent); add_task(test_select_sign_result); function promiseNavToolboxStatus(aExpectedStatus) { @@ -215,53 +207,6 @@ async function test_sign_escape() { await BrowserTestUtils.removeTab(tab); } -async function test_register_direct_cancel() { - // Open a new tab. - let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - - // Request a new credential with direct attestation and wait for the prompt. - let active = true; - let promise = promiseWebAuthnMakeCredential(tab, "direct") - .then(arrivingHereIsBad) - .catch(expectNotAllowedError) - .then(() => (active = false)); - await promiseNotification("webauthn-prompt-register-direct"); - - // Cancel the request. - ok(active, "request should still be active"); - PopupNotifications.panel.firstElementChild.secondaryButton.click(); - await promise; - - // Close tab. - await BrowserTestUtils.removeTab(tab); -} - -async function test_register_direct_presence() { - // Open a new tab. - let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - - // Request a new credential with direct attestation and wait for the prompt. - let active = true; - let promise = promiseWebAuthnMakeCredential(tab, "direct") - .then(arrivingHereIsBad) - .catch(expectNotAllowedError) - .then(() => (active = false)); - await promiseNotification("webauthn-prompt-register-direct"); - - // Click "proceed" and wait for presence prompt - let presence = promiseNotification("webauthn-prompt-presence"); - PopupNotifications.panel.firstElementChild.button.click(); - await presence; - - // Cancel the request. - ok(active, "request should still be active"); - PopupNotifications.panel.firstElementChild.button.click(); - await promise; - - // Close tab. - await BrowserTestUtils.removeTab(tab); -} - // Add two tabs, open WebAuthn in the first, switch, assert the prompt is // not visible, switch back, assert the prompt is there and cancel it. async function test_tab_switching() { @@ -359,7 +304,7 @@ async function test_window_switching() { await BrowserTestUtils.removeTab(tab); } -async function test_register_direct_proceed() { +async function test_register_direct_with_consent() { // Open a new tab. let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); @@ -367,7 +312,7 @@ async function test_register_direct_proceed() { let request = promiseWebAuthnMakeCredential(tab, "direct"); await promiseNotification("webauthn-prompt-register-direct"); - // Proceed. + // Click "Allow". PopupNotifications.panel.firstElementChild.button.click(); // Ensure we got "direct" attestation. @@ -377,7 +322,7 @@ async function test_register_direct_proceed() { await BrowserTestUtils.removeTab(tab); } -async function test_register_direct_proceed_anon() { +async function test_register_direct_without_consent() { // Open a new tab. let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); @@ -385,9 +330,8 @@ async function test_register_direct_proceed_anon() { let request = promiseWebAuthnMakeCredential(tab, "direct"); await promiseNotification("webauthn-prompt-register-direct"); - // Check "anonymize anyway" and proceed. - PopupNotifications.panel.firstElementChild.checkbox.checked = true; - PopupNotifications.panel.firstElementChild.button.click(); + // Click "Block". + PopupNotifications.panel.firstElementChild.secondaryButton.click(); // Ensure we got "none" attestation. await request.then(verifyAnonymizedCertificate); @@ -438,23 +382,22 @@ async function test_fullscreen_show_nav_toolbar() { await navToolboxHiddenPromise; - // Request a new credential and wait for the direct attestation consent - // prompt. + // Request a new credential with direct attestation. The consent prompt will + // keep the request active until we can verify that the nav toolbar is shown. let promptPromise = promiseNotification("webauthn-prompt-register-direct"); let navToolboxShownPromise = promiseNavToolboxStatus("shown"); let active = true; - let requestPromise = promiseWebAuthnMakeCredential(tab, "direct") - .then(arrivingHereIsBad) - .catch(expectNotAllowedError) - .then(() => (active = false)); + let requestPromise = promiseWebAuthnMakeCredential(tab, "direct").then( + () => (active = false) + ); await Promise.all([promptPromise, navToolboxShownPromise]); ok(active, "request is active"); ok(window.fullScreen, "window is fullscreen"); - // Cancel the request. + // Proceed through the consent prompt. PopupNotifications.panel.firstElementChild.secondaryButton.click(); await requestPromise; @@ -475,23 +418,22 @@ async function test_no_fullscreen_dom() { await fullScreenPaintPromise; ok(!!document.fullscreenElement, "a DOM element is fullscreen"); - // Request a new credential and wait for the direct attestation consent - // prompt. + // Request a new credential with direct attestation. The consent prompt will + // keep the request active until we can verify that we've left fullscreen. let promptPromise = promiseNotification("webauthn-prompt-register-direct"); fullScreenPaintPromise = promiseFullScreenPaint(); let active = true; - let requestPromise = promiseWebAuthnMakeCredential(tab, "direct") - .then(arrivingHereIsBad) - .catch(expectNotAllowedError) - .then(() => (active = false)); + let requestPromise = promiseWebAuthnMakeCredential(tab, "direct").then( + () => (active = false) + ); await Promise.all([promptPromise, fullScreenPaintPromise]); ok(active, "request is active"); ok(!document.fullscreenElement, "no DOM element is fullscreen"); - // Cancel the request. + // Proceed through the consent prompt. await waitForPopupNotificationSecurityDelay(); PopupNotifications.panel.firstElementChild.secondaryButton.click(); await requestPromise; -- cgit v1.2.3