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

[PoC] Add biometrics authentication method #9681

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

vxgmichel
Copy link
Contributor

Port of #6557

The new authentication method is called biometrics because this is how bitwarden calls it.

In practice it's an interface to Windows Hello, and more specifically the Windows.Security.Credentials.KeyCredential API.

To quote the docs:

An application cannot provision a key credential until the user has successfully done the following:

  • Connected their user account to their Microsoft account.
  • Provided an unlock gesture (PIN or biometric) to protect the container that stores their key credential.

That means that the parsec device key file becomes protected by the machine TPM which gets unlocked by either facial recognition, fingerprint recognition, or PIN code.

The is_biometrics_available function is exposed to specifically check those requirements.

This PR also adds the parsec device change-authentication method for testing purposes.

Logging using the biometrics access strategy is also supported in the electron app, but it's not yet available when saving a new device. In practice it would make sense to use biometrics when available and fall back to keyring if necessary, without exposing both to the user.

@vxgmichel
Copy link
Contributor Author

Reviewers should pay attention and address those TODOs:

  1. Focusing the windows security prompt:
        // TODO: This Alt key hack has been recently removed from bitwarden:
        //   https://github.com/bitwarden/clients/pull/12255
        // We should investigate if it's still necessary, or if the approach can be improved.
  1. Identifying the credential using its public key:
    // TODO: The approach here is to create a new key credential if it doesn't exist.
    // This allows us to expose only a single `derive_key_from_biometrics` API for
    // both loading and saving.
    // However, this means we cannot detect when an existing key credential has been
    // removed from the system (e.g when the pin code has been removed and recreated).
    // Another approach would be to retrieve the corresponding public key when saving
    // a new device using `KeyCredential::RetrievePublicKey()`. This way, this information
    // could be stored in the device key file and use it to detect when the key credential
    // has been removed or changed.
  1. Use an async API to not block during the prompt:
// TODO: this function is synchronous but will block the thread while waiting for the
// user to enter this pin code. Either this function should be async or we make sure
// the caller uses `tokio::task::spawn_blocking`
  1. Choose a challenge format:
    // TODO: Is this the formatting we want for the challenge?
    // Is the device ID the right thing to use here?
    let challenge = format!("BIOMETRICS_CHALLENGE:{}", device_id.hex());

@vxgmichel vxgmichel marked this pull request as ready for review February 16, 2025 15:41
@vxgmichel vxgmichel requested review from a team as code owners February 16, 2025 15:41
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.

1 participant