On Wed, May 06, 2015 at 02:26:30PM -0400, Stephen Gallagher wrote:
Patch 0001: LDAP: Support returning referral information
Some callers may be interested in the raw referral values returned from
a lookup. This patch allows interested consumers to get these referrals
back and process them if they wish. It does not implement a generic
automatic following of referrals.
Patch 0002: AD GPO: Support processing referrals
For GPOs assigned to a site, it's possible that their definition
actually exists in another domain. To retrieve this information,
we need to follow the referral and perform a base search on
another domain controller.
Resolves:
https://fedorahosted.org/sssd/ticket/2645
Thanks a lot for the patches! I have one questions about the first patch
and two nitpicks about the second. I'm also still waiting for the
Coverity results, but the queue seems to be quite long..
From 3f8826061d34639ddaaf245947085ea577e77fbe Mon Sep 17 00:00:00
2001
From: Stephen Gallagher <sgallagh(a)redhat.com>
Date: Fri, 1 May 2015 11:42:06 -0400
Subject: [PATCH 1/2] LDAP: Support returning referral information
Some callers may be interested in the raw referral values returned from
a lookup. This patch allows interested consumers to get these referrals
back and process them if they wish. It does not implement a generic
automatic following of referrals.
[...]
} else if (result == LDAP_REFERRAL) {
- if (refs != NULL) {
- for (i = 0; refs[i]; i++) {
- DEBUG(SSSDBG_TRACE_LIBS, "Ref: %s\n", refs[i]);
- }
- ldap_memvfree((void **) refs);
+ ret = sdap_get_generic_ext_add_references(state, refs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "sdap_get_generic_ext_add_references failed: %s(%d)",
+ sss_strerror(ret), ret);
+ tevent_req_error(req, ret);
}
- ldap_memfree(errmsg);
- tevent_req_error(req, ERR_REFERRAL);
- return;
This is a change in behaviour. Previously, we would always return
ERR_REFERRAL and let the caller handle the error code based on whether
the caller ignores referrals (like the AD Provider does by default) or
not.
Do you think it's OK to always treat referrals as a success now, even
for the plain LDAP provider? If so, then I guess we can remove the
ERR_REFERRAL special-case and the error code as well.
+ /* For referrals, we need to fall through as if it was
LDAP_SUCCESS */
} else if (result != LDAP_SUCCESS && result != LDAP_NO_SUCH_OBJECT) {
DEBUG(SSSDBG_OP_FAILURE,
"Unexpected result from ldap: %s(%d), %s\n",
sss_ldap_err2string(result), result,
errmsg ? errmsg : "no errmsg set");
From 9a45e8523540c9eff8a8b9d84da81c9ee77a91de Mon Sep 17 00:00:00
2001
From: Stephen Gallagher <sgallagh(a)redhat.com>
Date: Fri, 1 May 2015 16:26:36 -0400
Subject: [PATCH 2/2] AD GPO: Support processing referrals
For GPOs assigned to a site, it's possible that their definition
actually exists in another domain. To retrieve this information,
we need to follow the referral and perform a base search on
another domain controller.
Resolves:
https://fedorahosted.org/sssd/ticket/2645
[...]
+static void
+ad_gpo_get_sd_referral_conn_done(struct tevent_req *subreq)
+{
+ errno_t ret;
+ int dp_error;
+ const char *attrs[] = AD_GPO_ATTRS;
+
+ struct tevent_req *req =
+ tevent_req_callback_data(subreq, struct tevent_req);
+ struct ad_gpo_get_sd_referral_state *state =
+ tevent_req_data(req, struct ad_gpo_get_sd_referral_state);
+
+ ret = sdap_id_op_connect_recv(subreq, &dp_error);
+ talloc_zfree(subreq);
+ if (ret != EOK) {
+ if (dp_error == DP_ERR_OFFLINE) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Backend is marked offline, retry later!\n");
+ tevent_req_done(req);
+ } else {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cross-realm GPO processing failed to connect to " \
+ "referred LDAP server: (%d)[%s]\n",
+ ret, sss_strerror(ret));
+ tevent_req_error(req, ret);
+ }
+ return;
+ }
+
+ /* Request the referred GPO data */
+ subreq = sdap_sd_search_send(state, state->ev, state->opts,
+ sdap_id_op_handle(state->ref_op),
+ state->gpo_dn,
+ SECINFO_DACL,
+ attrs,
+ state->timeout);
You missed a NULL check here.
+ tevent_req_set_callback(subreq,
ad_gpo_get_sd_referral_search_done, req);
+
+}
+
+static void
+ad_gpo_get_sd_referral_search_done(struct tevent_req *subreq)
+{
+ errno_t ret;
+ int dp_error;
+ size_t num_results, num_refs;
+ struct sysdb_attrs **results = NULL;
+ char **refs;
+ struct tevent_req *req =
+ tevent_req_callback_data(subreq, struct tevent_req);
+ struct ad_gpo_get_sd_referral_state *state =
+ tevent_req_data(req, struct ad_gpo_get_sd_referral_state);
+
+ ret = sdap_sd_search_recv(subreq, NULL,
+ &num_results, &results,
+ &num_refs, &refs);
And a 'ret' check here.
+ talloc_zfree(subreq);
+
+ ret = sdap_id_op_done(state->ref_op, ret, &dp_error);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Unable to get GPO attributes: [%d](%s)\n",
+ ret, sss_strerror(ret));
+ ret = ENOENT;
+ goto done;
+ }
+