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

Add test vectors #2209

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add test vectors #2209

wants to merge 6 commits into from

Conversation

emlun
Copy link
Member

@emlun emlun commented Nov 20, 2024

Closes #1633. Sorry it took so long!

The test vectors proposed in #1633 use RP IDs of real websites unaffiliated with W3C, which felt out of place to me, so I chose to generate new ones instead. Also in order to pre-empt any worry that there could be something nefarious hidden in these values, they are all generated deterministically from disclosed PRNG seeds. Consequently the attestation statements are synthetic values rather than real attestations from the corresponding trusted source, which unfortunately means there's more room for error, but I think it's worth it to have the examples self-contained and transparent. I invite library authors to try running their registration and authentication procedures on these examples so that we may work out any inconsistencies.

I plan to also share the code used to generate these, but I needed to patch some of the libraries I used, so I need to resolve that first.


Preview | Diff

index.bs Outdated
; extra_client_data is included iff bit 0x01 of client_data_gen_flags is 1
extra_client_data = h'bac89435a89550bda8c99dfa860362b5' ; Derived by: HKDF-SHA-256(IKM='WebAuthn test vectors', salt=h'06', info='none.ES256', L=16)
clientDataJSON = h'7b2274797065223a22776562617574686e2e637265617465222c226368616c6c656e6765223a226845624d756173647333523143794e6e5f3238364837464c69544e796c516f4b6556333134366d5677314d222c226f726967696e223a2268747470733a2f2f6578616d706c652e6f7267222c2263726f73734f726967696e223a66616c73652c22657874726144617461223a22636c69656e74446174614a534f4e206d617920626520657874656e6465642077697468206164646974696f6e616c206669656c647320696e20746865206675747572652c207375636820617320746869733a20757369554e616956554c326f795a333668674e6974513d3d227d'
attestation_private_key = h'39c0e7521417ba54d43e8dc95174f423dee9bf3cd804ff6d65c857c9abf4d408' ; Derived by: HKDF-SHA-256(IKM='WebAuthn test vectors', salt=h'07', info='none.ES256', L=32)
Copy link
Contributor

@zacknewman zacknewman Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do attestation_private_key and attestation_cert_serial_number exist if this is supposed to be "ES256 Credential with No Attestation" (emphasis added)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it seems I forgot to push commit 8377288 which fixed this among other things.

</xmp>


### ES256 Credential with "crossOrigin": true in clientDataJSON ### {#sctn-test-vectors-none-es256-crossOrigin}
Copy link
Contributor

@zacknewman zacknewman Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these next 3 sections, I'm assuming no attestation is used despite the existence of the variables attestation_private_key and attestation_cert_serial_number? If so, it would be nice to add that in the section names. If they do have attestation, then the actual attestation should be added. For example if no attestation is used, this section should be called 'ES256 Credential with No Attestation and "crossOrigin": true in clientDataJSON'. Additionally like my first comment, it's less confusing if attestation_private_key and attestation_cert_serial_number were removed.

I say this since it's not uncommon for RPs to not support attestation—at least non-self attestation—and it would be nice to immediately know which tests are inapplicable/need-to-be-amended without actually parsing attestationObject to see if and what attestation is used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, see #2209 (comment). I've also added a mention that no attestation is used unless otherwise noted.

@zacknewman
Copy link
Contributor

Other than the two comments I made, this looks good to me. My RP library only supports no attestation and self attestation, and my library does not support ES512. My library successfully verified the first 5 tests. It also verified the next 5 sans the ES512 test after manually removing the attestation portions from the attestation objects.

Thanks for this. I'm sure many will benefit from being able to add these tests to their own unit tests.

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

Successfully merging this pull request may close these issues.

Missing Test Vectors
2 participants