[SSSD] [PATCH] AD: use GC for SID requests as well
Sumit Bose
sbose at redhat.com
Fri Jan 30 13:24:34 UTC 2015
On Fri, Jan 30, 2015 at 02:23:43PM +0100, Sumit Bose wrote:
> On Fri, Jan 30, 2015 at 01:27:34PM +0100, Jakub Hrozek wrote:
> > On Wed, Jan 28, 2015 at 08:18:04PM +0100, Sumit Bose wrote:
> > > On Wed, Jan 28, 2015 at 04:21:53PM +0100, Sumit Bose wrote:
> > > > On Wed, Jan 28, 2015 at 03:27:44PM +0100, Jakub Hrozek wrote:
> > > > > On Wed, Jan 28, 2015 at 03:05:00PM +0100, Sumit Bose wrote:
> > > > > > Hi,
> > > > > >
> > > > > > another issue found by Steeve during testing. To reproduce this you need
> > > > > > a universal group with members from different domains. Then either look
> > > > > > up the group by SID e.g. with
> > > > > >
> > > > > > python -c "import pysss_nss_idmap; print pysss_nss_idmap.getnamebysid('S-1-5-21-3456664713-2053453454-4165325232-1234')"
> > > > > >
> > > > > > and then with getent group groupname.
> > > > > >
> > > > > > Or use IPA views, override the group name in the 'default trust view'
> > > > > > on the IPA server and look up the group by the overridden name. In both
> > > > > > case the group should not already be in the cache. Only members from the
> > > > > > domain of the group should be show without the patch.
> > > > > >
> > > > > > bye,
> > > > > > Sumit
> > > > >
> > > > > The patch is correct, but I'm worried about the implications. What kind
> > > > > of requests by SID does the server receive? Do we also resolve requests
> > > > > for users by SID? In that case, we might be surprised that some POSIX
> > > > > attributes are not available in GC..
> > > >
> > > > good point, I'm working on a group only alternative.
> > >
> > > Please find attached a slightly extended version. It tries to switch to
> > > the LDAP connection in the common LDAP code. We already do similar
> > > things during group requests in the common LDAP code so I hope this is
> > > ok. This switch only happens during user_and_group request where it is
> > > not clear if the match will be a user or a group, e.g. a search by SID.
> > > I have to admit that there is a side effect because the AD provider uses
> > > GC lookups with LDAP fallback for these types of requests and will be
> > > affected by this change as well. But since the more detailed user
> > > information will be available via LDAP this might even be a benefit.
> > >
> > > bye,
> > > Sumit
> >
> > [...]
> >
> > > - if (state->sdap_ret == ENOENT) {
> > > + if (state->sdap_ret == ENOENT && state->noexist_delete == true) {
> >
> > I don't think this change is correct, the noexist_delete check should be
> > in its own nested if, otherwise looking up a SID that matches neither user
> > nor group fails with EIO:
>
> ah sorry, of course you are right new version attached.
and now with a patch.
>
> bye,
> Sumit
>
> > Breakpoint 3, get_user_and_group_users_done (subreq=0x18beca0) at src/providers/ldap/ldap_id.c:1799
> > 1799 struct tevent_req *req = tevent_req_callback_data(subreq,
> > (gdb) n
> > 1801 struct get_user_and_group_state *state = tevent_req_data(req,
> > (gdb)
> > 1805 ret = users_get_recv(subreq, &state->dp_error, &state->sdap_ret);
> > (gdb)
> > 1806 talloc_zfree(subreq);
> > (gdb)
> > 1808 if (ret != EOK) {
> > (gdb)
> > 1812 if (state->sdap_ret == ENOENT && state->noexist_delete == true) {
> > (gdb)
> > 1822 } else if (state->sdap_ret != EOK) {
> > (gdb)
> > 1823 tevent_req_error(req, EIO);
> > (gdb)
> > 1824 return;
> > (gdb) p state->sdap_ret
> > $1 = 2
> > (gdb) p state->noexist_delete
> > $2 = false
> > (gdb) quit
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From 44d917806f8b8ce0f2fe536a901ef278aadf40c4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 28 Jan 2015 14:04:45 +0100
Subject: [PATCH] AD: use GC for SID requests as well
If a universal group is looked up by SID the cross-domain members must
be resolved with the help of the Global Catalog.
Related to https://fedorahosted.org/sssd/ticket/2514
---
src/providers/ipa/ipa_subdomains_id.c | 1 +
src/providers/ldap/ldap_id.c | 38 ++++++++++++++++++++++++++---------
2 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c
index c8714a216daff7506f00248e25c281529d0479c4..0508e14b690c144f4bace9ed14a326ac724eb910 100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -603,6 +603,7 @@ ipa_get_ad_acct_send(TALLOC_CTX *mem_ctx,
*/
switch (state->ar->entry_type & BE_REQ_TYPE_MASK) {
case BE_REQ_INITGROUPS:
+ case BE_REQ_BY_SECID:
case BE_REQ_GROUP:
clist = ad_gc_conn_list(req, ad_id_ctx, state->obj_dom);
if (clist == NULL) {
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 2e58f4e49eb33a85cbb8b4144c69004c6b5b312b..5ce462d77867f115fe5c0214fcb95b72a4370472 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -33,6 +33,7 @@
#include "providers/ldap/sdap_async.h"
#include "providers/ldap/sdap_idmap.h"
#include "providers/ldap/sdap_users.h"
+#include "providers/ad/ad_common.h"
/* =Users-Related-Functions-(by-name,by-uid)============================== */
@@ -1745,6 +1746,8 @@ static void get_user_and_group_groups_done(struct tevent_req *subreq)
struct get_user_and_group_state *state = tevent_req_data(req,
struct get_user_and_group_state);
int ret;
+ struct ad_id_ctx *ad_id_ctx;
+ struct sdap_id_conn_ctx *user_conn;
ret = groups_get_recv(subreq, &state->dp_error, &state->sdap_ret);
talloc_zfree(subreq);
@@ -1764,8 +1767,22 @@ static void get_user_and_group_groups_done(struct tevent_req *subreq)
/* Now the search finished fine but did not find an entry.
* Retry with users. */
+
+ user_conn = state->conn;
+ /* Prefer LDAP over GC for users */
+ if (state->id_ctx->opts->schema_type == SDAP_SCHEMA_AD
+ && state->sdom->pvt != NULL) {
+ ad_id_ctx = talloc_get_type(state->sdom->pvt, struct ad_id_ctx);
+ if (ad_id_ctx != NULL && ad_id_ctx->ldap_ctx != NULL
+ && state->conn == ad_id_ctx->gc_ctx) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Switching to LDAP connection for user lookup.\n");
+ user_conn = ad_id_ctx->ldap_ctx;
+ }
+ }
+
subreq = users_get_send(req, state->ev, state->id_ctx,
- state->sdom, state->conn,
+ state->sdom, user_conn,
state->filter_val, state->filter_type, NULL,
state->attrs_type, state->noexist_delete);
if (subreq == NULL) {
@@ -1792,16 +1809,17 @@ static void get_user_and_group_users_done(struct tevent_req *subreq)
tevent_req_error(req, ret);
return;
}
-
if (state->sdap_ret == ENOENT) {
- /* The search ran to completion, but nothing was found.
- * Delete the existing entry, if any. */
- ret = sysdb_delete_by_sid(state->sysdb, state->domain,
- state->filter_val);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "Could not delete entry by SID!\n");
- tevent_req_error(req, ret);
- return;
+ if (state->noexist_delete == true) {
+ /* The search ran to completion, but nothing was found.
+ * Delete the existing entry, if any. */
+ ret = sysdb_delete_by_sid(state->sysdb, state->domain,
+ state->filter_val);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Could not delete entry by SID!\n");
+ tevent_req_error(req, ret);
+ return;
+ }
}
} else if (state->sdap_ret != EOK) {
tevent_req_error(req, EIO);
--
2.1.0
More information about the sssd-devel
mailing list