[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