diff --git a/proxmox-tfa/Cargo.toml b/proxmox-tfa/Cargo.toml index ce7e0bbc..81c79c24 100644 --- a/proxmox-tfa/Cargo.toml +++ b/proxmox-tfa/Cargo.toml @@ -19,11 +19,12 @@ serde = "1.0" serde_plain = "1.0" serde_json = { version = "1.0", optional = true } libc = { version = "0.2", optional = true } +url = "2.2" proxmox-schema = { version = "1", path = "../proxmox-schema", features = [ "api-macro" ], optional = true } proxmox-time = { version = "1", path = "../proxmox-time", optional = true } proxmox-uuid = { version = "1", path = "../proxmox-uuid", optional = true } -webauthn-rs = { version = "0.2.5", optional = true } +webauthn-rs = { version = "0.3", optional = true } [features] default = [] diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs index 48bac1c6..8918faef 100644 --- a/proxmox-tfa/src/api/mod.rs +++ b/proxmox-tfa/src/api/mod.rs @@ -421,7 +421,7 @@ impl TfaUserData { fn webauthn_registration_challenge( &mut self, access: A, - mut webauthn: Webauthn, + webauthn: Webauthn, userid: &str, description: String, ) -> Result { @@ -436,6 +436,7 @@ impl TfaUserData { userid.to_owned(), Some(cred_ids), Some(UserVerificationPolicy::Discouraged), + None, )?; let challenge_string = challenge.public_key.challenge.to_string(); @@ -587,7 +588,7 @@ impl TfaUserData { &mut self, access: A, userid: &str, - mut webauthn: Webauthn, + webauthn: Webauthn, ) -> Result, Error> { if self.webauthn.is_empty() { return Ok(None); @@ -602,8 +603,8 @@ impl TfaUserData { return Ok(None); } - let (challenge, state) = webauthn - .generate_challenge_authenticate(creds, Some(UserVerificationPolicy::Discouraged))?; + let (challenge, state) = webauthn.generate_challenge_authenticate(creds)?; + let challenge_string = challenge.public_key.challenge.to_string(); let mut data = access.open(userid)?; data.get_mut() @@ -699,7 +700,7 @@ impl TfaUserData { &mut self, access: A, userid: &str, - mut webauthn: Webauthn, + webauthn: Webauthn, mut response: Value, ) -> Result<(), Error> { let expire_before = proxmox_time::epoch_i64() - CHALLENGE_TIMEOUT_SECS; @@ -738,10 +739,9 @@ impl TfaUserData { data.save() .map_err(|err| format_err!("failed to save challenge file: {}", err))?; - match webauthn.authenticate_credential(response, challenge.state)? { - Some((_cred, _counter)) => Ok(()), - None => bail!("webauthn authentication failed"), - } + webauthn.authenticate_credential(&response, &challenge.state)?; + + Ok(()) } /// Verify a recovery key. @@ -1010,8 +1010,8 @@ impl TfaUserChallenges { bail!("no such challenge"); } - let credential = - webauthn.register_credential(response, reg.state, |id| -> Result { + let (credential, _authenticator) = + webauthn.register_credential(&response, ®.state, |id| -> Result { Ok(existing_registrations .iter() .any(|cred| cred.entry.cred_id == *id)) diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs index 54ed067b..c691e422 100644 --- a/proxmox-tfa/src/api/webauthn.rs +++ b/proxmox-tfa/src/api/webauthn.rs @@ -3,14 +3,30 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "api-types")] -use proxmox_schema::{api, Updater}; +use proxmox_schema::{api, Updater, UpdaterType}; -use webauthn_rs::crypto::COSEKey; -use webauthn_rs::proto::{Credential, CredentialID}; +use url::Url; + +use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy}; use super::IsExpired; -#[cfg_attr(feature = "api-types", api)] +#[derive(Clone, Deserialize, Serialize)] +/// Origin URL for WebauthnConfig +pub struct OriginUrl(Url); + +#[cfg(feature = "api-types")] +impl UpdaterType for OriginUrl { + type Updater = Option; +} + +#[cfg_attr(feature = "api-types", api( + properties: { + rp: { type: String }, + origin: { type: String }, + id: { type: String }, + } +))] #[cfg_attr(feature = "api-types", derive(Updater))] /// Server side webauthn server configuration. #[derive(Clone, Deserialize, Serialize)] @@ -25,7 +41,7 @@ pub struct WebauthnConfig { /// users type in their browsers to access the web interface. /// /// Changing this *may* break existing credentials. - pub origin: String, + pub origin: OriginUrl, /// Relying part ID. Must be the domain name without protocol, port or location. /// @@ -38,16 +54,16 @@ pub struct WebauthnConfig { /// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by /// the connecting client. impl webauthn_rs::WebauthnConfig for WebauthnConfig { - fn get_relying_party_name(&self) -> String { - self.rp.clone() + fn get_relying_party_name(&self) -> &str { + &self.rp } - fn get_origin(&self) -> &String { - &self.origin + fn get_origin(&self) -> &Url { + &self.origin.0 } - fn get_relying_party_id(&self) -> String { - self.id.clone() + fn get_relying_party_id(&self) -> &str { + &self.id } } @@ -132,6 +148,7 @@ pub struct WebauthnCredential { pub counter: u32, } +/// ignores verified and registration_policy fields for now impl From for WebauthnCredential { fn from(cred: Credential) -> Self { Self { @@ -142,12 +159,15 @@ impl From for WebauthnCredential { } } +/// always sets verified to false and registration_policy to Discouraged for now impl From for Credential { fn from(val: WebauthnCredential) -> Self { Credential { cred_id: val.cred_id, cred: val.cred, counter: val.counter, + verified: false, + registration_policy: UserVerificationPolicy::Discouraged, } } }