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

Conformance testing with FIDO-alliance tool #13

Closed
abergs opened this issue Aug 14, 2018 · 30 comments
Closed

Conformance testing with FIDO-alliance tool #13

abergs opened this issue Aug 14, 2018 · 30 comments

Comments

@abergs
Copy link
Collaborator

abergs commented Aug 14, 2018

Will update this issues with testing progress

@aseigler
Copy link
Collaborator

I consider "passing" to mean all tests pass the tool check with zero exceptions thrown for the expected pass tests and expected fail tests result in a thrown exception from the library.

These set of tests should all be passing:

ServerAuthenticatorAttestationResponse-Resp-1
ServerAuthenticatorAttestationResponse-Resp-2
ServerAuthenticatorAttestationResponse-Resp-3

@abergs
Copy link
Collaborator Author

abergs commented Aug 19, 2018

Latest run:

  • ✅Server-ServerAuthenticatorAssertionResponse-Resp-1 Test server processing ServerAuthenticatorAssertionResponse structure
  • ✅ Server-ServerAuthenticatorAssertionResponse-Resp-2 Test server processing CollectClientData
  • ❌ Server-ServerAuthenticatorAssertionResponse-Resp-3 Test server processing authenticatorData
  • ✅ Server-ServerPublicKeyCredentialGetOptionsResponse-Req-1 Test server generating ServerPublicKeyCredentialGetOptionsResponse
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-1 Test server processing ServerAuthenticatorAttestationResponse structure
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-2 Test server processing CollectClientData
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-3 Test server processing AttestationObject
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-4 Test server support of the authentication algorithms
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-6 Test server processing "packed" SELF(SURROGATE) attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-7 Test server processing "none" attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-8 Test server processing "fido-u2f" attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-9 Test server processing "tpm" attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-A Test server processing "android-key" attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-B Test server processing "android-safetynet" attestation
  • ✅ Server-ServerPublicKeyCredentialCreationOptions-Req-1 Test server generating ServerPublicKeyCredentialCreationOptionsRequest

Note: Strange errors when running Server-ServerAuthenticatorAssertionResponse-Resp-3.
image

@aseigler
Copy link
Collaborator

I am not sure I agree with some of the test results compared to the spec. I may open an issue or two against the conformance tool.

@aseigler
Copy link
Collaborator

Not mine but I'm not the only one.

fido-alliance/conformance-test-tools-resources#375

@aseigler
Copy link
Collaborator

A bunch more of these should be passing now. However, some of the failure tests are passing not because we are defending properly but because they are hitting other unexpected exceptions. It's tedious to track which ones are "good" pass and which are "bad" pass.

@abergs
Copy link
Collaborator Author

abergs commented Aug 23, 2018

Latest run:

  • ✅ Server-ServerPublicKeyCredentialCreationOptions-Req-1 Test server generating ServerPublicKeyCredentialCreationOptionsRequest
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-1 Test server processing ServerAuthenticatorAttestationResponse structure
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-2 Test server processing CollectClientData
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-3 Test server processing AttestationObject
  • ❌(1) Server-ServerAuthenticatorAttestationResponse-Resp-4 Test server support of the authentication algorithms
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation
  • ❌ (1) Server-ServerAuthenticatorAttestationResponse-Resp-6 Test server processing "packed" SELF(SURROGATE) attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-7 Test server processing "none" attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-8 Test server processing "fido-u2f" attestation
  • ❌(1) Server-ServerAuthenticatorAttestationResponse-Resp-9 Test server processing "tpm" attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-A Test server processing "android-key" attestation
  • ❌ Server-ServerAuthenticatorAttestationResponse-Resp-B Test server processing "android-safetynet" attestation
  • ✅ Server-ServerPublicKeyCredentialGetOptionsResponse-Req-1 Test server generating ServerPublicKeyCredentialGetOptionsResponse
  • ✅ Server-ServerAuthenticatorAssertionResponse-Resp-1 Test server processing ServerAuthenticatorAssertionResponse structure
  • ❌ Server-ServerAuthenticatorAssertionResponse-Resp-2 Test server processing CollectClientData
  • ❌ Server-ServerAuthenticatorAssertionResponse-Resp-3 Test server processing authenticatorData

