Skip to content

Commit

Permalink
Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn r=qdo…
Browse files Browse the repository at this point in the history
…t,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
  • Loading branch information
jcjones committed Oct 12, 2017
1 parent f05cfb1 commit c3de846
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 86 deletions.
36 changes: 36 additions & 0 deletions dom/webauthn/WebAuthnCoseIdentifiers.h
Original file line number Diff line number Diff line change
@@ -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<CoseAlgorithmIdentifier>(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
87 changes: 15 additions & 72 deletions dom/webauthn/WebAuthnManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"

Expand Down Expand Up @@ -53,28 +53,6 @@ NS_IMPL_ISUPPORTS(WebAuthnManager, nsIIPCBackgroundChildCreateCallback,
* Utility Functions
**********************************************************************/

template<class OOS>
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)
Expand Down Expand Up @@ -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<PublicKeyCredentialParameters> 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<PublicKeyCredentialParameters> 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();
}
Expand Down
2 changes: 1 addition & 1 deletion dom/webauthn/tests/browser/tab_webauthn_success.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
user: {id: new Uint8Array(), name: "none", icon: "none", displayName: "none"},
challenge: gCredentialChallenge,
timeout: 5000, // the minimum timeout is actually 15 seconds
pubKeyCredParams: [{type: "public-key", alg: "ES256"}],
pubKeyCredParams: [{type: "public-key", alg: cose_alg_ECDSA_w_SHA256}],
};

navigator.credentials.create({publicKey: makeCredentialOptions})
Expand Down
4 changes: 2 additions & 2 deletions dom/webauthn/tests/test_webauthn_loopback.html
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
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,
Expand All @@ -157,7 +157,7 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
function testMakeDuplicate(aCredInfo) {
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,
Expand Down
11 changes: 6 additions & 5 deletions dom/webauthn/tests/test_webauthn_make_credential.html
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ <h1>Test for MakeCredential for W3C Web Authentication</h1>

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 = [
Expand All @@ -87,14 +87,15 @@ <h1>Test for MakeCredential for W3C Web Authentication</h1>
.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
Expand Down
2 changes: 1 addition & 1 deletion dom/webauthn/tests/test_webauthn_no_token.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ <h1>Test for W3C Web Authentication with no token</h1>
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]
};
Expand Down
2 changes: 1 addition & 1 deletion dom/webauthn/tests/test_webauthn_sameorigin.html
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ <h1>Test Same Origin Policy for W3C Web Authentication</h1>
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() {
Expand Down
3 changes: 3 additions & 0 deletions dom/webauthn/tests/u2futil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions dom/webidl/WebAuthentication.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -124,6 +124,4 @@ typedef long COSEAlgorithmIdentifier;

typedef sequence<AAGUID> AuthenticatorSelectionList;

typedef BufferSource AAGUID;

typedef (boolean or DOMString) WebAuthnAlgorithmID; // Switch to COSE in Bug 1381190
typedef BufferSource AAGUID;

0 comments on commit c3de846

Please sign in to comment.