[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