MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.65k stars 138 forks source link

feat/fido-mds-3 #144

Closed MasterKale closed 3 years ago

MasterKale commented 3 years ago

This PR updates MetadataService to support v3 of the FIDO Alliance Metadata Service.

See https://fidoalliance.org/metadata/ for specifications and more info about use of the API

Addresses #143.

Breaking Changes

MetadataService.initialize()

SettingsService.setRootCertificates()/.getRootCertificates()

TODO

MasterKale commented 3 years ago

FIDO Conformance tests against MDSv3 require using an experimental version. I found an experimental v1.6.33 build which seems to work somewhat. I'm still trying to tell if the failing tests are my code, or a WIP test suite...

aseigler commented 3 years ago

This is not as fun as it seems at first glance.

MasterKale commented 3 years ago

This is not as fun as it seems at first glance.

Yeah, I get why we're not allowed to know which request corresponds to which test (because apparently companies were gaming it instead of writing good code??), but it's making it real difficult to have any sort of confidence in results of the tests.

Case in point: Metadata Service test F-1 succeeds when it should be failing:

Screen Shot 2021-08-21 at 10 13 23 PM

The code to check for these statuses is still there, and has definitely worked in the past, but none of the AAGUID's I'm being given for these v3 tests include any of the bad statuses in their status reports:

Getting statement for 4f086657-563a-4420-9971-d33e8e0d34f3
Status reports:
- FIDO_CERTIFIED_L1plus
Getting statement for 29b5e4a2-094e-4837-879f-a914c7a18274
Status reports:
- FIDO_CERTIFIED
Getting statement for 29b5e4a2-094e-4837-879f-a914c7a18274
Status reports:
- FIDO_CERTIFIED
Getting statement for e68036d3-0a5b-4e03-b184-ce3291dd3668
Status reports:
- SELF_ASSERTION_SUBMITTED
Getting statement for 45c9578d-8d5d-4e46-a862-b21d2e3961e9
Status reports:
- FIDO_CERTIFIED_L1plus
Getting statement for 5a30ae78-348e-4ed1-835e-16bddf96bbed
Status reports:
- FIDO_CERTIFIED_L1
Getting statement for d8e43bbc-cc1e-4309-915d-9f87e578166a
Status reports:
- FIDO_CERTIFIED_L3
Getting statement for 182a476e-0fab-4fdc-97b7-4abf8e4219c5
Status reports:
- FIDO_CERTIFIED_L3plus

I think at this point I'm not going to let this block delivery of this feature. I know the code in this PR is able to parse the v3 BLOBs, statements appear to be referenced appropriately in a majority of tests...I'm going to spend a bit more time with v1.6.33 tomorrow to work on a couple of errors that seem like legitimate issues with my code, but if I eyeball the rest afterwards and it still looks like it's the test suite's fault I'll still prepare to merge this.

MasterKale commented 3 years ago

All TPM tests in v1.6.33 appear to contain invalid certificate paths, too:

Getting statement for a7d6d93a-8a0d-11e8-9a94-a6cf71072f73
Sub. Issuer:  /C=US/ST=MY/L=Wakefield/O=FIDO Alliance/OU=CWG/CN=FIDO Fake TPM Root Certificate Authority 2018/E=conformance-tools@fidoalliance.org
Iss. Subject: /CN=Sample Attestation Root/O=FIDO Alliance/OU=UAF TWG,/L=Palo Alto/ST=CA/C=US
RP - attestation: Could not validate certificate path with any metadata root certificates (TPM)

☹️

aseigler commented 3 years ago

Case in point: Metadata Service test F-1 succeeds when it should be failing:

I am experiencing same issues. My guess is that SELF_ASSERTION_SUBMITTED is considered an undesirable status, but I am not certain.

MasterKale commented 3 years ago

@aseigler I've submitted a couple of issues for these:

...In case you wanted to follow along.

MasterKale commented 3 years ago

I'm managing to pass 96% of Conformance Tests via v1.6.33 with the changes in this PR:

Screen Shot 2021-08-22 at 4 36 43 PM

Of the three tests that are failing:

All of these were mentioned above as potential issues with an experimental build of Conformance Testing Tools.

I'm choosing to interpret this as satisfactory support for MDSv3 🎉