There's a TODO in WebAuthnManager.cpp:GetAlgorithmName to coerce JS objects to strings and get their names, similar to what WebCrypto does. This is a spec compliance requirement. This will require piping JS objects to the GetAlgorithmName function.

WD-07 changes this to COSE types

Comment on attachment 8918030 [details] Bug 1381190 - Remove WebAuthnRequest dead code https://reviewboard.mozilla.org/r/188928/#review194432 Sweet.

Comment on attachment 8918031 [details] Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn https://reviewboard.mozilla.org/r/188930/#review194822 ::: dom/webauthn/WebAuthnManager.cpp:21 (Diff revision 1) > #include "mozilla/dom/U2FUtil.h" > #include "mozilla/dom/WebAuthnCBORUtil.h" > #include "mozilla/dom/WebAuthnManager.h" > #include "mozilla/dom/WebAuthnTransactionChild.h" > #include "mozilla/dom/WebAuthnUtil.h" > #include "mozilla/dom/WebCryptoCommon.h" We can probably remove this now. ::: dom/webauthn/WebAuthnManager.cpp:344 (Diff revision 1) > if (NS_WARN_IF(NS_FAILED(srv))) { > promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR); > return promise.forget(); > } > > // Process each element of cryptoParameters using the following steps, to Is `cryptoParameters` an old reference to `mPubKeyCredParams`? ::: dom/webauthn/WebAuthnManager.cpp:358 (Diff revision 1) > // 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. That paragraph can probably go? ::: dom/webauthn/WebAuthnManager.cpp:365 (Diff revision 1) > - if (NS_FAILED(GetAlgorithmName(aOptions.mPubKeyCredParams[a].mAlg, > + if (NS_FAILED(CoseAlgorithmToWebCryptoId(aOptions.mPubKeyCredParams[a].mAlg, > - algName))) { > + algName))) { So we assign to `algName` here but then don't use that value anywhere, right? ::: dom/webauthn/WebAuthnManager.cpp:370 (Diff revision 1) > - // Add a new object of type PublicKeyCredentialParameters to > - // normalizedParameters, with type set to current.type and algorithm set to > + if (!acceptableParams.AppendElement(aOptions.mPubKeyCredParams[a], > + mozilla::fallible)){ If all we only check `acceptableParams.IsEmpty()`, maybe we should make that a boolean and change the condition? We don't need to maintain a list. ::: dom/webauthn/tests/browser/tab_webauthn_success.html:41 (Diff revision 1) > let makeCredentialOptions = { > rp: {id: document.domain, name: "none", icon: "none"}, > 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: -7}], Can we use constants here and elsewhere? Maybe defined in a common JS file?

Comment on attachment 8918031 [details] Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn https://reviewboard.mozilla.org/r/188930/#review194822 > Is `cryptoParameters` an old reference to `mPubKeyCredParams`? Yup, caught the GetAssertion uses, but not these.. oops > So we assign to `algName` here but then don't use that value anywhere, right? Yes, but this code needs to move to U2F where it really belongs. I'm going to let this remain unused for now and add a comment pointing to a newly filed Bug 1409220. > If all we only check `acceptableParams.IsEmpty()`, maybe we should make that a boolean and change the condition? We don't need to maintain a list. Dropping this in favor of Bug 1409220. > Can we use constants here and elsewhere? Maybe defined in a common JS file? Good idea. Updated.

Comment on attachment 8918031 [details] Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn https://reviewboard.mozilla.org/r/188930/#review195370

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 2 changesets with 10 changes to 10 files remote: remote: WebIDL file dom/webidl/WebAuthentication.webidl altered in changeset ed61bc39a21c without DOM peer review remote: remote: remote: remote: ************************** ERROR **************************** remote: remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=... remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding.. remote: remote: ************************************************************* remote: remote: remote: transaction abort! remote: rollback completed remote: pretxnchangegroup.d_webidl hook failed abort: push failed on remote

(In reply to Mozilla Autoland from comment #9) > remote: WebIDL file dom/webidl/WebAuthentication.webidl altered in changeset > ed61bc39a21c without DOM peer review Agh. Change-blinded to the one line WebIDL update. r?qdot, thanks.

Comment on attachment 8918031 [details] Bug 1381190 - Change to COSE Algorithm identifiers for WebAuthn https://reviewboard.mozilla.org/r/188930/#review195548

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c09ea1671fc3 Remove WebAuthnRequest dead code r=ttaubert https://hg.mozilla.org/integration/autoland/rev/35f1751b91a9 Change to COSE Algorithm identifiers for WebAuthn r=qdot,ttaubert

https://hg.mozilla.org/mozilla-central/rev/c09ea1671fc3 https://hg.mozilla.org/mozilla-central/rev/35f1751b91a9