search

web-auth / webauthn-framework

FIDO-U2F / FIDO2 / Webauthn Framework
MIT License
397 stars 51 forks source link

fix: detect realType AuthenticatorAttestationResponse #559

Closed joostdebruijn closed 5 months ago

joostdebruijn commented 5 months ago

Target branch: 4.8.0 Resolves issue: #558

I think this is a better check to determine if the response is an AuthenticatorAttestationResponse.

joostdebruijn commented 5 months ago

@Spomky I was looking further into this one because I want to know why there were no failing tests and I noticed all tests have AuthenticatorAttestationResponses without an AuthenticatorData-attribute. Therefore no tests are failing for v4.8.0. I see some test cases are really specific, so I'm not sure how to provide new ones.

Looking at the (latest) version of the spec, the AuthenticatorData-attribute must be in the AuthenticatorAttestationResponseJSON according to the dictionary (see: https://www.w3.org/TR/webauthn-3/#iface-pkcredential). I think the proposed change makes sense as an AuthenticatorAttestationResponseJSON must have the attestationObject and must not have a signature, while the AuthenticatorAssertionResponseJSON must have the authenticatorData and signature attributes and may have an attestationObject.

Maybe this is just implemented in Chrome or the spec may have changed on this point.

Spomky commented 5 months ago

Hi @joostdebruijn,

Many thnaks for this PR and the other one. I will take time to review them all.

I see some test cases are really specific, so I'm not sure how to provide new ones.

Except tests I wrote in the early versions of the project, most of them come from the conformance test tool provided by the FIDO Alliance. I managed to get this project to pass all tests. Note that when it was done, the version 2 of the specification was the latest version.

the AuthenticatorData-attribute must be in the AuthenticatorAttestationResponseJSON according to the dictionary

I think you are right and the authenticatorData should not be used to identify the response. There are a couple of new attributes in the latest draft (version 3). The authenticatorAttachment is not part of the version 2. The version 3 will be progressively be supported in the next major version.