From c3de84620ba8b184d15535985d3282f733e5987d Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Thu, 12 Oct 2017 15:21:06 -0700 Subject: [PATCH] Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn r=qdot,ttaubert The WD-06 (and later) WebAuthn specs choose to move to integer algorithm identifiers for the signatures [1], with a handful of algorithms identified [2]. U2F devices only support ES256 (e.g., COSE ID "-7"), so that's all that is implemented here. Note that the spec also now requires that we accept empty lists of parameters, and in that case, the RP says they aren't picky, so this changes what happens when the parameter list is empty (but still aborts when the list is non-empty but doesn't have anything we can use) [3]. There's a follow-on to move parameter-validation logic into the U2FTokenManager in Bug 1409220. [1] https://w3c.github.io/webauthn/#dictdef-publickeycredentialparameters [2] https://w3c.github.io/webauthn/#alg-identifier [3] https://w3c.github.io/webauthn/#createCredential bullet #12 MozReview-Commit-ID: KgL7mQ9u1uq --HG-- extra : rebase_source : 2a1767805779a9f8049102723011193f113f0713 --- dom/webauthn/WebAuthnCoseIdentifiers.h | 36 ++++++++ dom/webauthn/WebAuthnManager.cpp | 87 ++++--------------- .../tests/browser/tab_webauthn_success.html | 2 +- .../tests/test_webauthn_loopback.html | 4 +- .../tests/test_webauthn_make_credential.html | 11 +-- .../tests/test_webauthn_no_token.html | 2 +- .../tests/test_webauthn_sameorigin.html | 2 +- dom/webauthn/tests/u2futil.js | 3 + dom/webidl/WebAuthentication.webidl | 6 +- 9 files changed, 67 insertions(+), 86 deletions(-) create mode 100644 dom/webauthn/WebAuthnCoseIdentifiers.h diff --git a/dom/webauthn/WebAuthnCoseIdentifiers.h b/dom/webauthn/WebAuthnCoseIdentifiers.h new file mode 100644 index 0000000000000..265d52f69102f --- /dev/null +++ b/dom/webauthn/WebAuthnCoseIdentifiers.h @@ -0,0 +1,36 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et cindent: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_dom_WebAuthnCoseIdentifiers_h +#define mozilla_dom_WebAuthnCoseIdentifiers_h + +#include "mozilla/dom/WebCryptoCommon.h" + +namespace mozilla { +namespace dom { + +// From https://www.iana.org/assignments/cose/cose.xhtml#algorithms +enum class CoseAlgorithmIdentifier : int32_t { + ES256 = -7 +}; + +static nsresult +CoseAlgorithmToWebCryptoId(const int32_t& aId, /* out */ nsString& aName) +{ + switch(static_cast(aId)) { + case CoseAlgorithmIdentifier::ES256: + aName.AssignLiteral(JWK_ALG_ECDSA_P_256); + break; + default: + return NS_ERROR_DOM_NOT_SUPPORTED_ERR; + } + return NS_OK; +} + +} // namespace dom +} // namespace mozilla + +#endif // mozilla_dom_WebAuthnCoseIdentifiers_h diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 04a7bc817804b..c5c5c64aa5a26 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -8,6 +8,7 @@ #include "nsICryptoHash.h" #include "nsNetCID.h" #include "nsThreadUtils.h" +#include "WebAuthnCoseIdentifiers.h" #include "mozilla/ClearOnShutdown.h" #include "mozilla/dom/AuthenticatorAttestationResponse.h" #include "mozilla/dom/Promise.h" @@ -17,7 +18,6 @@ #include "mozilla/dom/WebAuthnManager.h" #include "mozilla/dom/WebAuthnTransactionChild.h" #include "mozilla/dom/WebAuthnUtil.h" -#include "mozilla/dom/WebCryptoCommon.h" #include "mozilla/ipc/BackgroundChild.h" #include "mozilla/ipc/PBackgroundChild.h" @@ -53,28 +53,6 @@ NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback, * Utility Functions **********************************************************************/ -template -static nsresult -GetAlgorithmName(const OOS& aAlgorithm, - /* out */ nsString& aName) -{ - if (aAlgorithm.IsString()) { - // If string, then treat as algorithm name - aName.Assign(aAlgorithm.GetAsString()); - } else { - // TODO: Coerce to string and extract name. See WebCryptoTask.cpp - } - - // Only ES256 is currently supported - if (NORMALIZED_EQUALS(aName, JWK_ALG_ECDSA_P_256)) { - aName.AssignLiteral(JWK_ALG_ECDSA_P_256); - } else { - return NS_ERROR_DOM_SYNTAX_ERR; - } - - return NS_OK; -} - static nsresult AssembleClientData(const nsAString& aOrigin, const CryptoBuffer& aChallenge, /* out */ nsACString& aJsonOut) @@ -365,75 +343,40 @@ WebAuthnManager::MakeCredential(nsPIDOMWindowInner* aParent, return promise.forget(); } - // Process each element of cryptoParameters using the following steps, to - // produce a new sequence normalizedParameters. - nsTArray normalizedParams; + + // TODO: Move this logic into U2FTokenManager in Bug 1409220. + + // Process each element of mPubKeyCredParams using the following steps, to + // produce a new sequence acceptableParams. + nsTArray acceptableParams; for (size_t a = 0; a < aOptions.mPubKeyCredParams.Length(); ++a) { // Let current be the currently selected element of - // cryptoParameters. + // mPubKeyCredParams. // If current.type does not contain a PublicKeyCredentialType // supported by this implementation, then stop processing current and move - // on to the next element in cryptoParameters. + // on to the next element in mPubKeyCredParams. if (aOptions.mPubKeyCredParams[a].mType != PublicKeyCredentialType::Public_key) { continue; } - // Let normalizedAlgorithm be the result of normalizing an algorithm using - // the procedure defined in [WebCryptoAPI], with alg set to - // current.algorithm and op set to 'generateKey'. If an error occurs during - // this procedure, then stop processing current and move on to the next - // element in cryptoParameters. - nsString algName; - if (NS_FAILED(GetAlgorithmName(aOptions.mPubKeyCredParams[a].mAlg, - algName))) { + if (NS_FAILED(CoseAlgorithmToWebCryptoId(aOptions.mPubKeyCredParams[a].mAlg, + algName))) { continue; } - // Add a new object of type PublicKeyCredentialParameters to - // normalizedParameters, with type set to current.type and algorithm set to - // normalizedAlgorithm. - PublicKeyCredentialParameters normalizedObj; - normalizedObj.mType = aOptions.mPubKeyCredParams[a].mType; - normalizedObj.mAlg.SetAsString().Assign(algName); - - if (!normalizedParams.AppendElement(normalizedObj, mozilla::fallible)){ + if (!acceptableParams.AppendElement(aOptions.mPubKeyCredParams[a], + mozilla::fallible)){ promise->MaybeReject(NS_ERROR_OUT_OF_MEMORY); return promise.forget(); } } - // If normalizedAlgorithm is empty and cryptoParameters was not empty, cancel + // If acceptableParams is empty and mPubKeyCredParams was not empty, cancel // the timer started in step 2, reject promise with a DOMException whose name // is "NotSupportedError", and terminate this algorithm. - if (normalizedParams.IsEmpty() && !aOptions.mPubKeyCredParams.IsEmpty()) { - promise->MaybeReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR); - return promise.forget(); - } - - // TODO: The following check should not be here. This is checking for - // parameters specific to the soft key, and should be put in the soft key - // manager in the parent process. Still need to serialize - // PublicKeyCredentialParameters first. - - // Check if at least one of the specified combinations of - // PublicKeyCredentialParameters and cryptographic parameters is supported. If - // not, return an error code equivalent to NotSupportedError and terminate the - // operation. - - bool isValidCombination = false; - - for (size_t a = 0; a < normalizedParams.Length(); ++a) { - if (normalizedParams[a].mType == PublicKeyCredentialType::Public_key && - normalizedParams[a].mAlg.IsString() && - normalizedParams[a].mAlg.GetAsString().EqualsLiteral( - JWK_ALG_ECDSA_P_256)) { - isValidCombination = true; - break; - } - } - if (!isValidCombination) { + if (acceptableParams.IsEmpty() && !aOptions.mPubKeyCredParams.IsEmpty()) { promise->MaybeReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR); return promise.forget(); } diff --git a/dom/webauthn/tests/browser/tab_webauthn_success.html b/dom/webauthn/tests/browser/tab_webauthn_success.html index 3a211e65f1660..3735cac12bfbb 100644 --- a/dom/webauthn/tests/browser/tab_webauthn_success.html +++ b/dom/webauthn/tests/browser/tab_webauthn_success.html @@ -38,7 +38,7 @@

Full-run test for MakeCredential/GetAssertion for W3C Web Authentication

Full-run test for MakeCredential/GetAssertion for W3C Web AuthenticationFull-run test for MakeCredential/GetAssertion for W3C Web AuthenticationTest for MakeCredential for W3C Web Authentication let rp = {id: document.domain, name: "none", icon: "none"}; let user = {id: new Uint8Array(64), name: "none", icon: "none", displayName: "none"}; - let param = {type: "public-key", alg: "es256"}; - let unsupportedParam = {type: "public-key", alg: "3DES"}; + let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; + let unsupportedParam = {type: "public-key", alg: cose_alg_ECDSA_w_SHA512}; let badParam = {type: "SimplePassword", alg: "MaxLength=2"}; var testFuncs = [ @@ -87,14 +87,15 @@

Test for MakeCredential for W3C Web Authentication

.catch(expectTypeError); }, - // Test without a parameter + // Test without any parameters; this is acceptable meaning the RP ID is + // happy to take any credential type function() { let makeCredentialOptions = { rp: rp, user: user, challenge: gCredentialChallenge, pubKeyCredParams: [] }; return credm.create({publicKey: makeCredentialOptions}) - .then(arrivingHereIsBad) - .catch(expectNotSupportedError); + .then(arrivingHereIsGood) + .catch(arrivingHereIsBad); }, // Test without a parameter array at all diff --git a/dom/webauthn/tests/test_webauthn_no_token.html b/dom/webauthn/tests/test_webauthn_no_token.html index 2ed95f430f5f2..34c7402fba7ab 100644 --- a/dom/webauthn/tests/test_webauthn_no_token.html +++ b/dom/webauthn/tests/test_webauthn_no_token.html @@ -45,7 +45,7 @@

Test for W3C Web Authentication with no token

function testMakeCredential() { let rp = {id: document.domain, name: "none", icon: "none"}; let user = {name: "none", icon: "none", displayName: "none"}; - let param = {type: "public-key", alg: "es256"}; + let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; let makeCredentialOptions = { rp: rp, user: user, challenge: credentialChallenge, pubKeyCredParams: [param] }; diff --git a/dom/webauthn/tests/test_webauthn_sameorigin.html b/dom/webauthn/tests/test_webauthn_sameorigin.html index 336b353f2e243..007814f2fd92e 100644 --- a/dom/webauthn/tests/test_webauthn_sameorigin.html +++ b/dom/webauthn/tests/test_webauthn_sameorigin.html @@ -61,7 +61,7 @@

Test Same Origin Policy for W3C Web Authentication

window.crypto.getRandomValues(chall); let user = {id: new Uint8Array(16), name: "none", icon: "none", displayName: "none"}; - let param = {type: "public-key", alg: "Es256"}; + let param = {type: "public-key", alg: cose_alg_ECDSA_w_SHA256}; var testFuncs = [ function() { diff --git a/dom/webauthn/tests/u2futil.js b/dom/webauthn/tests/u2futil.js index 83deb13bb440f..04ae56ec21b80 100644 --- a/dom/webauthn/tests/u2futil.js +++ b/dom/webauthn/tests/u2futil.js @@ -6,6 +6,9 @@ const flag_TUP = 0x01; const flag_UV = 0x04; const flag_AT = 0x40; +const cose_alg_ECDSA_w_SHA256 = -7; +const cose_alg_ECDSA_w_SHA512 = -36; + function handleEventMessage(event) { if ("test" in event.data) { let summary = event.data.test + ": " + event.data.msg; diff --git a/dom/webidl/WebAuthentication.webidl b/dom/webidl/WebAuthentication.webidl index 415f81f455458..bb11c3913d97e 100644 --- a/dom/webidl/WebAuthentication.webidl +++ b/dom/webidl/WebAuthentication.webidl @@ -41,7 +41,7 @@ interface AuthenticatorAssertionResponse : AuthenticatorResponse { dictionary PublicKeyCredentialParameters { required PublicKeyCredentialType type; - required WebAuthnAlgorithmID alg; // Switch to COSE in Bug 1381190 + required COSEAlgorithmIdentifier alg; }; dictionary MakePublicKeyCredentialOptions { @@ -124,6 +124,4 @@ typedef long COSEAlgorithmIdentifier; typedef sequence AuthenticatorSelectionList; -typedef BufferSource AAGUID; - -typedef (boolean or DOMString) WebAuthnAlgorithmID; // Switch to COSE in Bug 1381190 +typedef BufferSource AAGUID; \ No newline at end of file