Repository: lldap/lldap (language: rust)

Issue:
[BUG] Presence filters are not properly handled by lldap
Presence filters (`(unix-uid=*)`) are not respected by lldap.  Instead they are currently executed as a schema check rather than a data check.

[Reference](https://ldap.com/ldap-filters/) [documentation](https://ldapwiki.com/wiki/Wiki.jsp?page=Presence%20SearchFilter) for Presence Filters.

**To Reproduce**

1. Create a custom attribute.
2. Populate it with a value on some users and not all users.
3. Perform ldapsearch `(your-attribute=*)`
4. Observe that all users are returned, not just the ones with the attribute set.

**Expected behavior**

lldap should return only the users with the attribute present when queried with a presence filter.

**Additional context**

This silent failure misbehavior has security consequences.  For instance, the documentation for `libnss-ldapd` contains this passage:

```text
For  example,  to  check  that  the  user has a proper authorizedService value if the attribute is present (this almost emulates the pam_check_service_attr option in PADL's
pam_ldap):

(&(objectClass=posixAccount)(uid=$username)(|(authorizedService=$service)(!(authorizedService=*))))
```

If users were to trust this (or indeed chose to use `libpam-ldap` instead which likely does something similar _without_ showing the user the exact details of the query when `authorizedService` or `host` checks are performed), lldap would be exposing them to explicitly unauthorized logins on Hosts B-Z from users that should be authorized only on Host A.


Discussion / hints:

Fix patch:
diff --git a/server/src/domain/handler.rs b/server/src/domain/handler.rs
index 06ee8798..c5d5ad39 100644
--- a/server/src/domain/handler.rs
+++ b/server/src/domain/handler.rs
@@ -61,6 +61,7 @@ pub enum UserRequestFilter {
     MemberOf(GroupName),
     // Same, by id.
     MemberOfId(GroupId),
+    CustomAttributePresent(AttributeName),
 }
 
 impl From<bool> for UserRequestFilter {
@@ -85,6 +86,7 @@ pub enum GroupRequestFilter {
     // Check if the group contains a user identified by uid.
     Member(UserId),
     AttributeEquality(AttributeName, Serialized),
+    CustomAttributePresent(AttributeName),
 }
 
 impl From<bool> for GroupRequestFilter {
diff --git a/server/src/domain/ldap/group.rs b/server/src/domain/ldap/group.rs
index 313d40df..a3584ea7 100644
--- a/server/src/domain/ldap/group.rs
+++ b/server/src/domain/ldap/group.rs
@@ -228,10 +228,13 @@ fn convert_group_filter(
         LdapFilter::Not(filter) => Ok(GroupRequestFilter::Not(Box::new(rec(filter)?))),
         LdapFilter::Present(field) => {
             let field = AttributeName::from(field.as_str());
-            Ok(GroupRequestFilter::from(!matches!(
-                map_group_field(&field, schema),
-                GroupFieldType::NoMatch
-            )))
+            Ok(match map_group_field(&field, schema) {
+                GroupFieldType::Attribute(name, _, _) => {
+                    GroupRequestFilter::CustomAttributePresent(name)
+                }
+                GroupFieldType::NoMatch => GroupRequestFilter::from(false),
+                _ => GroupRequestFilter::from(true),
+            })
         }
         LdapFilter::Substring(field, substring_filter) => {
             let field = AttributeName::from(field.as_str());
diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs
index bbd6c698..325200b9 100644
--- a/server/src/domain/ldap/user.rs
+++ b/server/src/domain/ldap/user.rs
@@ -250,13 +250,13 @@ fn convert_user_filter(
         }
         LdapFilter::Present(field) => {
             let field = AttributeName::from(field.as_str());
-            // Check that it's a field we support.
-            Ok(UserRequestFilter::from(
-                field.as_str() == "objectclass"
-                    || field.as_str() == "dn"
-                    || field.as_str() == "distinguishedname"
-                    || !matches!(map_user_field(&field, schema), UserFieldType::NoMatch),
-            ))
+            Ok(match map_user_field(&field, schema) {
+                UserFieldType::Attribute(name, _, _) => {
+                    UserRequestFilter::CustomAttributePresent(name)
+                }
+                UserFieldType::NoMatch => UserRequestFilter::from(false),
+                _ => UserRequestFilter::from(true),
+            })
         }
         LdapFilter::Substring(field, substring_filter) => {
             let field = AttributeName::from(field.as_str());
diff --git a/server/src/domain/sql_group_backend_handler.rs b/server/src/domain/sql_group_backend_handler.rs
index 6f88fb27..ffa707af 100644
--- a/server/src/domain/sql_group_backend_handler.rs
+++ b/server/src/domain/sql_group_backend_handler.rs
@@ -16,14 +16,18 @@ use sea_orm::{
 };
 use tracing::instrument;
 
-fn attribute_condition(name: AttributeName, value: Serialized) -> Cond {
+fn attribute_condition(name: AttributeName, value: Option<Serialized>) -> Cond {
     Expr::in_subquery(
         Expr::col(GroupColumn::GroupId.as_column_ref()),
         model::GroupAttributes::find()
             .select_only()
             .column(model::GroupAttributesColumn::GroupId)
             .filter(model::GroupAttributesColumn::AttributeName.eq(name))
-            .filter(model::GroupAttributesColumn::Value.eq(value))
+            .filter(
+                value
+                    .map(|value| model::GroupAttributesColumn::Value.eq(value))
+                    .unwrap_or_else(|| SimpleExpr::Value(true.into())),
+            )
             .into_query(),
     )
     .into_condition()
@@ -71,7 +75,8 @@ fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond {
         ))))
         .like(filter.to_sql_filter())
         .into_condition(),
-        AttributeEquality(name, value) => attribute_condition(name, value),
+        AttributeEquality(name, value) => attribute_condition(name, Some(value)),
+        CustomAttributePresent(name) => attribute_condition(name, None),
     }
 }
 
diff --git a/server/src/domain/sql_schema_backend_handler.rs b/server/src/domain/sql_schema_backend_handler.rs
index c52f998c..288055ad 100644
--- a/server/src/domain/sql_schema_backend_handler.rs
+++ b/server/src/domain/sql_schema_backend_handler.rs
@@ -175,7 +175,9 @@ impl SqlBackendHandler {
 mod tests {
     use super::*;
     use crate::domain::{
-        handler::AttributeList, sql_backend_handler::tests::*, types::AttributeType,
+        handler::{AttributeList, UpdateUserRequest, UserBackendHandler, UserRequestFilter},
+        sql_backend_handler::tests::*,
+        types::{AttributeType, AttributeValue, Serialized},
     };
     use pretty_assertions::assert_eq;
 
@@ -268,6 +270,43 @@ mod tests {
             .contains(&expected_value));
     }
 
+    #[tokio::test]
+    async fn test_user_attribute_present_filter() {
+        let fixture = TestFixture::new().await;
+        let new_attribute = CreateAttributeRequest {
+            name: "new_attribute".into(),
+            attribute_type: AttributeType::Integer,
+            is_list: true,
+            is_visible: false,
+            is_editable: false,
+        };
+        fixture
+            .handler
+            .add_user_attribute(new_attribute)
+            .await
+            .unwrap();
+        fixture
+            .handler
+            .update_user(UpdateUserRequest {
+                user_id: "bob".into(),
+                insert_attributes: vec![AttributeValue {
+                    name: "new_attribute".into(),
+                    value: Serialized::from(&3),
+                }],
+                ..Default::default()
+            })
+            .await
+            .unwrap();
+        let users = get_user_names(
+            &fixture.handler,
+            Some(UserRequestFilter::CustomAttributePresent(
+                "new_attribute".into(),
+            )),
+        )
+        .await;
+        assert_eq!(users, vec!["bob"]);
+    }
+
     #[tokio::test]
     async fn test_group_attribute_add_and_delete() {
         let fixture = TestFixture::new().await;
diff --git a/server/src/domain/sql_user_backend_handler.rs b/server/src/domain/sql_user_backend_handler.rs
index 1ba60b6d..a3768b5e 100644
--- a/server/src/domain/sql_user_backend_handler.rs
+++ b/server/src/domain/sql_user_backend_handler.rs
@@ -22,14 +22,18 @@ use sea_orm::{
 use std::collections::HashSet;
 use tracing::instrument;
 
-fn attribute_condition(name: AttributeName, value: Serialized) -> Cond {
+fn attribute_condition(name: AttributeName, value: Option<Serialized>) -> Cond {
     Expr::in_subquery(
         Expr::col(UserColumn::UserId.as_column_ref()),
         model::UserAttributes::find()
             .select_only()
             .column(model::UserAttributesColumn::UserId)
             .filter(model::UserAttributesColumn::AttributeName.eq(name))
-            .filter(model::UserAttributesColumn::Value.eq(value))
+            .filter(
+                value
+                    .map(|value| model::UserAttributesColumn::Value.eq(value))
+                    .unwrap_or_else(|| SimpleExpr::Constant(true.into())),
+            )
             .into_query(),
     )
     .into_condition()
@@ -79,7 +83,7 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond {
                 ColumnTrait::eq(&column, value).into_condition()
             }
         }
-        AttributeEquality(column, value) => attribute_condition(column, value),
+        AttributeEquality(column, value) => attribute_condition(column, Some(value)),
         MemberOf(group) => user_id_subcondition(
             Expr::col((group_table, GroupColumn::LowercaseDisplayName))
                 .eq(group.as_str().to_lowercase())
@@ -98,6 +102,7 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond {
                 .like(filter.to_sql_filter())
                 .into_condition()
         }
+        CustomAttributePresent(name) => attribute_condition(name, None),
     }
 }
 


Test patch: