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 <sgall...@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 <sgall...@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@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel