From e2a1f1925cf3f903e28c4d8dba1ad027f0e7d531 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Tue, 16 Sep 2025 09:22:31 +0200 Subject: [PATCH 01/50] fix password reset grpc sending unparsed user agent (#1546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Filip Ślęzak --- .../defguard_core/src/grpc/password_reset.rs | 20 ++++++++++--------- crates/defguard_core/src/grpc/utils.rs | 3 ++- crates/defguard_core/src/handlers/mail.rs | 8 ++++---- crates/defguard_core/src/handlers/user.rs | 4 ++-- crates/defguard_core/src/headers.rs | 3 ++- crates/defguard_core/templates/base.tera | 2 +- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/crates/defguard_core/src/grpc/password_reset.rs b/crates/defguard_core/src/grpc/password_reset.rs index 6af863a8e8..0403857e59 100644 --- a/crates/defguard_core/src/grpc/password_reset.rs +++ b/crates/defguard_core/src/grpc/password_reset.rs @@ -18,6 +18,7 @@ use crate::{ mail::{send_password_reset_email, send_password_reset_success_email}, user::check_password_strength, }, + headers::get_device_info, mail::Mail, server_config, }; @@ -99,13 +100,14 @@ impl PasswordResetServer { debug!("Starting password reset request"); let ip_address; - let user_agent; + let device_info; if let Some(ref info) = req_device_info { ip_address = info.ip_address.clone(); - user_agent = info.user_agent.clone().unwrap_or_default(); + let agent = info.user_agent.clone().unwrap_or_default(); + device_info = get_device_info(&agent); } else { ip_address = String::new(); - user_agent = String::new(); + device_info = String::new(); } let email = request.email; @@ -152,14 +154,13 @@ impl PasswordResetServer { error!("Failed to commit transaction"); Status::internal("unexpected error") })?; - send_password_reset_email( &user, &self.mail_tx, config.enrollment_url.clone(), &enrollment.id, Some(&ip_address), - Some(&user_agent), + Some(&device_info), )?; info!( @@ -255,13 +256,14 @@ impl PasswordResetServer { let enrollment = self.validate_session(request.token.as_ref()).await?; let ip_address; - let user_agent; + let device_info; if let Some(ref info) = req_device_info { ip_address = info.ip_address.clone(); - user_agent = info.user_agent.clone().unwrap_or_default(); + let agent = info.user_agent.clone().unwrap_or_default(); + device_info = get_device_info(&agent); } else { ip_address = String::new(); - user_agent = String::new(); + device_info = String::new(); } if let Err(err) = check_password_strength(&request.password) { @@ -302,7 +304,7 @@ impl PasswordResetServer { &user, &self.mail_tx, Some(&ip_address), - Some(&user_agent), + Some(&device_info), )?; // Prepare event context and push the event diff --git a/crates/defguard_core/src/grpc/utils.rs b/crates/defguard_core/src/grpc/utils.rs index 226abbb117..0e5772d574 100644 --- a/crates/defguard_core/src/grpc/utils.rs +++ b/crates/defguard_core/src/grpc/utils.rs @@ -216,6 +216,7 @@ pub(crate) fn parse_client_info(info: &Option) -> Result<(IpAddr, St msg })?; let user_agent = info.user_agent.clone().unwrap_or_else(String::new); + let escaped_agent = tera::escape_html(&user_agent); - Ok((ip, user_agent)) + Ok((ip, escaped_agent)) } diff --git a/crates/defguard_core/src/handlers/mail.rs b/crates/defguard_core/src/handlers/mail.rs index 67aa52cddd..a50ff1989a 100644 --- a/crates/defguard_core/src/handlers/mail.rs +++ b/crates/defguard_core/src/handlers/mail.rs @@ -39,8 +39,8 @@ static EMAIL_MFA_CODE_EMAIL_SUBJECT: &str = "Your Multi-Factor Authentication Co static GATEWAY_DISCONNECTED: &str = "Defguard: Gateway disconnected"; static GATEWAY_RECONNECTED: &str = "Defguard: Gateway reconnected"; -pub static EMAIL_PASSOWRD_RESET_START_SUBJECT: &str = "Defguard: Password reset"; -pub static EMAIL_PASSOWRD_RESET_SUCCESS_SUBJECT: &str = "Defguard: Password reset success"; +pub static EMAIL_PASSWORD_RESET_START_SUBJECT: &str = "Defguard: Password reset"; +pub static EMAIL_PASSWORD_RESET_SUCCESS_SUBJECT: &str = "Defguard: Password reset success"; #[derive(Clone, Deserialize)] pub struct TestMail { @@ -461,7 +461,7 @@ pub fn send_password_reset_email( let mail = Mail { to: user.email.clone(), - subject: EMAIL_PASSOWRD_RESET_START_SUBJECT.into(), + subject: EMAIL_PASSWORD_RESET_START_SUBJECT.into(), content: templates::email_password_reset_mail(service_url, token, ip_address, device_info)?, attachments: Vec::new(), result_tx: None, @@ -491,7 +491,7 @@ pub fn send_password_reset_success_email( let mail = Mail { to: user.email.clone(), - subject: EMAIL_PASSOWRD_RESET_SUCCESS_SUBJECT.into(), + subject: EMAIL_PASSWORD_RESET_SUCCESS_SUBJECT.into(), content: templates::email_password_reset_success_mail(ip_address, device_info)?, attachments: Vec::new(), result_tx: None, diff --git a/crates/defguard_core/src/handlers/user.rs b/crates/defguard_core/src/handlers/user.rs index 909d109439..ecb90446e9 100644 --- a/crates/defguard_core/src/handlers/user.rs +++ b/crates/defguard_core/src/handlers/user.rs @@ -8,7 +8,7 @@ use serde_json::json; use super::{ AddUserData, ApiResponse, ApiResult, PasswordChange, PasswordChangeSelf, - StartEnrollmentRequest, Username, mail::EMAIL_PASSOWRD_RESET_START_SUBJECT, + StartEnrollmentRequest, Username, mail::EMAIL_PASSWORD_RESET_START_SUBJECT, user_for_admin_or_self, }; use crate::{ @@ -1086,7 +1086,7 @@ pub async fn reset_password( let mail = Mail { to: user.email.clone(), - subject: EMAIL_PASSOWRD_RESET_START_SUBJECT.into(), + subject: EMAIL_PASSWORD_RESET_START_SUBJECT.into(), content: templates::email_password_reset_mail( config.enrollment_url.clone(), enrollment.id.clone().as_str(), diff --git a/crates/defguard_core/src/headers.rs b/crates/defguard_core/src/headers.rs index c9b1bd796d..95c090a9b8 100644 --- a/crates/defguard_core/src/headers.rs +++ b/crates/defguard_core/src/headers.rs @@ -24,7 +24,8 @@ pub(crate) static USER_AGENT_PARSER: LazyLock = LazyLock::new(| #[must_use] pub(crate) fn get_device_info(user_agent: &str) -> String { - let client = USER_AGENT_PARSER.parse(user_agent); + let escaped = tera::escape_html(user_agent); + let client = USER_AGENT_PARSER.parse(&escaped); get_user_agent_device(&client) } diff --git a/crates/defguard_core/templates/base.tera b/crates/defguard_core/templates/base.tera index 181fee1de2..10b0f07e41 100644 --- a/crates/defguard_core/templates/base.tera +++ b/crates/defguard_core/templates/base.tera @@ -261,7 +261,7 @@ {% endif %} {% if device_type %}

- Device type: {{ device_type | safe }} + Device type: {{ device_type }}

{% endif %} From bdc6cabd94a171076cbb7962ddf003e70ac74733 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Wed, 17 Sep 2025 09:19:35 +0200 Subject: [PATCH 02/50] Fixes pentest issue DG25-10 from 2025-09-02 (#1579) * validate phone number during enrollment * also check phone numbers in core API endpoints --- crates/defguard_core/Cargo.toml | 1 + crates/defguard_core/src/db/models/mod.rs | 2 +- crates/defguard_core/src/grpc/enrollment.rs | 15 +++++++ crates/defguard_core/src/handlers/user.rs | 29 ++++++++++++- crates/defguard_core/src/lib.rs | 45 ++++++++++++++++++++- 5 files changed, 89 insertions(+), 3 deletions(-) diff --git a/crates/defguard_core/Cargo.toml b/crates/defguard_core/Cargo.toml index acd2a5d78c..10083cf94f 100644 --- a/crates/defguard_core/Cargo.toml +++ b/crates/defguard_core/Cargo.toml @@ -84,6 +84,7 @@ strum_macros = { workspace = true } bytes = { workspace = true } ed25519-dalek = { version = "2.2", features = ["rand_core"] } tower = "0.5" +regex = "1.10" [dev-dependencies] bytes = "1.6" diff --git a/crates/defguard_core/src/db/models/mod.rs b/crates/defguard_core/src/db/models/mod.rs index 265c027f6b..083645bff4 100644 --- a/crates/defguard_core/src/db/models/mod.rs +++ b/crates/defguard_core/src/db/models/mod.rs @@ -143,7 +143,7 @@ impl UserInfo { /// /// Return `true` if groups were changed, `false` otherwise. pub(crate) async fn handle_user_groups( - &mut self, + &self, transaction: &mut PgConnection, user: &mut User, ) -> Result { diff --git a/crates/defguard_core/src/grpc/enrollment.rs b/crates/defguard_core/src/grpc/enrollment.rs index 4490c08582..743be635a4 100644 --- a/crates/defguard_core/src/grpc/enrollment.rs +++ b/crates/defguard_core/src/grpc/enrollment.rs @@ -48,6 +48,7 @@ use crate::{ user::check_password_strength, }, headers::get_device_info, + is_valid_phone_number, mail::Mail, server_config, templates::{self, TemplateLocation}, @@ -343,6 +344,19 @@ impl EnrollmentServer { Ok(()) } + fn validate_activated_user(&self, request: &ActivateUserRequest) -> Result<(), Status> { + if let Some(ref phone_number) = request.phone_number { + if !is_valid_phone_number(phone_number) { + return Err(Status::new( + tonic::Code::InvalidArgument, + "invalid phone number", + )); + } + } + + Ok(()) + } + #[instrument(skip_all)] pub async fn activate_user( &self, @@ -351,6 +365,7 @@ impl EnrollmentServer { ) -> Result<(), Status> { debug!("Activating user account: {request:?}"); let enrollment = self.validate_session(request.token.as_ref()).await?; + self.validate_activated_user(&request)?; let ip_address; let device_info; diff --git a/crates/defguard_core/src/handlers/user.rs b/crates/defguard_core/src/handlers/user.rs index ecb90446e9..785bb46d74 100644 --- a/crates/defguard_core/src/handlers/user.rs +++ b/crates/defguard_core/src/handlers/user.rs @@ -31,6 +31,7 @@ use crate::{ }, error::WebError, events::{ApiEvent, ApiEventType, ApiRequestContext}, + is_valid_phone_number, mail::Mail, server_config, templates, }; @@ -280,6 +281,7 @@ pub async fn add_user( status: StatusCode::BAD_REQUEST, }); } + // check if email doesn't already exist if User::find_by_email(&appstate.pool, &user_data.email) .await? @@ -291,6 +293,18 @@ pub async fn add_user( status: StatusCode::BAD_REQUEST, }); } + + // check phone number + if let Some(ref phone) = user_data.phone { + if !is_valid_phone_number(phone) { + debug!("Invalid phone number for new user {username}: {phone}"); + return Ok(ApiResponse { + json: json!({}), + status: StatusCode::BAD_REQUEST, + }); + } + } + let password = match &user_data.password { Some(password) => { // check password strength @@ -645,11 +659,12 @@ pub async fn modify_user( context: ApiRequestContext, State(appstate): State, Path(username): Path, - Json(mut user_info): Json, + Json(user_info): Json, ) -> ApiResult { debug!("User {} updating user {username}", session.user.username); let mut user = user_for_admin_or_self(&appstate.pool, &session, &username).await?; let groups_before = UserInfo::from_user(&appstate.pool, &user).await?.groups; + // store user before mods let before = user.clone(); let old_username = user.username.clone(); @@ -660,6 +675,18 @@ pub async fn modify_user( status: StatusCode::BAD_REQUEST, }); } + + // check phone number + if let Some(ref phone) = user_info.phone { + if !is_valid_phone_number(phone) { + debug!("Invalid phone number for user {username}: {phone}"); + return Ok(ApiResponse { + json: json!({}), + status: StatusCode::BAD_REQUEST, + }); + } + } + let status_changing = user_info.is_active != user.is_active; let mut transaction = appstate.pool.begin().await?; diff --git a/crates/defguard_core/src/lib.rs b/crates/defguard_core/src/lib.rs index 68f82136ff..f6abd4ca90 100644 --- a/crates/defguard_core/src/lib.rs +++ b/crates/defguard_core/src/lib.rs @@ -3,7 +3,7 @@ #![allow(clippy::result_large_err)] use std::{ net::{IpAddr, Ipv4Addr, SocketAddr}, - sync::{Arc, Mutex, OnceLock, RwLock}, + sync::{Arc, LazyLock, Mutex, OnceLock, RwLock}, }; use crate::version::IncompatibleComponents; @@ -60,6 +60,7 @@ use handlers::{ yubikey::{delete_yubikey, rename_yubikey}, }; use ipnetwork::IpNetwork; +use regex::Regex; use secrecy::ExposeSecret; use semver::Version; use sqlx::PgPool; @@ -193,6 +194,11 @@ pub(crate) fn server_config() -> &'static DefGuardConfig { .expect("Server configuration not set yet") } +static PHONE_NUMBER_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r"^(\+?\d{1,3}\s?)?(\(\d{1,3}\)|\d{1,3})[-\s]?\d{1,4}[-\s]?\d{1,4}?$") + .expect("Failed to parse phone number regex") +}); + // WireGuard key length in bytes. pub(crate) const KEY_LENGTH: usize = 32; @@ -964,3 +970,40 @@ where .join(",") } } + +pub(crate) fn is_valid_phone_number(number: &str) -> bool { + PHONE_NUMBER_REGEX.is_match(number) +} + +#[cfg(test)] +mod test { + + use super::is_valid_phone_number; + + #[test] + fn test_is_valid_phone_number_dg25_10() { + let valid_numbers = &[ + "+48 (91) 123-456", + "123 456 7890", + "+1 (202) 555-0173", + "91-1234-5678", + "(22) 567 890", + ]; + for number in valid_numbers { + assert!(is_valid_phone_number(number)); + } + + let invalid_numbers = &[ + "4*4", + "+48 123456789", + "123-456-789-0000", + "(+48) 123 456", + "202.555.0173", + "(12345) 6789", + "+48 (91) 123-456 000 111", + ]; + for number in invalid_numbers { + assert!(!is_valid_phone_number(number)); + } + } +} From 8f96c05e8bdaf7d750e81fad5da3f060401445f2 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 17 Sep 2025 11:30:49 +0200 Subject: [PATCH 03/50] Do not display sensitive data from protos (#1580) --- Cargo.lock | 218 +++++++++++------- crates/defguard/src/main.rs | 2 +- crates/defguard_core/build.rs | 18 +- crates/defguard_core/src/auth/mod.rs | 7 +- crates/defguard_core/src/grpc/enrollment.rs | 5 +- crates/defguard_core/src/grpc/mod.rs | 8 +- .../defguard_core/src/grpc/password_reset.rs | 2 +- crates/defguard_core/src/version.rs | 4 +- 8 files changed, 162 insertions(+), 102 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 57464586f5..f4c4ff4e17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -569,9 +569,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.2.36" +version = "1.2.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5252b3d2648e5eedbc1a6f501e3c795e07025c1e93bbf8bbdd6eef7f447a6d54" +checksum = "65193589c6404eb80b450d618eaf9a2cafaaafd57ecce47370519ef674a7bd44" dependencies = [ "find-msvc-tools", "jobserver", @@ -1765,7 +1765,7 @@ dependencies = [ "js-sys", "libc", "r-efi", - "wasi 0.14.5+wasi-0.2.4", + "wasi 0.14.7+wasi-0.2.4", "wasm-bindgen", ] @@ -1845,7 +1845,7 @@ dependencies = [ "futures-core", "futures-sink", "http", - "indexmap 2.11.1", + "indexmap 2.11.3", "slab", "tokio", "tokio-util", @@ -2031,9 +2031,9 @@ dependencies = [ [[package]] name = "humantime" -version = "2.2.0" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b112acc8b3adf4b107a8ec20977da0273a8c386765a3ec0229bd500a1443f9f" +checksum = "135b12329e5e3ce057a9f972339ea52bc954fe1e9358ef27f95e89716fbc5424" [[package]] name = "hyper" @@ -2106,9 +2106,9 @@ dependencies = [ [[package]] name = "hyper-util" -version = "0.1.16" +version = "0.1.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d9b05277c7e8da2c93a568989bb6207bef0112e8d17df7a6eda4a3cf143bc5e" +checksum = "3c6995591a8f1380fcb4ba966a252a4b29188d51d2b89e3a252f5305be65aea8" dependencies = [ "base64 0.22.1", "bytes", @@ -2132,9 +2132,9 @@ dependencies = [ [[package]] name = "iana-time-zone" -version = "0.1.63" +version = "0.1.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0c919e5debc312ad217002b8048a17b7d83f80703865bbfcfebb0458b0b27d8" +checksum = "33e57f83510bb73707521ebaffa789ec8caf86f9657cad665b092b581d40e9fb" dependencies = [ "android_system_properties", "core-foundation-sys", @@ -2305,13 +2305,14 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.11.1" +version = "2.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "206a8042aec68fa4a62e8d3f7aa4ceb508177d9324faf261e1959e495b7a1921" +checksum = "92119844f513ffa41556430369ab02c295a3578af21cf945caa3e9e0c2481ac3" dependencies = [ "equivalent", "hashbrown 0.15.5", "serde", + "serde_core", ] [[package]] @@ -2401,9 +2402,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.78" +version = "0.3.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c0b063578492ceec17683ef2f8c5e89121fbd0b172cbc280635ab7567db2738" +checksum = "6247da8b8658ad4e73a186e747fcc5fc2a29f979d6fe6269127fdb5fd08298d0" dependencies = [ "once_cell", "wasm-bindgen", @@ -2561,9 +2562,9 @@ checksum = "f9fbbcab51052fe104eb5e5d351cf728d30a5be1fe14d9be8a3b097481fb97de" [[package]] name = "libredox" -version = "0.1.9" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "391290121bad3d37fbddad76d8f5d1c1c314cfc646d143d7e07a3086ddff0ce3" +checksum = "416f7e718bdb06000964960ffa43b4335ad4012ae8b99060261aa4a8088d5ccb" dependencies = [ "bitflags 2.9.4", "libc", @@ -3211,9 +3212,9 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "pest" -version = "2.8.1" +version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1db05f56d34358a8b1066f67cbb203ee3e7ed2ba674a6263a1d5ec6db2204323" +checksum = "21e0a3a33733faeaf8651dfee72dd0f388f0c8e5ad496a3478fa5a922f49cfa8" dependencies = [ "memchr", "thiserror 2.0.16", @@ -3222,9 +3223,9 @@ dependencies = [ [[package]] name = "pest_derive" -version = "2.8.1" +version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb056d9e8ea77922845ec74a1c4e8fb17e7c218cc4fc11a15c5d25e189aa40bc" +checksum = "bc58706f770acb1dbd0973e6530a3cff4746fb721207feb3a8a6064cd0b6c663" dependencies = [ "pest", "pest_generator", @@ -3232,9 +3233,9 @@ dependencies = [ [[package]] name = "pest_generator" -version = "2.8.1" +version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87e404e638f781eb3202dc82db6760c8ae8a1eeef7fb3fa8264b2ef280504966" +checksum = "6d4f36811dfe07f7b8573462465d5cb8965fffc2e71ae377a33aecf14c2c9a2f" dependencies = [ "pest", "pest_meta", @@ -3245,9 +3246,9 @@ dependencies = [ [[package]] name = "pest_meta" -version = "2.8.1" +version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edd1101f170f5903fde0914f899bb503d9ff5271d7ba76bbb70bea63690cc0d5" +checksum = "42919b05089acbd0a5dcd5405fb304d17d1053847b81163d09c4ad18ce8e8420" dependencies = [ "pest", "sha2", @@ -3260,7 +3261,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3672b37090dbd86368a4145bc067582552b29c27377cad4e0a306c97f9bd7772" dependencies = [ "fixedbitset", - "indexmap 2.11.1", + "indexmap 2.11.3", ] [[package]] @@ -3430,12 +3431,12 @@ checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" [[package]] name = "plist" -version = "1.7.4" +version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3af6b589e163c5a788fab00ce0c0366f6efbb9959c2f9874b224936af7fce7e1" +checksum = "740ebea15c5d1428f910cd1a5f52cebf8d25006245ed8ade92702f4943d91e07" dependencies = [ "base64 0.22.1", - "indexmap 2.11.1", + "indexmap 2.11.3", "quick-xml", "serde", "time", @@ -3498,9 +3499,9 @@ dependencies = [ [[package]] name = "proc-macro-crate" -version = "3.3.0" +version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edce586971a4dfaa28950c6f18ed55e0406c1ab88bbce2c6f6293a7aaba73d35" +checksum = "219cb19e96be00ab2e37d6e299658a0cfa83e52429179969b0f0121b4ac46983" dependencies = [ "toml_edit", ] @@ -4061,9 +4062,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.4" +version = "0.103.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a17884ae0c1b773f1ccd2bd4a8c72f16da897310a98b0e84bf349ad5ead92fc" +checksum = "8572f3c2cb9934231157b45499fc41e1f58c589fdfb81a844ba873265e80f8eb" dependencies = [ "ring", "rustls-pki-types", @@ -4193,19 +4194,21 @@ dependencies = [ [[package]] name = "semver" -version = "1.0.26" +version = "1.0.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56e6fa9c48d24d85fb3de5ad847117517440f6beceb7798af16b4a87d616b8d0" +checksum = "d767eb0aabc880b29956c35734170f26ed551a859dbd361d140cdbeca61ab1e2" dependencies = [ "serde", + "serde_core", ] [[package]] name = "serde" -version = "1.0.219" +version = "1.0.225" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f0e2c6ed6606019b4e29e69dbaba95b11854410e5347d525002456dbbb786b6" +checksum = "fd6c24dee235d0da097043389623fb913daddf92c76e9f5a1db88607a0bcbd1d" dependencies = [ + "serde_core", "serde_derive", ] @@ -4221,11 +4224,12 @@ dependencies = [ [[package]] name = "serde_bytes" -version = "0.11.17" +version = "0.11.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8437fd221bde2d4ca316d61b90e337e9e702b3820b87d63caa9ba6c02bd06d96" +checksum = "a5d440709e79d88e51ac01c4b72fc6cb7314017bb7da9eeff678aa94c10e3ea8" dependencies = [ "serde", + "serde_core", ] [[package]] @@ -4238,11 +4242,20 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_core" +version = "1.0.225" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "659356f9a0cb1e529b24c01e43ad2bdf520ec4ceaf83047b83ddcc2251f96383" +dependencies = [ + "serde_derive", +] + [[package]] name = "serde_derive" -version = "1.0.219" +version = "1.0.225" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00" +checksum = "0ea936adf78b1f766949a4977b91d2f5595825bd6ec079aa9543ad2685fc4516" dependencies = [ "proc-macro2", "quote", @@ -4251,37 +4264,39 @@ dependencies = [ [[package]] name = "serde_html_form" -version = "0.2.7" +version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d2de91cf02bbc07cde38891769ccd5d4f073d22a40683aa4bc7a95781aaa2c4" +checksum = "b2f2d7ff8a2140333718bb329f5c40fc5f0865b84c426183ce14c97d2ab8154f" dependencies = [ "form_urlencoded", - "indexmap 2.11.1", + "indexmap 2.11.3", "itoa", "ryu", - "serde", + "serde_core", ] [[package]] name = "serde_json" -version = "1.0.143" +version = "1.0.145" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a" +checksum = "402a6f66d8c709116cf22f558eab210f5a50187f702eb4d7e5ef38d9a7f1c79c" dependencies = [ "itoa", "memchr", "ryu", "serde", + "serde_core", ] [[package]] name = "serde_path_to_error" -version = "0.1.17" +version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59fab13f937fa393d08645bf3a84bdfe86e296747b506ada67bb15f10f218b2a" +checksum = "10a9ff822e371bb5403e391ecd83e182e0e77ba7f6fe0160b795797109d1b457" dependencies = [ "itoa", "serde", + "serde_core", ] [[package]] @@ -4326,7 +4341,7 @@ dependencies = [ "chrono", "hex", "indexmap 1.9.3", - "indexmap 2.11.1", + "indexmap 2.11.3", "schemars 0.9.0", "schemars 1.0.4", "serde", @@ -4354,7 +4369,7 @@ version = "0.9.34+deprecated" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" dependencies = [ - "indexmap 2.11.1", + "indexmap 2.11.3", "itoa", "ryu", "serde", @@ -4604,7 +4619,7 @@ dependencies = [ "futures-util", "hashbrown 0.15.5", "hashlink", - "indexmap 2.11.1", + "indexmap 2.11.3", "ipnetwork", "log", "memchr", @@ -5180,18 +5195,31 @@ dependencies = [ [[package]] name = "toml_datetime" -version = "0.6.11" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +checksum = "a197c0ec7d131bfc6f7e82c8442ba1595aeab35da7adbf05b6b73cd06a16b6be" +dependencies = [ + "serde_core", +] [[package]] name = "toml_edit" -version = "0.22.27" +version = "0.23.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +checksum = "c2ad0b7ae9cfeef5605163839cb9221f453399f15cfb5c10be9885fcf56611f9" dependencies = [ - "indexmap 2.11.1", + "indexmap 2.11.3", "toml_datetime", + "toml_parser", + "winnow", +] + +[[package]] +name = "toml_parser" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b551886f449aa90d4fe2bdaa9f4a2577ad2dde302c61ecf262d80b116db95c10" +dependencies = [ "winnow", ] @@ -5299,7 +5327,7 @@ checksum = "d039ad9159c98b70ecfd540b2573b97f7f52c3e8d9f8ad57a24b916a536975f9" dependencies = [ "futures-core", "futures-util", - "indexmap 2.11.1", + "indexmap 2.11.3", "pin-project-lite", "slab", "sync_wrapper", @@ -5617,7 +5645,7 @@ version = "5.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fcc29c80c21c31608227e0912b2d7fddba57ad76b606890627ba8ee7964e993" dependencies = [ - "indexmap 2.11.1", + "indexmap 2.11.3", "serde", "serde_json", "utoipa-gen", @@ -5757,18 +5785,18 @@ checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" [[package]] name = "wasi" -version = "0.14.5+wasi-0.2.4" +version = "0.14.7+wasi-0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4494f6290a82f5fe584817a676a34b9d6763e8d9d18204009fb31dceca98fd4" +checksum = "883478de20367e224c0090af9cf5f9fa85bed63a95c1abf3afc5c083ebc06e8c" dependencies = [ "wasip2", ] [[package]] name = "wasip2" -version = "1.0.0+wasi-0.2.4" +version = "1.0.1+wasi-0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03fa2761397e5bd52002cd7e73110c71af2109aca4e521a9f40473fe685b0a24" +checksum = "0562428422c63773dad2c345a1882263bbf4d65cf3f42e90921f787ef5ad58e7" dependencies = [ "wit-bindgen", ] @@ -5781,9 +5809,9 @@ checksum = "b8dad83b4f25e74f184f64c43b150b91efe7647395b42289f38e50566d82855b" [[package]] name = "wasm-bindgen" -version = "0.2.101" +version = "0.2.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e14915cadd45b529bb8d1f343c4ed0ac1de926144b746e2710f9cd05df6603b" +checksum = "4ad224d2776649cfb4f4471124f8176e54c1cca67a88108e30a0cd98b90e7ad3" dependencies = [ "cfg-if", "once_cell", @@ -5794,9 +5822,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-backend" -version = "0.2.101" +version = "0.2.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e28d1ba982ca7923fd01448d5c30c6864d0a14109560296a162f80f305fb93bb" +checksum = "3a1364104bdcd3c03f22b16a3b1c9620891469f5e9f09bc38b2db121e593e732" dependencies = [ "bumpalo", "log", @@ -5808,9 +5836,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.51" +version = "0.4.52" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ca85039a9b469b38336411d6d6ced91f3fc87109a2a27b0c197663f5144dffe" +checksum = "9c0a08ecf5d99d5604a6666a70b3cde6ab7cc6142f5e641a8ef48fc744ce8854" dependencies = [ "cfg-if", "js-sys", @@ -5821,9 +5849,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.101" +version = "0.2.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c3d463ae3eff775b0c45df9da45d68837702ac35af998361e2c84e7c5ec1b0d" +checksum = "0d7ab4ca3e367bb1ed84ddbd83cc6e41e115f8337ed047239578210214e36c76" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -5831,9 +5859,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.101" +version = "0.2.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7bb4ce89b08211f923caf51d527662b75bdc9c9c7aab40f86dcb9fb85ac552aa" +checksum = "4a518014843a19e2dbbd0ed5dfb6b99b23fb886b14e6192a00803a3e14c552b0" dependencies = [ "proc-macro2", "quote", @@ -5844,9 +5872,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.101" +version = "0.2.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f143854a3b13752c6950862c906306adb27c7e839f7414cec8fea35beab624c1" +checksum = "255eb0aa4cc2eea3662a00c2bbd66e93911b7361d5e0fcd62385acfd7e15dcee" dependencies = [ "unicode-ident", ] @@ -5866,9 +5894,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.78" +version = "0.3.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77e4b637749ff0d92b8fad63aa1f7cff3cbe125fd49c175cd6345e7272638b12" +checksum = "50462a022f46851b81d5441d1a6f5bac0b21a1d72d64bd4906fbdd4bf7230ec7" dependencies = [ "js-sys", "wasm-bindgen", @@ -6015,15 +6043,15 @@ dependencies = [ [[package]] name = "windows-core" -version = "0.61.2" +version = "0.62.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" +checksum = "57fe7168f7de578d2d8a05b07fd61870d2e73b4020e9f49aa00da8471723497c" dependencies = [ "windows-implement", "windows-interface", - "windows-link 0.1.3", - "windows-result", - "windows-strings", + "windows-link 0.2.0", + "windows-result 0.4.0", + "windows-strings 0.5.0", ] [[package]] @@ -6067,8 +6095,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b8a9ed28765efc97bbc954883f4e6796c33a06546ebafacbabee9696967499e" dependencies = [ "windows-link 0.1.3", - "windows-result", - "windows-strings", + "windows-result 0.3.4", + "windows-strings 0.4.2", ] [[package]] @@ -6080,6 +6108,15 @@ dependencies = [ "windows-link 0.1.3", ] +[[package]] +name = "windows-result" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7084dcc306f89883455a206237404d3eaf961e5bd7e0f312f7c91f57eb44167f" +dependencies = [ + "windows-link 0.2.0", +] + [[package]] name = "windows-strings" version = "0.4.2" @@ -6089,6 +6126,15 @@ dependencies = [ "windows-link 0.1.3", ] +[[package]] +name = "windows-strings" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7218c655a553b0bed4426cf54b20d7ba363ef543b52d515b3e48d7fd55318dda" +dependencies = [ + "windows-link 0.2.0", +] + [[package]] name = "windows-sys" version = "0.48.0" @@ -6331,9 +6377,9 @@ dependencies = [ [[package]] name = "wit-bindgen" -version = "0.45.1" +version = "0.46.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c573471f125075647d03df72e026074b7203790d41351cd6edc96f46bcccd36" +checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" [[package]] name = "writeable" @@ -6515,7 +6561,7 @@ dependencies = [ "arbitrary", "crc32fast", "flate2", - "indexmap 2.11.1", + "indexmap 2.11.3", "memchr", "zopfli", ] diff --git a/crates/defguard/src/main.rs b/crates/defguard/src/main.rs index 84a176a73d..d59c944b97 100644 --- a/crates/defguard/src/main.rs +++ b/crates/defguard/src/main.rs @@ -109,7 +109,7 @@ async fn main() -> Result<(), anyhow::Error> { let gateway_state = Arc::new(Mutex::new(GatewayMap::new())); let client_state = Arc::new(Mutex::new(ClientMap::new())); - let incompatible_components: Arc> = Default::default(); + let incompatible_components: Arc> = Arc::default(); // initialize admin user User::init_admin_user(&pool, config.default_admin_password.expose_secret()).await?; diff --git a/crates/defguard_core/build.rs b/crates/defguard_core/build.rs index a4c2030a96..9551d43add 100644 --- a/crates/defguard_core/build.rs +++ b/crates/defguard_core/build.rs @@ -6,9 +6,25 @@ fn main() -> Result<(), Box> { Emitter::default().add_instructions(&git2)?.emit()?; tonic_prost_build::configure() + // These types contain sensitive data. + .skip_debug([ + "ActivateUserRequest", + "AuthInfoResponse", + "AuthenticateRequest", + "AuthenticateResponse", + "ClientMfaFinishResponse", + "CodeMfaSetupStartResponse", + "CodeMfaSetupFinishResponse", + "CoreRequest", + "CoreResponse", + "DeviceConfigResponse", + "InstanceInfoResponse", + "NewDevice", + "PasswordResetRequest", + ]) .protoc_arg("--experimental_allow_proto3_optional") .type_attribute( - "license.LicenseLimits", + "LicenseLimits", "#[derive(serde::Serialize, serde::Deserialize)]", ) .compile_protos( diff --git a/crates/defguard_core/src/auth/mod.rs b/crates/defguard_core/src/auth/mod.rs index b0fd990e20..41b01e2fd4 100644 --- a/crates/defguard_core/src/auth/mod.rs +++ b/crates/defguard_core/src/auth/mod.rs @@ -422,11 +422,10 @@ where &client, &oauth2token, ))); - } else { - return Err(WebError::Authorization( - "OAuth2 client not found".into(), - )); } + return Err(WebError::Authorization( + "OAuth2 client not found".into(), + )); } } Ok(None) => { diff --git a/crates/defguard_core/src/grpc/enrollment.rs b/crates/defguard_core/src/grpc/enrollment.rs index 743be635a4..3947b1dc66 100644 --- a/crates/defguard_core/src/grpc/enrollment.rs +++ b/crates/defguard_core/src/grpc/enrollment.rs @@ -363,7 +363,7 @@ impl EnrollmentServer { request: ActivateUserRequest, req_device_info: Option, ) -> Result<(), Status> { - debug!("Activating user account: {request:?}"); + debug!("Activating user account"); let enrollment = self.validate_session(request.token.as_ref()).await?; self.validate_activated_user(&request)?; @@ -486,7 +486,7 @@ impl EnrollmentServer { request: NewDevice, req_device_info: Option, ) -> Result { - debug!("Adding new user device: {request:?}"); + debug!("Adding new user device"); let enrollment_token = self.validate_session(request.token.as_ref()).await?; // fetch related users @@ -855,7 +855,6 @@ impl EnrollmentServer { ), token: Some(token.token), }; - debug!("{response:?}."); // Prepare event context and push the event let (ip, user_agent) = parse_client_info(&req_device_info).map_err(Status::internal)?; diff --git a/crates/defguard_core/src/grpc/mod.rs b/crates/defguard_core/src/grpc/mod.rs index c414f078f3..cf32efd637 100644 --- a/crates/defguard_core/src/grpc/mod.rs +++ b/crates/defguard_core/src/grpc/mod.rs @@ -162,8 +162,7 @@ async fn handle_proxy_message_loop( break 'message; } Ok(Some(received)) => { - info!("Received message from proxy."); - debug!("Received the following message from proxy: {received:?}"); + debug!("Received message from proxy; ID={}", received.id); let payload = match received.payload { // rpc CodeMfaSetupStart return (CodeMfaSetupStartResponse) Some(core_request::Payload::CodeMfaSetupStart(request)) => { @@ -192,7 +191,7 @@ async fn handle_proxy_message_loop( Some(core_response::Payload::CodeMfaSetupFinishResponse(response)) } Err(err) => { - error!("Register mfa finish error {err}"); + error!("Register MFA finish error {err}"); Some(core_response::Payload::CoreError(err.into())) } } @@ -650,9 +649,8 @@ pub async fn run_grpc_bidi_stream( // Sleep before trying to reconnect sleep(TEN_SECS).await; continue; - } else { - IncompatibleComponents::remove_proxy(&incompatible_components); } + IncompatibleComponents::remove_proxy(&incompatible_components); info!("Connected to proxy at {}", endpoint.uri()); let mut resp_stream = response.into_inner(); diff --git a/crates/defguard_core/src/grpc/password_reset.rs b/crates/defguard_core/src/grpc/password_reset.rs index 0403857e59..69e2a9b555 100644 --- a/crates/defguard_core/src/grpc/password_reset.rs +++ b/crates/defguard_core/src/grpc/password_reset.rs @@ -252,7 +252,7 @@ impl PasswordResetServer { request: PasswordResetRequest, req_device_info: Option, ) -> Result<(), Status> { - debug!("Starting password reset: {request:?}"); + debug!("Starting password reset"); let enrollment = self.validate_session(request.token.as_ref()).await?; let ip_address; diff --git a/crates/defguard_core/src/version.rs b/crates/defguard_core/src/version.rs index c4eda787d1..5285e2e2eb 100644 --- a/crates/defguard_core/src/version.rs +++ b/crates/defguard_core/src/version.rs @@ -229,6 +229,7 @@ impl Hash for IncompatibleGatewayData { } impl IncompatibleGatewayData { + #[must_use] pub fn new( version: Option, hostname: Option, @@ -238,8 +239,8 @@ impl IncompatibleGatewayData { Self { version, hostname, - created, network_id, + created, } } @@ -276,6 +277,7 @@ impl PartialEq for IncompatibleProxyData { impl Eq for IncompatibleProxyData {} impl IncompatibleProxyData { + #[must_use] pub fn new(version: Option) -> Self { let created = Utc::now().naive_utc(); Self { version, created } From 99e451e99cd2c855c9ddd124afe2f94ac421146f Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Wed, 17 Sep 2025 14:27:36 +0200 Subject: [PATCH 04/50] Don't send empty strings when phone number is not provided (#1583) * don't send empty strings when phone number is not providecleand * use zod trim() instead of trimObjectStrings helper --- .../ProfileDetailsForm/ProfileDetailsForm.tsx | 7 +++--- .../components/AddUserForm/AddUserForm.tsx | 23 ++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/web/src/pages/users/UserProfile/ProfileDetails/ProfileDetailsForm/ProfileDetailsForm.tsx b/web/src/pages/users/UserProfile/ProfileDetails/ProfileDetailsForm/ProfileDetailsForm.tsx index eb84d88e64..16e3ad5242 100644 --- a/web/src/pages/users/UserProfile/ProfileDetails/ProfileDetailsForm/ProfileDetailsForm.tsx +++ b/web/src/pages/users/UserProfile/ProfileDetails/ProfileDetailsForm/ProfileDetailsForm.tsx @@ -34,8 +34,8 @@ import { QueryKeys } from '../../../../../shared/queries'; import type { OAuth2AuthorizedApps } from '../../../../../shared/types'; import { invalidateMultipleQueries } from '../../../../../shared/utils/invalidateMultipleQueries'; import { omitNull } from '../../../../../shared/utils/omitNull'; +import { removeEmptyStrings } from '../../../../../shared/utils/removeEmptyStrings'; import { titleCase } from '../../../../../shared/utils/titleCase'; -import { trimObjectStrings } from '../../../../../shared/utils/trimObjectStrings'; import { ProfileDetailsFormAppsField } from './ProfileDetailsFormAppsField'; interface Inputs { @@ -189,16 +189,15 @@ export const ProfileDetailsForm = () => { }, [LL.userPage.userDetails.fields.status]); const onValidSubmit: SubmitHandler = (values) => { - values = trimObjectStrings(values); if (userProfile?.user) { setUserProfile({ loading: true }); mutate({ username: userProfile.user.username, - data: { + data: removeEmptyStrings({ ...userProfile.user, ...values, totp_enabled: userProfile.user.totp_enabled, - }, + }), }); } }; diff --git a/web/src/pages/users/UsersOverview/modals/AddUserModal/components/AddUserForm/AddUserForm.tsx b/web/src/pages/users/UsersOverview/modals/AddUserModal/components/AddUserForm/AddUserForm.tsx index fc0954ac18..3d26891584 100644 --- a/web/src/pages/users/UsersOverview/modals/AddUserModal/components/AddUserForm/AddUserForm.tsx +++ b/web/src/pages/users/UsersOverview/modals/AddUserModal/components/AddUserForm/AddUserForm.tsx @@ -28,7 +28,7 @@ import { } from '../../../../../../../shared/patterns'; import { QueryKeys } from '../../../../../../../shared/queries'; import { invalidateMultipleQueries } from '../../../../../../../shared/utils/invalidateMultipleQueries'; -import { trimObjectStrings } from '../../../../../../../shared/utils/trimObjectStrings'; +import { removeEmptyStrings } from '../../../../../../../shared/utils/removeEmptyStrings'; import { passwordValidator } from '../../../../../../../shared/validators/password'; import { useAddUserModal } from '../../hooks/useAddUserModal'; @@ -58,6 +58,7 @@ export const AddUserForm = () => { .object({ username: z .string() + .trim() .min(1, LL.form.error.minimumLength()) .max(64, LL.form.error.maximumLength()) .regex(patternSafeUsernameCharacters, LL.form.error.forbiddenCharacter()), @@ -74,9 +75,9 @@ export const AddUserForm = () => { } return true; }, LL.modals.addUser.form.error.emailReserved()), - last_name: z.string().min(1, LL.form.error.required()), - first_name: z.string().min(1, LL.form.error.required()), - phone: z.string(), + last_name: z.string().trim().min(1, LL.form.error.required()), + first_name: z.string().trim().min(1, LL.form.error.required()), + phone: z.string().trim(), enable_enrollment: z.boolean(), }) .superRefine((val, ctx) => { @@ -187,23 +188,23 @@ export const AddUserForm = () => { }); const onSubmit: SubmitHandler = (data) => { - const trimmed = trimObjectStrings(data); - if (reservedUserNames.current.includes(trimmed.username)) { + const clean = removeEmptyStrings(data); + if (reservedUserNames.current.includes(clean.username)) { void trigger('username', { shouldFocus: true }); } else { - usernameAvailable(trimmed.username) + usernameAvailable(clean.username) .then(() => { setCheckingUsername(false); - if (trimmed.enable_enrollment) { - const userData = omit(trimmed, ['password', 'enable_enrollment']); + if (clean.enable_enrollment) { + const userData = omit(clean, ['password', 'enable_enrollment']); addUserMutation.mutate(userData); } else { - addUserMutation.mutate(omit(trimmed, ['enable_enrollment'])); + addUserMutation.mutate(omit(clean, ['enable_enrollment'])); } }) .catch(() => { setCheckingUsername(false); - reservedUserNames.current = [...reservedUserNames.current, trimmed.username]; + reservedUserNames.current = [...reservedUserNames.current, clean.username]; void trigger('username', { shouldFocus: true }); }); } From 0910c8cd73863085f0c4277e744cfca9499b1a83 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Thu, 18 Sep 2025 08:25:54 +0200 Subject: [PATCH 05/50] Fixes pentest issue DG25-17 from 2025-09-02 (#1581) * fix open redirect pentest issue * add tests and handling of get requests, allow redirects if url is allowed for the client * compare the whole url, not just domain * cargo clippy fixes * wip fix openid flow tests * fix panic in the contains_redirect_url method * cleanup eprintln statements * bring back the other openid flow test * state-based fallback url in openid test --- .../src/db/models/oauth2client.rs | 20 ++ .../defguard_core/src/handlers/openid_flow.rs | 54 ++-- .../tests/integration/api/openid.rs | 280 +++++++++++++++++- 3 files changed, 314 insertions(+), 40 deletions(-) diff --git a/crates/defguard_core/src/db/models/oauth2client.rs b/crates/defguard_core/src/db/models/oauth2client.rs index ce3561e601..0adea97e73 100644 --- a/crates/defguard_core/src/db/models/oauth2client.rs +++ b/crates/defguard_core/src/db/models/oauth2client.rs @@ -1,4 +1,5 @@ use model_derive::Model; +use reqwest::Url; use sqlx::{Error as SqlxError, PgExecutor, PgPool, query_as}; use super::NewOpenIDClient; @@ -101,6 +102,25 @@ impl OAuth2Client { .fetch_optional(pool) .await } + + /// Checks if `url` matches client config (ignoring trailing slashes) + pub(crate) fn contains_redirect_url(&self, url: &str) -> bool { + let parsed_redirect_uris: Vec = self + .redirect_uri + .iter() + .map(|uri| uri.trim_end_matches('/').into()) + .collect(); + + // extract origin from url + let Ok(url) = Url::parse(url) else { + return false; + }; + let url = url.origin().ascii_serialization(); + + !url.split(' ') + .map(|uri| uri.trim_end_matches('/')) + .all(|uri| !parsed_redirect_uris.iter().any(|u| u == uri)) + } } // Safe to show for not privileged users diff --git a/crates/defguard_core/src/handlers/openid_flow.rs b/crates/defguard_core/src/handlers/openid_flow.rs index dce8b6d569..818ce14f07 100644 --- a/crates/defguard_core/src/handlers/openid_flow.rs +++ b/crates/defguard_core/src/handlers/openid_flow.rs @@ -271,18 +271,7 @@ impl AuthenticationRequest { // assume `client_id` is the same here and in `oauth2client` - // check `redirect_uri` matches client config (ignoring trailing slashes) - let parsed_redirect_uris: Vec = oauth2client - .redirect_uri - .iter() - .map(|uri| uri.trim_end_matches('/').into()) - .collect(); - if self - .redirect_uri - .split(' ') - .map(|uri| uri.trim_end_matches('/')) - .all(|uri| !parsed_redirect_uris.iter().any(|u| u == uri)) - { + if !oauth2client.contains_redirect_url(&self.redirect_uri) { error!( "Invalid redirect_uri for client {}: {} not in [{}]", oauth2client.name, @@ -392,9 +381,11 @@ pub async fn authorization( private_cookies: PrivateCookieJar, ) -> Result<(StatusCode, HeaderMap, PrivateCookieJar), WebError> { let error; + let mut is_redirect_allowed = false; if let Some(oauth2client) = OAuth2Client::find_by_client_id(&appstate.pool, &data.client_id).await? { + is_redirect_allowed = oauth2client.contains_redirect_url(&data.redirect_uri); match data.validate_for_client(&oauth2client) { Ok(()) => { match &data.prompt { @@ -516,9 +507,12 @@ pub async fn authorization( error = CoreAuthErrorResponseType::UnauthorizedClient; } - let mut url = - Url::parse(&data.redirect_uri).map_err(|_| WebError::Http(StatusCode::BAD_REQUEST))?; - + let mut url = if is_redirect_allowed { + Url::parse(&data.redirect_uri).map_err(|_| WebError::Http(StatusCode::BAD_REQUEST))? + } else { + // Don't allow open redirects (DG25-17) + server_config().url.clone() + }; { let mut query_pairs = url.query_pairs_mut(); query_pairs.append_pair("error", error.as_ref()); @@ -552,13 +546,13 @@ pub async fn secure_authorization( Query(data): Query, private_cookies: PrivateCookieJar, ) -> Result<(StatusCode, HeaderMap, PrivateCookieJar), WebError> { - let mut url = - Url::parse(&data.redirect_uri).map_err(|_| WebError::Http(StatusCode::BAD_REQUEST))?; let error; - if data.allow { - if let Some(oauth2client) = - OAuth2Client::find_by_client_id(&appstate.pool, &data.client_id).await? - { + let mut is_redirect_allowed = false; + if let Some(oauth2client) = + OAuth2Client::find_by_client_id(&appstate.pool, &data.client_id).await? + { + is_redirect_allowed = oauth2client.contains_redirect_url(&data.redirect_uri); + if data.allow { match data.validate_for_client(&oauth2client) { Ok(()) => { if OAuth2AuthorizedApp::find_by_user_and_oauth2client_id( @@ -602,20 +596,26 @@ pub async fn secure_authorization( } } } else { - error!( - "User {} tried to log in with non-existent OIDC client id {}", + info!( + "User {} denied OIDC login with app id {}", session_info.user.username, data.client_id ); - error = CoreAuthErrorResponseType::UnauthorizedClient; + error = CoreAuthErrorResponseType::AccessDenied; } } else { - info!( - "User {} denied OIDC login with app id {}", + error!( + "User {} tried to log in with non-existent OIDC client id {}", session_info.user.username, data.client_id ); - error = CoreAuthErrorResponseType::AccessDenied; + error = CoreAuthErrorResponseType::UnauthorizedClient; } + let mut url = if is_redirect_allowed { + Url::parse(&data.redirect_uri).map_err(|_| WebError::Http(StatusCode::BAD_REQUEST))? + } else { + // Don't allow open redirects (DG25-17) + server_config().url.clone() + }; { let mut query_pairs = url.query_pairs_mut(); query_pairs.append_pair("error", error.as_ref()); diff --git a/crates/defguard_core/tests/integration/api/openid.rs b/crates/defguard_core/tests/integration/api/openid.rs index 7b701bba14..4aae97eaf8 100644 --- a/crates/defguard_core/tests/integration/api/openid.rs +++ b/crates/defguard_core/tests/integration/api/openid.rs @@ -19,13 +19,15 @@ use openidconnect::{ http::Method, }; use reqwest::{ - StatusCode, + StatusCode, Url, header::{AUTHORIZATION, CONTENT_TYPE, HeaderName, USER_AGENT}, }; use rsa::RsaPrivateKey; use serde::Deserialize; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; +use crate::api::common::client::TestResponse; + use super::common::{ client::TestClient, make_client, make_client_with_state, make_test_client, setup_pool, }; @@ -101,13 +103,13 @@ async fn test_openid_client(_: PgPoolOptions, options: PgConnectOptions) { async fn test_openid_flow(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; - let client = make_client(pool).await; + let (client, state) = make_client_with_state(pool).await; let auth = Auth::new("admin", "pass123"); let response = client.post("/api/v1/auth").json(&auth).send().await; assert_eq!(response.status(), StatusCode::OK); let openid_client = NewOpenIDClient { name: "Test".into(), - redirect_uri: vec!["http://localhost:3000/".into()], + redirect_uri: vec!["http://localhost:3000/".into(), "http://safe.net".into()], scope: vec!["openid".into()], enabled: true, }; @@ -251,6 +253,13 @@ async fn test_openid_flow(_: PgPoolOptions, options: PgConnectOptions) { let response = client.post("/api/v1/auth").json(&auth).send().await; assert_eq!(response.status(), StatusCode::OK); + let fallback_url = state + .config + .url + .to_string() + .trim_end_matches('/') + .to_string(); + // check code cannot be reused let response = client .post("/api/v1/oauth/token") @@ -286,34 +295,91 @@ async fn test_openid_flow(_: PgPoolOptions, options: PgConnectOptions) { .unwrap() .to_str() .unwrap(); + assert_eq!(response.status(), StatusCode::FOUND); + assert!(location.starts_with(&fallback_url)); assert!(location.contains("error")); - // test wrong invalid uri + // test invalid redirect uri let response = client - .post( + .post(format!( "/api/v1/oauth/authorize?\ response_type=code&\ - client_id=1&\ + client_id={}&\ redirect_uri=http%3A%2F%example%3A3000%2F&\ scope=openid&\ state=ABCDEF&\ nonce=blabla", - ) + openid_client.client_id + )) .send() .await; - assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert_eq!(response.status(), StatusCode::FOUND); + assert!(location.starts_with(&fallback_url)); + + // test non-whitelisted uri + let response = client + .post(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http%3A%2F%2Fexample%3A3000%3Fvalue1=one%26value2=two&\ + scope=openid&\ + state=ABCDEF&\ + nonce=blabla", + openid_client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + let location = response + .headers() + .get("Location") + .unwrap() + .to_str() + .unwrap(); + assert!(location.starts_with(&fallback_url)); + assert!(location.contains("error=access_denied")); + + // test whitelisted uri, invalid scope + let response = client + .post(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http://safe.net%3Fvalue1=one%26value2=two&\ + scope=profile&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + openid_client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + let location = response + .headers() + .get("Location") + .unwrap() + .to_str() + .unwrap(); + assert!(location.starts_with("http://safe.net")); + assert!(location.contains("error=invalid_scope")); + assert!(location.contains("value1=one")); + assert!(location.contains("value2=two")); // test wrong redirect uri let response = client - .post( + .post(format!( "/api/v1/oauth/authorize?\ response_type=code&\ - client_id=1&\ + client_id={}&\ redirect_uri=http%3A%2F%2Fexample%3A3000%3Fvalue1=one%26value2=two&\ scope=openid&\ state=ABCDEF&\ + allow=true&\ nonce=blabla", - ) + openid_client.client_id + )) .send() .await; assert_eq!(response.status(), StatusCode::FOUND); @@ -323,9 +389,8 @@ async fn test_openid_flow(_: PgPoolOptions, options: PgConnectOptions) { .unwrap() .to_str() .unwrap(); + assert!(location.starts_with(&fallback_url)); assert!(location.contains("error=access_denied")); - assert!(location.contains("value1=")); - assert!(location.contains("value2=")); // test allow false let response = client @@ -349,6 +414,7 @@ async fn test_openid_flow(_: PgPoolOptions, options: PgConnectOptions) { .unwrap() .to_str() .unwrap(); + assert!(location.starts_with("http://localhost:3000")); assert!(location.contains("error=access_denied")); } @@ -763,6 +829,194 @@ async fn dg25_23_test_openid_client_scope_change_clears_authorizations( ); } +#[sqlx::test] +async fn dg25_17_test_openid_open_redirects(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (client, state) = make_client_with_state(pool).await; + let _admin = User::find_by_username(&state.pool, "admin") + .await + .unwrap() + .unwrap(); + + // Authenticate admin + let auth = Auth::new("admin", "pass123"); + let response = client.post("/api/v1/auth").json(&auth).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // Create OAuth2 client + let oauth2client = NewOpenIDClient { + name: "Test Client".into(), + redirect_uri: vec!["http://localhost:3000/".into(), "http://safe.net/".into()], + scope: vec!["openid".into(), "email".into()], + enabled: true, + }; + + let response = client + .post("/api/v1/oauth") + .json(&oauth2client) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + let oauth2client: OAuth2Client = response.json().await; + + fn redirect_url(response: &TestResponse) -> String { + Url::parse( + response + .headers() + .get(reqwest::header::LOCATION) + .unwrap() + .to_str() + .unwrap(), + ) + .unwrap() + .origin() + .ascii_serialization() + } + + let fallback_url = state + .config + .url + .to_string() + .trim_end_matches('/') + .to_string(); + + // Try to authorize with allowed redirect url - invalid client id + let response = client + .post( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id=xxx&\ + redirect_uri=http://localhost:3000&\ + scope=openid email&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + ) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url,); + + let response = client + .get( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id=xxx&\ + redirect_uri=http://localhost:3000&\ + scope=openid email&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + ) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url); + + // Try to authorize with forbidden redirect url - invalid client id + let response = client + .post( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id=xxx&\ + redirect_uri=http://isec.pl&\ + scope=openid email&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + ) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url); + + let response = client + .get( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id=xxx&\ + redirect_uri=http://isec.pl&\ + scope=openid email&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + ) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url); + + // Try to authorize with forbidden redirect url - invalid scope + let response = client + .post(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http://isec.pl&\ + scope=profile&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + oauth2client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url); + + let response = client + .get(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http://isec.pl&\ + scope=profile&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + oauth2client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), fallback_url); + + // Same with allowed redirect_uri + let response = client + .post(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http://safe.net&\ + scope=profile&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + oauth2client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), "http://safe.net",); + + let response = client + .get(format!( + "/api/v1/oauth/authorize?\ + response_type=code&\ + client_id={}&\ + redirect_uri=http://safe.net&\ + scope=profile&\ + state=ABCDEF&\ + allow=true&\ + nonce=blabla", + oauth2client.client_id + )) + .send() + .await; + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(redirect_url(&response), "http://safe.net",); +} + #[sqlx::test] async fn dg25_22_test_respect_openid_scope_in_userinfo( _: PgPoolOptions, From bf0db2b0df8b0a3549c6053d571ca164df3af4f1 Mon Sep 17 00:00:00 2001 From: Jacek Chmielewski Date: Thu, 18 Sep 2025 08:27:14 +0200 Subject: [PATCH 06/50] ensure openid client names don't contain HTML (#1587) --- Cargo.lock | 187 ++++++++++++++++++ crates/defguard_core/Cargo.toml | 1 + .../src/handlers/openid_clients.rs | 20 ++ .../tests/integration/api/openid.rs | 68 +++++++ 4 files changed, 276 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index f4c4ff4e17..574a08e36d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -89,6 +89,19 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "ammonia" +version = "4.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6b346764dd0814805de8abf899fe03065bcee69bb1a4771c785817e39f3978f" +dependencies = [ + "cssparser", + "html5ever", + "maplit", + "tendril", + "url", +] + [[package]] name = "android_system_properties" version = "0.1.5" @@ -921,6 +934,29 @@ dependencies = [ "typenum", ] +[[package]] +name = "cssparser" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e901edd733a1472f944a45116df3f846f54d37e67e68640ac8bb69689aca2aa" +dependencies = [ + "cssparser-macros", + "dtoa-short", + "itoa", + "phf", + "smallvec", +] + +[[package]] +name = "cssparser-macros" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13b588ba4ac1a99f7f2964d24b3d896ddc6bf847ee3855dbd4366f058cfcd331" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "ctr" version = "0.9.2" @@ -1044,6 +1080,7 @@ dependencies = [ name = "defguard_core" version = "1.5.0" dependencies = [ + "ammonia", "anyhow", "argon2", "axum", @@ -1358,6 +1395,21 @@ dependencies = [ "zeroize", ] +[[package]] +name = "dtoa" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6add3b8cff394282be81f3fc1a0605db594ed69890078ca6e2cab1c408bcf04" + +[[package]] +name = "dtoa-short" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd1511a7b6a56299bd043a9c167a6d2bfb37bf84a6dfceaba651168adfb43c87" +dependencies = [ + "dtoa", +] + [[package]] name = "dyn-clone" version = "1.0.20" @@ -1622,6 +1674,16 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" +[[package]] +name = "futf" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df420e2e84819663797d1ec6544b13c5be84629e7bb00dc960d6917db2987843" +dependencies = [ + "mac", + "new_debug_unreachable", +] + [[package]] name = "futures" version = "0.3.31" @@ -1968,6 +2030,17 @@ dependencies = [ "windows-link 0.1.3", ] +[[package]] +name = "html5ever" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55d958c2f74b664487a2035fe1dadb032c48718a03b63f3ab0b8537db8549ed4" +dependencies = [ + "log", + "markup5ever", + "match_token", +] + [[package]] name = "http" version = "1.3.1" @@ -2642,6 +2715,40 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" +[[package]] +name = "mac" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" + +[[package]] +name = "maplit" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" + +[[package]] +name = "markup5ever" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "311fe69c934650f8f19652b3946075f0fc41ad8757dbb68f1ca14e7900ecc1c3" +dependencies = [ + "log", + "tendril", + "web_atoms", +] + +[[package]] +name = "match_token" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac84fd3f360fcc43dc5f5d186f02a94192761a080e8bc58621ad4d12296a58cf" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "matchers" version = "0.2.0" @@ -2761,6 +2868,12 @@ dependencies = [ "tempfile", ] +[[package]] +name = "new_debug_unreachable" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086" + [[package]] name = "nom" version = "7.1.3" @@ -3338,6 +3451,7 @@ version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1fd6780a80ae0c52cc120a26a1a42c1ae51b247a253e4e06113d23d2c2edd078" dependencies = [ + "phf_macros", "phf_shared", ] @@ -3361,6 +3475,19 @@ dependencies = [ "rand 0.8.5", ] +[[package]] +name = "phf_macros" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f84ac04429c13a7ff43785d75ad27569f2951ce0ffd30a3321230db2fc727216" +dependencies = [ + "phf_generator", + "phf_shared", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "phf_shared" version = "0.11.3" @@ -3478,6 +3605,12 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "precomputed-hash" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c" + [[package]] name = "prettyplease" version = "0.2.37" @@ -4846,6 +4979,31 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "string_cache" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf776ba3fa74f83bf4b63c3dcbbf82173db2632ed8452cb2d891d33f459de70f" +dependencies = [ + "new_debug_unreachable", + "parking_lot", + "phf_shared", + "precomputed-hash", + "serde", +] + +[[package]] +name = "string_cache_codegen" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c711928715f1fe0fe509c53b43e993a9a557babc2d0a3567d0a3006f1ac931a0" +dependencies = [ + "phf_generator", + "phf_shared", + "proc-macro2", + "quote", +] + [[package]] name = "stringprep" version = "0.1.5" @@ -4981,6 +5139,17 @@ dependencies = [ "windows-sys 0.61.0", ] +[[package]] +name = "tendril" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d24a120c5fc464a3458240ee02c299ebcb9d67b5249c8848b09d639dca8d7bb0" +dependencies = [ + "futf", + "mac", + "utf-8", +] + [[package]] name = "tera" version = "1.20.0" @@ -5627,6 +5796,12 @@ dependencies = [ "serde", ] +[[package]] +name = "utf-8" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09cc8ee72d2a9becf2f2febe0205bbed8fc6615b7cb429ad062dc7b7ddd036a9" + [[package]] name = "utf8_iter" version = "1.0.4" @@ -5912,6 +6087,18 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "web_atoms" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57ffde1dc01240bdf9992e3205668b235e59421fd085e8a317ed98da0178d414" +dependencies = [ + "phf", + "phf_codegen", + "string_cache", + "string_cache_codegen", +] + [[package]] name = "webauthn-attestation-ca" version = "0.5.2" diff --git a/crates/defguard_core/Cargo.toml b/crates/defguard_core/Cargo.toml index 10083cf94f..3ee6c10de5 100644 --- a/crates/defguard_core/Cargo.toml +++ b/crates/defguard_core/Cargo.toml @@ -85,6 +85,7 @@ bytes = { workspace = true } ed25519-dalek = { version = "2.2", features = ["rand_core"] } tower = "0.5" regex = "1.10" +ammonia = "4.1.1" [dev-dependencies] bytes = "1.6" diff --git a/crates/defguard_core/src/handlers/openid_clients.rs b/crates/defguard_core/src/handlers/openid_clients.rs index cc6a0bd52e..e0a911590d 100644 --- a/crates/defguard_core/src/handlers/openid_clients.rs +++ b/crates/defguard_core/src/handlers/openid_clients.rs @@ -26,6 +26,16 @@ pub async fn add_openid_client( "User {} adding OpenID client {}", session.user.username, data.name ); + if ammonia::is_html(&data.name) { + warn!( + "User {} attempted to create openid client with name containing HTML: {}", + session.user.username, data.name + ); + return Ok(ApiResponse { + json: json!({"msg": "invalid name"}), + status: StatusCode::BAD_REQUEST, + }); + } let client = OAuth2Client::from_new(data).save(&appstate.pool).await?; info!( "User {} added OpenID client {}", @@ -89,6 +99,16 @@ pub async fn change_openid_client( "User {} updating OpenID client {client_id}...", session.user.username ); + if ammonia::is_html(&data.name) { + warn!( + "User {} attempted to edit openid client with name containing HTML: {}", + session.user.username, data.name + ); + return Ok(ApiResponse { + json: json!({"msg": "invalid name"}), + status: StatusCode::BAD_REQUEST, + }); + } let mut transaction = appstate.pool.begin().await?; let status = match OAuth2Client::find_by_client_id(&mut *transaction, &client_id).await? { Some(mut client) => { diff --git a/crates/defguard_core/tests/integration/api/openid.rs b/crates/defguard_core/tests/integration/api/openid.rs index 4aae97eaf8..ec2ba54daf 100644 --- a/crates/defguard_core/tests/integration/api/openid.rs +++ b/crates/defguard_core/tests/integration/api/openid.rs @@ -1197,6 +1197,74 @@ async fn dg25_22_test_respect_openid_scope_in_userinfo( assert!(claims.phone_number().is_none()); } +#[sqlx::test] +async fn dg25_21_test_openid_html_injection(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + + let client = make_client(pool).await; + let auth = Auth::new("admin", "pass123"); + let response = client.post("/api/v1/auth").json(&auth).send().await; + assert_eq!(response.status(), StatusCode::OK); + + let invalid_names = &[ + "Test Click", + "Test Click", + "Test ", + "Test