Sanitize ldap strings, graceful activity log restart (1.6)

.github/workflows/build-docker.yml

@@ -72,7 +72,7 @@ jobs:
           cache-to: type=registry,mode=max,ref=${{ env.GHCR_REPO }}:cache-${{ matrix.tag }}-${{ env.SAFE_REF }}
 
       - name: Scan image with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
           image-ref: "${{ env.GHCR_REPO }}:${{ github.sha }}-${{ matrix.tag }}"
           format: "table"

.github/workflows/ci.yml

@@ -5,15 +5,15 @@ on:
     branches:
       - main
       - dev
-      - 'release/**'
+      - "release/**"
     paths-ignore:
       - "*.md"
       - "LICENSE"
   pull_request:
     branches:
       - main
       - dev
-      - 'release/**'
+      - "release/**"
     paths-ignore:
       - "*.md"
       - "LICENSE"
@@ -56,10 +56,10 @@ jobs:
           submodules: recursive
 
       - name: Scan code with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
-          scan-type: 'fs'
-          scan-ref: '.'
+          scan-type: "fs"
+          scan-ref: "."
           exit-code: "1"
           ignore-unfixed: true
           severity: "CRITICAL,HIGH,MEDIUM"

.github/workflows/sbom.yml

@@ -35,43 +35,43 @@ jobs:
           submodules: recursive
 
       - name: Create SBOM with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
-          scan-type: 'fs'
-          format: 'spdx-json'
+          scan-type: "fs"
+          format: "spdx-json"
           output: "defguard-${{ steps.vars.outputs.VERSION }}.sbom.json"
-          scan-ref: '.'
+          scan-ref: "."
           severity: "CRITICAL,HIGH,MEDIUM,LOW"
           scanners: "vuln"
           skip-dirs: "e2e"
 
       - name: Create docker image SBOM with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
           image-ref: "ghcr.io/defguard/defguard:${{ steps.vars.outputs.VERSION }}"
-          scan-type: 'image'
-          format: 'spdx-json'
+          scan-type: "image"
+          format: "spdx-json"
           output: "defguard-${{ steps.vars.outputs.VERSION }}-docker.sbom.json"
           severity: "CRITICAL,HIGH,MEDIUM,LOW"
           scanners: "vuln"
 
       - name: Create security advisory file with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
-          scan-type: 'fs'
-          format: 'json'
+          scan-type: "fs"
+          format: "json"
           output: "defguard-${{ steps.vars.outputs.VERSION }}.advisories.json"
-          scan-ref: '.'
+          scan-ref: "."
           severity: "CRITICAL,HIGH,MEDIUM,LOW"
           scanners: "vuln"
           skip-dirs: "e2e"
 
       - name: Create docker image security advisory file with Trivy
-        uses: aquasecurity/trivy-action@0.34.2
+        uses: aquasecurity/trivy-action@0.35.0
         with:
           image-ref: "ghcr.io/defguard/defguard:${{ steps.vars.outputs.VERSION }}"
-          scan-type: 'image'
-          format: 'json'
+          scan-type: "image"
+          format: "json"
           output: "defguard-${{ steps.vars.outputs.VERSION }}-docker.advisories.json"
           severity: "CRITICAL,HIGH,MEDIUM,LOW"
           scanners: "vuln"

crates/defguard_core/src/enterprise/ldap/error.rs

@@ -1,10 +1,21 @@
 use sqlx::error::Error as SqlxError;
 use thiserror::Error;
 
