-----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. - -- 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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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 <sgall...@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
0001-Add-utility-function-to-sanitize-LDAP-LDB-filters.patch.sig
Description: PGP signature
0002-Add-sysdb-utility-function-for-sanitizing-DN.patch.sig
Description: PGP signature
0003-Sanitize-search-filters-for-the-sysdb.patch.sig
Description: PGP signature
0004-Sanitize-sysdb-search-filters-in-the-IPA-provider.patch.sig
Description: PGP signature
0005-Sanitize-sysdb-filters-in-the-LDAP-provider.patch.sig
Description: PGP signature
0006-Sanitize-sysdb-DN-helpers.patch.sig
Description: PGP signature
0007-Sanitize-search-filters-in-memberOf-plugin.patch.sig
Description: PGP signature
0008-Sanitize-sysdb-dn-for-memberof-lookup.patch.sig
Description: PGP signature
0009-Add-unit-tests-for-users-and-groups-with-odd-charact.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
