From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 49E0E81FC3 for ; Fri, 26 Nov 2021 14:56:25 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 35ECB19465 for ; Fri, 26 Nov 2021 14:55:55 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E2551191F7 for ; Fri, 26 Nov 2021 14:55:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id BB9BE46A84 for ; Fri, 26 Nov 2021 14:55:42 +0100 (CET) From: Wolfgang Bumiller To: pmg-devel@lists.proxmox.com Date: Fri, 26 Nov 2021 14:55:22 +0100 Message-Id: <20211126135524.117846-19-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211126135524.117846-1-w.bumiller@proxmox.com> References: <20211126135524.117846-1-w.bumiller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.429 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, methods.rs, self.id, webauthn.rs] Subject: [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Nov 2021 13:56:25 -0000 and add a webauthn origin override parameter to all methods accessing it Signed-off-by: Wolfgang Bumiller --- proxmox-tfa/src/api/methods.rs | 21 ++++++++++++--- proxmox-tfa/src/api/mod.rs | 45 +++++++++++++++++++++------------ proxmox-tfa/src/api/webauthn.rs | 45 +++++++++++++++++++++++---------- 3 files changed, 79 insertions(+), 32 deletions(-) diff --git a/proxmox-tfa/src/api/methods.rs b/proxmox-tfa/src/api/methods.rs index 56971f1..b63d56e 100644 --- a/proxmox-tfa/src/api/methods.rs +++ b/proxmox-tfa/src/api/methods.rs @@ -317,6 +317,7 @@ pub fn add_tfa_entry( value: Option, challenge: Option, r#type: TfaType, + origin: Option<&url::Url>, ) -> Result { match r#type { TfaType::Totp => { @@ -331,7 +332,15 @@ pub fn add_tfa_entry( bail!("'totp' parameter is invalid for 'webauthn' entries"); } - add_webauthn(config, access, userid, description, challenge, value) + add_webauthn( + config, + access, + userid, + description, + challenge, + value, + origin, + ) } TfaType::U2f => { if totp.is_some() { @@ -436,10 +445,16 @@ fn add_webauthn( description: Option, challenge: Option, value: Option, + origin: Option<&url::Url>, ) -> Result { match challenge { None => config - .webauthn_registration_challenge(access, &userid, need_description(description)?) + .webauthn_registration_challenge( + access, + &userid, + need_description(description)?, + origin, + ) .map(|c| TfaUpdateInfo { challenge: Some(c), ..Default::default() @@ -449,7 +464,7 @@ fn add_webauthn( format_err!("missing 'value' parameter (webauthn challenge response missing)") })?; config - .webauthn_registration_finish(access, &userid, &challenge, &value) + .webauthn_registration_finish(access, &userid, &challenge, &value, origin) .map(TfaUpdateInfo::id) } } diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs index e0084b9..b591a23 100644 --- a/proxmox-tfa/src/api/mod.rs +++ b/proxmox-tfa/src/api/mod.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; use anyhow::{bail, format_err, Error}; use serde::{Deserialize, Serialize}; use serde_json::Value; +use url::Url; use webauthn_rs::{proto::UserVerificationPolicy, Webauthn}; @@ -28,6 +29,7 @@ pub mod methods; pub use recovery::RecoveryState; pub use u2f::U2fConfig; +use webauthn::WebauthnConfigInstance; pub use webauthn::{WebauthnConfig, WebauthnCredential}; #[cfg(feature = "api-types")] @@ -93,15 +95,22 @@ fn check_u2f(u2f: &Option) -> Result { /// Helper to get a `Webauthn` instance from a `WebauthnConfig`, or `None` if there isn't one /// configured. -fn get_webauthn(waconfig: &Option) -> Option> { - waconfig.clone().map(Webauthn::new) +fn get_webauthn<'a, 'config: 'a, 'origin: 'a>( + waconfig: &'config Option, + origin: Option<&'origin Url>, +) -> Option>, Error>> { + Some(waconfig.as_ref()?.instantiate(origin).map(Webauthn::new)) } /// Helper to get a u2f instance from a u2f config. /// /// This is outside of `TfaConfig` to not borrow its `&self`. -fn check_webauthn(waconfig: &Option) -> Result, Error> { - get_webauthn(waconfig).ok_or_else(|| format_err!("no webauthn configuration available")) +fn check_webauthn<'a, 'config: 'a, 'origin: 'a>( + waconfig: &'config Option, + origin: Option<&'origin Url>, +) -> Result>, Error> { + get_webauthn(waconfig, origin) + .ok_or_else(|| format_err!("no webauthn configuration available"))? } impl TfaConfig { @@ -142,8 +151,9 @@ impl TfaConfig { access: A, user: &str, description: String, + origin: Option<&Url>, ) -> Result { - let webauthn = check_webauthn(&self.webauthn)?; + let webauthn = check_webauthn(&self.webauthn, origin)?; self.users .entry(user.to_owned()) @@ -158,8 +168,9 @@ impl TfaConfig { userid: &str, challenge: &str, response: &str, + origin: Option<&Url>, ) -> Result { - let webauthn = check_webauthn(&self.webauthn)?; + let webauthn = check_webauthn(&self.webauthn, origin)?; let response: webauthn_rs::proto::RegisterPublicKeyCredential = serde_json::from_str(response) @@ -208,12 +219,13 @@ impl TfaConfig { &mut self, access: A, userid: &str, + origin: Option<&Url>, ) -> Result, Error> { match self.users.get_mut(userid) { Some(udata) => udata.challenge( access, userid, - get_webauthn(&self.webauthn), + get_webauthn(&self.webauthn, origin), get_u2f(&self.u2f).as_ref(), ), None => Ok(None), @@ -227,6 +239,7 @@ impl TfaConfig { userid: &str, challenge: &TfaChallenge, response: TfaResponse, + origin: Option<&Url>, ) -> Result { match self.users.get_mut(userid) { Some(user) => match response { @@ -239,7 +252,7 @@ impl TfaConfig { None => bail!("no u2f factor available for user '{}'", userid), }, TfaResponse::Webauthn(value) => { - let webauthn = check_webauthn(&self.webauthn)?; + let webauthn = check_webauthn(&self.webauthn, origin)?; user.verify_webauthn(access.clone(), userid, webauthn, value) } TfaResponse::Recovery(value) => { @@ -424,7 +437,7 @@ impl TfaUserData { fn webauthn_registration_challenge( &mut self, access: A, - webauthn: Webauthn, + webauthn: Webauthn, userid: &str, description: String, ) -> Result { @@ -463,7 +476,7 @@ impl TfaUserData { fn webauthn_registration_finish( &mut self, access: A, - webauthn: Webauthn, + webauthn: Webauthn, userid: &str, challenge: &str, response: webauthn_rs::proto::RegisterPublicKeyCredential, @@ -555,11 +568,11 @@ impl TfaUserData { } /// Generate a generic TFA challenge. See the [`TfaChallenge`] description for details. - pub fn challenge( + fn challenge( &mut self, access: A, userid: &str, - webauthn: Option>, + webauthn: Option, Error>>, u2f: Option<&u2f::U2f>, ) -> Result, Error> { if self.is_empty() { @@ -570,7 +583,7 @@ impl TfaUserData { totp: self.totp.iter().any(|e| e.info.enable), recovery: RecoveryState::from(&self.recovery), webauthn: match webauthn { - Some(webauthn) => self.webauthn_challenge(access.clone(), userid, webauthn)?, + Some(webauthn) => self.webauthn_challenge(access.clone(), userid, webauthn?)?, None => None, }, u2f: match u2f { @@ -591,7 +604,7 @@ impl TfaUserData { &mut self, access: A, userid: &str, - webauthn: Webauthn, + webauthn: Webauthn, ) -> Result, Error> { if self.webauthn.is_empty() { return Ok(None); @@ -703,7 +716,7 @@ impl TfaUserData { &mut self, access: A, userid: &str, - webauthn: Webauthn, + webauthn: Webauthn, mut response: Value, ) -> Result<(), Error> { let expire_before = proxmox_time::epoch_i64() - CHALLENGE_TIMEOUT_SECS; @@ -995,7 +1008,7 @@ impl TfaUserChallenges { /// `webauthn_registration_challenge`. The response should come directly from the client. fn webauthn_registration_finish( &mut self, - webauthn: Webauthn, + webauthn: Webauthn, challenge: &str, response: webauthn_rs::proto::RegisterPublicKeyCredential, existing_registrations: &[TfaEntry], diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs index 486e4f5..aed2885 100644 --- a/proxmox-tfa/src/api/webauthn.rs +++ b/proxmox-tfa/src/api/webauthn.rs @@ -1,6 +1,6 @@ //! Webauthn configuration and challenge data. -use anyhow::Error; +use anyhow::{format_err, Error}; use serde::{Deserialize, Serialize}; use url::Url; use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy}; @@ -50,7 +50,7 @@ impl Into for OriginUrl { #[cfg_attr(feature = "api-types", api( properties: { rp: { type: String }, - origin: { type: String }, + origin: { type: String, optional: true }, id: { type: String }, } ))] @@ -68,7 +68,8 @@ pub struct WebauthnConfig { /// users type in their browsers to access the web interface. /// /// Changing this *may* break existing credentials. - pub origin: OriginUrl, + #[serde(skip_serializing_if = "Option::is_none")] + pub origin: Option, /// Relying party ID. Must be the domain name without protocol, port or location. /// @@ -78,31 +79,49 @@ pub struct WebauthnConfig { impl WebauthnConfig { pub fn digest(&self) -> [u8; 32] { - let data = format!( - "rp={:?}\norigin={:?}\nid={:?}\n", - self.rp, - self.origin.0.as_str(), - self.id, - ); + let mut data = format!("rp={:?}\nid={:?}\n", self.rp, self.id,); + if let Some(origin) = &self.origin { + data.push_str(&format!("origin={:?}\n", origin.as_str())); + } openssl::sha::sha256(data.as_bytes()) } + + /// Instantiate a usable webauthn configuration instance. + pub(super) fn instantiate<'a, 'this: 'a, 'origin: 'a>( + &'this self, + origin: Option<&'origin Url>, + ) -> Result, Error> { + Ok(WebauthnConfigInstance { + origin: origin + .or_else(|| self.origin.as_ref().map(|u| &u.0)) + .ok_or_else(|| format_err!("missing webauthn origin"))?, + rp: &self.rp, + id: &self.id, + }) + } +} + +pub(super) struct WebauthnConfigInstance<'a> { + rp: &'a str, + origin: &'a Url, + id: &'a str, } /// For now we just implement this on the configuration this way. /// /// 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 { +impl<'a> webauthn_rs::WebauthnConfig for WebauthnConfigInstance<'a> { fn get_relying_party_name(&self) -> &str { - &self.rp + self.rp } fn get_origin(&self) -> &Url { - &self.origin.0 + self.origin } fn get_relying_party_id(&self) -> &str { - &self.id + self.id } } -- 2.30.2