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

authenticator_selection should require discoverable credentials; with optional hooks to customize by implementors #7

Closed
tcannonfodder opened this issue Jul 18, 2023 · 0 comments · Fixed by #8

Comments

@tcannonfodder
Copy link
Contributor

Currently, the authenticator_selection does not require that discoverable credentials be used.

This can be changed by updating the authenticator_selection to have resident_key: "required"

For example, this is where we'd change it in the RegistrationHelpers: https://github.com/ruby-passkeys/warden-webauthn/blob/main/lib/warden/webauthn/registration_helpers.rb#L11

authenticator_selection: { resident_key: "required", user_verification: "required" }

By default, warden-webauthn should explicitly require that the credentials must be discoverable:

Passkeys are primarily driven by the use of discoverable credentials. Discoverable credentials are a mechanism provided by the WebAuthn specification that allows for seamless authentication without the user having to provide either a username or password. In fact, WebAuthn credentials are determined to be passkeys based on their “discoverability”.
...
It’s important in our context of passkeys to focus primarily on discoverable credentials; a WebAuthn credential is not considered a passkey unless it’s discoverable.
https://developers.yubico.com/Passkeys/Passkey_concepts/Discoverable_vs_non-discoverable_credentials.html

This detail, and the desire to have the gem as secure by default, means that the resident_key should be required. Some problems with non-discoverable credentials are:

  • We would have make assumptions on what user identifier is used (eg: username, email, phone number, employee ID, etc.), which makes this gem less flexible & usable in a variety of contexts
  • It opens up the possibility for dictionary/phishing on those user identifiers. If there is an endpoint where someone can query to return a list of credentials for an identifier, they can determine if someone is using a service by the presence of credentials.

However, there is this caveat in the Yubico documentation:

Supporting non-discoverable credential flows is important for two distinct reasons:

While supporting this edge case is not something we want to implement in this gem, we should make it possible for an implementor to write their own strategy that does support it, for the cases where this truly is a need for their implementation. So we should make sure:

  • There are places to make these overrides
  • There is some documentation on how you'd make a new strategy to support this edge case

Note

"Discoverable Credentials" were previously known as "Resident Credentials", see: https://www.w3.org/TR/webauthn-2/#discoverable-credential

This is cleaning up a discussion that's been spread across a number of issues & PRs, pinging @heliocola:

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 a pull request may close this issue.

1 participant