webauthn: out-of-order keys in CBOR map.

RESOLVED FIXED in Firefox 59

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: agl, Assigned: agl)

Tracking

(Blocks 1 bug)

Trunk
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [webauthn][webauthn-wd07])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Using the Nightly build, below is a dump of the response.attestationObject from a webauthn enrollment. As specified, it's a CBOR map. However the first key is 686175746844617461 (i.e. "authData") which is followed by 63666d74 ("fmt").

However, the CTAP2 spec[1] says that "If two keys have different lengths, the shorter one sorts earlier". It could be argued that this CBOR is specified by webauthn and not CTAP, but all the arguments about wanting to avoid ambiguity around duplicate keys and reduced test coverage of allow arbitrary orders apply here too. Also, it would be nice to be able to use the same CBOR parser throughout.

[1] https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#message-encoding

a368617574684461746158ca12ca17b49af2289436f303e0166030a21e525d266e209267433801a8fd4071a041000000000000000000000000000000000000000000404653e0117fb6deb1cf4967e0669cd816bfabfe500d80da35c57923fd638849cccb6854050c5053e36834eb4cbdf625e022c7da0a56a3eaae0693ac682803c1f6a363616c6765455332353661785820578c3b41309593d00bcf9dfe89b74bb0a2ccceaf511baeedbd130f6b67690ad76179582051bf2e1586643767bd56cb592044a4226d5e9590a30baf59542231c6350ff3e363666d74686669646f2d7532666761747453746d74a263783563815902313082022d30820117a003020102020405b60579300b06092a864886f70d01010b302e312c302a0603550403132359756269636f2055324620526f6f742043412053657269616c203435373230303633313020170d3134303830313030303030305a180f32303530303930343030303030305a30283126302406035504030c1d59756269636f205532462045452053657269616c2039353831353033333059301306072a8648ce3d020106082a8648ce3d03010703420004fdb8deb3a1ed70eb636c066eb6006996a5f970fcb5db88fc3b305d41e5966f0c1b54b852fef0a0907ed17f3bffc29d4d321b9cf8a84a2ceaa038cabd35d598dea3263024302206092b0601040182c40a020415312e332e362e312e342e312e34313438322e312e31300b06092a864886f70d01010b03820101007ed3fb6ccc252013f82f218c2a37da6031d20e7f3081dafcaeb128fc7f9b233914bfb64d6135f17ce221fa764f453ef1273a8ce965956442bb2f1e47483f737dcbc98b585377fef50b270e0289f88436f1adcf49b2621ee5e302df555b9ab74272e069f918149b3dec4f12228b10c0f88de36af58a74bb442b85ae005364bda6702058fc1f2d879b530111ea60e86c63f17fa5944cc83f0aa269848b3ee388a6c09e6b05953fcbb8f47e83a27e0072a63c32ad64864e926d7112fa1997f7839656fbb32be8f7889d0f0145519a27afdd8e46b04ca4290d8540b634b886161e7588c86299dcdd6435d1678a3a6f0a74829c4dd3f70c3524d1ddf16d78add21b64637369675846304402206154783bba9d06e92c8b8d0eb8a4023e2a6a18bef30ea9dd5fc5f13b5dbb2be80220481f114b7812245803fd96634d8db5198e6071136f41463e02f2340aa4d504bd
Priority: -- → P2
Whiteboard: [webauthn][webauthn-wd07]
(In reply to Adam Langley from comment #0)
> Using the Nightly build, below is a dump of the response.attestationObject
> from a webauthn enrollment. As specified, it's a CBOR map. However the first
> key is 686175746844617461 (i.e. "authData") which is followed by 63666d74
> ("fmt").
> 
> However, the CTAP2 spec[1] says that "If two keys have different lengths,
> the shorter one sorts earlier".

The spec also says that "Sorting is performed on the bytes of the representation of the key data items". And later: "If the major types are different, the one with the lower value in numerical order sorts earlier."

"fmt" is of major type 3 == "text string". "authData" is of major type 2 == "byte string".

So, isn't it correct that "authData" sorts earlier than "fmt"?

Our "attStmt" map isn't sorted correctly, IIUIC. The byte string(2) "sig" should sort earlier than the array(4) "x5c".
Sorry, disregard what I said. I didn't realize one is supposed to compare the whole map item and its entire length. So you're of course right and "fmt" needs to be sorted earlier in the above example. Our CBOR encoder is rather simple and we'd have to sort maps ourselves. We'll have to swap it out and extend/use our Rust library.
(Assignee)

Comment 3

2 years ago
> I didn't realize one is supposed to compare the whole map item and its entire length.

The spec for the CBOR canonicalisation is here: https://fidoalliance.org/specs/fido-v2.0-rd-20170927/fido-client-to-authenticator-protocol-v2.0-rd-20170927.html#message-encoding

(It's similar, but not the same, as the RFC rules: https://tools.ietf.org/html/rfc7049#section-3.9)

The CTAP2 document says to sort *keys* by major type, then length, then lexically. So I believe that this list of keys would be considered to be correctly ordered:

1
256
-1
-256
"z"
"abc"

The CBOR RFC rules say "Sorting is performed on the bytes of the representation of the key data items without paying attention to the 3/5 bit splitting for major types."

I think that's saying that you only consider the serialised keys, not the whole key+data pair, same as the CTAP2 document. (I.e. it's "key data-items", not "key-data items".)

Unfortunately the CTAP2 sort order is different(!) because the RFC sorts by length before (implicitly) sorting by major type. Thus I think the items above would be in this order under the RFC rules:

1
-1
-256
"z"
256
"abc"

(I mention the RFC rules only for completeness. I believe the CTAP2 rules control here.)

There is an added complication if one considers that keys in a CBOR map may have one or more optional tags preceding them. I'm just assuming that tags aren't going to be used in webauthn CBOR to keep things simple.
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
Comment on attachment 8939264 [details]
Bug 1420760 - order webauthn CBOR map keys;

https://reviewboard.mozilla.org/r/209692/#review215414

Simple; that works. Thanks!
Attachment #8939264 - Flags: review?(jjones) → review+
Assignee: nobody → agl

Comment 7

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f94505f76fa4
Order webauthn CBOR map keys. r=jcj
Keywords: checkin-needed

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f94505f76fa4
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.