[SSSD] [PATCHES] Sanitize search filters in the sysdb
Sumit Bose
sbose at redhat.com
Thu Nov 11 21:38:58 UTC 2010
On Thu, Nov 11, 2010 at 12:04:23PM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 11/10/2010 10:07 AM, Sumit Bose wrote:
>
> >> I have only minor comment, please find them below ...
>
> > Why do you create a tmp_ctx on the two cases above and just use NULL in
> > the following three cases? I think using NULL is sufficient in all
> > cases.
>
>
> I've changed sysdb_custom_subtree_dn() to use NULL, but I left
> sysdb_custom_dn() alone with using tmp_ctx. This is mostly just because
> it reads cleaner, since there are two sanitized values in this function.
>
> All other recommended changes are made. Thank you for the review. New
> patches attached.
>
Thanks.
ACK
bye,
Sumit
> - --
> Stephen Gallagher
> RHCE 804006346421761
>
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkzcIhYACgkQeiVVYja6o6PUKgCfcyU3P3ZDt0lt2bkCLig2LfRr
> j3IAoIMpKp6vTXVrHsyQ/SUuIK2T+mTU
> =53CM
> -----END PGP SIGNATURE-----
> From 92817ea0da8062fba1835c35fd12bb6085364582 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Thu, 28 Oct 2010 12:12:12 -0400
> Subject: [PATCH 1/9] Add utility function to sanitize LDAP/LDB filters
>
> Also adds a unit test.
> ---
> src/tests/util-tests.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/util.c | 52 ++++++++++++++++++++++++++++++++++++
> src/util/util.h | 11 ++++++++
> 3 files changed, 131 insertions(+), 0 deletions(-)
>
> diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
> index bfc48bbac4e2a9ec12ff1a7c6cbe87bd14580881..cf96f0e356942c2bcd2667a2b778a65a91f46e2d 100644
> --- a/src/tests/util-tests.c
> +++ b/src/tests/util-tests.c
> @@ -175,6 +175,73 @@ START_TEST(test_diff_string_lists)
> }
> END_TEST
>
> +
> +START_TEST(test_sss_filter_sanitize)
> +{
> + errno_t ret;
> + char *sanitized = NULL;
> +
> + TALLOC_CTX *test_ctx = talloc_new(NULL);
> + fail_if (test_ctx == NULL, "Out of memory");
> +
> + const char no_specials[] = "username";
> + ret = sss_filter_sanitize(test_ctx, no_specials, &sanitized);
> + fail_unless(ret == EOK, "no_specials error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(no_specials, sanitized)==0,
> + "Expected [%s], got [%s]",
> + no_specials, sanitized);
> +
> + const char has_asterisk[] = "*username";
> + const char has_asterisk_expected[] = "\\2ausername";
> + ret = sss_filter_sanitize(test_ctx, has_asterisk, &sanitized);
> + fail_unless(ret == EOK, "has_asterisk error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(has_asterisk_expected, sanitized)==0,
> + "Expected [%s], got [%s]",
> + has_asterisk_expected, sanitized);
> +
> + const char has_lparen[] = "user(name";
> + const char has_lparen_expected[] = "user\\28name";
> + ret = sss_filter_sanitize(test_ctx, has_lparen, &sanitized);
> + fail_unless(ret == EOK, "has_lparen error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(has_lparen_expected, sanitized)==0,
> + "Expected [%s], got [%s]",
> + has_lparen_expected, sanitized);
> +
> + const char has_rparen[] = "user)name";
> + const char has_rparen_expected[] = "user\\29name";
> + ret = sss_filter_sanitize(test_ctx, has_rparen, &sanitized);
> + fail_unless(ret == EOK, "has_rparen error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(has_rparen_expected, sanitized)==0,
> + "Expected [%s], got [%s]",
> + has_rparen_expected, sanitized);
> +
> + const char has_backslash[] = "username\\";
> + const char has_backslash_expected[] = "username\\5c";
> + ret = sss_filter_sanitize(test_ctx, has_backslash, &sanitized);
> + fail_unless(ret == EOK, "has_backslash error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(has_backslash_expected, sanitized)==0,
> + "Expected [%s], got [%s]",
> + has_backslash_expected, sanitized);
> +
> + const char has_all[] = "\\(user)*name";
> + const char has_all_expected[] = "\\5c\\28user\\29\\2aname";
> + ret = sss_filter_sanitize(test_ctx, has_all, &sanitized);
> + fail_unless(ret == EOK, "has_all error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(strcmp(has_all_expected, sanitized)==0,
> + "Expected [%s], got [%s]",
> + has_all_expected, sanitized);
> +
> + talloc_free(test_ctx);
> +}
> +END_TEST
> +
> +
> Suite *util_suite(void)
> {
> Suite *s = suite_create("util");
> @@ -182,6 +249,7 @@ Suite *util_suite(void)
> TCase *tc_util = tcase_create("util");
>
> tcase_add_test (tc_util, test_diff_string_lists);
> + tcase_add_test (tc_util, test_sss_filter_sanitize);
> tcase_set_timeout(tc_util, 60);
>
> suite_add_tcase (s, tc_util);
> diff --git a/src/util/util.c b/src/util/util.c
> index 06eea283770601997e3ff90df73db58a4fb25bba..772a8b73f3a99d17b9a1cb9bc5d6543ef78ebc87 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -515,3 +515,55 @@ errno_t sss_hash_create(TALLOC_CTX *mem_ctx,
> talloc_free(internal_ctx);
> return ret;
> }
> +
> +errno_t sss_filter_sanitize(TALLOC_CTX *mem_ctx,
> + const char *input,
> + char **sanitized)
> +{
> + char *output;
> + size_t i = 0;
> + size_t j = 0;
> +
> + /* Assume the worst-case. We'll resize it later, once */
> + output = talloc_array(mem_ctx, char, strlen(input) * 3 + 1);
> + if (!output) {
> + return ENOMEM;
> + }
> +
> + while (input[i]) {
> + switch(input[i]) {
> + case '*':
> + output[j++] = '\\';
> + output[j++] = '2';
> + output[j++] = 'a';
> + break;
> + case '(':
> + output[j++] = '\\';
> + output[j++] = '2';
> + output[j++] = '8';
> + break;
> + case ')':
> + output[j++] = '\\';
> + output[j++] = '2';
> + output[j++] = '9';
> + break;
> + case '\\':
> + output[j++] = '\\';
> + output[j++] = '5';
> + output[j++] = 'c';
> + break;
> + default:
> + output[j++] = input[i];
> + }
> +
> + i++;
> + }
> + output[j] = '\0';
> + *sanitized = talloc_realloc(mem_ctx, output, char, j+1);
> + if (!*sanitized) {
> + talloc_free(output);
> + return ENOMEM;
> + }
> +
> + return EOK;
> +}
> diff --git a/src/util/util.h b/src/util/util.h
> index e93f6f86373aa3679b01e6f4ade7dfe5118006c7..53a6b1c97dc42459c9bcb99424fd75a4b3d8e107 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -366,4 +366,15 @@ errno_t diff_string_lists(TALLOC_CTX *memctx,
> char ***string1_only,
> char ***string2_only,
> char ***both_strings);
> +
> +/* Sanitize an input string (e.g. a username) for use in
> + * an LDAP/LDB filter
> + * 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(TALLOC_CTX *mem_ctx,
> + const char *input,
> + char **sanitized);
> +
> #endif /* __SSSD_UTIL_H__ */
> --
> 1.7.3.2
>
> From ec17e83efed97ffa85c13a69f2a041de23d6ee04 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 29 Oct 2010 10:11:05 -0400
> Subject: [PATCH 2/9] Add sysdb utility function for sanitizing DN
>
> ---
> src/db/sysdb.c | 24 ++++++++++++++++++++++++
> src/db/sysdb.h | 3 +++
> 2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/src/db/sysdb.c b/src/db/sysdb.c
> index bc6f8fc973f525385055c42851d42224c7ccb0a5..b2691526a4a02874f9784c2c41a63c8be237bc0c 100644
> --- a/src/db/sysdb.c
> +++ b/src/db/sysdb.c
> @@ -25,6 +25,30 @@
> #include "confdb/confdb.h"
> #include <time.h>
>
> +errno_t sysdb_dn_sanitize(void *mem_ctx, const char *input,
> + char **sanitized)
> +{
> + struct ldb_val val;
> + errno_t ret = EOK;
> +
> + val.data = (uint8_t *)talloc_strdup(mem_ctx, input);
> + if (!val.data) {
> + return ENOMEM;
> + }
> +
> + /* We can't include the trailing NULL because it would
> + * be escaped and result in an unterminated string
> + */
> + val.length = strlen(input);
> +
> + *sanitized = ldb_dn_escape_value(mem_ctx, val);
> + if (!*sanitized) {
> + ret = ENOMEM;
> + }
> +
> + talloc_free(val.data);
> + return ret;
> +}
>
> struct ldb_dn *sysdb_custom_subtree_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain,
> diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> index 0d8b29c96b8c459d8c84477425d8238f42b4c1ee..fde27b93e95774ff5d2cf16609981e739ec2f2fe 100644
> --- a/src/db/sysdb.h
> +++ b/src/db/sysdb.h
> @@ -697,4 +697,7 @@ errno_t sysdb_netgr_to_entries(TALLOC_CTX *mem_ctx,
> struct ldb_result *res,
> struct sysdb_netgroup_ctx ***entries);
>
> +errno_t sysdb_dn_sanitize(void *mem_ctx, const char *input,
> + char **sanitized);
> +
> #endif /* __SYS_DB_H__ */
> --
> 1.7.3.2
>
> From c9f1414ed527d8f85fbbaa5fa81819f271a58d3f Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 5 Nov 2010 11:05:38 -0400
> Subject: [PATCH 3/9] Sanitize search filters for the sysdb
>
> ---
> src/db/sysdb_search.c | 45 +++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> index e983b395741d9204a5a9dc1e122cc347e6fcb9ce..dfae4ddf0433947602a50859ebdd35757eca6e76 100644
> --- a/src/db/sysdb_search.c
> +++ b/src/db/sysdb_search.c
> @@ -37,6 +37,7 @@ int sysdb_getpwnam(TALLOC_CTX *mem_ctx,
> static const char *attrs[] = SYSDB_PW_ATTRS;
> struct ldb_dn *base_dn;
> struct ldb_result *res;
> + char *sanitized_name;
> int ret;
>
> if (!domain) {
> @@ -55,8 +56,14 @@ int sysdb_getpwnam(TALLOC_CTX *mem_ctx,
> goto done;
> }
>
> + ret = sss_filter_sanitize(tmpctx, name, &sanitized_name);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> ret = ldb_search(ctx->ldb, tmpctx, &res, base_dn,
> - LDB_SCOPE_SUBTREE, attrs, SYSDB_PWNAM_FILTER, name);
> + LDB_SCOPE_SUBTREE, attrs, SYSDB_PWNAM_FILTER,
> + sanitized_name);
> if (ret) {
> ret = sysdb_error_to_errno(ret);
> goto done;
> @@ -206,6 +213,7 @@ int sysdb_getgrnam(TALLOC_CTX *mem_ctx,
> TALLOC_CTX *tmpctx;
> static const char *attrs[] = SYSDB_GRSRC_ATTRS;
> const char *fmt_filter;
> + char *sanitized_name;
> struct ldb_dn *base_dn;
> struct ldb_result *res;
> int ret;
> @@ -233,8 +241,14 @@ int sysdb_getgrnam(TALLOC_CTX *mem_ctx,
> goto done;
> }
>
> + ret = sss_filter_sanitize(tmpctx, name, &sanitized_name);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> ret = ldb_search(ctx->ldb, tmpctx, &res, base_dn,
> - LDB_SCOPE_SUBTREE, attrs, fmt_filter, name);
> + LDB_SCOPE_SUBTREE, attrs, fmt_filter,
> + sanitized_name);
> if (ret) {
> ret = sysdb_error_to_errno(ret);
> goto done;
> @@ -472,6 +486,7 @@ int sysdb_get_user_attr(TALLOC_CTX *mem_ctx,
> TALLOC_CTX *tmpctx;
> struct ldb_dn *base_dn;
> struct ldb_result *res;
> + char *sanitized_name;
> int ret;
>
> if (!domain) {
> @@ -490,9 +505,14 @@ int sysdb_get_user_attr(TALLOC_CTX *mem_ctx,
> goto done;
> }
>
> + ret = sss_filter_sanitize(tmpctx, name, &sanitized_name);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> ret = ldb_search(ctx->ldb, tmpctx, &res, base_dn,
> LDB_SCOPE_SUBTREE, attributes,
> - SYSDB_PWNAM_FILTER, name);
> + SYSDB_PWNAM_FILTER, sanitized_name);
> if (ret) {
> ret = sysdb_error_to_errno(ret);
> goto done;
> @@ -769,6 +789,7 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> static const char *attrs[] = SYSDB_NETGR_ATTRS;
> struct ldb_dn *base_dn;
> struct ldb_result *result;
> + char *sanitized_netgroup;
> char *netgroup_dn;
> int lret;
> errno_t ret;
> @@ -790,8 +811,13 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> goto done;
> }
>
> + ret = sss_filter_sanitize(tmp_ctx, netgroup, &sanitized_netgroup);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> netgroup_dn = talloc_asprintf(tmp_ctx, SYSDB_TMPL_NETGROUP,
> - netgroup, domain->name);
> + sanitized_netgroup, domain->name);
> if (!netgroup_dn) {
> ret = ENOMEM;
> goto done;
> @@ -800,7 +826,7 @@ errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> lret = ldb_search(ctx->ldb, tmp_ctx, &result, base_dn,
> LDB_SCOPE_SUBTREE, attrs,
> SYSDB_NETGR_TRIPLES_FILTER,
> - netgroup, netgroup_dn);
> + sanitized_netgroup, netgroup_dn);
> ret = sysdb_error_to_errno(lret);
> if (ret != EOK) {
> goto done;
> @@ -824,6 +850,7 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
> TALLOC_CTX *tmpctx;
> struct ldb_dn *base_dn;
> struct ldb_result *result;
> + char *sanitized_netgroup;
> int ret;
>
> if (!domain) {
> @@ -842,9 +869,15 @@ int sysdb_get_netgroup_attr(TALLOC_CTX *mem_ctx,
> goto done;
> }
>
> + ret = sss_filter_sanitize(tmpctx, netgrname, &sanitized_netgroup);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> ret = ldb_search(ctx->ldb, tmpctx, &result, base_dn,
> LDB_SCOPE_SUBTREE, attributes,
> - SYSDB_NETGR_FILTER, netgrname);
> + SYSDB_NETGR_FILTER,
> + sanitized_netgroup);
> if (ret) {
> ret = sysdb_error_to_errno(ret);
> goto done;
> --
> 1.7.3.2
>
> From d6957ceffa69ef32a2ba9b0391209f9082a8d4fb Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Thu, 28 Oct 2010 20:28:59 -0400
> Subject: [PATCH 4/9] Sanitize sysdb search filters in the IPA provider
>
> ---
> src/providers/ipa/ipa_access.c | 19 +++++++++++++++++--
> 1 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
> index 979959fb12f6383edd86ce2405de217462b9a5a6..47e98cf95f5f3fcae98dbeaed17cf7aa5b175ac9 100644
> --- a/src/providers/ipa/ipa_access.c
> +++ b/src/providers/ipa/ipa_access.c
> @@ -735,6 +735,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
> struct tevent_req *subreq = NULL;
> struct hbac_get_host_info_state *state;
> struct sdap_handle *sdap_handle;
> + char *host;
> int ret;
> int i;
>
> @@ -763,14 +764,20 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
> goto fail;
> }
> for (i = 0; hostnames[i] != NULL; i++) {
> + ret = sss_filter_sanitize(state->host_filter, hostnames[i], &host);
> + if (ret != EOK) {
> + goto fail;
> + }
> +
> state->host_filter = talloc_asprintf_append(state->host_filter,
> "(&(objectclass=ipaHost)"
> "(|(fqdn=%s)(serverhostname=%s)))",
> - hostnames[i], hostnames[i]);
> + host, host);
> if (state->host_filter == NULL) {
> ret = ENOMEM;
> goto fail;
> }
> + talloc_zfree(host);
> }
> state->host_filter = talloc_asprintf_append(state->host_filter, ")");
> if (state->host_filter == NULL) {
> @@ -1028,6 +1035,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
> struct tevent_req *subreq = NULL;
> struct hbac_get_rules_state *state;
> struct sdap_handle *sdap_handle;
> + char *host_dn_clean;
> int ret;
> int i;
>
> @@ -1084,16 +1092,23 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
> state->hbac_attrs[16] = SYSDB_ORIG_DN;
> state->hbac_attrs[17] = NULL;
>
> + ret = sss_filter_sanitize(state, host_dn, &host_dn_clean);
> + if (ret != EOK) {
> + goto fail;
> + }
> +
> state->hbac_filter = talloc_asprintf(state,
> "(&(objectclass=ipaHBACRule)"
> "(%s=%s)(|(%s=%s)(%s=%s)",
> IPA_ENABLED_FLAG, IPA_TRUE_VALUE,
> IPA_HOST_CATEGORY, "all",
> - IPA_MEMBER_HOST, host_dn);
> + IPA_MEMBER_HOST, host_dn_clean);
> if (state->hbac_filter == NULL) {
> ret = ENOMEM;
> goto fail;
> }
> + talloc_zfree(host_dn_clean);
> +
> for (i = 0; memberof[i] != NULL; i++) {
> state->hbac_filter = talloc_asprintf_append(state->hbac_filter,
> "(%s=%s)",
> --
> 1.7.3.2
>
> From bbffef14f776a3867864c4c2fd27f9ca73d030e0 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Thu, 28 Oct 2010 20:34:45 -0400
> Subject: [PATCH 5/9] Sanitize sysdb filters in the LDAP provider
>
> ---
> src/providers/ldap/sdap_async_accounts.c | 13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
> index 6b14161cefb8a272edea34ac117550e5d53139a0..ab599f8c8775bb8d28da8de98c2ac5170f2f09b9 100644
> --- a/src/providers/ldap/sdap_async_accounts.c
> +++ b/src/providers/ldap/sdap_async_accounts.c
> @@ -2590,6 +2590,7 @@ static errno_t sdap_nested_group_process_step(struct tevent_req *req)
> errno_t ret;
> struct sdap_nested_group_ctx *state =
> tevent_req_data(req, struct sdap_nested_group_ctx);
> + char *member_dn;
> char *filter;
> static const char *attrs[] = SYSDB_PW_ATTRS;
> size_t count;
> @@ -2636,10 +2637,15 @@ static errno_t sdap_nested_group_process_step(struct tevent_req *req)
>
> } while (has_key);
>
> + ret = sss_filter_sanitize(state, state->member_dn, &member_dn);
> + if (ret != EOK) {
> + goto error;
> + }
> +
> /* Check for the specified origDN in the sysdb */
> filter = talloc_asprintf(NULL, "(%s=%s)",
> SYSDB_ORIG_DN,
> - state->member_dn);
> + member_dn);
> if (!filter) {
> ret = ENOMEM;
> goto error;
> @@ -2657,11 +2663,13 @@ static errno_t sdap_nested_group_process_step(struct tevent_req *req)
>
> filter = talloc_asprintf(NULL, "(%s=%s)",
> SYSDB_ORIG_DN,
> - state->member_dn);
> + member_dn);
> if (!filter) {
> ret = ENOMEM;
> goto error;
> }
> + talloc_zfree(member_dn);
> +
> ret = sysdb_search_groups(state, state->sysdb, state->domain,
> filter, attrs, &count, &msgs);
> talloc_zfree(filter);
> @@ -2710,6 +2718,7 @@ static errno_t sdap_nested_group_process_step(struct tevent_req *req)
>
> return EAGAIN;
> }
> + talloc_zfree(member_dn);
>
> /* We found a user with this origDN in the sysdb */
>
> --
> 1.7.3.2
>
> From d6c7ce0d95711051f67a3319d89c1cdffb5dbdc6 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 5 Nov 2010 11:15:42 -0400
> Subject: [PATCH 6/9] Sanitize sysdb DN helpers
>
> ---
> src/db/sysdb.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/src/db/sysdb.c b/src/db/sysdb.c
> index b2691526a4a02874f9784c2c41a63c8be237bc0c..07f7e9b1ca8e971e8502c45bf8815ebfd7e2c1f4 100644
> --- a/src/db/sysdb.c
> +++ b/src/db/sysdb.c
> @@ -54,33 +54,109 @@ struct ldb_dn *sysdb_custom_subtree_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain,
> const char *subtree_name)
> {
> - return ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
> - subtree_name, domain);
> + errno_t ret;
> + char *clean_subtree;
> + struct ldb_dn *dn = NULL;
> +
> + ret = sysdb_dn_sanitize(NULL, subtree_name, &clean_subtree);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> + dn = ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
> + clean_subtree, domain);
> + talloc_free(clean_subtree);
> +
> + return dn;
> }
> struct ldb_dn *sysdb_custom_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain, const char *object_name,
> const char *subtree_name)
> {
> - return ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_CUSTOM, object_name,
> - subtree_name, domain);
> + errno_t ret;
> + TALLOC_CTX *tmp_ctx;
> + char *clean_name;
> + char *clean_subtree;
> + struct ldb_dn *dn = NULL;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (!tmp_ctx) {
> + return NULL;
> + }
> +
> + ret = sysdb_dn_sanitize(tmp_ctx, object_name, &clean_name);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> + ret = sysdb_dn_sanitize(tmp_ctx, subtree_name, &clean_subtree);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> + dn = ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_CUSTOM, clean_name,
> + clean_subtree, domain);
> +
> +done:
> + talloc_free(tmp_ctx);
> + return dn;
> }
>
> struct ldb_dn *sysdb_user_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain, const char *name)
> {
> - return ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_USER, name, domain);
> + errno_t ret;
> + char *clean_name;
> + struct ldb_dn *dn;
> +
> + ret = sysdb_dn_sanitize(NULL, name, &clean_name);
> + if (ret != EOK) {
> + return NULL;
> + }
> +
> + dn = ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_USER,
> + clean_name, domain);
> + talloc_free(clean_name);
> +
> + return dn;
> }
>
> struct ldb_dn *sysdb_group_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain, const char *name)
> {
> - return ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_GROUP, name, domain);
> + errno_t ret;
> + char *clean_name;
> + struct ldb_dn *dn;
> +
> + ret = sysdb_dn_sanitize(NULL, name, &clean_name);
> + if (ret != EOK) {
> + return NULL;
> + }
> +
> + dn = ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_GROUP,
> + clean_name, domain);
> + talloc_free(clean_name);
> +
> + return dn;
> }
>
> struct ldb_dn *sysdb_netgroup_dn(struct sysdb_ctx *ctx, void *memctx,
> const char *domain, const char *name)
> {
> - return ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_NETGROUP, name, domain);
> + errno_t ret;
> + char *clean_name;
> + struct ldb_dn *dn;
> +
> + ret = sysdb_dn_sanitize(NULL, name, &clean_name);
> + if (ret != EOK) {
> + return NULL;
> + }
> +
> + dn = ldb_dn_new_fmt(memctx, ctx->ldb, SYSDB_TMPL_NETGROUP,
> + clean_name, domain);
> + talloc_free(clean_name);
> +
> + return dn;
> }
>
> struct ldb_dn *sysdb_netgroup_base_dn(struct sysdb_ctx *ctx, void *memctx,
> --
> 1.7.3.2
>
> From 47535dc8688576d68f9dc6af95a5b031989958c0 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Mon, 1 Nov 2010 14:47:09 -0400
> Subject: [PATCH 7/9] Sanitize search filters in memberOf plugin
>
> ---
> Makefile.am | 4 +++-
> src/ldb_modules/memberof.c | 22 ++++++++++++++++++++--
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index c9fd0a165f8af7fd620a132a613ecf4a23765c99..102149a8cd2734b9080c34f318a2e97ff79a78ba 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -933,7 +933,9 @@ proxy_child_LDADD = \
> $(SSSD_LIBS)
>
> memberof_la_SOURCES = \
> - src/ldb_modules/memberof.c
> + $(SSSD_DEBUG_OBJ) \
> + src/ldb_modules/memberof.c \
> + src/util/util.c
> memberof_la_CFLAGS = \
> $(AM_CFLAGS)
> memberof_la_LIBADD = $(LDB_LIBS) $(DHASH_LIBS)
> diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
> index 1e28593dd1c8d1522c9aefa1434d544e91412737..372aa544f31f40698c96619ce37e01df7cb7929f 100644
> --- a/src/ldb_modules/memberof.c
> +++ b/src/ldb_modules/memberof.c
> @@ -1167,9 +1167,11 @@ static int memberof_del(struct ldb_module *module, struct ldb_request *req)
> struct ldb_request *search;
> char *expression;
> const char *dn;
> + char *clean_dn;
> struct mbof_del_ctx *del_ctx;
> struct mbof_ctx *ctx;
> int ret;
> + errno_t sret;
>
> if (ldb_dn_is_special(req->op.del.dn)) {
> /* do not manipulate our control entries */
> @@ -1206,13 +1208,21 @@ static int memberof_del(struct ldb_module *module, struct ldb_request *req)
> talloc_free(ctx);
> return LDB_ERR_OPERATIONS_ERROR;
> }
> +
> + sret = sss_filter_sanitize(del_ctx, dn, &clean_dn);
> + if (sret != 0) {
> + talloc_free(ctx);
> + return LDB_ERR_OPERATIONS_ERROR;
> + }
> +
> expression = talloc_asprintf(del_ctx,
> "(|(distinguishedName=%s)(%s=%s))",
> - dn, DB_MEMBER, dn);
> + clean_dn, DB_MEMBER, clean_dn);
> if (!expression) {
> talloc_free(ctx);
> return LDB_ERR_OPERATIONS_ERROR;
> }
> + talloc_zfree(clean_dn);
>
> ret = ldb_build_search_req(&search, ldb, del_ctx,
> NULL, LDB_SCOPE_SUBTREE,
> @@ -1586,6 +1596,7 @@ static int mbof_del_execute_op(struct mbof_del_operation *delop)
> struct ldb_request *search;
> char *expression;
> const char *dn;
> + char *clean_dn;
> static const char *attrs[] = { DB_OC, DB_NAME,
> DB_MEMBER, DB_MEMBEROF, NULL };
> int ret;
> @@ -1599,12 +1610,19 @@ static int mbof_del_execute_op(struct mbof_del_operation *delop)
> if (!dn) {
> return LDB_ERR_OPERATIONS_ERROR;
> }
> +
> + ret = sss_filter_sanitize(del_ctx, dn, &clean_dn);
> + if (ret != 0) {
> + return LDB_ERR_OPERATIONS_ERROR;
> + }
> +
> expression = talloc_asprintf(del_ctx,
> "(|(distinguishedName=%s)(%s=%s))",
> - dn, DB_MEMBER, dn);
> + clean_dn, DB_MEMBER, clean_dn);
> if (!expression) {
> return LDB_ERR_OPERATIONS_ERROR;
> }
> + talloc_zfree(clean_dn);
>
> ret = ldb_build_search_req(&search, ldb, delop,
> NULL, LDB_SCOPE_SUBTREE,
> --
> 1.7.3.2
>
> From 6cdd932d8209716f43e6224138ad1151413788f9 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 5 Nov 2010 10:51:39 -0400
> Subject: [PATCH 8/9] Sanitize sysdb dn for memberof lookup
>
> ---
> src/providers/ldap/sdap_async_accounts.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
> index ab599f8c8775bb8d28da8de98c2ac5170f2f09b9..f4d6d052585ee9dc39ab41c33b88c779fa0d3e1c 100644
> --- a/src/providers/ldap/sdap_async_accounts.c
> +++ b/src/providers/ldap/sdap_async_accounts.c
> @@ -1915,6 +1915,7 @@ static void sdap_initgr_rfc2307_process(struct tevent_req *subreq)
> struct ldb_message_element *groups;
> size_t count;
> const char *attrs[2];
> + char *clean_dn;
> int ret;
> int i;
>
> @@ -1967,14 +1968,23 @@ static void sdap_initgr_rfc2307_process(struct tevent_req *subreq)
>
> /* Get a list of the groups by groupname only */
> for (i=0; i < groups->num_values; i++) {
> + ret = sysdb_dn_sanitize(state,
> + (const char *)groups->values[i].data,
> + &clean_dn);
> + if (ret != EOK) {
> + tevent_req_error(req, ret);
> + return;
> + }
> +
> ret = sysdb_group_dn_name(state->sysdb,
> sysdb_grouplist,
> - (const char *)groups->values[i].data,
> + clean_dn,
> &sysdb_grouplist[i]);
> if (ret != EOK) {
> tevent_req_error(req, ENOMEM);
> return;
> }
> + talloc_zfree(clean_dn);
> }
> sysdb_grouplist[groups->num_values] = NULL;
> }
> --
> 1.7.3.2
>
> From 1aff80fb0aa890e17f9977fb5fb84660186b173a Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 5 Nov 2010 11:05:47 -0400
> Subject: [PATCH 9/9] Add unit tests for users and groups with odd characters
>
> ---
> src/tests/sysdb-tests.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 145 insertions(+), 0 deletions(-)
>
> diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
> index dd76d97afeddfeee47313f8015f42071bfea3035..f8710a1c52ce8ead9a3382ebceb4b4c5e9b3a2c1 100644
> --- a/src/tests/sysdb-tests.c
> +++ b/src/tests/sysdb-tests.c
> @@ -2714,6 +2714,148 @@ START_TEST(test_sysdb_remove_netgroup_member)
> }
> END_TEST
>
> +START_TEST(test_odd_characters)
> +{
> + errno_t ret;
> + struct sysdb_test_ctx *test_ctx;
> + struct ldb_result *res;
> + struct ldb_message *msg;
> + struct ldb_val *val;
> + const char odd_username[] = "*(odd)\\user,name";
> + const char odd_groupname[] = "*(odd\\*)\\group,name";
> + const char odd_netgroupname[] = "*(odd\\*)\\netgroup,name";
> + const char *received_user;
> + const char *received_group;
> + static const char *user_attrs[] = SYSDB_PW_ATTRS;
> + static const char *netgr_attrs[] = SYSDB_NETGR_ATTRS;
> +
> + /* Setup */
> + ret = setup_sysdb_tests(&test_ctx);
> + if (ret != EOK) {
> + fail("Could not set up the test");
> + return;
> + }
> +
> + /* ===== Groups ===== */
> +
> + /* Add */
> + ret = sysdb_add_incomplete_group(test_ctx->sysdb, test_ctx->domain,
> + odd_groupname, 20000);
> + fail_unless(ret == EOK, "sysdb_add_incomplete_group error [%d][%s]",
> + ret, strerror(ret));
> +
> + /* Retrieve */
> + ret = sysdb_search_group_by_name(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_groupname, NULL, &msg);
> + fail_unless(ret == EOK, "sysdb_search_group_by_name error [%d][%s]",
> + ret, strerror(ret));
> + talloc_zfree(msg);
> +
> + ret = sysdb_getgrnam(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_groupname, &res);
> + fail_unless(ret == EOK, "sysdb_getgrnam error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(res->count == 1, "Received [%d] responses",
> + res->count);
> + received_group = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
> + fail_unless(strcmp(received_group, odd_groupname) == 0,
> + "Expected [%s], got [%s]",
> + odd_groupname, received_group);
> + talloc_free(res);
> +
> +
> + /* ===== Users ===== */
> +
> + /* Add */
> + ret = sysdb_add_basic_user(test_ctx,
> + test_ctx->sysdb,
> + test_ctx->domain,
> + odd_username,
> + 10000, 10000,
> + "","","");
> + fail_unless(ret == EOK, "sysdb_add_fake_user error [%d][%s]",
> + ret, strerror(ret));
> +
> + /* Retrieve */
> + ret = sysdb_search_user_by_name(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_username, NULL, &msg);
> + fail_unless(ret == EOK, "sysdb_search_user_by_name error [%d][%s]",
> + ret, strerror(ret));
> + val = ldb_dn_get_component_val(msg->dn, 0);
> + fail_unless(strcmp((char *)val->data, odd_username)==0,
> + "Expected [%s] got [%s]\n",
> + odd_username, (char *)val->data);
> + talloc_zfree(msg);
> +
> + /* Add to the group */
> + ret = sysdb_add_group_member(test_ctx->sysdb, test_ctx->domain,
> + odd_groupname, odd_username,
> + SYSDB_MEMBER_USER);
> + fail_unless(ret == EOK, "sysdb_add_group_member error [%d][%s]",
> + ret, strerror(ret));
> +
> + ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_username, &res);
> + fail_unless(ret == EOK, "sysdb_getpwnam error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(res->count == 1, "Received [%d] responses",
> + res->count);
> + received_user = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
> + fail_unless(strcmp(received_user, odd_username) == 0,
> + "Expected [%s], got [%s]",
> + odd_username, received_user);
> + talloc_zfree(res);
> +
> + /* Attributes */
> + ret = sysdb_get_user_attr(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_username, user_attrs, &res);
> + fail_unless(ret == EOK, "sysdb_get_user_attr error [%d][%s]",
> + ret, strerror(ret));
> + talloc_free(res);
> +
> + /* Delete User */
> + ret = sysdb_delete_user(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_username, 10000);
> + fail_unless(ret == EOK, "sysdb_delete_user error [%d][%s]",
> + ret, strerror(ret));
> +
> +
> + /* Delete Group */
> + ret = sysdb_delete_group(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_groupname, 20000);
> + fail_unless(ret == EOK, "sysdb_delete_group error [%d][%s]",
> + ret, strerror(ret));
> +
> + /* ===== Netgroups ===== */
> + /* Add */
> + ret = sysdb_add_netgroup(test_ctx->sysdb, test_ctx->domain,
> + odd_netgroupname, "No description",
> + NULL, 30);
> + fail_unless(ret == EOK, "sysdb_add_netgroup error [%d][%s]",
> + ret, strerror(ret));
> +
> + /* Retrieve */
> + ret = sysdb_getnetgr(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_netgroupname, &res);
> + fail_unless(ret == EOK, "sysdb_getnetgr error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(res->count == 1, "Received [%d] responses",
> + res->count);
> + talloc_zfree(res);
> +
> + ret = sysdb_get_netgroup_attr(test_ctx, test_ctx->sysdb, test_ctx->domain,
> + odd_netgroupname, netgr_attrs, &res);
> + fail_unless(ret == EOK, "sysdb_get_netgroup_attr error [%d][%s]",
> + ret, strerror(ret));
> + fail_unless(res->count == 1, "Received [%d] responses",
> + res->count);
> + talloc_zfree(res);
> +
> + /* ===== Arbitrary Entries ===== */
> +
> + talloc_free(test_ctx);
> +}
> +END_TEST
>
> Suite *create_sysdb_suite(void)
> {
> @@ -2836,6 +2978,9 @@ Suite *create_sysdb_suite(void)
>
> tcase_add_test(tc_sysdb, test_sysdb_attrs_to_list);
>
> + /* Test unusual characters */
> + tcase_add_test(tc_sysdb, test_odd_characters);
> +
> /* ===== NETGROUP TESTS ===== */
>
> /* Create a new netgroup */
> --
> 1.7.3.2
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
More information about the sssd-devel
mailing list