From 21b8a0a334337138b2ad075420fab434a632fc39 Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Tue, 2 Apr 2024 02:28:52 +0200 Subject: [PATCH 1/4] remove u2f to webauthn migration --- src/db/models/two_factor.rs | 69 ------------------------------------- src/main.rs | 1 - 2 files changed, 70 deletions(-) diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index 530e35b4..ff353ac2 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -147,73 +147,4 @@ impl TwoFactor { .map_res("Error deleting twofactors") }} } - - pub async fn migrate_u2f_to_webauthn(conn: &mut DbConn) -> EmptyResult { - let u2f_factors = db_run! { conn: { - twofactor::table - .filter(twofactor::atype.eq(TwoFactorType::U2f as i32)) - .load::(conn) - .expect("Error loading twofactor") - .from_db() - }}; - - use crate::api::core::two_factor::webauthn::U2FRegistration; - use crate::api::core::two_factor::webauthn::{get_webauthn_registrations, WebauthnRegistration}; - use webauthn_rs::proto::*; - - for mut u2f in u2f_factors { - let mut regs: Vec = serde_json::from_str(&u2f.data)?; - // If there are no registrations or they are migrated (we do the migration in batch so we can consider them all migrated when the first one is) - if regs.is_empty() || regs[0].migrated == Some(true) { - continue; - } - - let (_, mut webauthn_regs) = get_webauthn_registrations(&u2f.user_uuid, conn).await?; - - // If the user already has webauthn registrations saved, don't overwrite them - if !webauthn_regs.is_empty() { - continue; - } - - for reg in &mut regs { - let x: [u8; 32] = reg.reg.pub_key[1..33].try_into().unwrap(); - let y: [u8; 32] = reg.reg.pub_key[33..65].try_into().unwrap(); - - let key = COSEKey { - type_: COSEAlgorithm::ES256, - key: COSEKeyType::EC_EC2(COSEEC2Key { - curve: ECDSACurve::SECP256R1, - x, - y, - }), - }; - - 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, - }, - }; - - webauthn_regs.push(new_reg); - - reg.migrated = Some(true); - } - - u2f.data = serde_json::to_string(®s)?; - u2f.save(conn).await?; - - TwoFactor::new(u2f.user_uuid.clone(), TwoFactorType::Webauthn, serde_json::to_string(&webauthn_regs)?) - .save(conn) - .await?; - } - - Ok(()) - } } diff --git a/src/main.rs b/src/main.rs index 12953979..f2784d9d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,7 +88,6 @@ async fn main() -> Result<(), Error> { let pool = create_db_pool().await; schedule_jobs(pool.clone()); - crate::db::models::TwoFactor::migrate_u2f_to_webauthn(&mut pool.get().await.unwrap()).await.unwrap(); launch_rocket(pool, extra_debug).await // Blocks until program termination. } From 8b3a3a15243c40e2c877e3b8eb9ec56276fb39fd Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Sat, 6 Apr 2024 11:33:25 +0200 Subject: [PATCH 2/4] update webauthn-rs crate --- Cargo.lock | 192 ++++++++++++++++++++- Cargo.toml | 2 +- src/api/core/two_factor/webauthn.rs | 251 +++++----------------------- src/config.rs | 18 ++ src/error.rs | 4 +- src/main.rs | 2 +- 6 files changed, 251 insertions(+), 218 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1afed44f..4363b866 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -86,6 +86,45 @@ dependencies = [ "password-hash", ] +[[package]] +name = "asn1-rs" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30ff05a702273012438132f449575dbc804e27b2f3cbe3069aa237d26c98fa33" +dependencies = [ + "asn1-rs-derive", + "asn1-rs-impl", + "displaydoc", + "nom", + "num-traits", + "rusticata-macros", + "thiserror", + "time", +] + +[[package]] +name = "asn1-rs-derive" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db8b7511298d5b7784b40b092d9e9dcd3a627a5707e4b5e507931ab0d44eeebf" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", + "synstructure", +] + +[[package]] +name = "asn1-rs-impl" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2777730b2039ac0f95f093556e61b6d26cebed5393ca6f152717777cec3a42ed" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "async-channel" version = "1.9.0" @@ -379,6 +418,17 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c3c1a368f70d6cf7302d78f8f7093da241fb8e8807c05cc9e51a125895a6d5b" +[[package]] +name = "base64urlsafedata" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18b3d30abb74120a9d5267463b9e0045fdccc4dd152e7249d966612dc1721384" +dependencies = [ + "base64 0.21.7", + "serde", + "serde_json", +] + [[package]] name = "bigdecimal" version = "0.4.3" @@ -582,6 +632,23 @@ dependencies = [ "stacker", ] +[[package]] +name = "compact_jwt" +version = "0.2.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7aa76ef19968577838a34d02848136bb9b6bdbfd7675fb968fe9c931bc434b33" +dependencies = [ + "base64 0.13.1", + "base64urlsafedata", + "hex", + "openssl", + "serde", + "serde_json", + "tracing", + "url", + "uuid", +] + [[package]] name = "concurrent-queue" version = "2.4.0" @@ -768,6 +835,20 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c297a1c74b71ae29df00c3e22dd9534821d60eb9af5a0192823fa2acea70c2a" +[[package]] +name = "der-parser" +version = "7.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe398ac75057914d7d07307bf67dc7f3f574a26783b4fc7805a20ffa9f506e82" +dependencies = [ + "asn1-rs", + "displaydoc", + "nom", + "num-bigint", + "num-traits", + "rusticata-macros", +] + [[package]] name = "deranged" version = "0.3.11" @@ -887,6 +968,17 @@ dependencies = [ "subtle", ] +[[package]] +name = "displaydoc" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.58", +] + [[package]] name = "dotenvy" version = "0.15.7" @@ -1388,6 +1480,12 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d231dfb89cfffdbc30e7fc41579ed6066ad03abda9e567ccafae602b97ec5024" +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "hickory-proto" version = "0.24.0" @@ -2181,6 +2279,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "oid-registry" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38e20717fa0541f39bd146692035c37bedfa532b3e5071b35761082407546b2a" +dependencies = [ + "asn1-rs", +] + [[package]] name = "once_cell" version = "1.19.0" @@ -3011,6 +3118,15 @@ version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" +[[package]] +name = "rusticata-macros" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "faf0c4a6ece9950b9abdb62b1cfcf2a68b3b67a10ba445b3bb85be2a293d0632" +dependencies = [ + "nom", +] + [[package]] name = "rustix" version = "0.37.27" @@ -3185,10 +3301,10 @@ dependencies = [ ] [[package]] -name = "serde_cbor" -version = "0.11.2" +name = "serde_cbor_2" +version = "0.12.0-dev" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bef2ebfde456fb76bbcf9f59315333decc4fda0b2b44b420243c11e0f5ec1f5" +checksum = "b46d75f449e01f1eddbe9b00f432d616fbbd899b809c837d0fbc380496a0dd55" dependencies = [ "half", "serde", @@ -3426,6 +3542,18 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +[[package]] +name = "synstructure" +version = "0.12.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", + "unicode-xid", +] + [[package]] name = "syslog" version = "6.1.0" @@ -3933,6 +4061,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a183cf7feeba97b4dd1c0d46788634f6221d87fa961b305bed08c851829efcc0" dependencies = [ "getrandom", + "serde", ] [[package]] @@ -4142,21 +4271,52 @@ dependencies = [ [[package]] name = "webauthn-rs" -version = "0.3.2" +version = "0.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90b266eccb4b32595876f5c73ea443b0516da0b1df72ca07bc08ed9ba7f96ec1" +checksum = "2db00711c712414e93b019c4596315085792215bc2ac2d5872f9e8913b0a6316" +dependencies = [ + "base64urlsafedata", + "serde", + "tracing", + "url", + "uuid", + "webauthn-rs-core", +] + +[[package]] +name = "webauthn-rs-core" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "294c78c83f12153a51e1cf1e6970b5da1397645dada39033a9c3173a8fc4fc2b" dependencies = [ "base64 0.13.1", + "base64urlsafedata", + "compact_jwt", + "der-parser", "nom", "openssl", "rand", "serde", - "serde_cbor", - "serde_derive", + "serde_cbor_2", "serde_json", "thiserror", "tracing", "url", + "uuid", + "webauthn-rs-proto", + "x509-parser", +] + +[[package]] +name = "webauthn-rs-proto" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d24e638361a63ba5c0a0be6a60229490fcdf33740ed63df5bb6bdb627b52a138" +dependencies = [ + "base64urlsafedata", + "serde", + "serde_json", + "url", ] [[package]] @@ -4402,6 +4562,24 @@ version = "0.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" +[[package]] +name = "x509-parser" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fb9bace5b5589ffead1afb76e43e34cff39cd0f3ce7e170ae0c29e53b88eb1c" +dependencies = [ + "asn1-rs", + "base64 0.13.1", + "data-encoding", + "der-parser", + "lazy_static", + "nom", + "oid-registry", + "rusticata-macros", + "thiserror", + "time", +] + [[package]] name = "yansi" version = "1.0.1" diff --git a/Cargo.toml b/Cargo.toml index 16546a7f..d27f39cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -109,7 +109,7 @@ totp-lite = "2.0.1" yubico = { version = "0.11.0", features = ["online-tokio"], default-features = false } # WebAuthn libraries -webauthn-rs = "0.3.2" +webauthn-rs = { version = "0.4.8", features = ["danger-allow-state-serialisation"] } # 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 14ba8514..3c765db1 100644 --- a/src/api/core/two_factor/webauthn.rs +++ b/src/api/core/two_factor/webauthn.rs @@ -1,8 +1,7 @@ use rocket::serde::json::Json; use rocket::Route; use serde_json::Value; -use url::Url; -use webauthn_rs::{base64_data::Base64UrlSafeData, proto::*, AuthenticationState, RegistrationState, Webauthn}; +use webauthn_rs::prelude::*; use crate::{ api::{ @@ -16,93 +15,13 @@ use crate::{ }, error::Error, util::NumberOrString, - CONFIG, + CONFIG, WEBAUTHN, }; 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: Vec, - 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, -} - -struct WebauthnConfig { - url: String, - origin: Url, - rpid: String, -} - -impl WebauthnConfig { - fn load() -> Webauthn { - let domain = CONFIG.domain(); - let domain_origin = CONFIG.domain_origin(); - Webauthn::new(Self { - rpid: Url::parse(&domain).map(|u| u.domain().map(str::to_owned)).ok().flatten().unwrap_or_default(), - url: domain, - origin: Url::parse(&domain_origin).unwrap(), - }) - } -} - -impl webauthn_rs::WebauthnConfig for WebauthnConfig { - fn get_relying_party_name(&self) -> &str { - &self.url - } - - fn get_origin(&self) -> &Url { - &self.origin - } - - fn get_relying_party_id(&self) -> &str { - &self.rpid - } - - /// We have WebAuthn configured to discourage user verification - /// if we leave this enabled, it will cause verification issues when a keys send UV=1. - /// Upstream (the library they use) ignores this when set to discouraged, so we should too. - fn get_require_uv_consistency(&self) -> bool { - false - } -} - -#[derive(Debug, Serialize, Deserialize)] -pub struct WebauthnRegistration { - pub id: i32, - pub name: String, - pub migrated: bool, - - pub credential: Credential, -} - -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() { @@ -135,21 +54,16 @@ async fn generate_webauthn_challenge( data.validate(&user, false, &mut conn).await?; - let registrations = get_webauthn_registrations(&user.uuid, &mut conn) + let registrations: Vec = get_webauthn_registrations(&user.uuid, &mut conn) .await? .1 .into_iter() - .map(|r| r.credential.cred_id) // We return the credentialIds to the clients to avoid double registering + .map(|r| r.security_key.cred_id().clone()) // We return the credentialIds to the clients to avoid double registering .collect(); - let (challenge, state) = WebauthnConfig::load().generate_challenge_register_options( - user.uuid.as_bytes().to_vec(), - user.email, - user.name, - Some(registrations), - None, - None, - )?; + let user_uuid = Uuid::parse_str(&user.uuid)?; + let (challenge, state) = + WEBAUTHN.start_securitykey_registration(user_uuid, &user.email, &user.name, Some(registrations), None, None)?; let type_ = TwoFactorType::WebauthnRegisterChallenge; TwoFactor::new(user.uuid, type_, serde_json::to_string(&state)?).save(&mut conn).await?; @@ -165,90 +79,11 @@ async fn generate_webauthn_challenge( struct EnableWebauthnData { Id: NumberOrString, // 1..5 Name: String, - DeviceResponse: RegisterPublicKeyCredentialCopy, + DeviceResponse: RegisterPublicKeyCredential, MasterPasswordHash: Option, Otp: Option, } -// This is copied from RegisterPublicKeyCredential to change the Response objects casing -#[derive(Debug, Deserialize)] -#[allow(non_snake_case)] -struct RegisterPublicKeyCredentialCopy { - pub Id: String, - pub RawId: Base64UrlSafeData, - pub Response: AuthenticatorAttestationResponseRawCopy, - pub Type: String, -} - -// This is copied from AuthenticatorAttestationResponseRaw to change clientDataJSON to clientDataJson -#[derive(Debug, Deserialize)] -#[allow(non_snake_case)] -pub struct AuthenticatorAttestationResponseRawCopy { - pub AttestationObject: Base64UrlSafeData, - pub ClientDataJson: Base64UrlSafeData, -} - -impl From for RegisterPublicKeyCredential { - fn from(r: RegisterPublicKeyCredentialCopy) -> Self { - Self { - id: r.Id, - raw_id: r.RawId, - response: AuthenticatorAttestationResponseRaw { - attestation_object: r.Response.AttestationObject, - client_data_json: r.Response.ClientDataJson, - }, - type_: r.Type, - } - } -} - -// This is copied from PublicKeyCredential to change the Response objects casing -#[derive(Debug, Deserialize)] -#[allow(non_snake_case)] -pub struct PublicKeyCredentialCopy { - pub Id: String, - pub RawId: Base64UrlSafeData, - pub Response: AuthenticatorAssertionResponseRawCopy, - pub Extensions: Option, - pub Type: String, -} - -// This is copied from AuthenticatorAssertionResponseRaw to change clientDataJSON to clientDataJson -#[derive(Debug, Deserialize)] -#[allow(non_snake_case)] -pub struct AuthenticatorAssertionResponseRawCopy { - pub AuthenticatorData: Base64UrlSafeData, - pub ClientDataJson: Base64UrlSafeData, - pub Signature: Base64UrlSafeData, - pub UserHandle: Option, -} - -#[derive(Debug, Deserialize)] -#[allow(non_snake_case)] -pub struct AuthenticationExtensionsClientOutputsCopy { - #[serde(default)] - pub Appid: bool, -} - -impl From for PublicKeyCredential { - fn from(r: PublicKeyCredentialCopy) -> Self { - Self { - id: r.Id, - raw_id: r.RawId, - response: AuthenticatorAssertionResponseRaw { - authenticator_data: r.Response.AuthenticatorData, - client_data_json: r.Response.ClientDataJson, - signature: r.Response.Signature, - user_handle: r.Response.UserHandle, - }, - extensions: r.Extensions.map(|e| AuthenticationExtensionsClientOutputs { - appid: e.Appid, - }), - type_: r.Type, - } - } -} - #[post("/two-factor/webauthn", data = "")] async fn activate_webauthn(data: JsonUpcase, headers: Headers, mut conn: DbConn) -> JsonResult { let data: EnableWebauthnData = data.into_inner().data; @@ -265,7 +100,7 @@ async fn activate_webauthn(data: JsonUpcase, headers: Header 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: RegistrationState = serde_json::from_str(&tf.data)?; + let state: SecurityKeyRegistration = serde_json::from_str(&tf.data)?; tf.delete(&mut conn).await?; state } @@ -273,8 +108,7 @@ async fn activate_webauthn(data: JsonUpcase, headers: Header }; // Verify the credentials with the saved state - let (credential, _data) = - WebauthnConfig::load().register_credential(&data.DeviceResponse.into(), &state, |_| Ok(false))?; + let security_key = WEBAUTHN.finish_securitykey_registration(&data.DeviceResponse, &state)?; let mut registrations: Vec<_> = get_webauthn_registrations(&user.uuid, &mut conn).await?.1; // TODO: Check for repeated ID's @@ -283,7 +117,7 @@ async fn activate_webauthn(data: JsonUpcase, headers: Header name: data.Name, migrated: false, - credential, + security_key, }); // Save the registrations and return them @@ -334,27 +168,11 @@ 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.credential.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!({ @@ -364,7 +182,26 @@ async fn delete_webauthn(data: JsonUpcase, headers: Headers, mut }))) } -pub async fn get_webauthn_registrations( +#[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( user_uuid: &str, conn: &mut DbConn, ) -> Result<(bool, Vec), Error> { @@ -377,16 +214,16 @@ pub async fn get_webauthn_registrations( pub async fn generate_webauthn_login(user_uuid: &str, conn: &mut DbConn) -> JsonResult { // Load saved credentials - let creds: Vec = - get_webauthn_registrations(user_uuid, conn).await?.1.into_iter().map(|r| r.credential).collect(); + let creds: Vec = + get_webauthn_registrations(user_uuid, conn).await?.1.into_iter().map(|r| r.security_key).collect(); if creds.is_empty() { err!("No Webauthn devices registered") } // Generate a challenge based on the credentials - let ext = RequestAuthenticationExtensions::builder().appid(format!("{}/app-id.json", &CONFIG.domain())).build(); - let (response, state) = WebauthnConfig::load().generate_challenge_authenticate_options(creds, Some(ext))?; + //let ext = RequestAuthenticationExtensions::builder().appid(format!("{}/app-id.json", &CONFIG.domain())).build(); + let (response, state) = WEBAUTHN.start_securitykey_authentication(&creds)?; //, Some(ext))?; // Save the challenge state for later validation TwoFactor::new(user_uuid.into(), TwoFactorType::WebauthnLoginChallenge, serde_json::to_string(&state)?) @@ -401,7 +238,7 @@ pub async fn validate_webauthn_login(user_uuid: &str, response: &str, conn: &mut let type_ = TwoFactorType::WebauthnLoginChallenge as i32; let state = match TwoFactor::find_by_user_and_type(user_uuid, type_, conn).await { Some(tf) => { - let state: AuthenticationState = serde_json::from_str(&tf.data)?; + let state: SecurityKeyAuthentication = serde_json::from_str(&tf.data)?; tf.delete(conn).await?; state } @@ -413,19 +250,17 @@ pub async fn validate_webauthn_login(user_uuid: &str, response: &str, conn: &mut ), }; - let rsp: crate::util::UpCase = serde_json::from_str(response)?; - let rsp: PublicKeyCredential = rsp.data.into(); + let rsp: PublicKeyCredential = serde_json::from_str(response)?; + // let rsp: PublicKeyCredential = rsp.data.into(); let mut registrations = get_webauthn_registrations(user_uuid, conn).await?.1; - // If the credential we received is migrated from U2F, enable the U2F compatibility - //let use_u2f = registrations.iter().any(|r| r.migrated && r.credential.cred_id == rsp.raw_id.0); - let (cred_id, auth_data) = WebauthnConfig::load().authenticate_credential(&rsp, &state)?; + let auth_result = WEBAUTHN.finish_securitykey_authentication(&rsp, &state)?; for reg in &mut registrations { - if ®.credential.cred_id == cred_id { - reg.credential.counter = auth_data.counter; - + if reg.security_key.cred_id() == auth_result.cred_id() + && reg.security_key.update_credential(&auth_result).is_some() + { TwoFactor::new(user_uuid.to_string(), TwoFactorType::Webauthn, serde_json::to_string(®istrations)?) .save(conn) .await?; diff --git a/src/config.rs b/src/config.rs index 489a229d..bd4c3133 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,6 +5,7 @@ use std::sync::RwLock; use job_scheduler_ng::Schedule; use once_cell::sync::Lazy; use reqwest::Url; +use webauthn_rs::{Webauthn, WebauthnBuilder}; use crate::{ db::DbConnType, @@ -24,6 +25,23 @@ pub static CONFIG: Lazy = Lazy::new(|| { }) }); +pub static WEBAUTHN: Lazy = Lazy::new(|| { + let domain = CONFIG.domain(); + let domain_origin = CONFIG.domain_origin(); + let android_app_url = Url::parse("android:apk-key-hash:dUGFzUzf3lmHSLBDBIv+WaFyZMI").unwrap(); + let ios_app_url = Url::parse("ios:bundle-id:com.8bit.bitwarden").unwrap(); + + let rp_id = Url::parse(&domain).map(|u| u.domain().map(str::to_owned)).ok().flatten().unwrap_or_default(); + let rp_name = domain; + let rp_origin = Url::parse(&domain_origin).unwrap(); + let builder = WebauthnBuilder::new(&rp_id, &rp_origin) + .expect("Invalid configuration") + .rp_name(&rp_name) + .append_allowed_origin(&ios_app_url) + .append_allowed_origin(&android_app_url); + builder.build().expect("Invalid configuration") +}); + pub type Pass = String; macro_rules! make_config { diff --git a/src/error.rs b/src/error.rs index 784aad6a..f0b5d6d0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -52,7 +52,8 @@ use rocket::error::Error as RocketErr; use serde_json::{Error as SerdeErr, Value}; use std::io::Error as IoErr; use std::time::SystemTimeError as TimeErr; -use webauthn_rs::error::WebauthnError as WebauthnErr; +use uuid::Error as UuidErr; +use webauthn_rs::prelude::WebauthnError as WebauthnErr; use yubico::yubicoerror::YubicoError as YubiErr; #[derive(Serialize)] @@ -87,6 +88,7 @@ make_error! { Smtp(SmtpErr): _has_source, _api_error, OpenSSL(SSLErr): _has_source, _api_error, Rocket(RocketErr): _has_source, _api_error, + Uuid(UuidErr): _has_source, _api_error, DieselCon(DieselConErr): _has_source, _api_error, Webauthn(WebauthnErr): _has_source, _api_error, diff --git a/src/main.rs b/src/main.rs index f2784d9d..f9fc68ff 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,7 +53,7 @@ mod util; use crate::api::purge_auth_requests; use crate::api::{WS_ANONYMOUS_SUBSCRIPTIONS, WS_USERS}; -pub use config::CONFIG; +pub use config::{CONFIG, WEBAUTHN}; pub use error::{Error, MapResult}; use rocket::data::{Limits, ToByteUnit}; use std::sync::Arc; From 54d4612fc82aa5564c9fd385a5695c146d0639ab Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Mon, 22 Apr 2024 19:57:37 +0200 Subject: [PATCH 3/4] Revert "remove u2f to webauthn migration" This reverts commit 21b8a0a334337138b2ad075420fab434a632fc39. --- src/db/models/two_factor.rs | 69 +++++++++++++++++++++++++++++++++++++ src/main.rs | 1 + 2 files changed, 70 insertions(+) diff --git a/src/db/models/two_factor.rs b/src/db/models/two_factor.rs index ff353ac2..530e35b4 100644 --- a/src/db/models/two_factor.rs +++ b/src/db/models/two_factor.rs @@ -147,4 +147,73 @@ impl TwoFactor { .map_res("Error deleting twofactors") }} } + + pub async fn migrate_u2f_to_webauthn(conn: &mut DbConn) -> EmptyResult { + let u2f_factors = db_run! { conn: { + twofactor::table + .filter(twofactor::atype.eq(TwoFactorType::U2f as i32)) + .load::(conn) + .expect("Error loading twofactor") + .from_db() + }}; + + use crate::api::core::two_factor::webauthn::U2FRegistration; + use crate::api::core::two_factor::webauthn::{get_webauthn_registrations, WebauthnRegistration}; + use webauthn_rs::proto::*; + + for mut u2f in u2f_factors { + let mut regs: Vec = serde_json::from_str(&u2f.data)?; + // If there are no registrations or they are migrated (we do the migration in batch so we can consider them all migrated when the first one is) + if regs.is_empty() || regs[0].migrated == Some(true) { + continue; + } + + let (_, mut webauthn_regs) = get_webauthn_registrations(&u2f.user_uuid, conn).await?; + + // If the user already has webauthn registrations saved, don't overwrite them + if !webauthn_regs.is_empty() { + continue; + } + + for reg in &mut regs { + let x: [u8; 32] = reg.reg.pub_key[1..33].try_into().unwrap(); + let y: [u8; 32] = reg.reg.pub_key[33..65].try_into().unwrap(); + + let key = COSEKey { + type_: COSEAlgorithm::ES256, + key: COSEKeyType::EC_EC2(COSEEC2Key { + curve: ECDSACurve::SECP256R1, + x, + y, + }), + }; + + 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, + }, + }; + + webauthn_regs.push(new_reg); + + reg.migrated = Some(true); + } + + u2f.data = serde_json::to_string(®s)?; + u2f.save(conn).await?; + + TwoFactor::new(u2f.user_uuid.clone(), TwoFactorType::Webauthn, serde_json::to_string(&webauthn_regs)?) + .save(conn) + .await?; + } + + Ok(()) + } } diff --git a/src/main.rs b/src/main.rs index f9fc68ff..7b8bf37a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,6 +88,7 @@ async fn main() -> Result<(), Error> { let pool = create_db_pool().await; schedule_jobs(pool.clone()); + crate::db::models::TwoFactor::migrate_u2f_to_webauthn(&mut pool.get().await.unwrap()).await.unwrap(); launch_rocket(pool, extra_debug).await // Blocks until program termination. } From 0020d36c24f6694dc2997e4e81061b64f79acc1a Mon Sep 17 00:00:00 2001 From: Stefan Melmuk Date: Mon, 22 Apr 2024 21:00:28 +0200 Subject: [PATCH 4/4] 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 { 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, + 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, +} + #[post("/two-factor/get-webauthn", data = "")] async fn get_webauthn(data: JsonUpcase, headers: Headers, mut conn: DbConn) -> JsonResult { if !CONFIG.domain_set() { @@ -168,11 +190,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().0); + 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!({ @@ -183,7 +221,7 @@ async fn delete_webauthn(data: JsonUpcase, 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), 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 = 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);