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 support for pinUvAuthProtocol(0x04) in HmacSecretInput #1

Closed
wants to merge 2 commits into from

Conversation

robin-nitrokey
Copy link
Member

Submitted to upstream as trussed-dev#1 but duplicated here for an early release.

The deserialization of authenticatorGetAssertion requests using the
hmac-secret extension as sent by e. g. python-fido2 currently fails.
This patch adds some tests with the offending input.
The December 08, 2020 version of the Client to Authenticator Protocol
(CTAP) specification added the pinUvAuthProtocol(0x04) field to the
input data for the hmac-secret extension in the
authenticatorGetAssertion command.  This patch adds this new field to
the HmacSecretInput struct to make it possible to parse requests using
this new field.
	https://fidoalliance.org/specs/fido-v2.1-rd-20201208/fido-client-to-authenticator-protocol-v2.1-rd-20201208.html

Fixes #1.
Copy link
Member

@szszszsz szszszsz left a comment

Choose a reason for hiding this comment

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

Looks good!

Potential improvements:

  • describe input data for tests (it's not immediately obvious), or
  • better would be to provide the input structures in human-readable form and serialize them to CBOR (e.g. JSON string converted during tests execution to CBOR).

@robin-nitrokey
Copy link
Member Author

Merged upstream in trussed-dev#2.

@szszszsz
Copy link
Member

@robin-nitrokey Please check and remove the hmac-secret branch, if it is not needed anymore

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

Successfully merging this pull request may close these issues.

2 participants