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

MakeCredentialPinAuthProtocolTest return code mismatch #582

Closed
bunnie opened this issue Jan 30, 2023 · 3 comments
Closed

MakeCredentialPinAuthProtocolTest return code mismatch #582

bunnie opened this issue Jan 30, 2023 · 3 comments

Comments

@bunnie
Copy link

bunnie commented Jan 30, 2023

Expected Behavior

This test from the CTAP2 test tool:

https://github.com/google/CTAP2-test-tool/blob/1afd50bd1b700dc86d6dffeca70a331771d24e45/src/tests/make_credential.cc#L503-L515

Should conclude with the correct error code.

Actual Behavior

It is reporting an incorrect return code mismatch:

 Test successful: Tests the response on an empty PIN auth without a PIN in MakeCredential.
 Test successful: Tests if the PIN protocol parameter is checked in MakeCredential.
 Expected error code `CTAP2_ERR_PIN_AUTH_INVALID`, got `CTAP1_ERR_INVALID_PARAMETER`.

Steps to Reproduce the Problem

  1. Run the CTAP2-test-tool against an OpenSK implementation on the develop branch
  2. Search for the error in the log

Specifications

  • Version: develop fork
  • Platform: Precursor
bunnie added a commit to betrusted-io/xous-core that referenced this issue Jan 30, 2023
See the above issue for more details
@kaczmarczyck
Copy link
Collaborator

Hi, the error code changed between CTAP 2.0 and 2.1. OpenSK is more up to date than the Test Tool. I double checked the specification, and it matches OpenSK:
inv_param

Fixing this is part of this issue on the Test Tool. I propose to close this issue and the corresponding PR?

@kaczmarczyck
Copy link
Collaborator

One general comment: The error code checks in the Test Tool are currently too strict. The specification shuffled error codes around a bit between 2.0 and 2.1. So even a correctly implemented security key will return wrong error codes, if you compare against the wrong CTAP version.

@bunnie
Copy link
Author

bunnie commented Feb 7, 2023

Yep, sounds good. I had assumed the test tool was the ground truth, but it sounds like the test tool isn't up to date. We can close this out if that's the case.

@bunnie bunnie closed this as completed Feb 7, 2023
bunnie added a commit to betrusted-io/xous-core that referenced this issue Feb 7, 2023
Turns out the testbench failures are due to the testbench
being out of date. OpenSK is correct, the tests are wrong.

See discussions here:

google/OpenSK#586
google/OpenSK#584
google/OpenSK#582

We'll await updates to the test tool and fold those into CI,
and leave the current failures standing.
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

2 participants