+/// LDAP server responses (especially `LdapResult.text` and `LdapResult.matched`) may contain
+/// null bytes and non-printable control characters that corrupt log output. This function
+/// filters out all control characters except `\n` and `\t`.
+pub(super) fn sanitize_ldap_string(s: &str) -> String {
+    s.chars()
+        .filter(|c| !c.is_control() || *c == '\n' || *c == '\t')
+        .collect()
+}
+
 #[derive(Debug, Error)]
 pub enum LdapError {
+    // Stores a sanitized string to avoid null bytes / control chars from LDAP responses
+    // corrupting log output.
     #[error("LDAP error: {0}")]
-    Ldap(#[from] ldap3::LdapError),
+    Ldap(String),
     #[error("Object not found: {0}")]
     ObjectNotFound(String),
     #[error("Missing required LDAP settings: {0}")]
@@ -31,3 +42,41 @@ pub enum LdapError {
     #[error("User {0} does not belong to the defined synchronization groups in {1}")]
     UserNotInLDAPSyncGroups(String, &'static str),
 }
+
+impl From<ldap3::LdapError> 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 are stripped
+        assert_eq!(sanitize_ldap_string("hello\0world"), "helloworld");
+
+        // Other non-printable control chars are stripped
+        assert_eq!(sanitize_ldap_string("text\x01\x02\x03"), "text");
+
+        // Realistic LDAP error string containing null bytes is cleaned correctly
+        let dirty = "80090308: LdapErr: DSID-0C09044E, comment: AcceptSecurityContext error, data 52e, v2580\0";
+        let clean = "80090308: LdapErr: DSID-0C09044E, comment: AcceptSecurityContext error, data 52e, v2580";
+        assert_eq!(sanitize_ldap_string(dirty), clean);
+
+        // \n and \t are preserved
+        assert_eq!(
+            sanitize_ldap_string("line1\nline2\ttabbed"),
+            "line1\nline2\ttabbed"
+        );
+
+        // Normal ASCII and Unicode pass through unchanged
+        assert_eq!(sanitize_ldap_string("hello world"), "hello world");
+        assert_eq!(
+            sanitize_ldap_string("zażółć gęślą jaźń"),
+            "zażółć gęślą jaźń"
+        );
+    }
+}

crates/defguard_core/src/enterprise/ldap/mod.rs

@@ -15,7 +15,7 @@ use rand::Rng;
 use sqlx::PgPool;
 use sync::{get_ldap_sync_status, is_ldap_desynced, set_ldap_sync_status};
 
-use self::error::LdapError;
+use self::error::{LdapError, sanitize_ldap_string};
 use crate::{
     db::{self, User},
     enterprise::{is_business_license_active, ldap::model::extract_dn_path, limits::update_counts},
@@ -582,7 +582,9 @@ impl LDAPConnection {
                 info!("Found LDAP user with DN: {}", dn);
                 User::from_searchentry(&entry, &user.username, None)
             }
-            None => Err(LdapError::ObjectNotFound(format!("User {dn} not found",))),
+            None => Err(LdapError::ObjectNotFound(sanitize_ldap_string(&format!(
+                "User {dn} not found",
+            )))),
         }
     }
 
@@ -626,9 +628,11 @@ impl LDAPConnection {
         if !self.is_username_available(&user.username).await?
             || self.user_exists_by_dn(&user_dn).await?
         {
-            return Err(LdapError::ObjectAlreadyExists(format!(
-                "User with username {} or DN {user_dn} already exists",
-                user.username
+            return Err(LdapError::ObjectAlreadyExists(sanitize_ldap_string(
+                &format!(
+                    "User with username {} or DN {user_dn} already exists",
+                    user.username
+                ),
             )));
         }
         self.add(
@@ -704,9 +708,9 @@ impl LDAPConnection {
             .and_then(|v| v.first())
             .map(ToString::to_string)
             .ok_or_else(|| {
-                LdapError::ObjectNotFound(format!(
+                LdapError::ObjectNotFound(sanitize_ldap_string(&format!(
                     "Couldn't extract a group name from searchentry {entry:?}."
-                ))
+                )))
             })
     }
 

crates/defguard_core/src/enterprise/ldap/model.rs

@@ -4,7 +4,10 @@ use defguard_common::db::{Id, models::Settings};
 use ldap3::{Mod, SearchEntry};
 use sqlx::{Error as SqlxError, PgExecutor};
 
-use super::{LDAPConfig, error::LdapError};
+use super::{
+    LDAPConfig,
+    error::{LdapError, sanitize_ldap_string},
+};
 use crate::{db::User, handlers::user::check_username, hashset};
 
 pub(crate) enum UserObjectClass {
@@ -69,12 +72,12 @@ impl User {
         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() {