[SSSD] [PATCH] Require existence of GID number in group searches

Sumit Bose sbose at redhat.com
Mon Mar 14 14:29:42 UTC 2011


On Mon, Mar 14, 2011 at 10:00:12AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 03/11/2011 10:51 AM, Sumit Bose wrote:
> > On Fri, Mar 11, 2011 at 05:56:57AM -0500, Stephen Gallagher wrote:
> > This is most commonly seen with ActiveDirectory?. The 'group'
> > objectClass does not have a mandatory GID attribute, and SSSD was
> > throwing errors when trying to process groups without them (which is
> > necessary for use on a POSIX system).
> > 
> > This patch updates the group filters so that we include "gidNumber=*" to
> > filter out groups that are missing this information.
> > 
> >> Although it is quite elegant to let the server do the work I wonder if
> >> we shouldn't read all groups and let sssd do the checks. This way we can
> >> give detailed debug messages and maybe reduce the number of bug reports.
> > 
> >> Maybe we should add the name of the group to this check too and a check
> >> for user id and user name, just to be on the safe side.
> > 
> >> bye,
> >> Sumit
> > 
> > 
> > Fixes https://fedorahosted.org/sssd/ticket/824
> 
> 
> After some discussion in IRC, we decided that the correct behavior is to
> skip groups that are missing mandatory arguments to play better with
> ActiveDirectory. The new version of the patch now checks for both group
> ID and group name.
> 
> Also, I've added a second patch to mandate username, UID and GID for
> users during enumeration as well. There's no need to restrict this for
> non-enumerated lookups, as users are always queried directly via
> username or gid, and would thus benefit from the existing failure
> logging if the requested user was missing attributes.
> 

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/
> 
> iEYEARECAAYFAk1+H2gACgkQeiVVYja6o6NDVACgkjXklSiaA+n8iSobaICZeUNB
> LM8Aniz9qiGSVQUi3wmRnbrSwxyCBDnl
> =Z5WU
> -----END PGP SIGNATURE-----

