diff --git a/dom/webauthn/WebAuthnCBORUtil.cpp b/dom/webauthn/WebAuthnCBORUtil.cpp index ea9761679187..0f4c3c7b32c7 100644 --- a/dom/webauthn/WebAuthnCBORUtil.cpp +++ b/dom/webauthn/WebAuthnCBORUtil.cpp @@ -47,10 +47,10 @@ CBOREncodePublicKeyObj(const CryptoBuffer& aPubKeyBuf, } nsresult -CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf, - const CryptoBuffer& aAttestationCertBuf, - const CryptoBuffer& aSignatureBuf, - /* out */ CryptoBuffer& aAttestationObj) +CBOREncodeFidoU2FAttestationObj(const CryptoBuffer& aAuthDataBuf, + const CryptoBuffer& aAttestationCertBuf, + const CryptoBuffer& aSignatureBuf, + /* out */ CryptoBuffer& aAttestationObj) { /* Attestation Object, encoded in CBOR (description is CDDL) @@ -97,5 +97,39 @@ CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf, return NS_OK; } +nsresult +CBOREncodeNoneAttestationObj(const CryptoBuffer& aAuthDataBuf, + /* out */ CryptoBuffer& aAttestationObj) +{ + /* + Attestation Object, encoded in CBOR (description is CDDL) + + $$attStmtType //= ( + fmt: "none", + attStmt: emptyMap + ) + + emptyMap = {} + */ + cbor::output_dynamic cborAttOut; + cbor::encoder encoder(cborAttOut); + encoder.write_map(3); + { + encoder.write_string("fmt"); + encoder.write_string("none"); + + encoder.write_string("attStmt"); + encoder.write_map(0); + + encoder.write_string("authData"); + encoder.write_bytes(aAuthDataBuf.Elements(), aAuthDataBuf.Length()); + } + + if (!aAttestationObj.Assign(cborAttOut.data(), cborAttOut.size())) { + return NS_ERROR_OUT_OF_MEMORY; + } + return NS_OK; +} + } } diff --git a/dom/webauthn/WebAuthnCBORUtil.h b/dom/webauthn/WebAuthnCBORUtil.h index 2a71a8257de6..ecde64c42da0 100644 --- a/dom/webauthn/WebAuthnCBORUtil.h +++ b/dom/webauthn/WebAuthnCBORUtil.h @@ -21,10 +21,14 @@ CBOREncodePublicKeyObj(const CryptoBuffer& aPubKeyBuf, /* out */ CryptoBuffer& aPubKeyObj); nsresult -CBOREncodeAttestationObj(const CryptoBuffer& aAuthDataBuf, - const CryptoBuffer& aAttestationCertBuf, - const CryptoBuffer& aSignatureBuf, - /* out */ CryptoBuffer& aAttestationObj); +CBOREncodeFidoU2FAttestationObj(const CryptoBuffer& aAuthDataBuf, + const CryptoBuffer& aAttestationCertBuf, + const CryptoBuffer& aSignatureBuf, + /* out */ CryptoBuffer& aAttestationObj); + +nsresult +CBOREncodeNoneAttestationObj(const CryptoBuffer& aAuthDataBuf, + /* out */ CryptoBuffer& aAttestationObj); } // namespace dom } // namespace mozilla diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index b1315032c519..8d314d5c23d3 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -397,9 +397,24 @@ WebAuthnManager::MakeCredential(const PublicKeyCredentialCreationOptions& aOptio bool requestDirectAttestation = attestation == AttestationConveyancePreference::Direct; - // In Bug 1430150, if requestDirectAttestation is true, we will need to prompt - // the user for permission to proceed. For now, we ignore it. - Unused << requestDirectAttestation; + // XXX Bug 1430150. Need something that allows direct attestation + // for tests until we implement a permission dialog we can click. + if (requestDirectAttestation) { + nsresult rv; + nsCOMPtr prefService = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv); + + if (NS_SUCCEEDED(rv)) { + nsCOMPtr branch; + rv = prefService->GetBranch("security.webauth.", getter_AddRefs(branch)); + + if (NS_SUCCEEDED(rv)) { + rv = branch->GetBoolPref("webauthn_testing_allow_direct_attestation", + &requestDirectAttestation); + } + } + + requestDirectAttestation &= NS_SUCCEEDED(rv); + } // Create and forward authenticator selection criteria. WebAuthnAuthenticatorSelection authSelection(selection.mRequireResidentKey, @@ -425,6 +440,7 @@ WebAuthnManager::MakeCredential(const PublicKeyCredentialCreationOptions& aOptio mTransaction = Some(WebAuthnTransaction(promise, rpIdHash, clientDataJSON, + requestDirectAttestation, signal)); mChild->SendRequestRegister(mTransaction.ref().mId, info); @@ -604,6 +620,7 @@ WebAuthnManager::GetAssertion(const PublicKeyCredentialRequestOptions& aOptions, mTransaction = Some(WebAuthnTransaction(promise, rpIdHash, clientDataJSON, + false /* aDirectAttestation */, signal)); mChild->SendRequestSign(mTransaction.ref().mId, info); @@ -653,8 +670,8 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId, RejectTransaction(NS_ERROR_OUT_OF_MEMORY); return; } - // TODO: Adjust the AAGUID from all zeroes in Bug 1381575 (if needed) - // See https://github.com/w3c/webauthn/issues/506 + // FIDO U2F devices have no AAGUIDs, so they'll be all zeros until we add + // support for CTAP2 devices. for (int i=0; i<16; i++) { aaguidBuf.AppendElement(0x00, mozilla::fallible); } @@ -730,11 +747,16 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId, return; } - // The Authentication Data buffer gets CBOR-encoded with the Cert and - // Signature to build the Attestation Object. + // Direct attestation might have been requested by the RP. mDirectAttestation + // will be true only if the user consented via the permission UI. CryptoBuffer attObj; - rv = CBOREncodeAttestationObj(authDataBuf, attestationCertBuf, signatureBuf, - attObj); + if (mTransaction.ref().mDirectAttestation) { + rv = CBOREncodeFidoU2FAttestationObj(authDataBuf, attestationCertBuf, + signatureBuf, attObj); + } else { + rv = CBOREncodeNoneAttestationObj(authDataBuf, attObj); + } + if (NS_FAILED(rv)) { RejectTransaction(rv); return; diff --git a/dom/webauthn/WebAuthnManager.h b/dom/webauthn/WebAuthnManager.h index 461a14773626..f15906239891 100644 --- a/dom/webauthn/WebAuthnManager.h +++ b/dom/webauthn/WebAuthnManager.h @@ -57,10 +57,12 @@ public: WebAuthnTransaction(const RefPtr& aPromise, const nsTArray& aRpIdHash, const nsCString& aClientData, + bool aDirectAttestation, AbortSignal* aSignal) : mPromise(aPromise) , mRpIdHash(aRpIdHash) , mClientData(aClientData) + , mDirectAttestation(aDirectAttestation) , mSignal(aSignal) , mId(NextId()) { @@ -76,6 +78,10 @@ public: // Client data used to assemble reply objects. nsCString mClientData; + // The RP might request direct attestation. + // Only used by the MakeCredential operation. + bool mDirectAttestation; + // An optional AbortSignal instance. RefPtr mSignal; diff --git a/dom/webauthn/tests/browser/browser_webauthn_telemetry.js b/dom/webauthn/tests/browser/browser_webauthn_telemetry.js index 8862a7279e34..8e14455d1445 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_telemetry.js +++ b/dom/webauthn/tests/browser/browser_webauthn_telemetry.js @@ -68,6 +68,7 @@ add_task(async function test_loopback() { Services.prefs.setBoolPref("security.webauth.webauthn", true); Services.prefs.setBoolPref("security.webauth.webauthn_enable_softtoken", true); Services.prefs.setBoolPref("security.webauth.webauthn_enable_usbtoken", false); + Services.prefs.setBoolPref("security.webauth.webauthn_testing_allow_direct_attestation", true); await executeTestPage(testPage); diff --git a/dom/webauthn/tests/browser/tab_webauthn_success.html b/dom/webauthn/tests/browser/tab_webauthn_success.html index 3735cac12bfb..98c84608b5d3 100644 --- a/dom/webauthn/tests/browser/tab_webauthn_success.html +++ b/dom/webauthn/tests/browser/tab_webauthn_success.html @@ -39,6 +39,7 @@ let makeCredentialOptions = { challenge: gCredentialChallenge, timeout: 5000, // the minimum timeout is actually 15 seconds pubKeyCredParams: [{type: "public-key", alg: cose_alg_ECDSA_w_SHA256}], + attestation: "direct" }; navigator.credentials.create({publicKey: makeCredentialOptions}) @@ -113,4 +114,4 @@ navigator.credentials.create({publicKey: makeCredentialOptions}) - \ No newline at end of file + diff --git a/dom/webauthn/tests/test_webauthn_attestation_conveyance.html b/dom/webauthn/tests/test_webauthn_attestation_conveyance.html index 969ba8d248ac..92743721179e 100644 --- a/dom/webauthn/tests/test_webauthn_attestation_conveyance.html +++ b/dom/webauthn/tests/test_webauthn_attestation_conveyance.html @@ -24,6 +24,7 @@ function getAttestationCertFromAttestationBuffer(aAttestationBuffer) { return webAuthnDecodeCBORAttestation(aAttestationBuffer) .then((aAttestationObj) => { + is("fido-u2f", aAttestationObj.fmt, "Is a FIDO U2F Attestation"); let attestationCertDER = aAttestationObj.attStmt.x5c[0]; let certDERBuffer = attestationCertDER.slice(0, attestationCertDER.byteLength).buffer; let certAsn1 = org.pkijs.fromBER(certDERBuffer); @@ -32,9 +33,12 @@ } function verifyAnonymizedCertificate(aResult) { - // TODO: Update this logic with Bug 1430150. - // Until then, all certificates are direct. - return verifyDirectCertificate(aResult); + return webAuthnDecodeCBORAttestation(aResult.response.attestationObject) + .then(({fmt, attStmt}) => { + is("none", fmt, "Is a None Attestation"); + is("object", typeof(attStmt), "attStmt is a map"); + is(0, Object.keys(attStmt).length, "attStmt is empty"); + }); } function verifyDirectCertificate(aResult) { @@ -59,6 +63,7 @@ ["security.webauth.webauthn", true], ["security.webauth.webauthn_enable_softtoken", true], ["security.webauth.webauthn_enable_usbtoken", false], + ["security.webauth.webauthn_testing_allow_direct_attestation", true], ]}); }); diff --git a/dom/webauthn/tests/test_webauthn_loopback.html b/dom/webauthn/tests/test_webauthn_loopback.html index f002a3e212f3..9551f429b307 100644 --- a/dom/webauthn/tests/test_webauthn_loopback.html +++ b/dom/webauthn/tests/test_webauthn_loopback.html @@ -24,7 +24,8 @@ SimpleTest.waitForExplicitFinish(); SpecialPowers.pushPrefEnv({"set": [["security.webauth.webauthn", true], ["security.webauth.webauthn_enable_softtoken", true], - ["security.webauth.webauthn_enable_usbtoken", false]]}, + ["security.webauth.webauthn_enable_usbtoken", false], + ["security.webauth.webauthn_testing_allow_direct_attestation", true]]}, function() { is(navigator.authentication, undefined, "navigator.authentication does not exist any longer"); isnot(navigator.credentials, undefined, "Credential Management API endpoint must exist"); @@ -149,7 +150,8 @@ function() { rp: rp, user: user, challenge: gCredentialChallenge, - pubKeyCredParams: [param] + pubKeyCredParams: [param], + attestation: "direct" }; credm.create({publicKey: makeCredentialOptions}) .then(decodeCreatedCredential) diff --git a/dom/webauthn/tests/u2futil.js b/dom/webauthn/tests/u2futil.js index 8b90ce537826..7a245ce9f569 100644 --- a/dom/webauthn/tests/u2futil.js +++ b/dom/webauthn/tests/u2futil.js @@ -146,13 +146,22 @@ function hexDecode(str) { return new Uint8Array(str.match(/../g).map(x => parseInt(x, 16))); } +function hasOnlyKeys(obj, ...keys) { + let okeys = new Set(Object.keys(obj)); + return keys.length == okeys.size && + keys.every(k => okeys.has(k)); +} + function webAuthnDecodeCBORAttestation(aCborAttBuf) { let attObj = CBOR.decode(aCborAttBuf); console.log(":: Attestation CBOR Object ::"); - if (!("authData" in attObj && "fmt" in attObj && "attStmt" in attObj)) { + if (!hasOnlyKeys(attObj, "authData", "fmt", "attStmt")) { return Promise.reject("Invalid CBOR Attestation Object"); } - if (!("sig" in attObj.attStmt && "x5c" in attObj.attStmt)) { + if (attObj.fmt == "fido-u2f" && !hasOnlyKeys(attObj.attStmt, "sig", "x5c")) { + return Promise.reject("Invalid CBOR Attestation Statement"); + } + if (attObj.fmt == "none" && Object.keys(attObj.attStmt).length > 0) { return Promise.reject("Invalid CBOR Attestation Statement"); }