Blame SOURCES/0040-UTIL-DN-sanitization.patch

bcb322
From d0cc9f3b8b8b7aebb4980d1eb7228f0df2051672 Mon Sep 17 00:00:00 2001
bcb322
From: Tomas Halman <thalman@redhat.com>
bcb322
Date: Fri, 31 Jul 2020 11:12:02 +0200
bcb322
Subject: [PATCH 40/41] UTIL: DN sanitization
bcb322
bcb322
Some of the ldap servers returns DN in attributes such as isMemberOf
bcb322
with spaces like dc=example, dc=com. That should be fine and we
bcb322
should ignore them (cut them out) instead of escaping.
bcb322
bcb322
Resolves:
bcb322
https://github.com/SSSD/sssd/issues/5261
bcb322
(cherry picked from commit 882307cdc1b596ba0cc346a0001f4fc014818d82)
bcb322
---
bcb322
 src/tests/cmocka/test_utils.c |  70 +++++++++++++++++++
bcb322
 src/util/util.c               | 127 ++++++++++++++++++++++++++++++++++
bcb322
 src/util/util.h               |  20 ++++++
bcb322
 3 files changed, 217 insertions(+)
bcb322
bcb322
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
bcb322
index bd2c9e65d..aa245f00b 100644
bcb322
--- a/src/tests/cmocka/test_utils.c
bcb322
+++ b/src/tests/cmocka/test_utils.c
bcb322
@@ -1935,6 +1935,73 @@ static void test_sss_get_domain_mappings_content(void **state)
bcb322
      * capaths might not be as expected. */
bcb322
 }
bcb322
 
