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

Fix #319: CTAP2.0 bug in preflighting, which can omit credential data #320

Merged
merged 1 commit into from
Nov 18, 2023

Conversation

msirringhaus
Copy link
Contributor

@msirringhaus msirringhaus commented Nov 17, 2023

Fix bug in preflighting, where CTAP2.0 devices can omit the credential data if the allow_list is of length one. Without this fix, those answer get ignored and the device discarded as being not useable.

Adding a bunch of prefligh-tests. For this, I had to extend the mock device to be able to skip the low-level byte-by-byte comparison of incoming and outgoing data, and instead use CTAP requests and responses directly, for higher-level business-logic testing.
Now, we can add what requests we expect, and what responses the mock device should give.
For this, CTAP-responses have to be marked with their own marker trait, so that we can collect all different responses as dyn std::any::Any in a single Vec. For that, responses have to be marked 'static (required by Any), but since all of our Responses are 'static, this is currently not a problem.

And a new TestDevice-trait has been added, which is a no-op implementation in release-mode, but gives us the ability to skip the low-level serialization of requests. Only the mock-device actually implements something.
It is currently decided upon creation of the device (using the two new*()-functions), if it should skip the serialization or not.

…al data

Extend the mock device to be able to skip the low-level byte-by-byte
comparison of incoming and outgoing data, and instead use CTAP requests
and responses directly, for higher-level business-logic testing.
Add tests for preflighting.
Copy link
Collaborator

@jschanck jschanck left a comment

Choose a reason for hiding this comment

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

I would prefer to move away from mock device tests and use VirtualFidoDevice instead (#298). But that's a larger project. So this looks good to me.

@jschanck jschanck merged commit 32f8e14 into mozilla:ctap2-2021 Nov 18, 2023
8 checks passed
@xchapron-ledger
Copy link

@msirringhaus @jschanck Thanks!

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.

3 participants