We seem to have some regression, e.g. - ❌ Server-ServerAuthenticatorAssertionResponse-Resp-2 Test server processing CollectClientData. (please note that order of tests was changed between the versions of the compat tool)

I agree with you about finding "holes" in the defense where we catch another error in the response before we reach what test is supposed to test for. I guess the best way to completely solve that would be to add test cases in the conformance tool.

@aseigler
Copy link
Collaborator

I am of the opinion that something is broken in Server-ServerAuthenticatorAssertionResponse-Resp-3. I get the same results you do and the test does not seem to be hitting any interesting code at all.

@abergs
Copy link
Collaborator Author

abergs commented Aug 24, 2018

I've been talking to Ackermann Yuriy and sent images and a error report, hopefully they are able to correct the test.

@aseigler
Copy link
Collaborator

Lots more should be passing now. Most of Server-ServerAuthenticatorAssertionResponse-Resp-3 is still broken, but it seems that is a known issue now.

@abergs
Copy link
Collaborator Author

abergs commented Aug 29, 2018

@aseigler I'm figuring out the issue with help form Ackerman. Error message seems to be because of the naive way we store credentials, so I'm updating the testcontroller.

@abergs
Copy link
Collaborator Author

abergs commented Aug 29, 2018

@aseigler this branch (https://github.com/abergs/fido2-net-lib/tree/test-fixes) contains a commit that makes the tests pass. It's still a bit ugly and the UV/UP tests seems to fail a lot.

I pushed it to a branch because I too eagerly made a change from Raw.Id to Raw.RawId and want to fix that before merging to master but you can use it if you want to run tests.

@abergs
Copy link
Collaborator Author

abergs commented Aug 29, 2018

image

@aseigler
Copy link
Collaborator

I will take a look at the branch when I get a chance. I am quite honestly much more useful in the crypto and server side data structure handling aspects of this project than I am in the client or web aspects, I try to defer as much web related stuff as I can to your expertise.

@abergs
Copy link
Collaborator Author

abergs commented Aug 30, 2018

I agree with the vice-versa @aseigler ;) But feel free to use the branch to run the tests since they work in that branch.

@abergs
Copy link
Collaborator Author

abergs commented Aug 30, 2018

Latest run:

  • ✅ Server-ServerPublicKeyCredentialCreationOptions-Req-1 Test server generating ServerPublicKeyCredentialCreationOptionsRequest
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-1 Test server processing ServerAuthenticatorAttestationResponse structure
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-2 Test server processing CollectClientData
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-3 Test server processing AttestationObject
  • 💠 Server-ServerAuthenticatorAttestationResponse-Resp-4 Test server support of the authentication algorithms
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation
  • ❌ (1) Server-ServerAuthenticatorAttestationResponse-Resp-6 Test server processing "packed" SELF(SURROGATE) attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-7 Test server processing "none" attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-8 Test server processing "fido-u2f" attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-9 Test server processing "tpm" attestation
  • 💠 Server-ServerAuthenticatorAttestationResponse-Resp-A Test server processing "android-key" attestation
  • ✅ Server-ServerAuthenticatorAttestationResponse-Resp-B Test server processing "android-safetynet" attestation
  • ✅ Server-ServerPublicKeyCredentialGetOptionsResponse-Req-1 Test server generating ServerPublicKeyCredentialGetOptionsResponse
  • ✅ Server-ServerAuthenticatorAssertionResponse-Resp-1 Test server processing ServerAuthenticatorAssertionResponse structure
  • ✅ Server-ServerAuthenticatorAssertionResponse-Resp-2 Test server processing CollectClientData
  • ❌(2) Server-ServerAuthenticatorAssertionResponse-Resp-3 Test server processing authenticatorData

💠 Pending - What does this mean?
Server-ServerAuthenticatorAssertionResponse-Resp-3 fails on UV/UP flags.

@abergs
Copy link
Collaborator Author

abergs commented Sep 12, 2018

