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