-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 09/13/2010 04:36 PM, Simo Sorce wrote: > On Mon, 13 Sep 2010 15:43:33 -0400 > Stephen Gallagher <[email protected]> wrote: > >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> On 09/13/2010 03:41 PM, Simo Sorce wrote: >>> On Mon, 13 Sep 2010 15:09:28 -0400 >>> Stephen Gallagher <[email protected]> wrote: >>> >>>> Sorry, noticed a tiny oversight on patch 0001. I had only fixed the >>>> DEBUG message for the add case, and missed the del case. The new >>>> patch 0001 corrects this. >>>> >>> >>> Aren't those bugs a separate issue worth their own patch ? >>> (there is also no mention of this issue in the commit message >>> AFAICS) >>> >>> Simo. >>> >> >> I can split them out. I only included them because I needed to fix >> them to help identify the real problem. > > They are a potential crash bug, so it would be better if we separated > them as they are a fix on their own. > > If you split them you get as bonus an automatic ack for all three :) > > Simo. >
Split and attached for posterity. Thanks for the 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.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkyPZZMACgkQeiVVYja6o6PbKACfc31Om0Z5fAz11kyb9gjtwMjC FrwAoI1YFW2JiGdmpKF45GQKc2wgBvOr =p6um -----END PGP SIGNATURE-----
From 95e21e11bfc73a07fd0b700607868f89074b4248 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Mon, 13 Sep 2010 11:45:42 -0400 Subject: [PATCH 1/3] Request all group attributes during initgroups processing We tried to be too clever and only requested the name of the group, but we require the objectClass to validate the results. https://fedorahosted.org/sssd/ticket/622 --- src/providers/ldap/ldap_id.c | 1 + src/providers/ldap/sdap_async_accounts.c | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index d52dcec5b081ccca44e3995a3f7672390df33e5d..0c90773a50fc8a2dbb1ed9ddb58c26b2d8291360 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -619,6 +619,7 @@ static void groups_by_user_done(struct tevent_req *subreq) return; } + state->dp_error = DP_ERR_OK; tevent_req_done(req); } diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c index 8999ba015fc8bf7c7884245cd055b10033b331f0..4db4a4ccd53a05670b63568ef504969a0d04a80a 100644 --- a/src/providers/ldap/sdap_async_accounts.c +++ b/src/providers/ldap/sdap_async_accounts.c @@ -1042,7 +1042,8 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx, struct tevent_req *req, *subreq; struct sdap_initgr_rfc2307_state *state; const char *filter; - const char *attrs[2]; + const char **attrs; + errno_t ret; req = tevent_req_create(memctx, &state, struct sdap_initgr_rfc2307_state); if (!req) return NULL; @@ -1059,12 +1060,12 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx, return NULL; } - attrs[0] = talloc_strdup(state, opts->group_map[SDAP_AT_GROUP_NAME].name); - if (!attrs[0]) { - talloc_zfree(req); + ret = build_attrs_from_map(state, opts->group_map, + SDAP_OPTS_GROUP, &attrs); + if (ret != EOK) { + talloc_free(req); return NULL; } - attrs[1] = NULL; filter = talloc_asprintf(state, "(&(%s=%s)(objectclass=%s))", opts->group_map[SDAP_AT_GROUP_MEMBER].name, -- 1.7.2.2
From 3652c365be56f0d54d92fd67638bdac6377a75fd Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Tue, 14 Sep 2010 08:05:18 -0400 Subject: [PATCH 2/3] Fix missing variable substitution in DEBUG message --- src/db/sysdb_ops.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 017f8ebce9fefa4a747bf502c8c7f6ba61193ff8..d20375c32224f983b7fce4602c30f4e7aed4c4a4 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -2227,7 +2227,7 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb, add_groups[i], user); if (ret != EOK) { DEBUG(1, ("Could not add user [%s] to group [%s]. " - "Skipping.\n")); + "Skipping.\n", user, add_groups[i])); /* Continue on, we should try to finish the rest */ } } @@ -2240,7 +2240,7 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb, del_groups[i], user); if (ret != EOK) { DEBUG(1, ("Could not remove user [%s] from group [%s]. " - "Skipping\n")); + "Skipping\n", user, del_groups[i])); /* Continue on, we should try to finish the rest */ } } -- 1.7.2.2
From f1099285cfc859f0b995ad0ed6626caf54d1c272 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Tue, 14 Sep 2010 08:06:31 -0400 Subject: [PATCH 3/3] Initgroups on a non-cached user should go to the data provider We were accidentally returning an error when sysdb_getpwnam() returned zero results internally in sysdb_initgroups(). The correct behavior here is to return EOK and a result object with zero entries. --- src/db/sysdb_search.c | 12 +++++++++++- src/responder/nss/nsssrv_cmd.c | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 6029b99d810835d874d69bc5a60cad6529ce93e1..a24ea5b17103e1d190ec0a6d764fe378ed5b31af 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -383,10 +383,20 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx, ret = sysdb_getpwnam(tmpctx, ctx, domain, name, &res); if (ret != EOK) { + DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n", + ret, strerror(ret))); goto done; } - if (res->count != 1) { + + if (res->count == 0) { + /* User is not cached yet */ + *_res = talloc_steal(mem_ctx, res); + ret = EOK; + goto done; + + } else if (res->count != 1) { ret = EIO; + DEBUG(1, ("sysdb_getpwnam returned count: [%d]\n", res->count)); goto done; } diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 6df705fb6354dca0fe48c098264a6270d6160321..c3f35e13a48c9c00be9a6e5bde7968445c37e67f 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -2895,7 +2895,8 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx) ret = sysdb_initgroups(cmdctx, sysdb, dom, name, &dctx->res); if (ret != EOK) { - DEBUG(1, ("Failed to make request to our cache!\n")); + DEBUG(1, ("Failed to make request to our cache! [%d][%s]\n", + ret, strerror(ret))); return EIO; } -- 1.7.2.2
0001-Request-all-group-attributes-during-initgroups-proce.patch.sig
Description: PGP signature
0002-Fix-missing-variable-substitution-in-DEBUG-message.patch.sig
Description: PGP signature
0003-Initgroups-on-a-non-cached-user-should-go-to-the-dat.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
