Fix Webauthn/Passkey 2FA migration/validation issues (#6190)

* Apply Passkey fixes from zUnixorn

Applied SecurityKey to Passkey fixes from @zUnixorn

Co-authored-by: zUnixorn <77864446+zUnixorn@users.noreply.github.com>

Signed-off-by: BlackDex <black.dex@gmail.com>

* Fix Webauthn/Passkey 2FA migration issues

Because the webauthn-rs v0.3 crate did not know or store new flags currently used in v0.5, some verifications failed.
This mainly failed because of a check if a key was backuped or not, and if it was allowed to do so.

Most hardware keys like YubiKey's do not have this flag enabled and can't be duplicated or faked via software.
Since the rise of Passkey's, like Bitwarden's own implementation, and other platforms like Android, and Apple use Software keys which are shared between devices, they set these backup flags to true. This broke the login attempts, because the default during the migration was `false`, and cause an error during validation.

This PR checks for the flags during the response/verification step, and if these flags are `true`, then search for the stored key, adjust it's value, and also update the current challenge state to match, to prevent the first login attempt to fail.

This should not cause any issue, since the credential-id is checked and matched, and only updated when needed.

Fixes #6154

Signed-off-by: BlackDex <black.dex@gmail.com>

* Fix comments

Signed-off-by: BlackDex <black.dex@gmail.com>

---------

Signed-off-by: BlackDex <black.dex@gmail.com>

Cargo.toml

@@ -126,8 +126,7 @@ yubico = { package = "yubico_ng", version = "0.14.1", features = ["online-tokio"
 # WebAuthn libraries
 # danger-allow-state-serialisation is needed to save the state in the db
 # danger-credential-internals is needed to support U2F to Webauthn migration
-# danger-user-presence-only-security-keys is needed to disable UV
+webauthn-rs = { version = "0.5.2", features = ["danger-allow-state-serialisation", "danger-credential-internals"] }
-webauthn-rs = { version = "0.5.2", features = ["danger-allow-state-serialisation", "danger-credential-internals", "danger-user-presence-only-security-keys"] }
 webauthn-rs-proto = "0.5.2"
 webauthn-rs-core = "0.5.2"
 

src/api/core/two_factor/webauthn.rs

@@ -21,12 +21,12 @@ use std::sync::{Arc, LazyLock};
 use std::time::Duration;
 use url::Url;
 use uuid::Uuid;
-use webauthn_rs::prelude::{Base64UrlSafeData, SecurityKey, SecurityKeyAuthentication, SecurityKeyRegistration};
+use webauthn_rs::prelude::{Base64UrlSafeData, Credential, Passkey, PasskeyAuthentication, PasskeyRegistration};
 use webauthn_rs::{Webauthn, WebauthnBuilder};
 use webauthn_rs_proto::{
     AuthenticationExtensionsClientOutputs, AuthenticatorAssertionResponseRaw, AuthenticatorAttestationResponseRaw,
     PublicKeyCredential, RegisterPublicKeyCredential, RegistrationExtensionsClientOutputs,
-    RequestAuthenticationExtensions,
+    RequestAuthenticationExtensions, UserVerificationPolicy,
 };
 
 pub static WEBAUTHN_2FA_CONFIG: LazyLock<Arc<Webauthn>> = LazyLock::new(|| {
@@ -38,8 +38,7 @@ pub static WEBAUTHN_2FA_CONFIG: LazyLock<Arc<Webauthn>> = LazyLock::new(|| {
     let webauthn = WebauthnBuilder::new(&rp_id, &rp_origin)
         .expect("Creating WebauthnBuilder failed")
         .rp_name(&domain)
-        .timeout(Duration::from_millis(60000))
+        .timeout(Duration::from_millis(60000));
-        .danger_set_user_presence_only_security_keys(true);
 
     Arc::new(webauthn.build().expect("Building Webauthn failed"))
 });
@@ -78,7 +77,7 @@ pub struct WebauthnRegistration {
     pub name: String,
     pub migrated: bool,
 
-    pub credential: SecurityKey,
+    pub credential: Passkey,
 }
 
 impl WebauthnRegistration {
@@ -89,6 +88,24 @@ impl WebauthnRegistration {
             "migrated": self.migrated,
         })
     }
+
+    fn set_backup_eligible(&mut self, backup_eligible: bool, backup_state: bool) -> bool {
+        let mut changed = false;
+        let mut cred: Credential = self.credential.clone().into();
+
+        if cred.backup_state != backup_state {
+            cred.backup_state = backup_state;
+            changed = true;
+        }
+
+        if backup_eligible && !cred.backup_eligible {
+            cred.backup_eligible = true;
+            changed = true;
+        }
+
+        self.credential = cred.into();
+        changed
+    }
 }
 
 #[post("/two-factor/get-webauthn", data = "<data>")]
@@ -131,18 +148,27 @@ async fn generate_webauthn_challenge(
         .map(|r| r.credential.cred_id().to_owned()) // We return the credentialIds to the clients to avoid double registering
         .collect();
 
-    let (challenge, state) = webauthn.start_securitykey_registration(
+    let (mut challenge, state) = webauthn.start_passkey_registration(
         Uuid::from_str(&user.uuid).expect("Failed to parse UUID"), // Should never fail
         &user.email,
         &user.name,
         Some(registrations),
-        None,
-        None,
     )?;
 
+    let mut state = serde_json::to_value(&state)?;
+    state["rs"]["policy"] = Value::String("discouraged".to_string());
+    state["rs"]["extensions"].as_object_mut().unwrap().clear();
+
     let type_ = TwoFactorType::WebauthnRegisterChallenge;
     TwoFactor::new(user.uuid.clone(), type_, serde_json::to_string(&state)?).save(&mut conn).await?;
 
+    // Because for this flow we abuse the passkeys as 2FA, and use it more like a securitykey
+    // we need to modify some of the default settings defined by `start_passkey_registration()`.
+    challenge.public_key.extensions = None;
+    if let Some(asc) = challenge.public_key.authenticator_selection.as_mut() {
+        asc.user_verification = UserVerificationPolicy::Discouraged_DO_NOT_USE;
+    }
+
     let mut challenge_value = serde_json::to_value(challenge.public_key)?;
     challenge_value["status"] = "ok".into();
     challenge_value["errorMessage"] = "".into();
@@ -253,7 +279,7 @@ async fn activate_webauthn(
     let type_ = TwoFactorType::WebauthnRegisterChallenge as i32;
     let state = match TwoFactor::find_by_user_and_type(&user.uuid, type_, &mut conn).await {
         Some(tf) => {
-            let state: SecurityKeyRegistration = serde_json::from_str(&tf.data)?;
+            let state: PasskeyRegistration = serde_json::from_str(&tf.data)?;
             tf.delete(&mut conn).await?;
             state
         }
@@ -261,7 +287,7 @@ async fn activate_webauthn(
     };
 
     // Verify the credentials with the saved state
-    let credential = webauthn.finish_securitykey_registration(&data.device_response.into(), &state)?;
+    let credential = webauthn.finish_passkey_registration(&data.device_response.into(), &state)?;
 
     let mut registrations: Vec<_> = get_webauthn_registrations(&user.uuid, &mut conn).await?.1;
     // TODO: Check for repeated ID's
@@ -372,21 +398,25 @@ pub async fn generate_webauthn_login(
     conn: &mut DbConn,
 ) -> JsonResult {
     // Load saved credentials
-    let creds: Vec<_> = get_webauthn_registrations(user_id, conn).await?.1.into_iter().map(|r| r.credential).collect();
+    let creds: Vec<Passkey> =
+        get_webauthn_registrations(user_id, conn).await?.1.into_iter().map(|r| r.credential).collect();
 
     if creds.is_empty() {
         err!("No Webauthn devices registered")
     }
 
     // Generate a challenge based on the credentials
-    let (mut response, state) = webauthn.start_securitykey_authentication(&creds)?;
+    let (mut response, state) = webauthn.start_passkey_authentication(&creds)?;
 
     // Modify to discourage user verification
     let mut state = serde_json::to_value(&state)?;
+    state["ast"]["policy"] = Value::String("discouraged".to_string());
 
     // Add appid, this is only needed for U2F compatibility, so maybe it can be removed as well
     let app_id = format!("{}/app-id.json", &CONFIG.domain());
     state["ast"]["appid"] = Value::String(app_id.clone());
+
+    response.public_key.user_verification = UserVerificationPolicy::Discouraged_DO_NOT_USE;
     response
         .public_key
         .extensions
@@ -413,9 +443,9 @@ pub async fn validate_webauthn_login(
     conn: &mut DbConn,
 ) -> EmptyResult {
     let type_ = TwoFactorType::WebauthnLoginChallenge as i32;
-    let state = match TwoFactor::find_by_user_and_type(user_id, type_, conn).await {
+    let mut state = match TwoFactor::find_by_user_and_type(user_id, type_, conn).await {
         Some(tf) => {
-            let state: SecurityKeyAuthentication = serde_json::from_str(&tf.data)?;
+            let state: PasskeyAuthentication = serde_json::from_str(&tf.data)?;
             tf.delete(conn).await?;
             state
         }
@@ -432,7 +462,12 @@ pub async fn validate_webauthn_login(
 
     let mut registrations = get_webauthn_registrations(user_id, conn).await?.1;
 
-    let authentication_result = webauthn.finish_securitykey_authentication(&rsp, &state)?;
+    // We need to check for and update the backup_eligible flag when needed.
+    // Vaultwarden did not have knowledge of this flag prior to migrating to webauthn-rs v0.5.x
+    // Because of this we check the flag at runtime and update the registrations and state when needed
+    check_and_update_backup_eligible(user_id, &rsp, &mut registrations, &mut state, conn).await?;
+
+    let authentication_result = webauthn.finish_passkey_authentication(&rsp, &state)?;
 
     for reg in &mut registrations {
         if ct_eq(reg.credential.cred_id(), authentication_result.cred_id()) {
@@ -454,3 +489,66 @@ pub async fn validate_webauthn_login(
         }
     )
 }
+
+async fn check_and_update_backup_eligible(
+    user_id: &UserId,
+    rsp: &PublicKeyCredential,
+    registrations: &mut Vec<WebauthnRegistration>,
+    state: &mut PasskeyAuthentication,
+    conn: &mut DbConn,
+) -> EmptyResult {
+    // The feature flags from the response
+    // For details see: https://www.w3.org/TR/webauthn-3/#sctn-authenticator-data
+    const FLAG_BACKUP_ELIGIBLE: u8 = 0b0000_1000;
+    const FLAG_BACKUP_STATE: u8 = 0b0001_0000;
+
+    if let Some(bits) = rsp.response.authenticator_data.get(32) {
+        let backup_eligible = 0 != (bits & FLAG_BACKUP_ELIGIBLE);
+        let backup_state = 0 != (bits & FLAG_BACKUP_STATE);
+
+        // If the current key is backup eligible, then we probably need to update one of the keys already stored in the database
+        // This is needed because Vaultwarden didn't store this information when using the previous version of webauthn-rs since it was a new addition to the protocol
+        // Because we store multiple keys in one json string, we need to fetch the correct key first, and update its information before we let it verify
+        if backup_eligible {
+            let rsp_id = rsp.raw_id.as_slice();
+            for reg in &mut *registrations {
+                if ct_eq(reg.credential.cred_id().as_slice(), rsp_id) {
+                    // Try to update the key, and if needed also update the database, before the actual state check is done
+                    if reg.set_backup_eligible(backup_eligible, backup_state) {
+                        TwoFactor::new(
+                            user_id.clone(),
+                            TwoFactorType::Webauthn,
+                            serde_json::to_string(&registrations)?,
+                        )
+                        .save(conn)
+                        .await?;
+
+                        // We also need to adjust the current state which holds the challenge used to start the authentication verification
+                        // Because Vaultwarden supports multiple keys, we need to loop through the deserialized state and check which key to update
+                        let mut raw_state = serde_json::to_value(&state)?;
+                        if let Some(credentials) = raw_state
+                            .get_mut("ast")
+                            .and_then(|v| v.get_mut("credentials"))
+                            .and_then(|v| v.as_array_mut())
+                        {
+                            for cred in credentials.iter_mut() {
+                                if cred.get("cred_id").is_some_and(|v| {
+                                    // Deserialize to a [u8] so it can be compared using `ct_eq` with the `rsp_id`
+                                    let cred_id_slice: Base64UrlSafeData = serde_json::from_value(v.clone()).unwrap();
+                                    ct_eq(cred_id_slice, rsp_id)
+                                }) {
+                                    cred["backup_eligible"] = Value::Bool(backup_eligible);
+                                    cred["backup_state"] = Value::Bool(backup_state);
+                                }
+                            }
+                        }
+
+                        *state = serde_json::from_value(raw_state)?;
+                    }
+                    break;
+                }
+            }
+        }
+    }
+    Ok(())
+}