> From 234a1e598c26a5382b0950872eef55a95e1da1bf Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Fri, 11 Mar 2011 05:06:48 -0500
> Subject: [PATCH 1/2] Require existence of GID number and name in group searches
> 
> https://fedorahosted.org/sssd/ticket/824
> ---
>  src/providers/ldap/ldap_id.c             |    9 ++++++---
>  src/providers/ldap/ldap_id_enum.c        |   28 ++++++++++++++++------------
>  src/providers/ldap/sdap_async_accounts.c |   30 ++++++++++++++++++++----------
>  3 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
> index 9a234280082f7396eda4307e9e4bb4bd63b5615c..776df1ac2d9e983a792fbba0f6773c082898708d 100644
> --- a/src/providers/ldap/ldap_id.c
> +++ b/src/providers/ldap/ldap_id.c
> @@ -335,9 +335,12 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
>          goto fail;
>      }
>  
> -    state->filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))",
> -                                    attr_name, clean_name,
> -                                    ctx->opts->group_map[SDAP_OC_GROUP].name);
> +    state->filter =
> +            talloc_asprintf(state, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
> +                            attr_name, clean_name,
> +                            ctx->opts->group_map[SDAP_OC_GROUP].name,
> +                            ctx->opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                            ctx->opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!state->filter) {
>          DEBUG(2, ("Failed to build filter\n"));
>          ret = ENOMEM;
> diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
> index f47ee9fbe170bae0058a682a3a051df21cfbc0d6..42c2911926602bfc2e3a33a0af837d6e809ee68b 100644
> --- a/src/providers/ldap/ldap_id_enum.c
> +++ b/src/providers/ldap/ldap_id_enum.c
> @@ -546,19 +546,23 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
>      state->op = op;
>  
>      if (ctx->srv_opts && ctx->srv_opts->max_group_value && !purge) {
> -        state->filter = talloc_asprintf(state,
> -                              "(&(%s=*)(objectclass=%s)(%s>=%s)(!(%s=%s)))",
> -                              ctx->opts->group_map[SDAP_AT_GROUP_NAME].name,
> -                              ctx->opts->group_map[SDAP_OC_GROUP].name,
> -                              ctx->opts->group_map[SDAP_AT_GROUP_USN].name,
> -                              ctx->srv_opts->max_group_value,
> -                              ctx->opts->group_map[SDAP_AT_GROUP_USN].name,
> -                              ctx->srv_opts->max_group_value);
> +        state->filter = talloc_asprintf(
> +                state,
> +                "(&(objectclass=%s)(%s=*)(%s=*)(%s>=%s)(!(%s=%s)))",
> +                ctx->opts->group_map[SDAP_OC_GROUP].name,
> +                ctx->opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                ctx->opts->group_map[SDAP_AT_GROUP_GID].name,
> +                ctx->opts->group_map[SDAP_AT_GROUP_USN].name,
> +                ctx->srv_opts->max_group_value,
> +                ctx->opts->group_map[SDAP_AT_GROUP_USN].name,
> +                ctx->srv_opts->max_group_value);
>      } else {
> -        state->filter = talloc_asprintf(state,
> -                              "(&(%s=*)(objectclass=%s))",
> -                              ctx->opts->group_map[SDAP_AT_GROUP_NAME].name,
> -                              ctx->opts->group_map[SDAP_OC_GROUP].name);
> +        state->filter = talloc_asprintf(
> +                state,
> +                "(&(objectclass=%s)(%s=*)(%s=*))",
> +                ctx->opts->group_map[SDAP_OC_GROUP].name,
> +                ctx->opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                ctx->opts->group_map[SDAP_AT_GROUP_GID].name);
>      }
>      if (!state->filter) {
>          DEBUG(2, ("Failed to build filter\n"));
> diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
> index 8e459598674d589c0cdfcece125c183f7c95bb4d..30c7a4986c4014becc97e2d4deb9b3d6eb03091f 100644
> --- a/src/providers/ldap/sdap_async_accounts.c
> +++ b/src/providers/ldap/sdap_async_accounts.c
> @@ -2007,10 +2007,12 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx,
>          return NULL;
>      }
>  
> -    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))",
> +    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
>                               opts->group_map[SDAP_AT_GROUP_MEMBER].name,
>                               clean_name,
> -                             opts->group_map[SDAP_OC_GROUP].name);
> +                             opts->group_map[SDAP_OC_GROUP].name,
> +                             opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                             opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!filter) {
>          talloc_zfree(req);
>          return NULL;
> @@ -2211,8 +2213,10 @@ static struct tevent_req *sdap_initgr_nested_send(TALLOC_CTX *memctx,
>          return NULL;
>      }
>  
> -    state->filter = talloc_asprintf(state, "(objectclass=%s)",
> -                                    opts->group_map[SDAP_OC_GROUP].name);
> +    state->filter = talloc_asprintf(state, "(&(objectclass=%s)(%s=*)(%s=*)",
> +                                    opts->group_map[SDAP_OC_GROUP].name,
> +                                    opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                                    opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!state->filter) {
>          talloc_zfree(req);
>          return NULL;
> @@ -3103,8 +3107,10 @@ static errno_t sdap_nested_group_lookup_group(struct tevent_req *req)
>      }
>  
>      filter = talloc_asprintf(
> -            sdap_attrs, "(objectclass=%s)",
> -            state->opts->group_map[SDAP_OC_GROUP].name);
> +            sdap_attrs, "(&(objectclass=%s)(%s=*)(%s=*))",
> +            state->opts->group_map[SDAP_OC_GROUP].name,
> +            state->opts->group_map[SDAP_AT_GROUP_NAME].name,
> +            state->opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!filter) {
>          talloc_free(sdap_attrs);
>          return ENOMEM;
> @@ -3435,10 +3441,12 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send(
>          return NULL;
>      }
>  
> -    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))",
> +    filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
>                               opts->group_map[SDAP_AT_GROUP_MEMBER].name,
>                               clean_orig_dn,
> -                             opts->group_map[SDAP_OC_GROUP].name);
> +                             opts->group_map[SDAP_OC_GROUP].name,
> +                             opts->group_map[SDAP_AT_GROUP_NAME].name,
> +                             opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!filter) {
>          talloc_zfree(req);
>          return NULL;
> @@ -3839,10 +3847,12 @@ static errno_t rfc2307bis_nested_groups_step(struct tevent_req *req)
>      }
>  
>      filter = talloc_asprintf(
> -            tmp_ctx, "(&(%s=%s)(objectclass=%s))",
> +            tmp_ctx, "(&(%s=%s)(objectclass=%s)(%s=*)(%s=*))",
>              state->opts->group_map[SDAP_AT_GROUP_MEMBER].name,
>              clean_orig_dn,
> -            state->opts->group_map[SDAP_OC_GROUP].name);
> +            state->opts->group_map[SDAP_OC_GROUP].name,
> +            state->opts->group_map[SDAP_AT_GROUP_NAME].name,
> +            state->opts->group_map[SDAP_AT_GROUP_GID].name);
>      if (!filter) {
>          ret = ENOMEM;
>          goto error;
> -- 
> 1.7.4
> 

> From f7f797f29e2db2be83f9b448fa481046d68f5686 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Mon, 14 Mar 2011 09:56:22 -0400
> Subject: [PATCH 2/2] Require existence of username, uid and gid for user enumeration
> 
> We will ignore users that do not have these three values.
> ---
>  src/providers/ldap/ldap_id_enum.c |   30 ++++++++++++++++++------------
>  1 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
> index 42c2911926602bfc2e3a33a0af837d6e809ee68b..6899b87c08b46c3c2b61fcd975ab14a4118cc918 100644
> --- a/src/providers/ldap/ldap_id_enum.c
> +++ b/src/providers/ldap/ldap_id_enum.c
> @@ -441,19 +441,25 @@ static struct tevent_req *enum_users_send(TALLOC_CTX *memctx,
>      state->op = op;
>  
>      if (ctx->srv_opts && ctx->srv_opts->max_user_value && !purge) {
> -        state->filter = talloc_asprintf(state,
> -                               "(&(%s=*)(objectclass=%s)(%s>=%s)(!(%s=%s)))",
> -                               ctx->opts->user_map[SDAP_AT_USER_NAME].name,
> -                               ctx->opts->user_map[SDAP_OC_USER].name,
> -                               ctx->opts->user_map[SDAP_AT_USER_USN].name,
> -                               ctx->srv_opts->max_user_value,
> -                               ctx->opts->user_map[SDAP_AT_USER_USN].name,
> -                               ctx->srv_opts->max_user_value);
> +        state->filter = talloc_asprintf(
> +                state,
> +                "(&(objectclass=%s)(%s=*)(%s=*)(%s=*)(%s>=%s)(!(%s=%s)))",
> +                ctx->opts->user_map[SDAP_OC_USER].name,
> +                ctx->opts->user_map[SDAP_AT_USER_NAME].name,
> +                ctx->opts->user_map[SDAP_AT_USER_UID].name,
> +                ctx->opts->user_map[SDAP_AT_USER_GID].name,
> +                ctx->opts->user_map[SDAP_AT_USER_USN].name,
> +                ctx->srv_opts->max_user_value,
> +                ctx->opts->user_map[SDAP_AT_USER_USN].name,
> +                ctx->srv_opts->max_user_value);
>      } else {
> -        state->filter = talloc_asprintf(state,
> -                               "(&(%s=*)(objectclass=%s))",
> -                               ctx->opts->user_map[SDAP_AT_USER_NAME].name,
> -                               ctx->opts->user_map[SDAP_OC_USER].name);
> +        state->filter = talloc_asprintf(
> +                state,
> +                "(&(objectclass=%s)(%s=*)(%s=*)(%s=*))",
> +                ctx->opts->user_map[SDAP_OC_USER].name,
> +                ctx->opts->user_map[SDAP_AT_USER_NAME].name,
> +                ctx->opts->user_map[SDAP_AT_USER_UID].name,
> +                ctx->opts->user_map[SDAP_AT_USER_GID].name);
>      }
>      if (!state->filter) {
>          DEBUG(2, ("Failed to build filter\n"));
> -- 
> 1.7.4
> 



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