[SSSD] [PATCHES] Support GPOs referred from other domains

Jakub Hrozek jhrozek at redhat.com
Thu May 14 15:42:21 UTC 2015


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 at 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 at 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;
> +    }
> +



More information about the sssd-devel mailing list