Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoding attestationObject #1614

Closed
cyberphone opened this issue May 21, 2021 · 14 comments
Closed

Decoding attestationObject #1614

cyberphone opened this issue May 21, 2021 · 14 comments
Assignees
Milestone

Comments

@cyberphone
Copy link

https://www.w3.org/TR/webauthn/#attestation-object

The mixing of fixed, variable length, and CBOR data in this object creates some issues. Maybe I missed something but doesn't the optional EXTENSIONS field require a non-standard CBOR parsing process for finding out the length of the preceding public key object?

My current code assumes (including testing the ED flag) that there are no EXTENSIONS but that feels like a potential problem.

Now it is of course [much] too late but if there ever will be a major revision I suggest that all data is expressed as CBOR and presented as UInt8Arrays in JavaScript..

@Firstyear
Copy link
Contributor

They should be base64'd because there is no way to get from json to a uint8array without JS having to decode it and manipulate it.

@cyberphone
Copy link
Author

That would work fine although I think it is about time for JS to to natively support base64url since this has become the norm for newer stuff like JOSE.

@agl
Copy link
Contributor

agl commented Jun 2, 2021

The authenticator data contains a non-CBOR prefix, optionally followed by a CBOR structure for the extensions. You should not need exceptions to CBOR parsing to parse it, but you will need to remove the non-CBOR prefix first.

The CBOR itself is a subset of full CBOR and a greatly reduced parser is sufficient: https://fidoalliance.org/specs/fido-v2.1-rd-20210309/fido-client-to-authenticator-protocol-v2.1-rd-20210309.html#ctap2-canonical-cbor-encoding-form

@agl
Copy link
Contributor

agl commented Jun 2, 2021

(I should also mention that https://www.w3.org/TR/2021/REC-webauthn-2-20210408/#sctn-public-key-easy may be helpful and may remove the need to parse CBOR at all.)

@nadalin nadalin added this to the L3-WD-01 milestone Jun 2, 2021
@cyberphone
Copy link
Author

AFAICT, there is a non-CBOR prefix followed by a public key in COSE format, then followed by an optional extension in CBOR format. This is, after removing the non-CBOR prefix, you need a CBOR parser that do not parse a single object, but potentially multiple objects. I doubt that there are many (any?) CBOR parsers out there that can do that. Arrays were designed to cope with such constructs.

Anyway, nobody seems to be using the extension field so maybe I just raised a zero issue.

@cyberphone
Copy link
Author

That is, if there is a problem(?), it is rather at the CTAP level than in WebAuthn.

@agl
Copy link
Contributor

agl commented Jun 2, 2021

Indeed the public-key that is encoded prior to the extensions is itself CBOR. But the length of those CBOR bytes is given explicitly in the authenticator data, so code can find its extent and pass it precisely to a CBOR parser. You know that it's exactly one CBOR object, as is the extensions map.

You should expect extensions to sometimes exist, even if you don't request any.

https://www.w3.org/TR/2021/REC-webauthn-2-20210408/#sctn-public-key-easy is hopefully helpful here.

@cyberphone
Copy link
Author

Yes, we have the length to the CBOR data but if this looks like this

{
 public key
}
{
  extension
}

it is not entirely obvious what to do. My parser consider this a syntax error but maybe other parsers are smarter and converts the result to an implicit array?

@agl
Copy link
Contributor

agl commented Jun 2, 2021

The length of the public-key map is given explicitly. The length of the extensions is then the remainder. The CBOR parse should only see one at a time.

(If useful: https://source.chromium.org/chromium/chromium/src/+/main:device/fido/authenticator_data.cc;l=51;drc=15c07004f30de337f50cb95ce590613bc7ffa953 )

@emlun
Copy link
Member

emlun commented Jun 2, 2021

The length of the public-key map is given explicitly.

I think you're mixing that up with the credential ID length. The byte length of the public-key map is not given explicitly as it is implicit from the CBOR structure, and CBOR encodes the number of pairs rather than the byte length of the map.

But yeah, you don't need any nonstandard CBOR parser but you do need one with an API that facilitates reading multiple concatenated values. For example the linked Chrome example seems to be reading a stateful byte stream so it can continue the second parse from where the first parse ended. Another option would be for the parser to return the end offset along with the parsed result (common in C APIs, for example).

@cyberphone
Copy link
Author

Thanx @emlun, then this (unfortunately) works exactly as I thought 😒

@agl
Copy link
Contributor

agl commented Jun 2, 2021

I think you're mixing that up with the credential ID length.

Yes, you're correct. Sorry, should have checked rather than using memory.

@yackermann
Copy link
Contributor

@cyberphone
Copy link
Author

cyberphone commented Jun 3, 2021

Thanx @herrjemand, this is what I mean with "non-standard" parsing:

let pubKeyLength = vanillacbor.decodeOnlyFirst(buffer).byteLength; 

Although it almost broke my engineering heart 💔 I had to add this to my parser as well 😁
https://github.com/cyberphone/openkeystore/blob/60db18a1bba52c420d84c2da682ca3e260555428/library/src/org/webpki/cbor/CBORObject.java#L353

COSE also comes with a bunch of surprises, nobody seems to know if ECDH uses Concat or HKDF and the RFC gives no clues at all 🙄 cose-wg/COSE-JAVA#104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants