[PATCHES] Sanitize search filters in the sysdb

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This is the first part of a fix for https://fedorahosted.org/sssd/ticket/639

These patches address the various search filters we're using in the
sysdb. Patches for LDAP search filters will be coming later. I wanted to
get these reviewed first.

Patch 0001: Adds a utility function to sanitize search filters using the
method described in RFC 4515. Includes a unit test.

Patch 0002: Sanitize search filters internal to the public sysdb APIs.
Includes unit tests.

Patch 0003: Sanitize sysdb search filters in the IPA provider used with
sysdb_search_custom()

Patch 0004: Sanitize sysdb search filters used when processing nested
groups.

- -- 
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.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkzKGl0ACgkQeiVVYja6o6OSLQCeI9d1ITiVS4JcrZesZTXsyTyG
> zS0AoIQeKBsatVL4Ed51D2nGwxaT7PVh
> =qbgv
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/28/2010 08:50 PM, Stephen Gallagher wrote:
> This is the first part of a fix for https://fedorahosted.org/sssd/ticket/639
>
> These patches address the various search filters we're using in the
> sysdb. Patches for LDAP search filters will be coming later. I wanted to
> get these reviewed first.
>
> Patch 0001: Adds a utility function to sanitize search filters using the
> method described in RFC 4515. Includes a unit test.
>
> Patch 0002: Sanitize search filters internal to the public sysdb APIs.
> Includes unit tests.
>
> Patch 0003: Sanitize sysdb search filters in the IPA provider used with
> sysdb_search_custom()
>
> Patch 0004: Sanitize sysdb search filters used when processing nested
> groups.

Sorry, withdrawing these patches. There's a problem with
sysdb_delete_user() and sysdb_delete_group(). I'm not sure how I missed
it, since the test I wrote is failing...

Unfortunately, I think I may have discovered a bug in libldb that I'll
have to work around. It looks like libldb is overzealous in sanitizing a
basedn with strange characters. I will dig into this tomorrow.

- -- 
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.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkzKH0sACgkQeiVVYja6o6Nn+gCgoXu8k93E4iEIJHa4ZK2847Bi
> PmMAoIxO9x8let7MXi16Zp67g8EkRpXD
> =f7kS
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/28/2010 09:11 PM, Stephen Gallagher wrote:
> On 10/28/2010 08:50 PM, Stephen Gallagher wrote:
> > This is the first part of a fix for https://fedorahosted.org/sssd/ticket/639
> > These patches address the various search filters we're using in the
> > sysdb. Patches for LDAP search filters will be coming later. I wanted to
> > get these reviewed first.

Patch 0001: Add utility function to sanitize LDAP/LDB filters. Contains
a unit test.

Patch 0002: Add sysdb utility function for sanitizing DN. No unit test,
since we're relying on an opaque function from libldb.

Patch 0003: Use sss_filter_sanitize() for all ldb searches internal to
the sysdb

Patch 0004: Use sss_filter_sanitize() for all ldb searches in the IPA
provider

Patch 0005: Use sss_filter_sanitize() for all ldb searches in the LDAP
provider

Patch 0006: Use sysdb_dn_sanitize() in the DN helpers

Patch 0007: Sanitize search filters in the memberOf plugin

Patch 0008: Sanitize a DN lookup for memberOf in initgroups processing

Patch 0009: Add unit tests for users/groups/netgroups with odd characters

- -- 
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/
>
> iEYEARECAAYFAkzUJFcACgkQeiVVYja6o6OqzACePcPef36XeMXYojTwqKAnnBKj
> dFQAnR87P/W/wCoaMk9C4RtKiyE0vxuS
> =ndLg
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Fri, Nov 05, 2010 at 11:35:52AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 10/28/2010 09:11 PM, Stephen Gallagher wrote:
> > On 10/28/2010 08:50 PM, Stephen Gallagher wrote:
> > > This is the first part of a fix for https://fedorahosted.org/sssd/ticket/639
> > > These patches address the various search filters we're using in the
> > > sysdb. Patches for LDAP search filters will be coming later. I wanted to
> > > get these reviewed first.
>
> Patch 0001: Add utility function to sanitize LDAP/LDB filters. Contains
> a unit test.
>
> Patch 0002: Add sysdb utility function for sanitizing DN. No unit test,
> since we're relying on an opaque function from libldb.
>
> Patch 0003: Use sss_filter_sanitize() for all ldb searches internal to
> the sysdb
>
> Patch 0004: Use sss_filter_sanitize() for all ldb searches in the IPA
> provider
>
> Patch 0005: Use sss_filter_sanitize() for all ldb searches in the LDAP
> provider
>
> Patch 0006: Use sysdb_dn_sanitize() in the DN helpers
>
> Patch 0007: Sanitize search filters in the memberOf plugin
>
> Patch 0008: Sanitize a DN lookup for memberOf in initgroups processing
>
> Patch 0009: Add unit tests for users/groups/netgroups with odd characters

I have only minor comment, please find them below ...

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/
>
> iEYEARECAAYFAkzUJFcACgkQeiVVYja6o6OqzACePcPef36XeMXYojTwqKAnnBKj
> dFQAnR87P/W/wCoaMk9C4RtKiyE0vxuS
> =ndLg
> -----END PGP SIGNATURE-----
> diff --git a/src/util/util.c b/src/util/util.c
> index 06eea283770601997e3ff90df73db58a4fb25bba..20b8c78a72611335be77d36e7b3dc420f26989d2 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -515,3 +515,55 @@ errno_t sss_hash_create(TALLOC_CTX *mem_ctx,

...
> }
> output[j] = '\0';
> *sanitized = talloc_realloc(mem_ctx, output, char, strlen(output)+1);

I think it is not necessary to call talloc_realloc() and if you really
want to call it you can use j instead of strlen(output).

...
> From 1516feab46b282ada2f3ed1d808f259474b942dc Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher 
> 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 |   17 +++++++++++++++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
> index 979959fb12f6383edd86ce2405de217462b9a5a6..c22f75efb0509c11f0073b6a5925730c1e42ee8d 100644
> --- a/src/providers/ipa/ipa_access.c
> +++ b/src/providers/ipa/ipa_access.c
> @@ -1084,12 +1092,17 @@ 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_free(host_dn_clean) ?

...
> diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
> index 6b14161cefb8a272edea34ac117550e5d53139a0..426bdf44e5072c19eb97aee03d3731fbf1dfebc9 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,7 +2663,7 @@ 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_free(member_dn) ?

...
> From 52934f59cdef9a9d90b253e323a2c914e22fb1f7 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher 
> Date: Fri, 5 Nov 2010 11:15:42 -0400
> Subject: [PATCH 6/9] Sanitize sysdb DN helpers
>
> src/db/sysdb.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 90 insertions(+), 7 deletions(-)
>
> diff --git a/src/db/sysdb.c b/src/db/sysdb.c
> index b2691526a4a02874f9784c2c41a63c8be237bc0c..71d4f8490a4cf3d68a85ae0a71836ff8e8d50b1d 100644
> --- a/src/db/sysdb.c
> +++ b/src/db/sysdb.c
> @@ -54,33 +54,116 @@ 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;
> TALLOC_CTX *tmp_ctx;
> char *clean_subtree;
> struct ldb_dn *dn = NULL;
> tmp_ctx = talloc_new(NULL);
> if (!tmp_ctx) {
>    return NULL;
> }
> 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_SUBTREE,
>                    clean_subtree, domain);
>
> +done:
> talloc_free(tmp_ctx);
> 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;
>
> }

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.
> 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 170fba6e14eeea0ff65917a8d7e5b753b260ef73 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher 
> 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 |   20 ++++++++++++++++++--
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
> index 1e28593dd1c8d1522c9aefa1434d544e91412737..e135cf8d0a0144447f91846e8cc134f2e895e81e 100644
> --- a/src/ldb_modules/memberof.c
> +++ b/src/ldb_modules/memberof.c
> @@ -1206,9 +1208,16 @@ 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_free(clean_dn) ?
> @@ -1586,6 +1595,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,9 +1609,15 @@ 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);

talloc_free(clean_dn) ?
>  if (!expression) {
>      return LDB_ERR_OPERATIONS_ERROR;
>  }
>
> -- 
> 1.7.3.2
> From d1644133be265567c81482ae755f6d1a4b7e5260 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher 
> 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 |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
> index 426bdf44e5072c19eb97aee03d3731fbf1dfebc9..6f4532eea31ff4a027c221983bf91ef06ea66f58 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,9 +1968,17 @@ 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);

talloc_free(clean_dn) ?

[Stephen Gallagher]

-----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-----

[Sumit Bose]

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 
> 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 
> 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 
> 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 
> 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 
> 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 
> 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 
> 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 
> 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 
> 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
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/11/2010 04:38 PM, Sumit Bose wrote:
> > Thanks.
> > ACK

I had a one-liner syntax error bug in the "Sanitize sysdb dn helpers"
patch. Correction in attached patch.

No need for re-review.

- -- 
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/
>
> iEYEARECAAYFAkzdU3kACgkQeiVVYja6o6PWTACcDrtWnzqyBxZuBT3ljb73vR1w
> wDcAn1Qb5cj1SA7oQ+uI8ovX/iAwRxXe
> =GLy/
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/12/2010 09:47 AM, Stephen Gallagher wrote:
> On 11/11/2010 04:38 PM, Sumit Bose wrote:
> > > Thanks.
> > > ACK
>
> I had a one-liner syntax error bug in the "Sanitize sysdb dn helpers"
> patch. Correction in attached patch.
>
> No need for re-review.

Note: I sent the wrong patch to this list. Here is the correct one that
I pushed.

Pushed all to master.

- -- 
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/
>
> iEYEARECAAYFAkzhSXwACgkQeiVVYja6o6NqigCfTnMX89wlqtHCLBbpS//7BFra
> KnEAn3qCZaOnE52QSE3OMdyYpNpKpONK
> =tu7P
> -----END PGP SIGNATURE-----