@aseigler Do you know what the pending things are?

@aseigler
Copy link
Collaborator

Functionally, off the top of my head, I know about these:

  1. Missing ALG_SIGN_ED25519_EDDSA_SHA512_RAW support
  2. Finish parsing Android key extensions
  3. What is going on in F-2 Send ServerAuthenticatorAttestationResponse with SELF "packed" attestation, that contains full attestation, and check that server returns an error fido-alliance/conformance-test-tools-resources#387
  4. Making use of MDS

MDS doesn't seem to have any authenticators registered with aaguid yet, but there is an update supposed to post on 18-Sept that may change that. I am a little baffled by the MDS conformance tests, they do not seem to work like the rest of the tests which are fairly straightforward.

@abergs
Copy link
Collaborator Author

abergs commented Sep 12, 2018

That's good. But I was as actually refering to the tests that report status as "pending" with the blue color. (so not "not passing" or "passing". just "pending")

@aseigler
Copy link
Collaborator

aseigler commented Sep 12, 2018

Ah, OK. Those blue marks coincide with 1 and 2 above. The first X is 3 and 4 is not referenced yet anywhere else.

@aseigler
Copy link
Collaborator

Android key should pass clean after tonight's PR. I don't see a good way to implement Ed25519 support without taking a dependency on something like https://github.com/CodesInChaos/Chaos.NaCl. There doesn't seem to be native support that I can find (and I looked, hard).

@aseigler
Copy link
Collaborator

With the metadata handling code from #42 minus the outstanding issues fido-alliance/conformance-test-tools-resources#395 and fido-alliance/conformance-test-tools-resources#396, and without figuring out EdDSA support, we should be clean for all 4 authenticator assertion and attestation tests.

@aseigler
Copy link
Collaborator

After #43 we should be clean for all server tests aside from metadata service tests, for all algorithms except ED25519. Please verify independently.

@abergs
Copy link
Collaborator Author

abergs commented Oct 8, 2018

🎉 100% test passes with all optional algorithms (except ED25519)
image

This calls for celebration. @aseigler an incredible well done job!
Closing this issue now. If you think we should add support for ED25519, please open a new issue.

@abergs abergs closed this as completed Oct 8, 2018
@aseigler
Copy link
Collaborator

aseigler commented Oct 9, 2018

I have a fix for EdDSA support I am testing here aseigler@e61e8fb and here aseigler@5743c4b.

@pham-vy
Copy link

pham-vy commented Jan 9, 2019

Need help with Metadata Service tests
At now the server does not check certificate revocation, so the F-5 (intermediate certificate is revoked) and F-6 (leaf certificate is revoked) in Metadata Service tests are failed.
But if I change the Revocation Mode to Online, the chain Status will be Revocation Status Unknown and Offline Revocation. It seems like no CRL distribution point in MDSROOT.crt
How can I get the CRL to check revocation in these test cases?

@aseigler
Copy link
Collaborator

aseigler commented Jan 9, 2019

There are links to the root and issuing CRLs here: https://fidoalliance.org/metadata/

But there is no in box way to use CRL file with X509Certificate2, and I haven't gotten the cycles to write out something that will fill the gap.

@pham-vy
Copy link

pham-vy commented Jan 12, 2019

How can I pass the case:
F-2 Send ServerAuthenticatorAttestationResponse with SELF "packed" attestation, that contains full attestation, and check that server returns an error

Already known that I have to use metadata for this case, but there is no suitable entry from metadata.

@aseigler
Copy link
Collaborator

aseigler commented Jan 12, 2019

You have to use the test metadata from the link in the conformance tool. See the custom metadata load stuff in the metadata code and the packed attestation code that interacts with metadata to see how that test was covered.

@aseigler
Copy link
Collaborator

I finally figured out how to pass ALL of the conformance tests, including the metadata service tests. You get a snazzy "submit results" button! I will document the metadata service test pass process so that it can be repeated by anyone. You do NOT need an MDS access key to pass the metadata service tests.

image

@daviddesmet
Copy link
Contributor

Really great news! 😃

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

No branches or pull requests

4 participants