1420763 - webauthn: credential public key not a COSE_Key

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect, P2)

Description

[Adam Langley]

Using a Nightly build, below is a credential public key from attestationObject.authData:

a363616c6765455332353661785820578c3b41309593d00bcf9dfe89b74bb0a2ccceaf511baeedbd130f6b67690ad76179582051bf2e1586643767bd56cb592044a4226d5e9590a30baf59542231c6350ff3e3

It's a CBOR map with keys "alg", "x", and "y". However, I think it should be a COSE_Key structure[1]. Based on the example[2] from the RFC, it looks like that should be an integer-keyed map.

[1] https://www.w3.org/TR/webauthn/#sec-attestation-data
[2] https://tools.ietf.org/html/rfc8152#appendix-C.7.1

Comment 1

[Adam Langley]

Attachment: Bug 1420763 - encode webauthn keys as a COSE key;

webauthn says[1] that public keys are encoded as COSE keys.  I find the COSE
RFC quite circuitous in many respects and so any reviews should check whether
they agree with my understanding of what should be in a COSE key.

The webauthn spec says that the key:

    “MUST contain the "alg" parameter and MUST NOT contain
     any other optional parameters.”

I don't believe that any of the parameters included are optional but, again, I
don't think the RFC is completely clear.

[1] https://www.w3.org/TR/webauthn/#sec-attested-credential-data

Review commit: https://reviewboard.mozilla.org/r/209714/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209714/

Comment 2

[Adam Langley]

https://reviewboard.mozilla.org/r/209714/

Comment 3

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8939282 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

https://reviewboard.mozilla.org/r/209714/#review215582

This is accurate, based on my reading of COSE and the CTAP canonicalization rules. Thank you again, Adam!

Comment 4

[Eliza Balazs [:ebalazs_]]

MozReview complains that a suitable reviewer has not given a "Ship It!"
I can't land the patch because of this.

Comment 5

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8939282 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

That is super weird; 1420760 landed just fine earlier today.

I guess I'll bother David; r?keeler?

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8939282 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

https://reviewboard.mozilla.org/r/209714/#review215914

Looking at RFC 8152 section 13.1.1, it seems crv, x, and y are indeed required, so I believe this is correct.

Comment 7

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s d441285b7018 -d 38b6a5527c29: rebasing 440875:d441285b7018 "Bug 1420763 - encode webauthn keys as a COSE key; r=jcj,keeler" (tip)
merging dom/webauthn/WebAuthnCBORUtil.cpp
warning: conflicts while merging dom/webauthn/WebAuthnCBORUtil.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 8

[J.C. Jones [:jcj] (he/him)]

Attachment: Bug 1420763 - encode webauthn keys as a COSE key;

webauthn says[1] that public keys are encoded as COSE keys.  I find the COSE
RFC quite circuitous in many respects and so any reviews should check whether
they agree with my understanding of what should be in a COSE key.

The webauthn spec says that the key:

    “MUST contain the "alg" parameter and MUST NOT contain
     any other optional parameters.”

I don't believe that any of the parameters included are optional but, again, I
don't think the RFC is completely clear.

[1] https://www.w3.org/TR/webauthn/#sec-attested-credential-data

Review commit: https://reviewboard.mozilla.org/r/210238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210238/

Comment 9

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8939964 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

https://reviewboard.mozilla.org/r/210238/#review215962

Comment 10

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8939964 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

https://reviewboard.mozilla.org/r/210238/#review216244

Comment 11

[J.C. Jones [:jcj] (he/him)]

https://webauthn.bin.coffee/ is now updated to use COSE structures.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd99a1f959ad
encode webauthn keys as a COSE key; r=keeler

Comment 13

[Cristina Coroiu [:ccoroiu]]

Backed out 1 changesets (bug 1420763) for failing dom/webauthn/tests/test_webauthn_loopback.html r=backout on a CLOSED TREE

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=cd99a1f959ad1de4166c493fd5bf1ca86fbd2500&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&selectedJob=154432326

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=154432281&repo=autoland&lineNumber=2169

Backout: https://hg.mozilla.org/integration/autoland/rev/58070175527e314d6061a04464ec4538232315b8

Comment 14

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8939964 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210238/diff/1-2/

Comment 15

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 8939964 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210238/diff/2-3/

Comment 16

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8939964 [details]
Bug 1420763 - encode webauthn keys as a COSE key;

https://reviewboard.mozilla.org/r/210238/#review216370

lgtm: https://treeherder.mozilla.org/#/jobs?repo=try&revision=42f07806d3fe

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6168b9358352
encode webauthn keys as a COSE key; r=keeler

Comment 18

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6168b9358352