From 035a843c71215ffbff0f12c2871dbc3aee27fd88 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Mon, 22 Apr 2024 21:00:28 +0200 Subject: [PATCH] fix u2f migration --- Cargo.lock | 1 + Cargo.toml | 3 +- src/api/core/two_factor/webauthn.rs | 80 +++++++++++++++++++++-------- src/db/models/two_factor.rs | 21 ++++---- 4 files changed, 73 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd0fd08c..31353895 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4143,6 +4143,7 @@ dependencies = [ "url", "uuid", "webauthn-rs", + "webauthn-rs-core", "which", "yubico", ] diff --git a/Cargo.toml b/Cargo.toml index 57ec9847..cc22f37d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -109,7 +109,8 @@ totp-lite = "2.0.1" yubico = { version = "0.11.0", features = ["online-tokio"], default-features = false } # WebAuthn libraries -webauthn-rs = { version = "0.4.8", features = ["danger-allow-state-serialisation"] } +webauthn-rs = { version = "0.4.8", features = ["danger-allow-state-serialisation", "danger-credential-internals", "resident-key-support"] } +webauthn-rs-core = { version = "0.4.9" } # Handling of URL's for WebAuthn and favicons url = "2.5.0" diff --git a/src/api/core/two_factor/webauthn.rs b/src/api/core/two_factor/webauthn.rs index 309b19ea..ce75fe6b 100644 --- a/src/api/core/two_factor/webauthn.rs +++ b/src/api/core/two_factor/webauthn.rs @@ -22,6 +22,47 @@ pub fn routes() -> Vec { routes![get_webauthn, generate_webauthn_challenge, activate_webauthn, activate_webauthn_put, delete_webauthn,] } +// Some old u2f structs still needed for migrating from u2f to WebAuthn +// Both `struct Registration` and `struct U2FRegistration` can be removed if we remove the u2f to WebAuthn migration +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Registration { + pub key_handle: CredentialID, + pub pub_key: Vec, + pub attestation_cert: Option>, + pub device_name: Option, +} + +#[derive(Serialize, Deserialize)] +pub struct U2FRegistration { + pub id: i32, + pub name: String, + #[serde(with = "Registration")] + pub reg: Registration, + pub counter: u32, + compromised: bool, + pub migrated: Option, +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct WebauthnRegistration { + pub id: i32, + pub name: String, + pub migrated: bool, + + pub security_key: SecurityKey, +} + +impl WebauthnRegistration { + fn to_json(&self) -> Value { + json!({ + "Id": self.id, + "Name": self.name, + "migrated": self.migrated, + }) + } +} + #[post("/two-factor/get-webauthn", data = "")] async fn get_webauthn(data: JsonUpcase, headers: Headers, mut conn: DbConn) -> JsonResult { if !CONFIG.domain_set() { @@ -168,11 +209,27 @@ async fn delete_webauthn(data: JsonUpcase, headers: Headers, mut None => err!("Webauthn entry not found"), }; - let _removed_item = data.remove(item_pos); + let removed_item = data.remove(item_pos); tf.data = serde_json::to_string(&data)?; tf.save(&mut conn).await?; drop(tf); + // If entry is migrated from u2f, delete the u2f entry as well + if let Some(mut u2f) = + TwoFactor::find_by_user_and_type(&headers.user.uuid, TwoFactorType::U2f as i32, &mut conn).await + { + let mut data: Vec = match serde_json::from_str(&u2f.data) { + Ok(d) => d, + Err(_) => err!("Error parsing U2F data"), + }; + + data.retain(|r| r.reg.key_handle != *removed_item.security_key.cred_id()); + let new_data_str = serde_json::to_string(&data)?; + + u2f.data = new_data_str; + u2f.save(&mut conn).await?; + } + let keys_json: Vec = data.iter().map(WebauthnRegistration::to_json).collect(); Ok(Json(json!({ @@ -182,26 +239,7 @@ async fn delete_webauthn(data: JsonUpcase, headers: Headers, mut }))) } -#[derive(Debug, Serialize, Deserialize)] -struct WebauthnRegistration { - pub id: i32, - pub name: String, - pub migrated: bool, - - pub security_key: SecurityKey, -} - -impl WebauthnRegistration { - fn to_json(&self) -> Value { - json!({ - "Id": self.id, - "Name": self.name, - "migrated": self.migrated, - }) - } -} - -async fn get_webauthn_registrations( +pub async fn get_webauthn_registrations( user_uuid: &str, conn: &mut DbConn, ) -> Result<(bool, Vec), Error> { diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index 530e35b4..777acf4a 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -159,7 +159,7 @@ impl TwoFactor { use crate::api::core::two_factor::webauthn::U2FRegistration; use crate::api::core::two_factor::webauthn::{get_webauthn_registrations, WebauthnRegistration}; - use webauthn_rs::proto::*; + use webauthn_rs_core::proto::*; for mut u2f in u2f_factors { let mut regs: Vec = serde_json::from_str(&u2f.data)?; @@ -183,22 +183,23 @@ impl TwoFactor { type_: COSEAlgorithm::ES256, key: COSEKeyType::EC_EC2(COSEEC2Key { curve: ECDSACurve::SECP256R1, - x, - y, + x: x.to_vec().into(), + y: y.to_vec().into(), }), }; + let credential = CredentialV3 { + counter: reg.counter, + verified: false, + cred: key, + cred_id: reg.reg.key_handle.clone().into(), + registration_policy: UserVerificationPolicy::Preferred, + }; let new_reg = WebauthnRegistration { id: reg.id, migrated: true, name: reg.name.clone(), - credential: Credential { - counter: reg.counter, - verified: false, - cred: key, - cred_id: reg.reg.key_handle.clone(), - registration_policy: UserVerificationPolicy::Discouraged, - }, + security_key: (Credential::from(credential).into()), }; webauthn_regs.push(new_reg);