From 0020d36c24f6694dc2997e4e81061b64f79acc1a Mon Sep 17 00:00:00 2001
From: Stefan Melmuk <stefan.melmuk@gmail.com>
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 | 44 +++++++++++++++++++++++++++--
 src/db/models/two_factor.rs         | 21 +++++++-------
 4 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 4363b866..c5c047ad 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4133,6 +4133,7 @@ dependencies = [
  "url",
  "uuid",
  "webauthn-rs",
+ "webauthn-rs-core",
  "which",
  "yubico",
 ]
diff --git a/Cargo.toml b/Cargo.toml
index d27f39cb..2633dcc9 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 3c765db1..3165fc0e 100644
--- a/src/api/core/two_factor/webauthn.rs
+++ b/src/api/core/two_factor/webauthn.rs
@@ -22,6 +22,28 @@ pub fn routes() -> Vec<Route> {
     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: Vec<u8>,
+    pub pub_key: Vec<u8>,
+    pub attestation_cert: Option<Vec<u8>>,
+    pub device_name: Option<String>,
+}
+
+#[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<bool>,
+}
+
 #[post("/two-factor/get-webauthn", data = "<data>")]
 async fn get_webauthn(data: JsonUpcase<PasswordOrOtpData>, headers: Headers, mut conn: DbConn) -> JsonResult {
     if !CONFIG.domain_set() {
@@ -168,11 +190,27 @@ async fn delete_webauthn(data: JsonUpcase<DeleteU2FData>, 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<U2FRegistration> = 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().0);
+        let new_data_str = serde_json::to_string(&data)?;
+
+        u2f.data = new_data_str;
+        u2f.save(&mut conn).await?;
+    }
+
     let keys_json: Vec<Value> = data.iter().map(WebauthnRegistration::to_json).collect();
 
     Ok(Json(json!({
@@ -183,7 +221,7 @@ async fn delete_webauthn(data: JsonUpcase<DeleteU2FData>, headers: Headers, mut
 }
 
 #[derive(Debug, Serialize, Deserialize)]
-struct WebauthnRegistration {
+pub struct WebauthnRegistration {
     pub id: i32,
     pub name: String,
     pub migrated: bool,
@@ -201,7 +239,7 @@ impl WebauthnRegistration {
     }
 }
 
-async fn get_webauthn_registrations(
+pub async fn get_webauthn_registrations(
     user_uuid: &str,
     conn: &mut DbConn,
 ) -> Result<(bool, Vec<WebauthnRegistration>), Error> {
diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs
index 530e35b4..51a7951b 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<U2FRegistration> = 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(),
+                    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);