Bug 1838945 - virtual authenticator test cleanup. r=keeler

Depends on D181006

Differential Revision: https://phabricator.services.mozilla.com/D181300
This commit is contained in:
John Schanck 2023-08-09 20:47:54 +00:00
Родитель 80906d58d2
Коммит 5a81fc2d12
8 изменённых файлов: 54 добавлений и 91 удалений

Просмотреть файл

@ -226,7 +226,7 @@ void WebAuthnController::Register(
if (NS_WARN_IF(NS_FAILED(rv))) {
// We haven't set mTransaction yet, so we can't use AbortTransaction
Unused << mTransactionParent->SendAbort(aTransactionId,
NS_ERROR_DOM_UNKNOWN_ERR);
NS_ERROR_DOM_NOT_ALLOWED_ERR);
return;
}
@ -370,14 +370,16 @@ void WebAuthnController::RunFinishRegister(
nsresult status;
nsresult rv = aResult->GetStatus(&status);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
if (NS_FAILED(status)) {
bool shouldCancelActiveDialog = true;
if (status == NS_ERROR_DOM_OPERATION_ERR) {
if (status == NS_ERROR_DOM_INVALID_STATE_ERR) {
// PIN-related errors. Let the dialog show to inform the user
shouldCancelActiveDialog = false;
} else {
status = NS_ERROR_DOM_NOT_ALLOWED_ERR;
}
Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
u"CTAPRegisterAbort"_ns, 1);
@ -390,14 +392,14 @@ void WebAuthnController::RunFinishRegister(
nsTArray<uint8_t> attObj;
rv = aResult->GetAttestationObject(attObj);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
nsTArray<uint8_t> credentialId;
rv = aResult->GetCredentialId(credentialId);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
@ -506,7 +508,7 @@ void WebAuthnController::RunFinishSign(
if (aResult.Length() == 0) {
Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
u"CTAPSignAbort"_ns, 1);
AbortTransaction(aTransactionId, NS_ERROR_DOM_UNKNOWN_ERR, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
@ -514,19 +516,20 @@ void WebAuthnController::RunFinishSign(
nsresult status;
nsresult rv = aResult[0]->GetStatus(&status);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
if (NS_FAILED(status)) {
bool shouldCancelActiveDialog = true;
if (status == NS_ERROR_DOM_OPERATION_ERR) {
if (status == NS_ERROR_DOM_INVALID_STATE_ERR) {
// PIN-related errors, e.g. blocked token. Let the dialog show to inform
// the user
shouldCancelActiveDialog = false;
}
Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
u"CTAPSignAbort"_ns, 1);
AbortTransaction(aTransactionId, status, shouldCancelActiveDialog);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR,
shouldCancelActiveDialog);
return;
}
mPendingSignResults = aResult.Clone();
@ -539,13 +542,13 @@ void WebAuthnController::RunFinishSign(
nsresult status;
nsresult rv = assertion->GetStatus(&status);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
if (NS_WARN_IF(NS_FAILED(status))) {
Telemetry::ScalarAdd(Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
u"CTAPSignAbort"_ns, 1);
AbortTransaction(aTransactionId, status, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
}
@ -606,28 +609,28 @@ void WebAuthnController::RunResumeWithSelectedSignResult(
nsTArray<uint8_t> credentialId;
nsresult rv = selected->GetCredentialId(credentialId);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
nsTArray<uint8_t> signature;
rv = selected->GetSignature(signature);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
nsTArray<uint8_t> authenticatorData;
rv = selected->GetAuthenticatorData(authenticatorData);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}
nsTArray<uint8_t> rpIdHash;
rv = selected->GetRpIdHash(rpIdHash);
if (NS_WARN_IF(NS_FAILED(rv))) {
AbortTransaction(aTransactionId, NS_ERROR_FAILURE, true);
AbortTransaction(aTransactionId, NS_ERROR_DOM_NOT_ALLOWED_ERR, true);
return;
}

Просмотреть файл

@ -608,6 +608,10 @@ already_AddRefed<Promise> WebAuthnManager::GetAssertion(
allowList.AppendElement(c);
}
}
if (allowList.Length() == 0 && aOptions.mAllowCredentials.Length() != 0) {
promise->MaybeReject(NS_ERROR_DOM_NOT_ALLOWED_ERR);
return promise.forget();
}
if (!MaybeCreateBackgroundActor()) {
promise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);

Просмотреть файл

@ -22,9 +22,8 @@ use authenticator::{
use moz_task::RunnableBuilder;
use nserror::{
nsresult, NS_ERROR_DOM_INVALID_STATE_ERR, NS_ERROR_DOM_NOT_ALLOWED_ERR,
NS_ERROR_DOM_NOT_SUPPORTED_ERR, NS_ERROR_DOM_OPERATION_ERR, NS_ERROR_DOM_UNKNOWN_ERR,
NS_ERROR_FAILURE, NS_ERROR_NOT_AVAILABLE, NS_ERROR_NOT_IMPLEMENTED, NS_ERROR_NULL_POINTER,
NS_OK,
NS_ERROR_DOM_NOT_SUPPORTED_ERR, NS_ERROR_DOM_UNKNOWN_ERR, NS_ERROR_FAILURE,
NS_ERROR_NOT_AVAILABLE, NS_ERROR_NOT_IMPLEMENTED, NS_ERROR_NULL_POINTER, NS_OK,
};
use nsstring::{nsACString, nsCString, nsString};
use serde_cbor;
@ -75,12 +74,12 @@ fn authrs_to_nserror(e: &AuthenticatorError) -> nsresult {
AuthenticatorError::U2FToken(U2FTokenError::NotSupported) => NS_ERROR_DOM_NOT_SUPPORTED_ERR,
AuthenticatorError::U2FToken(U2FTokenError::InvalidState) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::U2FToken(U2FTokenError::NotAllowed) => NS_ERROR_DOM_NOT_ALLOWED_ERR,
AuthenticatorError::PinError(PinError::PinRequired) => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::PinError(PinError::InvalidPin(_)) => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::PinError(PinError::PinAuthBlocked) => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::PinError(PinError::PinBlocked) => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::PinError(PinError::PinNotSet) => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::CredentialExcluded => NS_ERROR_DOM_OPERATION_ERR,
AuthenticatorError::PinError(PinError::PinRequired) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::PinError(PinError::InvalidPin(_)) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::PinError(PinError::PinAuthBlocked) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::PinError(PinError::PinBlocked) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::PinError(PinError::PinNotSet) => NS_ERROR_DOM_INVALID_STATE_ERR,
AuthenticatorError::CredentialExcluded => NS_ERROR_DOM_INVALID_STATE_ERR,
_ => NS_ERROR_DOM_UNKNOWN_ERR,
}
}
@ -497,12 +496,10 @@ impl AuthrsTransport {
unsafe { args.GetUserVerification(&mut *user_verification) }.to_result()?;
let user_verification_req = if user_verification.eq("required") {
UserVerificationRequirement::Required
} else if user_verification.eq("preferred") {
UserVerificationRequirement::Preferred
} else if user_verification.eq("discouraged") {
UserVerificationRequirement::Discouraged
} else {
return Err(NS_ERROR_FAILURE);
UserVerificationRequirement::Preferred
};
let mut authenticator_attachment = nsString::new();
@ -650,12 +647,10 @@ impl AuthrsTransport {
unsafe { args.GetUserVerification(&mut *user_verification) }.to_result()?;
let user_verification_req = if user_verification.eq("required") {
UserVerificationRequirement::Required
} else if user_verification.eq("preferred") {
UserVerificationRequirement::Preferred
} else if user_verification.eq("discouraged") {
UserVerificationRequirement::Discouraged
} else {
return Err(NS_ERROR_FAILURE);
UserVerificationRequirement::Preferred
};
let mut alternate_rp_id = None;

Просмотреть файл

@ -378,9 +378,13 @@ impl VirtualFidoDevice for TestToken {
} else {
// 12. Discoverable credential case
// return any number of assertions from credentials bound to this RP ID
// TODO(Bug 1838932) Until we have conditional mediation we actually don't want to
// return a list of credentials here. The UI to select one of the results blocks
// testing.
for credential in eligible_cred_iter.filter(|x| x.is_discoverable_credential) {
let assertion = credential.assert(&req.client_data_hash, flags).into();
assertions.push(assertion);
break;
}
}

Просмотреть файл

@ -140,10 +140,7 @@
// Test failure cases for get assertion.
add_task(async function test_get_assertion_failures() {
// Unknown user verification.
await requestGetAssertion("unknown")
.then(arrivingHereIsBad)
.catch(expectNotAllowedError);
// No failures currently tested
});
</script>

Просмотреть файл

@ -117,15 +117,15 @@
}]).then(arrivingHereIsGood)
.catch(arrivingHereIsBad);
// Request an assertion for a random credential. The token indicates
// consent even though this credential doesn't exist, so expect an
// InvalidStateError.
// Request an assertion for a random credential. This should be
// indistinguishable from the user denying consent for a known
// credential, so expect a NotAllowedError.
await requestGetAssertion([{
type: "public-key",
id: crypto.getRandomValues(new Uint8Array(16)),
transports: ["usb"],
}]).then(arrivingHereIsBad)
.catch(expectInvalidStateError);
.catch(expectNotAllowedError);
// Request an assertion for gCredential with transport=nfc.
// The softoken pretends to support all transports.

Просмотреть файл

@ -41,14 +41,6 @@
add_task(test_too_many_credentials);
add_task(test_unexpected_option_unknown_credential_type);
add_task(test_empty_credential_list);
add_task(() => {
// Enable USB tokens.
return SpecialPowers.pushPrefEnv({"set": [
["security.webauth.webauthn_enable_softtoken", false],
["security.webauth.webauthn_enable_usbtoken", true],
]});
});
add_task(test_usb_empty_credential_list);
function requestGetAssertion(params) {
return navigator.credentials.get(params);
@ -95,19 +87,6 @@
.then(res => validCred = {type: "public-key", id: res.rawId} );
}
// Test basic good call, but without giving a credential so expect failures
// this is OK by the standard, but not supported by U2F-backed authenticators
// like the soft token in use here.
async function test_without_credential() {
let publicKey = {
challenge: gAssertionChallenge
};
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
}
// Test with a valid credential
async function test_with_credential() {
let publicKey = {
@ -121,7 +100,7 @@
}
// Test with an unexpected option. That won't stop anything, and we'll
// fail with InvalidState just as if we had no valid credentials -- which
// fail with NotAllowed just as if we had no valid credentials -- which
// we don't.
async function test_unexpected_option() {
let publicKey = {
@ -131,7 +110,7 @@
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
.catch(expectNotAllowedError);
}
// Test with an unexpected option but a valid credential
@ -172,7 +151,7 @@
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
.catch(expectNotAllowedError);
}
// Test with an unknown credential
@ -184,7 +163,7 @@
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
.catch(expectNotAllowedError);
}
// Test with too many credentials
@ -210,12 +189,10 @@
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
.catch(expectNotAllowedError);
}
// Test with an empty credential list
// This will return InvalidStateError since the softotken consents, but
// there are no valid credentials.
async function test_empty_credential_list() {
let publicKey = {
challenge: gAssertionChallenge,
@ -224,28 +201,7 @@
await requestGetAssertion({publicKey})
.then(arrivingHereIsBad)
.catch(expectInvalidStateError);
}
// Test with an empty credential list
async function test_usb_empty_credential_list() {
let publicKey = {
challenge: gAssertionChallenge,
allowCredentials: []
};
let ctrl = new AbortController();
let request = requestGetAssertion({publicKey, signal: ctrl.signal})
.then(arrivingHereIsBad)
.catch(expectAbortError);
// Wait a tick for the statemachine to start.
await Promise.resolve();
// The request should time out. We'll abort it here and will fail or
// succeed upon resolution, when the error code is checked.
ctrl.abort();
await request;
.catch(expectNotAllowedError);
}
</script>

Просмотреть файл

@ -194,13 +194,15 @@
}
// Test with an unsupported parameter
// The result depends on the tokens that are available, so we
// expect a NotAllowedError so as not to leak this.
async function test_unsupported_parameter() {
let makeCredentialOptions = {
rp, user, challenge: gCredentialChallenge, pubKeyCredParams: [unsupportedParam]
};
return credm.create({publicKey: makeCredentialOptions})
.then(arrivingHereIsBad)
.catch(expectNotSupportedError);
.catch(expectNotAllowedError);
}
// Test with an unsupported parameter and a good one
@ -214,7 +216,7 @@
.catch(arrivingHereIsBad);
}
// Test with one unknown parameter.
// Test with one unknown (not "public-key") parameter.
async function test_one_unknown_parameter() {
let makeCredentialOptions = {
rp, user, challenge: gCredentialChallenge, pubKeyCredParams: [unknownParam]
@ -225,6 +227,8 @@
}
// Test with an unsupported parameter, and an unknown one
// The result depends on the tokens that are available, so we
// expect a NotAllowedError so as not to leak this.
async function test_one_unknown_one_unsupported_param() {
let makeCredentialOptions = {
rp, user, challenge: gCredentialChallenge,
@ -232,7 +236,7 @@
};
return credm.create({publicKey: makeCredentialOptions})
.then(arrivingHereIsBad)
.catch(expectNotSupportedError);
.catch(expectNotAllowedError);
}
// Test with an unsupported parameter, an unknown one, and a good one. This