bcb322
+
bcb322
+static void test_sss_filter_sanitize_dn(void **state)
bcb322
+{
bcb322
+    TALLOC_CTX *tmp_ctx;
bcb322
+    char *trimmed;
bcb322
+    int ret;
bcb322
+    const char *DN = "cn=user,ou=people,dc=example,dc=com";
bcb322
+
bcb322
+    tmp_ctx = talloc_new(NULL);
bcb322
+    assert_non_null(tmp_ctx);
bcb322
+
bcb322
+    /* test that we remove spaces around '=' and ','*/
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, DN, &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn=user,ou=people,dc=example,dc=com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn= user,ou =people,dc = example,dc  =  com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn=user, ou=people ,dc=example , dc=com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn=user,  ou=people  ,dc=example  ,   dc=com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn= user, ou =people ,dc = example  ,  dc  = com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, " cn=user,ou=people,dc=example,dc=com ", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "  cn=user, ou=people, dc=example, dc=com  ", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal(DN, trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    /* test that we keep spaces inside a value */
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn = user one, ou=people  branch, dc=example, dc=com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal("cn=user\\20one,ou=people\\20\\20branch,dc=example,dc=com", trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    /* test that we keep escape special chars like () */
bcb322
+    ret = sss_filter_sanitize_dn(tmp_ctx, "cn = user one, ou=p(e)ople, dc=example, dc=com", &trimmed);
bcb322
+    assert_int_equal(ret, EOK);
bcb322
+    assert_string_equal("cn=user\\20one,ou=p\\28e\\29ople,dc=example,dc=com", trimmed);
bcb322
+    talloc_free(trimmed);
bcb322
+
bcb322
+    talloc_free(tmp_ctx);
bcb322
+}
bcb322
+
bcb322
 int main(int argc, const char *argv[])
bcb322
 {
bcb322
     poptContext pc;
bcb322
@@ -2044,6 +2111,9 @@ int main(int argc, const char *argv[])
bcb322
         cmocka_unit_test_setup_teardown(test_sss_ptr_hash_without_cb,
bcb322
                                         setup_leak_tests,
bcb322
                                         teardown_leak_tests),
bcb322
+        cmocka_unit_test_setup_teardown(test_sss_filter_sanitize_dn,
bcb322
+                                        setup_leak_tests,
bcb322
+                                        teardown_leak_tests),
bcb322
     };
bcb322
 
bcb322
     /* Set debug level to invalid value so we can decide if -d 0 was used. */
bcb322
diff --git a/src/util/util.c b/src/util/util.c
bcb322
index e3efa7fef..4051c1f4e 100644
bcb322
--- a/src/util/util.c
bcb322
+++ b/src/util/util.c
bcb322
@@ -530,6 +530,133 @@ errno_t sss_filter_sanitize(TALLOC_CTX *mem_ctx,
bcb322
     return sss_filter_sanitize_ex(mem_ctx, input, sanitized, NULL);
bcb322
 }
bcb322
 
bcb322
+
bcb322
+/* There is similar function ldap_dn_normalize in openldap.
bcb322
+ * To avoid dependecies across project we have this own func.
bcb322
+ * Also ldb can do this but doesn't handle all the cases
bcb322
+ */
bcb322
+static errno_t sss_trim_dn(TALLOC_CTX *mem_ctx,
bcb322
+                           const char *input,
bcb322
+                           char **trimmed)
bcb322
+{
bcb322
+    int i = 0;
bcb322
+    int o = 0;
bcb322
+    int s;
bcb322
+    char *output;
bcb322
+    enum sss_trim_dn_state {
bcb322
+        SSS_TRIM_DN_STATE_READING_NAME,
bcb322
+        SSS_TRIM_DN_STATE_READING_VALUE
bcb322
+    } state = SSS_TRIM_DN_STATE_READING_NAME;
bcb322
+
bcb322
+    *trimmed = NULL;
bcb322
+
bcb322
+    output = talloc_array(mem_ctx, char, strlen(input) + 1);
bcb322
+    if (!output) {
bcb322
+        return ENOMEM;
bcb322
+    }
bcb322
+
bcb322
+    /* skip leading spaces */
bcb322
+    while(isspace(input[i])) {
bcb322
+        ++i;
bcb322
+    }
bcb322
+
bcb322
+    while(input[i] != '\0') {
bcb322
+        if (!isspace(input[i])) {
bcb322
+            switch (input[i]) {
bcb322
+            case '=':
bcb322
+                output[o++] = input[i++];
bcb322
+                if (state == SSS_TRIM_DN_STATE_READING_NAME) {
bcb322
+                    while (isspace(input[i])) {
bcb322
+                        ++i;
bcb322
+                    }
bcb322
+                    state = SSS_TRIM_DN_STATE_READING_VALUE;
bcb322
+                }
bcb322
+                break;
bcb322
+            case ',':
bcb322
+                output[o++] = input[i++];
bcb322
+                if (state == SSS_TRIM_DN_STATE_READING_VALUE) {
bcb322
+                    while (isspace(input[i])) {
bcb322
+                        ++i;
bcb322
+                    }
bcb322
+                    state = SSS_TRIM_DN_STATE_READING_NAME;
bcb322
+                }
bcb322
+                break;
bcb322
+            case '\\':
bcb322
+                output[o++] = input[i++];
bcb322
+                if (input[i] != '\0') {
bcb322
+                    output[o++] = input[i++];
bcb322
+                }
bcb322
+                break;
bcb322
+            default:
bcb322
+                if (input[i] != '\0') {
bcb322
+                    output[o++] = input[i++];
bcb322
+                }
bcb322
+                break;
bcb322
+            }
bcb322
+
bcb322
+            continue;
bcb322
+        }
bcb322
+
bcb322
+        /* non escaped space found */
bcb322
+        s = 1;
bcb322
+        while (isspace(input[i + s])) {
bcb322
+            ++s;
bcb322
+        }
bcb322
+
bcb322
+        switch (state) {
bcb322
+        case SSS_TRIM_DN_STATE_READING_NAME:
bcb322
+            if (input[i + s] != '=') {
bcb322
+                /* this is not trailing space - should not be removed */
bcb322
+                while (isspace(input[i])) {
bcb322
+                    output[o++] = input[i++];
bcb322
+                }
bcb322
+            } else {
bcb322
+                i += s;
bcb322
+            }
bcb322
+            break;
bcb322
+        case SSS_TRIM_DN_STATE_READING_VALUE:
bcb322
+            if (input[i + s] != ',') {
bcb322
+                /* this is not trailing space - should not be removed */
bcb322
+                while (isspace(input[i])) {
bcb322
+                    output[o++] = input[i++];
bcb322
+                }
bcb322
+            } else {
bcb322
+                i += s;
bcb322
+            }
bcb322
+            break;
bcb322
+        }
bcb322
+    }
bcb322
+
bcb322
+    output[o--] = '\0';
bcb322
+
bcb322
+    /* trim trailing space */
bcb322
+    while (o >= 0 && isspace(output[o])) {
bcb322
+        output[o--] = '\0';
bcb322
+    }
bcb322
+
bcb322
+    *trimmed = output;
bcb322
+    return EOK;
bcb322
+}
bcb322
+
bcb322
+errno_t sss_filter_sanitize_dn(TALLOC_CTX *mem_ctx,
bcb322
+                               const char *input,
bcb322
+                               char **sanitized)
bcb322
+{
bcb322
+    errno_t ret;
bcb322
+    char *trimmed_dn = NULL;
bcb322
+
bcb322
+    ret = sss_trim_dn(mem_ctx, input, &trimmed_dn);
bcb322
+    if (ret != EOK) {
bcb322
+        goto done;
bcb322
+    }
bcb322
+
bcb322
+    ret = sss_filter_sanitize_ex(mem_ctx, trimmed_dn, sanitized, NULL);
bcb322
+
bcb322
+ done:
bcb322
+    talloc_free(trimmed_dn);
bcb322
+    return ret;
bcb322
+}
bcb322
+
bcb322
 char *
bcb322
 sss_escape_ip_address(TALLOC_CTX *mem_ctx, int family, const char *addr)
bcb322
 {
bcb322
diff --git a/src/util/util.h b/src/util/util.h
bcb322
index 8dc887cab..d7d2017fa 100644
bcb322
--- a/src/util/util.h
bcb322
+++ b/src/util/util.h
bcb322
@@ -477,6 +477,26 @@ errno_t sss_filter_sanitize_for_dom(TALLOC_CTX *mem_ctx,
bcb322
                                     char **sanitized,
bcb322
                                     char **lc_sanitized);
bcb322
 
bcb322
+/* Sanitize an input string (e.g. a DN) for use in
bcb322
+ * an LDAP/LDB filter
bcb322
+ *
bcb322
+ * It is basically the same as sss_filter_sanitize(_ex),
bcb322
+ * just extra spaces inside DN around '=' and ',' are removed
bcb322
+ * before sanitizing other characters . According the documentation
bcb322
+ * spaces in DN are allowed and some ldap servers can return them
bcb322
+ * in isMemberOf or member attributes.
bcb322
+ *
bcb322
+ * (dc = my example, dc = com => dc=my\20example,dc=com)
bcb322
+ *
bcb322
+ * Returns a newly-constructed string attached to mem_ctx
bcb322
+ * It will fail only on an out of memory condition, where it
bcb322
+ * will return ENOMEM.
bcb322
+ *
bcb322
+ */
bcb322
+errno_t sss_filter_sanitize_dn(TALLOC_CTX *mem_ctx,
bcb322
+                               const char *input,
bcb322
+                               char **sanitized);
bcb322
+
bcb322
 char *
bcb322
 sss_escape_ip_address(TALLOC_CTX *mem_ctx, int family, const char *addr);
bcb322
 
bcb322
-- 
bcb322
2.21.3
bcb322