From fb8a9a446c7831229df58f1e24b9f51571317402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Mon, 19 May 2025 16:29:32 +0200 Subject: [PATCH 1/5] chore: bump version 1.3.1 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef9af4d6ce..4730531f8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1070,7 +1070,7 @@ dependencies = [ [[package]] name = "defguard" -version = "1.3.0" +version = "1.3.1" dependencies = [ "anyhow", "argon2", diff --git a/Cargo.toml b/Cargo.toml index 2a480c5337..af48eaa0f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "defguard" -version = "1.3.0" +version = "1.3.1" edition = "2021" license-file = "LICENSE.md" homepage = "https://defguard.net/" From d46facc1ded93136ef08afb851e189b86cd0ff67 Mon Sep 17 00:00:00 2001 From: Robert Olejnik Date: Mon, 19 May 2025 18:59:37 +0200 Subject: [PATCH 2/5] Update LICENSE.md --- src/enterprise/LICENSE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/enterprise/LICENSE.md b/src/enterprise/LICENSE.md index 7a386fefe8..3df1267b4a 100644 --- a/src/enterprise/LICENSE.md +++ b/src/enterprise/LICENSE.md @@ -1,4 +1,4 @@ -Copyright 2024 teonite ventures sp. z o. o. +Copyright ©️ defguard sp. z o. o. defguard enterprise license / defguard.net From 4c0662859a5afd2a240b6a30d88d97f556922df9 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 22 May 2025 13:21:56 +0200 Subject: [PATCH 3/5] Fix LDAP login (#1167) * remove some checks on ldap login, other minor fixes * bump version to 1.3.2 * Update Cargo.toml --- Cargo.lock | 2 +- Cargo.toml | 39 +++++++++++++++++++----- src/db/models/user.rs | 3 +- src/enterprise/ldap/client.rs | 1 + src/enterprise/ldap/mod.rs | 18 +++++++---- src/enterprise/ldap/utils.rs | 34 ++++++++++++--------- src/handlers/auth.rs | 57 +++++++++++++++-------------------- 7 files changed, 92 insertions(+), 62 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4730531f8a..754cc8efac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1070,7 +1070,7 @@ dependencies = [ [[package]] name = "defguard" -version = "1.3.1" +version = "1.3.2" dependencies = [ "anyhow", "argon2", diff --git a/Cargo.toml b/Cargo.toml index af48eaa0f3..e1904134e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "defguard" -version = "1.3.1" +version = "1.3.2" edition = "2021" license-file = "LICENSE.md" homepage = "https://defguard.net/" @@ -17,7 +17,10 @@ axum-client-ip = "0.7" axum-extra = { version = "0.10", features = ["cookie-private", "typed-header"] } base32 = "0.5" base64 = "0.22" -chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] } +chrono = { version = "0.4", default-features = false, features = [ + "clock", + "serde", +] } clap = { version = "4.5", features = ["derive", "env"] } dotenvy = "0.15" humantime = "2.1" @@ -30,7 +33,9 @@ lettre = { version = "0.11", features = ["tokio1-native-tls"] } md4 = "0.10" mime_guess = "2.0" model_derive = { path = "model-derive" } -openidconnect = { version = "4.0", default-features = false, optional = true, features = ["reqwest"] } +openidconnect = { version = "4.0", default-features = false, optional = true, features = [ + "reqwest", +] } parse_link_header = "0.4" paste = "1.0.15" pgp = "0.14" @@ -52,14 +57,26 @@ serde_json = "1.0" serde_urlencoded = "0.7" sha-1 = "0.10" sha256 = "1.5" -sqlx = { version = "0.8", features = ["chrono", "ipnetwork", "postgres", "runtime-tokio-native-tls", "uuid"] } +sqlx = { version = "0.8", features = [ + "chrono", + "ipnetwork", + "postgres", + "runtime-tokio-native-tls", + "uuid", +] } ssh-key = "0.6" struct-patch = "0.8" tera = "1.20" thiserror = "2.0" # match axum-extra -> cookies time = { version = "0.3", default-features = false } -tokio = { version = "1", features = ["macros", "parking_lot", "rt-multi-thread", "sync", "time"] } +tokio = { version = "1", features = [ + "macros", + "parking_lot", + "rt-multi-thread", + "sync", + "time", +] } tokio-stream = "0.1" tokio-util = "0.7" tonic = { version = "0.12", features = ["gzip", "tls-native-roots"] } @@ -75,7 +92,9 @@ utoipa = { version = "5", features = ["axum_extras", "chrono", "uuid"] } utoipa-swagger-ui = { version = "9", features = ["axum", "vendored"] } uuid = { version = "1.9", features = ["v4"] } webauthn-authenticator-rs = { version = "0.5" } -webauthn-rs = { version = "0.5", features = ["danger-allow-state-serialisation"] } +webauthn-rs = { version = "0.5", features = [ + "danger-allow-state-serialisation", +] } webauthn-rs-proto = "0.5" x25519-dalek = { version = "2.0", features = ["static_secrets"] } @@ -88,7 +107,13 @@ bytes = "1.6" claims = "0.8" matches = "0.1" regex = "1.10" -reqwest = { version = "0.12", features = ["cookies", "json", "multipart", "rustls-tls", "stream"], default-features = false } +reqwest = { version = "0.12", features = [ + "cookies", + "json", + "multipart", + "rustls-tls", + "stream", +], default-features = false } serde_qs = "0.13" webauthn-authenticator-rs = { version = "0.5", features = ["softpasskey"] } diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 62ed28b18e..a1d94a61d1 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -172,11 +172,12 @@ impl User { } pub(crate) fn verify_password(&self, password: &str) -> Result<(), HashError> { + debug!("Checking if password matches for user {}", self.username); if let Some(hash) = &self.password_hash { let parsed_hash = PasswordHash::new(hash)?; Argon2::default().verify_password(password.as_bytes(), &parsed_hash) } else { - error!("Password not set for user {}", self.username); + info!("User {} has no password set", self.username); Err(HashError::Password) } } diff --git a/src/enterprise/ldap/client.rs b/src/enterprise/ldap/client.rs index 78249abcb0..e6d42987fe 100644 --- a/src/enterprise/ldap/client.rs +++ b/src/enterprise/ldap/client.rs @@ -93,6 +93,7 @@ impl super::LDAPConnection { .success()?; debug!("LDAP user groups search result: {res}"); debug!("Performed LDAP group search with filter = {filter}"); + debug!("Found groups: {rs:?}"); Ok(rs.into_iter().map(SearchEntry::construct).collect()) } diff --git a/src/enterprise/ldap/mod.rs b/src/enterprise/ldap/mod.rs index 18b3a40998..0e84c31fbf 100644 --- a/src/enterprise/ldap/mod.rs +++ b/src/enterprise/ldap/mod.rs @@ -27,12 +27,6 @@ pub mod utils; pub(crate) async fn do_ldap_sync(pool: &PgPool) -> Result<(), LdapError> { debug!("Starting LDAP sync, if enabled"); let mut settings = Settings::get_current_settings(); - if !is_enterprise_enabled() { - info!("Enterprise features are disabled, not performing LDAP sync and automatically disabling it"); - settings.ldap_sync_enabled = false; - update_current_settings(pool, settings).await?; - return Err(LdapError::EnterpriseDisabled("LDAP sync".to_string())); - } // Mark as out of sync only if we can't propagate changes to LDAP, as it // doesn't matter for the sync status if we can't pull changes. @@ -49,6 +43,13 @@ pub(crate) async fn do_ldap_sync(pool: &PgPool) -> Result<(), LdapError> { return Ok(()); } + if !is_enterprise_enabled() { + info!("Enterprise features are disabled, not performing LDAP sync and automatically disabling it"); + settings.ldap_sync_enabled = false; + update_current_settings(pool, settings).await?; + return Err(LdapError::EnterpriseDisabled("LDAP sync".to_string())); + } + if is_ldap_desynced() { info!("LDAP is considered to be desynced, doing a full sync"); } else { @@ -379,6 +380,11 @@ impl LDAPConnection { }) .collect::>(); + debug!( + "User groups: {user_groups_names:?}, sync groups: {:?}", + self.config.ldap_sync_groups + ); + if user_groups_names .into_iter() .any(|group| self.config.ldap_sync_groups.contains(group)) diff --git a/src/enterprise/ldap/utils.rs b/src/enterprise/ldap/utils.rs index 08d8eb418d..f45a9bd907 100644 --- a/src/enterprise/ldap/utils.rs +++ b/src/enterprise/ldap/utils.rs @@ -19,28 +19,33 @@ pub(crate) async fn login_through_ldap( ) -> Result, LdapError> { debug!("Logging in user {username} through LDAP"); let mut ldap_connection = LDAPConnection::create().await?; - let ldap_user = ldap_connection + let mut ldap_user = ldap_connection .fetch_user_by_credentials(username, password) .await?; if !ldap_connection.user_in_ldap_sync_groups(&ldap_user).await? { + info!("User {username} is not in LDAP sync groups, not allowing to login through LDAP.",); return Err(LdapError::UserNotInLDAPSyncGroups( username.to_string(), "LDAP", )); } debug!("User {ldap_user} logged in through LDAP"); - let user = - if let Some(defguard_user) = User::find_by_username(pool, &ldap_user.username).await? { - if !defguard_user.ldap_sync_allowed(pool).await? { - return Err(LdapError::UserNotInLDAPSyncGroups( - ldap_user.to_string(), - "Defguard", - )); - } - defguard_user - } else { - ldap_user.save(pool).await? - }; + // The user is logging in through LDAP, so we can infer that there are no other login options (Defguard password), + // so we should mark them as from_ldap. + let user = if let Some(mut defguard_user) = + User::find_by_username(pool, &ldap_user.username).await? + { + debug!("User {defguard_user} already exists in Defguard, marking them as coming from LDAP and proceeding with login"); + defguard_user.from_ldap = true; + defguard_user.save(pool).await?; + defguard_user + } else { + debug!( + "User {ldap_user} doesn't exist in Defguard, creating them first based on LDAP data" + ); + ldap_user.from_ldap = true; + ldap_user.save(pool).await? + }; Ok(user) } @@ -53,7 +58,7 @@ pub(crate) async fn user_from_ldap( debug!("Getting user {username} from LDAP"); let mut ldap_connection = LDAPConnection::create().await?; - let ldap_user = ldap_connection + let mut ldap_user = ldap_connection .fetch_user_by_credentials(username, password) .await?; if !ldap_connection.user_in_ldap_sync_groups(&ldap_user).await? { @@ -62,6 +67,7 @@ pub(crate) async fn user_from_ldap( "LDAP", )); } + ldap_user.from_ldap = true; let user = ldap_user.save(pool).await?; debug!("User {user} found in LDAP"); diff --git a/src/handlers/auth.rs b/src/handlers/auth.rs index 4bb9f47921..92794459e8 100644 --- a/src/handlers/auth.rs +++ b/src/handlers/auth.rs @@ -141,24 +141,18 @@ pub(crate) async fn authenticate( let mut user = match User::find_by_username(&appstate.pool, &username_or_email).await { Ok(Some(user)) => match user.verify_password(&data.password) { - Ok(()) => { - if user.is_active { - user - } else { - info!("Failed to authenticate user {username_or_email}: user is disabled"); - return Err(WebError::Authorization("user not found".into())); - } - } + Ok(()) => user, Err(err) => { if settings.ldap_enabled { - if let Ok(user) = - login_through_ldap(&appstate.pool, &username_or_email, &data.password).await + match login_through_ldap(&appstate.pool, &username_or_email, &data.password) + .await { - user - } else { - info!("Failed to authenticate user {username_or_email}: {err}"); - log_failed_login_attempt(&appstate.failed_logins, &username_or_email); - return Err(WebError::Authorization(err.to_string())); + Ok(user) => user, + Err(err) => { + info!("Failed to authenticate user {username_or_email} through LDAP: {err}"); + log_failed_login_attempt(&appstate.failed_logins, &username_or_email); + return Err(WebError::Authorization(err.to_string())); + } } } else { info!("Failed to authenticate user {username_or_email}: {err}"); @@ -170,16 +164,7 @@ pub(crate) async fn authenticate( Ok(None) => { match User::find_by_email(&appstate.pool, &username_or_email).await { Ok(Some(user)) => match user.verify_password(&data.password) { - Ok(()) => { - if user.is_active { - user - } else { - info!( - "Failed to authenticate user {username_or_email}: user is disabled" - ); - return Err(WebError::Authorization("user not found".into())); - } - } + Ok(()) => user, Err(err) => { if settings.ldap_enabled { if let Ok(user) = @@ -207,14 +192,15 @@ pub(crate) async fn authenticate( debug!( "User not found in DB, authenticating user {username_or_email} with LDAP" ); - if let Ok(user) = - user_from_ldap(&appstate.pool, &username_or_email, &data.password).await - { - user - } else { - info!("Failed to authenticate user {username_or_email} with LDAP"); - log_failed_login_attempt(&appstate.failed_logins, &username_or_email); - return Err(WebError::Authorization("user not found".into())); + match user_from_ldap(&appstate.pool, &username_or_email, &data.password).await { + Ok(user) => user, + Err(err) => { + info!( + "Failed to authenticate user {username_or_email} with LDAP: {err}" + ); + log_failed_login_attempt(&appstate.failed_logins, &username_or_email); + return Err(WebError::Authorization(err.to_string())); + } } } Err(err) => { @@ -229,6 +215,11 @@ pub(crate) async fn authenticate( } }; + if !user.is_active { + info!("Failed to authenticate user {username_or_email}: user is disabled"); + return Err(WebError::Authorization("user not found".into())); + } + let (session, user_info, mfa_info) = create_session( &appstate.pool, &appstate.mail_tx, From be94713198d6e0540fe209334758128f4cbe51c0 Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Thu, 22 May 2025 15:45:05 +0200 Subject: [PATCH 4/5] Change user debug logging (#1168) --- src/db/models/user.rs | 54 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/db/models/user.rs b/src/db/models/user.rs index a1d94a61d1..08d7f7fac7 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -77,7 +77,7 @@ pub struct UserDiagnostic { pub enrolled: bool, } -#[derive(Clone, Debug, Model, PartialEq, Eq, Hash, Serialize, FromRow)] +#[derive(Clone, Model, PartialEq, Eq, Hash, Serialize, FromRow)] pub struct User { pub id: I, pub username: String, @@ -117,6 +117,58 @@ pub struct User { pub(crate) recovery_codes: Vec, } +// TODO: Refactor the user struct to use SecretStringWrapper instead of this +impl fmt::Debug for User { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { + id, + username, + password_hash: _, + last_name, + first_name, + email, + phone, + mfa_enabled, + is_active, + from_ldap, + ldap_pass_randomized, + ldap_rdn, + openid_sub, + totp_enabled, + email_mfa_enabled, + totp_secret: _, + email_mfa_secret: _, + mfa_method, + recovery_codes, + } = self; + + f.debug_struct("User") + .field("id", id) + .field("username", username) + .field("last_name", last_name) + .field("first_name", first_name) + .field("email", email) + .field("phone", phone) + .field("mfa_enabled", mfa_enabled) + .field("is_active", is_active) + .field("from_ldap", from_ldap) + .field("ldap_pass_randomized", ldap_pass_randomized) + .field("ldap_rdn", ldap_rdn) + .field("openid_sub", openid_sub) + .field("totp_enabled", totp_enabled) + .field("email_mfa_enabled", email_mfa_enabled) + .field("mfa_method", mfa_method) + .field( + "recovery_codes", + &format_args!("{} items", recovery_codes.len()), + ) + .field("password_hash", &"***") + .field("totp_secret", &"***") + .field("email_mfa_secret", &"***") + .finish() + } +} + fn hash_password(password: &str) -> Result { let salt = SaltString::generate(&mut OsRng); Ok(Argon2::default() From 22ae8235b3234b5dbd602391e3ffd4f7998e58ec Mon Sep 17 00:00:00 2001 From: Aleksander <170264518+t-aleksander@users.noreply.github.com> Date: Fri, 23 May 2025 11:18:55 +0200 Subject: [PATCH 5/5] Change init vpn command behavior (#1169) * change init vpn command * comment * revert to save method * formatting * grammar --- src/config.rs | 2 + src/lib.rs | 102 ++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 80 insertions(+), 24 deletions(-) diff --git a/src/config.rs b/src/config.rs index 19fd9ecb52..f5bc6ef94b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -188,6 +188,8 @@ pub struct InitVpnLocationArgs { pub dns: Option, #[arg(long)] pub allowed_ips: Vec, + #[arg(long)] + pub id: Option, } impl DefGuardConfig { diff --git a/src/lib.rs b/src/lib.rs index f1307a750c..eb40db3136 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -736,36 +736,90 @@ pub async fn init_dev_env(config: &DefGuardConfig) { /// Create a new VPN location. /// Meant to be used to automate setting up a new defguard instance. -/// Does not handle assigning device IPs, since no device should exist at this point. +/// If the network ID has been specified, it will be assumed that the user wants to update the existing network or create a new one with a predefined ID. +/// This is mainly used for deployment purposes where the network ID must be known beforehand. +/// +/// If there is no ID specified, the function will only create the network if no other network exists. +/// In other words, multiple networks can be created, but only if the ID is predefined for each network. pub async fn init_vpn_location( pool: &PgPool, args: &InitVpnLocationArgs, ) -> Result { - // check if a VPN location exists already - let networks = WireguardNetwork::all(pool).await?; - if !networks.is_empty() { - return Err(anyhow!( - "Failed to initialize first VPN location. A location already exists." - )); + // The ID is predefined + let network = if let Some(location_id) = args.id { + let mut transaction = pool.begin().await?; + // If the network already exists, update it, assuming that's the user's intent. + let network = if let Some(mut network) = + WireguardNetwork::find_by_id(&mut *transaction, location_id).await? + { + network.name = args.name.clone(); + network.address = vec![args.address]; + network.port = args.port; + network.endpoint = args.endpoint.clone(); + network.dns = args.dns.clone(); + network.allowed_ips = args.allowed_ips.clone(); + network.save(&mut *transaction).await?; + network.sync_allowed_devices(&mut transaction, None).await?; + network + } + // Otherwise create it with the predefined ID + else { + let network = WireguardNetwork::new( + args.name.clone(), + vec![args.address], + args.port, + args.endpoint.clone(), + args.dns.clone(), + args.allowed_ips.clone(), + false, + DEFAULT_KEEPALIVE_INTERVAL, + DEFAULT_DISCONNECT_THRESHOLD, + false, + false, + )? + .save(&mut *transaction) + .await?; + if network.id != location_id { + return Err(anyhow!( + "Failed to initialize VPN location. The ID of the newly created network ({}) does not match \ + the predefined ID ({location_id}). The predefined ID must be the next available ID.", + network.id + )); + } + network.add_all_allowed_devices(&mut transaction).await?; + network + }; + transaction.commit().await?; + network + } + // No predefined ID, add the network if no other networks are present + else { + // check if a VPN location exists already + let networks = WireguardNetwork::all(pool).await?; + if !networks.is_empty() { + return Err(anyhow!( + "Failed to initialize first VPN location. Location already exists." + )); + }; + + // create a new network + WireguardNetwork::new( + args.name.clone(), + vec![args.address], + args.port, + args.endpoint.clone(), + args.dns.clone(), + args.allowed_ips.clone(), + false, + DEFAULT_KEEPALIVE_INTERVAL, + DEFAULT_DISCONNECT_THRESHOLD, + false, + false, + )? + .save(pool) + .await? }; - // create a new network - let network = WireguardNetwork::new( - args.name.clone(), - vec![args.address], - args.port, - args.endpoint.clone(), - args.dns.clone(), - args.allowed_ips.clone(), - false, - DEFAULT_KEEPALIVE_INTERVAL, - DEFAULT_DISCONNECT_THRESHOLD, - false, - false, - )? - .save(pool) - .await?; - // generate gateway token let token = Claims::new( ClaimsType::Gateway,