Fix UV-skipping logic with required and preferred - #374
Open
micolous wants to merge 3 commits into
Open
Conversation
* Update `AuthenticatorOptions::user_verification` docs to use the CTAP 2.3 description, and note some caveats. * Add `AuthenticatorOptions::supports_uv()` helper.
…uraged and 'possible'.
* Don't skip UV when `uv = required` * Don't skip UV when `uv != discouraged` and UV is "possible" but unconfigured This is rewritten to be closer to the CTAP 2.3 spec's steps.
7 tasks
Collaborator
Author
|
CI failure looks unrelated, opened #375 for that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #373, matching the behaviour defined in the CTAP and WebAuthn specs:
Don't skip UV when
uv = required.Previously, UV would be skipped when a device did not have some form of UV configured, and required the RPs to reject the request (which they need to do anyway for security reasons).
Don't skip UV when
uv = preferredand an authenticator supports any form of user verification.Previously, this would be skipped when a device did not have some form of UV configured.
UV is still skipped for authenticators that don't support any form of user verification (CTAP 1, and some CTAP 2.0 devices). Supporting UV is mandatory in CTAP 2.1 and later, but this doesn't enforce those rules.
Unfortunately, this change will cause an authenticator that supports UV but is unconfigured to now fail those operations in Firefox, because it doesn't prompt to set PINs at enrolment time. These would previously proceed without UV.
Refactor the
can_skip_user_verificationchecks to make them closer to CTAP 2.3's validation steps.Update docs for
AuthenticatorOptions::user_verificationto use CTAP 2.3's wording, noting caveats with CTAP 2.0 and reformat it to look nicer inrustdoc.Testing
I tested this with the
ctap2.rsexample. All changes in bold marked with ⭐. Any authenticator options not specified are missing.clientPin,alwaysUv,!makeCredUvNotRqdclientPin,!alwaysUv,makeCredUvNotRqd!clientPin,!alwaysUv,makeCredUvNotRqdclientPin!clientPinclientPin!clientPin