From 72d5b7cebd65818469c87f2c7501edb2ec2838d0 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:37:29 +0200 Subject: [PATCH 1/4] sanitize ldap errors --- .../src/enterprise/ldap/error.rs | 49 ++++++++++++++++++- .../tests/proxy_manager/handler/polling.rs | 3 +- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/crates/defguard_core/src/enterprise/ldap/error.rs b/crates/defguard_core/src/enterprise/ldap/error.rs index 3b32d92486..c285527ba5 100644 --- a/crates/defguard_core/src/enterprise/ldap/error.rs +++ b/crates/defguard_core/src/enterprise/ldap/error.rs @@ -1,10 +1,22 @@ use defguard_common::db::models::settings::SettingsSaveError; use thiserror::Error; +/// Strips null bytes and non-printable control characters from LDAP error strings. +/// LDAP server responses may embed raw binary data or null terminators in diagnostic +/// fields (e.g. `LdapResult.text`, `LdapResult.matched`) that corrupt log output. +fn sanitize_ldap_string(s: &str) -> String { + s.chars() + .filter(|&c| c == '\n' || c == '\t' || !c.is_control()) + .collect() +} + #[derive(Debug, Error)] pub enum LdapError { + /// Sanitized string representation of an ldap3 error. Stored as String (rather than + /// the original ldap3::LdapError) so null bytes and control chars are stripped once + /// at conversion time and can never surface through Display or Debug. #[error("LDAP error: {0}")] - Ldap(#[from] ldap3::LdapError), + Ldap(String), #[error("Object not found: {0}")] ObjectNotFound(String), #[error("Missing required LDAP settings: {0}")] @@ -35,3 +47,38 @@ pub enum LdapError { #[error("User {0} does not belong to the defined synchronization groups in {1}")] UserNotInLDAPSyncGroups(String, &'static str), } + +impl From for LdapError { + fn from(err: ldap3::LdapError) -> Self { + Self::Ldap(sanitize_ldap_string(&err.to_string())) + } +} + +#[cfg(test)] +mod tests { + use super::sanitize_ldap_string; + + #[test] + fn sanitize_ldap_string_strips_control_chars() { + // Null bytes, bells, and other control chars should be removed. + assert_eq!(sanitize_ldap_string("hello\0world"), "helloworld"); + assert_eq!(sanitize_ldap_string("text\x01\x02\x03"), "text"); + assert_eq!( + sanitize_ldap_string("rc=49, dn: \"\0\", text: \"invalid\0creds\""), + "rc=49, dn: \"\", text: \"invalidcreds\"" + ); + + // Tabs and newlines should be kept. + assert_eq!( + sanitize_ldap_string("line1\nline2\ttabbed"), + "line1\nline2\ttabbed" + ); + + // Printable ASCII and Unicode should pass through unchanged. + assert_eq!( + sanitize_ldap_string("normal error text"), + "normal error text" + ); + assert_eq!(sanitize_ldap_string("ünïcödé"), "ünïcödé"); + } +} diff --git a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/polling.rs b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/polling.rs index 726f87b360..51192362a1 100644 --- a/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/polling.rs +++ b/crates/defguard_proxy_manager/src/tests/proxy_manager/handler/polling.rs @@ -1,9 +1,8 @@ -use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; - use defguard_proto::{ client_types::InstanceInfoRequest, proxy::{CoreRequest, core_request, core_response}, }; +use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use super::support::{ assert_error_response, clear_test_license, complete_proxy_handshake, create_device_for_user, From be17c666fe2cb769077ce8b9efab8fb2d096fac4 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:38:08 +0200 Subject: [PATCH 2/4] graceful activity log restart --- crates/defguard_event_logger/src/lib.rs | 972 ++++++++++++------------ 1 file changed, 488 insertions(+), 484 deletions(-) diff --git a/crates/defguard_event_logger/src/lib.rs b/crates/defguard_event_logger/src/lib.rs index 237dbdf53b..bee0c7b92e 100644 --- a/crates/defguard_event_logger/src/lib.rs +++ b/crates/defguard_event_logger/src/lib.rs @@ -88,8 +88,8 @@ fn map_vpn_event(event: VpnEvent) -> (EventType, Option) { /// Run the event logger service /// -/// This function runs in an infinite loop, receiving messages from the event_logger_rx channel -/// and storing them in the database as activity log events. +/// Runs in an infinite loop processing batches of activity log events. Any database errors +/// are logged and the loop restarts rather than propagating the error. pub async fn run_event_logger( pool: PgPool, mut event_logger_rx: UnboundedReceiver, @@ -97,7 +97,6 @@ pub async fn run_event_logger( ) -> Result<(), EventLoggerError> { info!("Starting activity log event logger service"); - // Receive messages in an infinite loop loop { // Collect multiple messages from the channel (up to MESSAGE_LIMIT at a time) let mut message_buffer: Vec = Vec::with_capacity(MESSAGE_LIMIT); @@ -105,522 +104,527 @@ pub async fn run_event_logger( .recv_many(&mut message_buffer, MESSAGE_LIMIT) .await; + // Channel has been closed, shut down gracefully + if message_count == 0 { + info!("Event logger channel closed, shutting down"); + return Ok(()); + } + debug!("Processing batch of {message_count} activity log events"); - let mut transaction = pool.begin().await?; - let mut serialized_activity_log_events = String::new(); + if let Err(e) = process_batch(&pool, message_buffer, &activity_log_messages_tx).await { + error!("Failed to process activity log event batch, batch will be discarded: {e}"); + } + } +} - // Process all messages in the batch - for message in message_buffer { - // Unpack shared event context - let EventContext { - user_id, - username, - location, - timestamp, - ip, - device, - } = message.context; +async fn process_batch( + pool: &PgPool, + message_buffer: Vec, + activity_log_messages_tx: &tokio::sync::broadcast::Sender, +) -> Result<(), EventLoggerError> { + let mut transaction = pool.begin().await?; + let mut serialized_activity_log_events = String::new(); + + // Process all messages in the batch + for message in message_buffer { + // Unpack shared event context + let EventContext { + user_id, + username, + location, + timestamp, + ip, + device, + } = message.context; - // Convert each message to a related activity log event - let activity_log_event = { - let (module, event, description, metadata) = match message.event { - LoggerEvent::Defguard(event) => { - let module = ActivityLogModule::Defguard; - let description = get_defguard_event_description(&event); + // Convert each message to a related activity log event + let activity_log_event = { + let (module, event, description, metadata) = match message.event { + LoggerEvent::Defguard(event) => { + let module = ActivityLogModule::Defguard; + let description = get_defguard_event_description(&event); - let (event_type, metadata) = match *event { - DefguardEvent::UserLogin => (EventType::UserLogin, None), - DefguardEvent::UserLoginFailed { message } => ( - EventType::UserLoginFailed, - serde_json::to_value(LoginFailedMetadata { message }).ok(), - ), - DefguardEvent::UserMfaLogin { mfa_method } => ( - EventType::UserMfaLogin, - serde_json::to_value(MfaLoginMetadata { mfa_method }).ok(), - ), - DefguardEvent::UserMfaLoginFailed { + let (event_type, metadata) = match *event { + DefguardEvent::UserLogin => (EventType::UserLogin, None), + DefguardEvent::UserLoginFailed { message } => ( + EventType::UserLoginFailed, + serde_json::to_value(LoginFailedMetadata { message }).ok(), + ), + DefguardEvent::UserMfaLogin { mfa_method } => ( + EventType::UserMfaLogin, + serde_json::to_value(MfaLoginMetadata { mfa_method }).ok(), + ), + DefguardEvent::UserMfaLoginFailed { + mfa_method, + message, + } => ( + EventType::UserMfaLoginFailed, + serde_json::to_value(MfaLoginFailedMetadata { mfa_method, message, - } => ( - EventType::UserMfaLoginFailed, - serde_json::to_value(MfaLoginFailedMetadata { - mfa_method, - message, - }) - .ok(), - ), - DefguardEvent::UserLogout => (EventType::UserLogout, None), - DefguardEvent::UserDeviceAdded { owner, device } => ( - EventType::DeviceAdded, - serde_json::to_value(DeviceMetadata { - owner: owner.into(), - device, - }) - .ok(), - ), - DefguardEvent::UserDeviceRemoved { owner, device } => ( - EventType::DeviceRemoved, - serde_json::to_value(DeviceMetadata { - owner: owner.into(), - device, - }) - .ok(), - ), - DefguardEvent::UserDeviceModified { - owner, + }) + .ok(), + ), + DefguardEvent::UserLogout => (EventType::UserLogout, None), + DefguardEvent::UserDeviceAdded { owner, device } => ( + EventType::DeviceAdded, + serde_json::to_value(DeviceMetadata { + owner: owner.into(), + device, + }) + .ok(), + ), + DefguardEvent::UserDeviceRemoved { owner, device } => ( + EventType::DeviceRemoved, + serde_json::to_value(DeviceMetadata { + owner: owner.into(), + device, + }) + .ok(), + ), + DefguardEvent::UserDeviceModified { + owner, + before, + after, + } => ( + EventType::DeviceModified, + serde_json::to_value(DeviceModifiedMetadata { + owner: owner.into(), before, after, - } => ( - EventType::DeviceModified, - serde_json::to_value(DeviceModifiedMetadata { - owner: owner.into(), - before, - after, - }) - .ok(), - ), - DefguardEvent::UserGroupsModified { - user, + }) + .ok(), + ), + DefguardEvent::UserGroupsModified { + user, + before, + after, + } => ( + EventType::UserGroupsModified, + serde_json::to_value(UserGroupsModifiedMetadata { + user: user.into(), before, after, - } => ( - EventType::UserGroupsModified, - serde_json::to_value(UserGroupsModifiedMetadata { - user: user.into(), - before, - after, - }) + }) + .ok(), + ), + DefguardEvent::RecoveryCodeUsed => (EventType::RecoveryCodeUsed, None), + DefguardEvent::PasswordChanged => (EventType::PasswordChanged, None), + DefguardEvent::PasswordChangedByAdmin { user } => ( + EventType::PasswordChangedByAdmin, + serde_json::to_value(PasswordChangedByAdminMetadata { + user: user.into(), + }) + .ok(), + ), + DefguardEvent::MfaDisabled => (EventType::MfaDisabled, None), + DefguardEvent::UserMfaDisabled { user } => ( + EventType::UserMfaDisabled, + serde_json::to_value(UserMfaDisabledMetadata { user: user.into() }) + .ok(), + ), + DefguardEvent::MfaTotpEnabled => (EventType::MfaTotpEnabled, None), + DefguardEvent::MfaTotpDisabled => (EventType::MfaTotpDisabled, None), + DefguardEvent::MfaEmailEnabled => (EventType::MfaEmailEnabled, None), + DefguardEvent::MfaEmailDisabled => (EventType::MfaEmailDisabled, None), + DefguardEvent::MfaSecurityKeyAdded { key } => ( + EventType::MfaSecurityKeyAdded, + serde_json::to_value(MfaSecurityKeyMetadata { key: key.into() }).ok(), + ), + DefguardEvent::MfaSecurityKeyRemoved { key } => ( + EventType::MfaSecurityKeyRemoved, + serde_json::to_value(MfaSecurityKeyMetadata { key: key.into() }).ok(), + ), + DefguardEvent::AuthenticationKeyAdded { key } => ( + EventType::AuthenticationKeyAdded, + serde_json::to_value(AuthenticationKeyMetadata { key: key.into() }) .ok(), - ), - DefguardEvent::RecoveryCodeUsed => (EventType::RecoveryCodeUsed, None), - DefguardEvent::PasswordChanged => (EventType::PasswordChanged, None), - DefguardEvent::PasswordChangedByAdmin { user } => ( - EventType::PasswordChangedByAdmin, - serde_json::to_value(PasswordChangedByAdminMetadata { - user: user.into(), - }) + ), + DefguardEvent::AuthenticationKeyRemoved { key } => ( + EventType::AuthenticationKeyRemoved, + serde_json::to_value(AuthenticationKeyMetadata { key: key.into() }) .ok(), - ), - DefguardEvent::MfaDisabled => (EventType::MfaDisabled, None), - DefguardEvent::UserMfaDisabled { user } => ( - EventType::UserMfaDisabled, - serde_json::to_value(UserMfaDisabledMetadata { user: user.into() }) - .ok(), - ), - DefguardEvent::MfaTotpEnabled => (EventType::MfaTotpEnabled, None), - DefguardEvent::MfaTotpDisabled => (EventType::MfaTotpDisabled, None), - DefguardEvent::MfaEmailEnabled => (EventType::MfaEmailEnabled, None), - DefguardEvent::MfaEmailDisabled => (EventType::MfaEmailDisabled, None), - DefguardEvent::MfaSecurityKeyAdded { key } => ( - EventType::MfaSecurityKeyAdded, - serde_json::to_value(MfaSecurityKeyMetadata { key: key.into() }) - .ok(), - ), - DefguardEvent::MfaSecurityKeyRemoved { key } => ( - EventType::MfaSecurityKeyRemoved, - serde_json::to_value(MfaSecurityKeyMetadata { key: key.into() }) - .ok(), - ), - DefguardEvent::AuthenticationKeyAdded { key } => ( - EventType::AuthenticationKeyAdded, - serde_json::to_value(AuthenticationKeyMetadata { key: key.into() }) - .ok(), - ), - DefguardEvent::AuthenticationKeyRemoved { key } => ( - EventType::AuthenticationKeyRemoved, - serde_json::to_value(AuthenticationKeyMetadata { key: key.into() }) - .ok(), - ), - DefguardEvent::AuthenticationKeyRenamed { - key, + ), + DefguardEvent::AuthenticationKeyRenamed { + key, + old_name, + new_name, + } => ( + EventType::AuthenticationKeyRenamed, + serde_json::to_value(AuthenticationKeyRenamedMetadata { + key: key.into(), old_name, new_name, - } => ( - EventType::AuthenticationKeyRenamed, - serde_json::to_value(AuthenticationKeyRenamedMetadata { - key: key.into(), - old_name, - new_name, - }) - .ok(), - ), - DefguardEvent::ApiTokenAdded { owner, token } => ( - EventType::ApiTokenAdded, - serde_json::to_value(ApiTokenMetadata { - owner: owner.into(), - token: token.into(), - }) - .ok(), - ), - DefguardEvent::ApiTokenRemoved { owner, token } => ( - EventType::ApiTokenRemoved, - serde_json::to_value(ApiTokenMetadata { - owner: owner.into(), - token: token.into(), - }) - .ok(), - ), - DefguardEvent::ApiTokenRenamed { - owner, - token, + }) + .ok(), + ), + DefguardEvent::ApiTokenAdded { owner, token } => ( + EventType::ApiTokenAdded, + serde_json::to_value(ApiTokenMetadata { + owner: owner.into(), + token: token.into(), + }) + .ok(), + ), + DefguardEvent::ApiTokenRemoved { owner, token } => ( + EventType::ApiTokenRemoved, + serde_json::to_value(ApiTokenMetadata { + owner: owner.into(), + token: token.into(), + }) + .ok(), + ), + DefguardEvent::ApiTokenRenamed { + owner, + token, + old_name, + new_name, + } => ( + EventType::ApiTokenRenamed, + serde_json::to_value(ApiTokenRenamedMetadata { + owner: owner.into(), + token: token.into(), old_name, new_name, - } => ( - EventType::ApiTokenRenamed, - serde_json::to_value(ApiTokenRenamedMetadata { - owner: owner.into(), - token: token.into(), - old_name, - new_name, - }) - .ok(), - ), - DefguardEvent::UserAdded { user } => ( - EventType::UserAdded, - serde_json::to_value(UserMetadata { user: user.into() }).ok(), - ), - DefguardEvent::UserRemoved { user } => ( - EventType::UserRemoved, - serde_json::to_value(UserMetadata { user: user.into() }).ok(), - ), - DefguardEvent::UserModified { before, after } => ( - EventType::UserModified, - serde_json::to_value(UserModifiedMetadata { - before: before.into(), - after: after.into(), - }) - .ok(), - ), - DefguardEvent::NetworkDeviceAdded { device, location } => ( - EventType::NetworkDeviceAdded, - serde_json::to_value(NetworkDeviceMetadata { device, location }) - .ok(), - ), - DefguardEvent::NetworkDeviceRemoved { device, location } => ( - EventType::NetworkDeviceRemoved, - serde_json::to_value(NetworkDeviceMetadata { device, location }) - .ok(), - ), - DefguardEvent::NetworkDeviceModified { - location, + }) + .ok(), + ), + DefguardEvent::UserAdded { user } => ( + EventType::UserAdded, + serde_json::to_value(UserMetadata { user: user.into() }).ok(), + ), + DefguardEvent::UserRemoved { user } => ( + EventType::UserRemoved, + serde_json::to_value(UserMetadata { user: user.into() }).ok(), + ), + DefguardEvent::UserModified { before, after } => ( + EventType::UserModified, + serde_json::to_value(UserModifiedMetadata { + before: before.into(), + after: after.into(), + }) + .ok(), + ), + DefguardEvent::NetworkDeviceAdded { device, location } => ( + EventType::NetworkDeviceAdded, + serde_json::to_value(NetworkDeviceMetadata { device, location }).ok(), + ), + DefguardEvent::NetworkDeviceRemoved { device, location } => ( + EventType::NetworkDeviceRemoved, + serde_json::to_value(NetworkDeviceMetadata { device, location }).ok(), + ), + DefguardEvent::NetworkDeviceModified { + location, + before, + after, + } => ( + EventType::NetworkDeviceModified, + serde_json::to_value(NetworkDeviceModifiedMetadata { before, after, - } => ( - EventType::NetworkDeviceModified, - serde_json::to_value(NetworkDeviceModifiedMetadata { - before, - after, - location, - }) - .ok(), - ), - DefguardEvent::VpnLocationAdded { location } => ( - EventType::VpnLocationAdded, - serde_json::to_value(VpnLocationMetadata { location }).ok(), - ), - DefguardEvent::VpnLocationRemoved { location } => ( - EventType::VpnLocationRemoved, - serde_json::to_value(VpnLocationMetadata { location }).ok(), - ), - DefguardEvent::VpnLocationModified { before, after } => ( - EventType::VpnLocationModified, - serde_json::to_value(VpnLocationModifiedMetadata { before, after }) - .ok(), - ), - DefguardEvent::OpenIdAppAdded { app } => ( - EventType::OpenIdAppAdded, - serde_json::to_value(OpenIdAppMetadata { app: app.into() }).ok(), - ), - DefguardEvent::OpenIdAppRemoved { app } => ( - EventType::OpenIdAppRemoved, - serde_json::to_value(OpenIdAppMetadata { app: app.into() }).ok(), - ), - DefguardEvent::OpenIdAppModified { before, after } => ( - EventType::OpenIdAppModified, - serde_json::to_value(OpenIdAppModifiedMetadata { - before: before.into(), - after: after.into(), - }) - .ok(), - ), - DefguardEvent::OpenIdAppStateChanged { app, enabled } => ( - EventType::OpenIdAppStateChanged, - serde_json::to_value(OpenIdAppStateChangedMetadata { - app: app.into(), - enabled, - }) - .ok(), - ), - DefguardEvent::OpenIdProviderModified { provider } => ( - EventType::OpenIdProviderModified, - serde_json::to_value(OpenIdProviderMetadata { - provider: provider.into(), - }) - .ok(), - ), - DefguardEvent::OpenIdProviderRemoved { provider } => ( - EventType::OpenIdProviderRemoved, - serde_json::to_value(OpenIdProviderMetadata { - provider: provider.into(), - }) - .ok(), - ), - DefguardEvent::SettingsUpdatedPartial { before, after } => ( - EventType::SettingsUpdatedPartial, - serde_json::to_value(SettingsUpdateMetadata { - before: before.into(), - after: after.into(), - }) - .ok(), - ), - DefguardEvent::SettingsUpdated { before, after } => ( - EventType::SettingsUpdated, - serde_json::to_value(SettingsUpdateMetadata { - before: before.into(), - after: after.into(), - }) - .ok(), - ), - DefguardEvent::SettingsDefaultBrandingRestored => { - (EventType::SettingsDefaultBrandingRestored, None) - } - DefguardEvent::ActivityLogStreamCreated { stream } => ( - EventType::ActivityLogStreamCreated, - serde_json::to_value(ActivityLogStreamMetadata { - stream: stream.into(), - }) - .ok(), - ), - DefguardEvent::ActivityLogStreamRemoved { stream } => ( - EventType::ActivityLogStreamRemoved, - serde_json::to_value(ActivityLogStreamMetadata { - stream: stream.into(), - }) - .ok(), - ), - DefguardEvent::ActivityLogStreamModified { before, after } => ( - EventType::ActivityLogStreamModified, - serde_json::to_value(ActivityLogStreamModifiedMetadata { - before: before.into(), - after: after.into(), - }) - .ok(), - ), - DefguardEvent::GroupsBulkAssigned { users, groups } => ( - EventType::GroupsBulkAssigned, - serde_json::to_value(GroupsBulkAssignedMetadata { - users: users.into_iter().map(Into::into).collect(), - groups, - }) - .ok(), - ), - DefguardEvent::GroupAdded { group } => ( - EventType::GroupAdded, - serde_json::to_value(GroupMetadata { group }).ok(), - ), - DefguardEvent::GroupModified { before, after } => ( - EventType::GroupModified, - serde_json::to_value(GroupModifiedMetadata { before, after }).ok(), - ), - DefguardEvent::GroupRemoved { group } => ( - EventType::GroupRemoved, - serde_json::to_value(GroupMetadata { group }).ok(), - ), - DefguardEvent::GroupMemberAdded { group, user } => ( - EventType::GroupMemberAdded, - serde_json::to_value(GroupAssignedMetadata { - group, - user: user.into(), - }) - .ok(), - ), - DefguardEvent::GroupMemberRemoved { group, user } => ( - EventType::GroupMemberRemoved, - serde_json::to_value(GroupAssignedMetadata { - group, - user: user.into(), - }) + location, + }) + .ok(), + ), + DefguardEvent::VpnLocationAdded { location } => ( + EventType::VpnLocationAdded, + serde_json::to_value(VpnLocationMetadata { location }).ok(), + ), + DefguardEvent::VpnLocationRemoved { location } => ( + EventType::VpnLocationRemoved, + serde_json::to_value(VpnLocationMetadata { location }).ok(), + ), + DefguardEvent::VpnLocationModified { before, after } => ( + EventType::VpnLocationModified, + serde_json::to_value(VpnLocationModifiedMetadata { before, after }) .ok(), - ), - DefguardEvent::GroupMembersModified { + ), + DefguardEvent::OpenIdAppAdded { app } => ( + EventType::OpenIdAppAdded, + serde_json::to_value(OpenIdAppMetadata { app: app.into() }).ok(), + ), + DefguardEvent::OpenIdAppRemoved { app } => ( + EventType::OpenIdAppRemoved, + serde_json::to_value(OpenIdAppMetadata { app: app.into() }).ok(), + ), + DefguardEvent::OpenIdAppModified { before, after } => ( + EventType::OpenIdAppModified, + serde_json::to_value(OpenIdAppModifiedMetadata { + before: before.into(), + after: after.into(), + }) + .ok(), + ), + DefguardEvent::OpenIdAppStateChanged { app, enabled } => ( + EventType::OpenIdAppStateChanged, + serde_json::to_value(OpenIdAppStateChangedMetadata { + app: app.into(), + enabled, + }) + .ok(), + ), + DefguardEvent::OpenIdProviderModified { provider } => ( + EventType::OpenIdProviderModified, + serde_json::to_value(OpenIdProviderMetadata { + provider: provider.into(), + }) + .ok(), + ), + DefguardEvent::OpenIdProviderRemoved { provider } => ( + EventType::OpenIdProviderRemoved, + serde_json::to_value(OpenIdProviderMetadata { + provider: provider.into(), + }) + .ok(), + ), + DefguardEvent::SettingsUpdatedPartial { before, after } => ( + EventType::SettingsUpdatedPartial, + serde_json::to_value(SettingsUpdateMetadata { + before: before.into(), + after: after.into(), + }) + .ok(), + ), + DefguardEvent::SettingsUpdated { before, after } => ( + EventType::SettingsUpdated, + serde_json::to_value(SettingsUpdateMetadata { + before: before.into(), + after: after.into(), + }) + .ok(), + ), + DefguardEvent::SettingsDefaultBrandingRestored => { + (EventType::SettingsDefaultBrandingRestored, None) + } + DefguardEvent::ActivityLogStreamCreated { stream } => ( + EventType::ActivityLogStreamCreated, + serde_json::to_value(ActivityLogStreamMetadata { + stream: stream.into(), + }) + .ok(), + ), + DefguardEvent::ActivityLogStreamRemoved { stream } => ( + EventType::ActivityLogStreamRemoved, + serde_json::to_value(ActivityLogStreamMetadata { + stream: stream.into(), + }) + .ok(), + ), + DefguardEvent::ActivityLogStreamModified { before, after } => ( + EventType::ActivityLogStreamModified, + serde_json::to_value(ActivityLogStreamModifiedMetadata { + before: before.into(), + after: after.into(), + }) + .ok(), + ), + DefguardEvent::GroupsBulkAssigned { users, groups } => ( + EventType::GroupsBulkAssigned, + serde_json::to_value(GroupsBulkAssignedMetadata { + users: users.into_iter().map(Into::into).collect(), + groups, + }) + .ok(), + ), + DefguardEvent::GroupAdded { group } => ( + EventType::GroupAdded, + serde_json::to_value(GroupMetadata { group }).ok(), + ), + DefguardEvent::GroupModified { before, after } => ( + EventType::GroupModified, + serde_json::to_value(GroupModifiedMetadata { before, after }).ok(), + ), + DefguardEvent::GroupRemoved { group } => ( + EventType::GroupRemoved, + serde_json::to_value(GroupMetadata { group }).ok(), + ), + DefguardEvent::GroupMemberAdded { group, user } => ( + EventType::GroupMemberAdded, + serde_json::to_value(GroupAssignedMetadata { group, - added, - removed, - } => ( - EventType::GroupMembersModified, - serde_json::to_value(GroupMembersModifiedMetadata { - group, - added: added.into_iter().map(Into::into).collect(), - removed: removed.into_iter().map(Into::into).collect(), - }) - .ok(), - ), - DefguardEvent::WebHookAdded { webhook } => ( - EventType::WebHookAdded, - serde_json::to_value(WebHookMetadata { webhook }).ok(), - ), - DefguardEvent::WebHookModified { before, after } => ( - EventType::WebHookModified, - serde_json::to_value(WebHookModifiedMetadata { before, after }) - .ok(), - ), - DefguardEvent::WebHookRemoved { webhook } => ( - EventType::WebHookRemoved, - serde_json::to_value(WebHookMetadata { webhook }).ok(), - ), - DefguardEvent::WebHookStateChanged { webhook, enabled } => ( - EventType::WebHookStateChanged, - serde_json::to_value(WebHookStateChangedMetadata { - webhook, - enabled, - }) - .ok(), - ), - DefguardEvent::PasswordReset { user } => ( - EventType::PasswordReset, - serde_json::to_value(PasswordResetMetadata { user: user.into() }) - .ok(), - ), - DefguardEvent::ClientConfigurationTokenAdded { user } => ( - EventType::ClientConfigurationTokenAdded, - serde_json::to_value(ClientConfigurationTokenMetadata { - user: user.into(), - }) - .ok(), - ), - DefguardEvent::UserSnatBindingAdded { user, binding } => ( - EventType::UserSnatBindingAdded, - serde_json::to_value(UserSnatBindingMetadata { - user: user.into(), - binding, - }) - .ok(), - ), - DefguardEvent::UserSnatBindingRemoved { user, binding } => ( - EventType::UserSnatBindingRemoved, - serde_json::to_value(UserSnatBindingMetadata { - user: user.into(), - binding, - }) + user: user.into(), + }) + .ok(), + ), + DefguardEvent::GroupMemberRemoved { group, user } => ( + EventType::GroupMemberRemoved, + serde_json::to_value(GroupAssignedMetadata { + group, + user: user.into(), + }) + .ok(), + ), + DefguardEvent::GroupMembersModified { + group, + added, + removed, + } => ( + EventType::GroupMembersModified, + serde_json::to_value(GroupMembersModifiedMetadata { + group, + added: added.into_iter().map(Into::into).collect(), + removed: removed.into_iter().map(Into::into).collect(), + }) + .ok(), + ), + DefguardEvent::WebHookAdded { webhook } => ( + EventType::WebHookAdded, + serde_json::to_value(WebHookMetadata { webhook }).ok(), + ), + DefguardEvent::WebHookModified { before, after } => ( + EventType::WebHookModified, + serde_json::to_value(WebHookModifiedMetadata { before, after }).ok(), + ), + DefguardEvent::WebHookRemoved { webhook } => ( + EventType::WebHookRemoved, + serde_json::to_value(WebHookMetadata { webhook }).ok(), + ), + DefguardEvent::WebHookStateChanged { webhook, enabled } => ( + EventType::WebHookStateChanged, + serde_json::to_value(WebHookStateChangedMetadata { webhook, enabled }) .ok(), - ), - DefguardEvent::UserSnatBindingModified { - user, + ), + DefguardEvent::PasswordReset { user } => ( + EventType::PasswordReset, + serde_json::to_value(PasswordResetMetadata { user: user.into() }).ok(), + ), + DefguardEvent::ClientConfigurationTokenAdded { user } => ( + EventType::ClientConfigurationTokenAdded, + serde_json::to_value(ClientConfigurationTokenMetadata { + user: user.into(), + }) + .ok(), + ), + DefguardEvent::UserSnatBindingAdded { user, binding } => ( + EventType::UserSnatBindingAdded, + serde_json::to_value(UserSnatBindingMetadata { + user: user.into(), + binding, + }) + .ok(), + ), + DefguardEvent::UserSnatBindingRemoved { user, binding } => ( + EventType::UserSnatBindingRemoved, + serde_json::to_value(UserSnatBindingMetadata { + user: user.into(), + binding, + }) + .ok(), + ), + DefguardEvent::UserSnatBindingModified { + user, + before, + after, + } => ( + EventType::UserSnatBindingModified, + serde_json::to_value(UserSnatBindingModifiedMetadata { + user: user.into(), before, after, - } => ( - EventType::UserSnatBindingModified, - serde_json::to_value(UserSnatBindingModifiedMetadata { - user: user.into(), - before, - after, - }) - .ok(), - ), - DefguardEvent::ProxyModified { before, after } => ( - EventType::ProxyModified, - serde_json::to_value(ProxyModifiedMetadata { before, after }).ok(), - ), - DefguardEvent::ProxyDeleted { proxy } => ( - EventType::ProxyDeleted, - serde_json::to_value(ProxyDeletedMetadata { proxy }).ok(), - ), - DefguardEvent::GatewayModified { before, after } => ( - EventType::GatewayModified, - serde_json::to_value(GatewayModifiedMetadata { before, after }) - .ok(), - ), - DefguardEvent::GatewayDeleted { gateway } => ( - EventType::GatewayDeleted, - serde_json::to_value(GatewayDeletedMetadata { gateway }).ok(), - ), - }; - (module, event_type, description, metadata) - } - LoggerEvent::Vpn(event) => { - let module = ActivityLogModule::Vpn; - let description = get_vpn_event_description(&event); - - let (event_type, metadata) = map_vpn_event(*event); - (module, event_type, description, metadata) - } - LoggerEvent::Enrollment(event) => { - let module = ActivityLogModule::Enrollment; - let description = get_enrollment_event_description(&event); + }) + .ok(), + ), + DefguardEvent::ProxyModified { before, after } => ( + EventType::ProxyModified, + serde_json::to_value(ProxyModifiedMetadata { before, after }).ok(), + ), + DefguardEvent::ProxyDeleted { proxy } => ( + EventType::ProxyDeleted, + serde_json::to_value(ProxyDeletedMetadata { proxy }).ok(), + ), + DefguardEvent::GatewayModified { before, after } => ( + EventType::GatewayModified, + serde_json::to_value(GatewayModifiedMetadata { before, after }).ok(), + ), + DefguardEvent::GatewayDeleted { gateway } => ( + EventType::GatewayDeleted, + serde_json::to_value(GatewayDeletedMetadata { gateway }).ok(), + ), + }; + (module, event_type, description, metadata) + } + LoggerEvent::Vpn(event) => { + let module = ActivityLogModule::Vpn; + let description = get_vpn_event_description(&event); - let (event_type, metadata) = match *event { - EnrollmentEvent::EnrollmentStarted => { - (EventType::EnrollmentStarted, None) - } - EnrollmentEvent::EnrollmentCompleted => { - (EventType::EnrollmentCompleted, None) - } - EnrollmentEvent::EnrollmentDeviceAdded { device } => ( - EventType::EnrollmentDeviceAdded, - serde_json::to_value(EnrollmentDeviceAddedMetadata { device }).ok(), - ), - EnrollmentEvent::PasswordResetRequested => { - (EventType::PasswordResetRequested, None) - } - EnrollmentEvent::PasswordResetStarted => { - (EventType::PasswordResetStarted, None) - } - EnrollmentEvent::PasswordResetCompleted => { - (EventType::PasswordResetCompleted, None) - } - EnrollmentEvent::TokenAdded { user } => ( - EventType::EnrollmentTokenAdded, - serde_json::to_value(EnrollmentTokenMetadata { user: user.into() }) - .ok(), - ), - }; - (module, event_type, description, metadata) - } - }; + let (event_type, metadata) = map_vpn_event(*event); + (module, event_type, description, metadata) + } + LoggerEvent::Enrollment(event) => { + let module = ActivityLogModule::Enrollment; + let description = get_enrollment_event_description(&event); - ActivityLogEvent { - id: NoId, - timestamp, - user_id, - username, - location, - ip: ip.map(Into::into), - event, - module, - device, - description, - metadata, + let (event_type, metadata) = match *event { + EnrollmentEvent::EnrollmentStarted => (EventType::EnrollmentStarted, None), + EnrollmentEvent::EnrollmentCompleted => { + (EventType::EnrollmentCompleted, None) + } + EnrollmentEvent::EnrollmentDeviceAdded { device } => ( + EventType::EnrollmentDeviceAdded, + serde_json::to_value(EnrollmentDeviceAddedMetadata { device }).ok(), + ), + EnrollmentEvent::PasswordResetRequested => { + (EventType::PasswordResetRequested, None) + } + EnrollmentEvent::PasswordResetStarted => { + (EventType::PasswordResetStarted, None) + } + EnrollmentEvent::PasswordResetCompleted => { + (EventType::PasswordResetCompleted, None) + } + EnrollmentEvent::TokenAdded { user } => ( + EventType::EnrollmentTokenAdded, + serde_json::to_value(EnrollmentTokenMetadata { user: user.into() }) + .ok(), + ), + }; + (module, event_type, description, metadata) } }; - match serde_json::to_string(&activity_log_event) { - Ok(serialized_activity_log_event) => { - serialized_activity_log_events += &(serialized_activity_log_event + "\n"); - } - Err(e) => { - error!("Failed to serialize activity log event. Reason: {e}"); - } + ActivityLogEvent { + id: NoId, + timestamp, + user_id, + username, + location, + ip: ip.map(Into::into), + event, + module, + device, + description, + metadata, } + }; - // Store activity log event in DB - // TODO: do batch inserts - activity_log_event.save(&mut *transaction).await?; - } - - // Send serialized events - if !serialized_activity_log_events.is_empty() { - let in_bytes = bytes::Bytes::from(serialized_activity_log_events); - if let Err(send_err) = activity_log_messages_tx.send(in_bytes) { - trace!( - "Sending serialized activity log events message failed. Most likely because there is no listeners. Reason: {send_err}" - ); + match serde_json::to_string(&activity_log_event) { + Ok(serialized_activity_log_event) => { + serialized_activity_log_events += &(serialized_activity_log_event + "\n"); + } + Err(e) => { + error!("Failed to serialize activity log event. Reason: {e}"); } } - // Commit the transaction - transaction.commit().await?; + // Store activity log event in DB + // TODO: do batch inserts + activity_log_event.save(&mut *transaction).await?; + } + + // Send serialized events + if !serialized_activity_log_events.is_empty() { + let in_bytes = bytes::Bytes::from(serialized_activity_log_events); + if let Err(send_err) = activity_log_messages_tx.send(in_bytes) { + trace!( + "Sending serialized activity log events message failed. Most likely because there is no listeners. Reason: {send_err}" + ); + } } + + transaction.commit().await?; + + Ok(()) } #[cfg(test)] From 8d24727f0c8d7b8c8b257812faf43cc4b047164d Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Tue, 21 Apr 2026 12:38:25 +0200 Subject: [PATCH 3/4] sanitize more --- crates/defguard_core/src/enterprise/ldap/error.rs | 2 +- crates/defguard_core/src/enterprise/ldap/model.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/defguard_core/src/enterprise/ldap/error.rs b/crates/defguard_core/src/enterprise/ldap/error.rs index c285527ba5..07e75d19cf 100644 --- a/crates/defguard_core/src/enterprise/ldap/error.rs +++ b/crates/defguard_core/src/enterprise/ldap/error.rs @@ -4,7 +4,7 @@ use thiserror::Error; /// Strips null bytes and non-printable control characters from LDAP error strings. /// LDAP server responses may embed raw binary data or null terminators in diagnostic /// fields (e.g. `LdapResult.text`, `LdapResult.matched`) that corrupt log output. -fn sanitize_ldap_string(s: &str) -> String { +pub(super) fn sanitize_ldap_string(s: &str) -> String { s.chars() .filter(|&c| c == '\n' || c == '\t' || !c.is_control()) .collect() diff --git a/crates/defguard_core/src/enterprise/ldap/model.rs b/crates/defguard_core/src/enterprise/ldap/model.rs index 0ead7b3821..a7eadbb63c 100644 --- a/crates/defguard_core/src/enterprise/ldap/model.rs +++ b/crates/defguard_core/src/enterprise/ldap/model.rs @@ -7,7 +7,7 @@ use defguard_common::db::{ use ldap3::{Mod, SearchEntry}; use sqlx::PgExecutor; -use super::{LDAPConfig, error::LdapError}; +use super::{LDAPConfig, error::{LdapError, sanitize_ldap_string}}; use crate::{handlers::user::check_username, hashset}; pub(crate) enum UserObjectClass { @@ -45,12 +45,12 @@ pub(crate) fn user_from_searchentry( if let Some(rdn) = extract_rdn_value(&entry.dn) { user.ldap_rdn = Some(rdn); } else { - return Err(LdapError::InvalidDN(entry.dn.clone())); + return Err(LdapError::InvalidDN(sanitize_ldap_string(&entry.dn))); } if let Some(dn_path) = extract_dn_path(&entry.dn) { user.ldap_user_path = Some(dn_path); } else { - return Err(LdapError::InvalidDN(entry.dn.clone())); + return Err(LdapError::InvalidDN(sanitize_ldap_string(&entry.dn))); } // Print the warning only if everything else checks out if check_username(username).is_err() { From 8f9ec9d3565e9eeb5426b0b487ae5a7b8b957101 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Tue, 21 Apr 2026 12:40:43 +0200 Subject: [PATCH 4/4] format --- crates/defguard_certs/src/lib.rs | 3 +-- crates/defguard_core/src/enterprise/ldap/model.rs | 5 ++++- crates/defguard_proxy_manager/src/handler.rs | 4 ++-- .../defguard_setup/tests/integration/auto_adoption_wizard.rs | 3 +-- .../tests/integration/auto_wizard_url_settings.rs | 5 ++--- crates/defguard_setup/tests/integration/initial_setup.rs | 3 +-- crates/defguard_setup/tests/integration/migration_wizard.rs | 2 +- 7 files changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/defguard_certs/src/lib.rs b/crates/defguard_certs/src/lib.rs index 99d64667e5..e526b38bf5 100644 --- a/crates/defguard_certs/src/lib.rs +++ b/crates/defguard_certs/src/lib.rs @@ -2,6 +2,7 @@ use std::{net::IpAddr, str::FromStr}; use base64::{Engine, prelude::BASE64_STANDARD}; use chrono::NaiveDateTime; +pub use rcgen::ExtendedKeyUsagePurpose; use rcgen::{ BasicConstraints, Certificate, CertificateParams, CertificateSigningRequestParams, IsCa, Issuer, KeyPair, KeyUsagePurpose, SigningKey, string::Ia5String, @@ -14,8 +15,6 @@ use x509_parser::{ parse_x509_certificate, }; -pub use rcgen::ExtendedKeyUsagePurpose; - const CA_NAME: &str = "Defguard CA"; const NOT_BEFORE_OFFSET_SECS: Duration = Duration::minutes(5); const DEFAULT_CERT_VALIDITY_DAYS: i64 = 1825; diff --git a/crates/defguard_core/src/enterprise/ldap/model.rs b/crates/defguard_core/src/enterprise/ldap/model.rs index a7eadbb63c..8fbc7631ac 100644 --- a/crates/defguard_core/src/enterprise/ldap/model.rs +++ b/crates/defguard_core/src/enterprise/ldap/model.rs @@ -7,7 +7,10 @@ use defguard_common::db::{ use ldap3::{Mod, SearchEntry}; use sqlx::PgExecutor; -use super::{LDAPConfig, error::{LdapError, sanitize_ldap_string}}; +use super::{ + LDAPConfig, + error::{LdapError, sanitize_ldap_string}, +}; use crate::{handlers::user::check_username, hashset}; pub(crate) enum UserObjectClass { diff --git a/crates/defguard_proxy_manager/src/handler.rs b/crates/defguard_proxy_manager/src/handler.rs index 81d804ff51..d7e8b67047 100644 --- a/crates/defguard_proxy_manager/src/handler.rs +++ b/crates/defguard_proxy_manager/src/handler.rs @@ -62,6 +62,8 @@ use tokio::{ time::sleep, }; use tokio_stream::wrappers::UnboundedReceiverStream; +#[cfg(test)] +use tonic::transport::Endpoint; use tonic::{ Code, Request, Streaming, service::interceptor::InterceptedService, transport::Channel, }; @@ -72,8 +74,6 @@ use crate::{ HandlerTxMap, ProxyError, ProxyTxSet, TEN_SECS, servers::{EnrollmentServer, PasswordResetServer}, }; -#[cfg(test)] -use tonic::transport::Endpoint; const VERSION_ZERO: Version = Version::new(0, 0, 0); diff --git a/crates/defguard_setup/tests/integration/auto_adoption_wizard.rs b/crates/defguard_setup/tests/integration/auto_adoption_wizard.rs index deaec3553c..cf61eb3510 100644 --- a/crates/defguard_setup/tests/integration/auto_adoption_wizard.rs +++ b/crates/defguard_setup/tests/integration/auto_adoption_wizard.rs @@ -28,9 +28,8 @@ use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use tokio::time::timeout; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; -use crate::common::SESSION_COOKIE_NAME; - use super::common::{SHUTDOWN_TIMEOUT, make_setup_test_client}; +use crate::common::SESSION_COOKIE_NAME; fn init_tracing_once() { static ONCE: Once = Once::new(); diff --git a/crates/defguard_setup/tests/integration/auto_wizard_url_settings.rs b/crates/defguard_setup/tests/integration/auto_wizard_url_settings.rs index 289558ded8..59e2854193 100644 --- a/crates/defguard_setup/tests/integration/auto_wizard_url_settings.rs +++ b/crates/defguard_setup/tests/integration/auto_wizard_url_settings.rs @@ -22,11 +22,10 @@ use reqwest::{ }; use serde_json::json; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; - -use crate::common::{SESSION_COOKIE_NAME, TestClient}; +use tokio::{sync::oneshot, time::timeout}; use super::common::{SHUTDOWN_TIMEOUT, make_setup_test_client}; -use tokio::{sync::oneshot, time::timeout}; +use crate::common::{SESSION_COOKIE_NAME, TestClient}; async fn bootstrap_wizard_to_url_settings( pool: &sqlx::PgPool, diff --git a/crates/defguard_setup/tests/integration/initial_setup.rs b/crates/defguard_setup/tests/integration/initial_setup.rs index 08816b2b3f..527c6d868f 100644 --- a/crates/defguard_setup/tests/integration/initial_setup.rs +++ b/crates/defguard_setup/tests/integration/initial_setup.rs @@ -34,9 +34,8 @@ use tokio::{ time::timeout, }; -use crate::common::SESSION_COOKIE_NAME; - use super::common::{SHUTDOWN_TIMEOUT, make_setup_test_client}; +use crate::common::SESSION_COOKIE_NAME; async fn assert_setup_step(pool: &sqlx::PgPool, expected: InitialSetupStep) { let step = InitialSetupState::get(pool) diff --git a/crates/defguard_setup/tests/integration/migration_wizard.rs b/crates/defguard_setup/tests/integration/migration_wizard.rs index d7f9d72c4d..356498254f 100644 --- a/crates/defguard_setup/tests/integration/migration_wizard.rs +++ b/crates/defguard_setup/tests/integration/migration_wizard.rs @@ -15,11 +15,11 @@ use reqwest::{ }; use serde_json::json; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; +use tokio::time::timeout; use super::common::{ SHUTDOWN_TIMEOUT, init_settings_with_secret_key, make_migration_test_client, seed_admin_user, }; -use tokio::time::timeout; async fn assert_migration_step(pool: &sqlx::PgPool, expected_variant: &str) { let state = MigrationWizardState::get(pool)