Skip to content

Commit 1dacfac

Browse files
authored
fix(fido2authenticator): fix cipher downgrades during fido2 assertion (#1287)
Ticket: https://bitwarden.atlassian.net/browse/PM-40750 Currently, cipher assertions are inadvertently using field-level encryption, and not enrolling into cipher-keys. This is a bug, and the correct public APIs of vault should be used. `encrypt_composite` will be removed as an impl on ciphers in a subsequent PR, making the API private.
1 parent 9b83f54 commit 1dacfac

1 file changed

Lines changed: 21 additions & 27 deletions

File tree

crates/bitwarden-fido/src/authenticator.rs

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::sync::Mutex;
22

33
use bitwarden_core::Client;
44
use bitwarden_crypto::CryptoError;
5-
use bitwarden_vault::{CipherError, CipherView, EncryptionContext};
5+
use bitwarden_vault::{CipherError, CipherView, EncryptError, VaultClientExt};
66
use itertools::Itertools;
77
use passkey::{
88
authenticator::{Authenticator, DiscoverabilitySupport, StoreInfo, UiHint, UserCheck},
@@ -446,15 +446,15 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
446446
) -> Result<(), StatusCode> {
447447
#[derive(Debug, Error)]
448448
enum InnerError {
449-
#[error("Client User Id has not been set")]
450-
MissingUserId,
451449
#[error(transparent)]
452450
FillCredential(#[from] FillCredentialError),
453451
#[error(transparent)]
454452
Cipher(#[from] CipherError),
455453
#[error(transparent)]
456454
Crypto(#[from] CryptoError),
457455
#[error(transparent)]
456+
Encrypt(#[from] EncryptError),
457+
#[error(transparent)]
458458
Fido2Callback(#[from] Fido2CallbackError),
459459

460460
#[error("No selected credential available")]
@@ -469,12 +469,6 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
469469
rp: passkey::types::ctap2::make_credential::PublicKeyCredentialRpEntity,
470470
options: passkey::types::ctap2::get_assertion::Options,
471471
) -> Result<(), InnerError> {
472-
let user_id = this
473-
.authenticator
474-
.client
475-
.internal
476-
.get_user_id()
477-
.ok_or(InnerError::MissingUserId)?;
478472
let cred = try_from_credential_full(cred, user, rp, options)?;
479473

480474
// Get the previously selected cipher and add the new credential to it
@@ -498,14 +492,17 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
498492
.replace(selected.clone());
499493

500494
// Encrypt the updated cipher before sending it to the clients to be stored
501-
let encrypted = key_store.encrypt(selected)?;
495+
let encryption_context = this
496+
.authenticator
497+
.client
498+
.vault()
499+
.ciphers()
500+
.encrypt(selected)
501+
.await?;
502502

503503
this.authenticator
504504
.credential_store
505-
.save_credential(EncryptionContext {
506-
cipher: encrypted,
507-
encrypted_for: user_id,
508-
})
505+
.save_credential(encryption_context)
509506
.await?;
510507

511508
Ok(())
@@ -522,8 +519,6 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
522519
async fn update_credential(&mut self, cred: Passkey) -> Result<(), StatusCode> {
523520
#[derive(Debug, Error)]
524521
enum InnerError {
525-
#[error("Client User Id has not been set")]
526-
MissingUserId,
527522
#[error(transparent)]
528523
InvalidGuid(#[from] InvalidGuidError),
529524
#[error("Credential ID does not match selected credential")]
@@ -535,6 +530,8 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
535530
#[error(transparent)]
536531
Crypto(#[from] CryptoError),
537532
#[error(transparent)]
533+
Encrypt(#[from] EncryptError),
534+
#[error(transparent)]
538535
Fido2Callback(#[from] Fido2CallbackError),
539536
#[error(transparent)]
540537
GetSelectedCredential(#[from] GetSelectedCredentialError),
@@ -545,12 +542,6 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
545542
this: &mut CredentialStoreImpl<'_>,
546543
cred: Passkey,
547544
) -> Result<(), InnerError> {
548-
let user_id = this
549-
.authenticator
550-
.client
551-
.internal
552-
.get_user_id()
553-
.ok_or(InnerError::MissingUserId)?;
554545
// Get the previously selected cipher and update the credential
555546
let selected = this.authenticator.get_selected_credential()?;
556547

@@ -576,14 +567,17 @@ impl passkey::authenticator::CredentialStore for CredentialStoreImpl<'_> {
576567
.replace(selected.clone());
577568

578569
// Encrypt the updated cipher before sending it to the clients to be stored
579-
let encrypted = key_store.encrypt(selected)?;
570+
let encryption_context = this
571+
.authenticator
572+
.client
573+
.vault()
574+
.ciphers()
575+
.encrypt(selected)
576+
.await?;
580577

581578
this.authenticator
582579
.credential_store
583-
.save_credential(EncryptionContext {
584-
cipher: encrypted,
585-
encrypted_for: user_id,
586-
})
580+
.save_credential(encryption_context)
587581
.await?;
588582

589583
Ok(())

0 commit comments

Comments
 (0)