Patch 0001: AD: Clean up ad_access_gpo Just a minor cleanup to ad_gpo_access_send to adhere to our tevent conventions. This is purely for aesthetic and maintainability reasons; it has no functional effect.
Patch 0002: AD: Always get domain-specific ID connection This one is a little tricky. It turns out that in some circumstances, ad_ctx->ldap_ctx may actually be pointing at a subdomain rather than the enrolled domain. I don't know the reasons for this (and it appears to be a race-condition, because I could only get it to happen if I was quick to test logins right after restarting SSSD). However, the fix is fairly straightforward: sdap_domain_get()->pvt->ldap_ctx always provides the real ldap_ctx for the requested domain (either the enrolled domain or any of the trusted domains). The IS_SUBDOMAIN() check and shortcut to ad_ctx->ldap_ctx was unnecessary and (thanks to the odd race) incorrect. This patch removes this conditional shortcut and forces us to get the correct ldap_ctx. This proved to be the last piece necessary to get Patch 0003 to work. Patch 0003: AD GPO: Always look up GPOs from machine domain When dealing with users from a child domain, SSSD was attempting to use the subdomain for lookups. However, all GPOs applicable to this machine are stored in the primary domain (the domain the host directly joined). This patch has the GPO processing use the primary domain instead of the user domain. Resolves: https://fedorahosted.org/sssd/ticket/2606
From 39a0dc5dd670cb251e3c9a3b35aca9dbb2ede061 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Tue, 14 Apr 2015 13:07:36 -0400 Subject: [PATCH 1/3] AD: Clean up ad_access_gpo Align goto usage with conventions in the rest of the source. --- src/providers/ad/ad_gpo.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index a881741a6ead9244ac123608234d1a0c35f830e3..54e5545a57b7e697f730431ae35a95ccabbe21db 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1532,12 +1532,10 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, "service %s maps to %s\n", service, gpo_map_type_string(gpo_map_type)); if (gpo_map_type == GPO_MAP_PERMIT) { ret = EOK; - tevent_req_done(req); - tevent_req_post(req, ev); goto immediately; } if (gpo_map_type == GPO_MAP_DENY) { switch (ctx->gpo_access_control_mode) { @@ -1549,12 +1547,10 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, sss_log_ext(SSS_LOG_WARNING, LOG_AUTHPRIV, "Warning: user would " \ "have been denied GPO-based logon access if the " \ "ad_gpo_access_control option were set to enforcing " \ "mode."); ret = EOK; - tevent_req_done(req); - tevent_req_post(req, ev); goto immediately; default: ret = EINVAL; goto immediately; } @@ -1590,19 +1586,21 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, ret, sss_strerror(ret)); goto immediately; } tevent_req_set_callback(subreq, ad_gpo_connect_done, req); - ret = EOK; + return req; immediately: - if (ret != EOK) { + if (ret == EOK) { + tevent_req_done(req); + } else { tevent_req_error(req, ret); - tevent_req_post(req, ev); } + tevent_req_post(req, ev); return req; } static errno_t process_offline_gpos(TALLOC_CTX *mem_ctx, -- 2.3.5
From 5e57bf4e92fd898a1879dc773c7a380b1f96b7ad Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Tue, 14 Apr 2015 21:50:36 -0400 Subject: [PATCH 2/3] AD: Always get domain-specific ID connection ad_get_dom_ldap_conn() assumed that ad_ctx->ldap_ctx always points at the LDAP connection for the primary domain, however it turns out that this is not always the case. It's currently unclear why, but this connection can sometimes be pointing at a subdomain. Since the value of subdom_id_ctx->ldap_ctx always points to the correct domain (including the primary domain case), there's no benefit to trying to shortcut to the ad_ctx->ldap_ctx when performing this lookup. This patch also makes a minor tweak to the tests so that the primary domain passes the sdap_domain_get() check for validity (since it needs to have a private member assigned). --- src/providers/ad/ad_common.c | 18 +++++++----------- src/tests/cmocka/test_ad_common.c | 1 + 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c index 120878977d08aab04bbd9e3cf87a00a4b018b6e4..5eeb8dd74d1df89a1a0afa50560b8341b0088778 100644 --- a/src/providers/ad/ad_common.c +++ b/src/providers/ad/ad_common.c @@ -1138,22 +1138,18 @@ ad_get_dom_ldap_conn(struct ad_id_ctx *ad_ctx, struct sss_domain_info *dom) { struct sdap_id_conn_ctx *conn; struct sdap_domain *sdom; struct ad_id_ctx *subdom_id_ctx; - if (IS_SUBDOMAIN(dom)) { - sdom = sdap_domain_get(ad_ctx->sdap_id_ctx->opts, dom); - if (sdom == NULL || sdom->pvt == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "No ID ctx available for [%s].\n", - dom->name); - return NULL; - } - subdom_id_ctx = talloc_get_type(sdom->pvt, struct ad_id_ctx); - conn = subdom_id_ctx->ldap_ctx; - } else { - conn = ad_ctx->ldap_ctx; + sdom = sdap_domain_get(ad_ctx->sdap_id_ctx->opts, dom); + if (sdom == NULL || sdom->pvt == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "No ID ctx available for [%s].\n", + dom->name); + return NULL; } + subdom_id_ctx = talloc_get_type(sdom->pvt, struct ad_id_ctx); + conn = subdom_id_ctx->ldap_ctx; return conn; } struct sdap_id_conn_ctx ** diff --git a/src/tests/cmocka/test_ad_common.c b/src/tests/cmocka/test_ad_common.c index 19a4d395ba3fc4eae6601b3ad7056c41384a5c4f..1c44bc34b9350c4c7bca1dfb3fedd3184d7f14f2 100644 --- a/src/tests/cmocka/test_ad_common.c +++ b/src/tests/cmocka/test_ad_common.c @@ -92,10 +92,11 @@ ad_common_test_setup(void **state) struct sdap_options); assert_non_null(ad_ctx->sdap_id_ctx->opts); ret = sdap_domain_add(ad_ctx->sdap_id_ctx->opts, test_ctx->dom, &sdom); assert_int_equal(ret, EOK); + sdom->pvt = ad_ctx; subdom_ad_ctx = talloc_zero(test_ctx, struct ad_id_ctx); assert_non_null(subdom_ad_ctx); subdom_ldap_ctx = talloc_zero(subdom_ad_ctx, struct sdap_id_conn_ctx); -- 2.3.5
From a3811325ff351520528ed01693ebba0481feab6a Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Fri, 10 Apr 2015 16:34:37 -0400 Subject: [PATCH 3/3] AD GPO: Always look up GPOs from machine domain When dealing with users from a child domain, SSSD was attempting to use the subdomain for lookups. However, all GPOs applicable to this machine are stored in the primary domain (the domain the host directly joined). This patch has the GPO processing use the primary domain instead of the user domain. Resolves: https://fedorahosted.org/sssd/ticket/2606 --- src/providers/ad/ad_gpo.c | 54 +++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 54e5545a57b7e697f730431ae35a95ccabbe21db..990acf94ae6d8fbd8f0e512354d22e1d0a71c292 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1399,11 +1399,12 @@ parse_policy_setting_value(TALLOC_CTX *mem_ctx, static errno_t ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx, enum gpo_access_control_mode gpo_mode, enum gpo_map_type gpo_map_type, const char *user, - struct sss_domain_info *domain) + struct sss_domain_info *user_domain, + struct sss_domain_info *host_domain) { int ret; const char *allow_key = NULL; char **allow_sids; int allow_size ; @@ -1414,33 +1415,34 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx, allow_key = gpo_map_option_entries[gpo_map_type].allow_key; DEBUG(SSSDBG_TRACE_ALL, "allow_key: %s\n", allow_key); deny_key = gpo_map_option_entries[gpo_map_type].deny_key; DEBUG(SSSDBG_TRACE_ALL, "deny_key: %s\n", deny_key); - ret = parse_policy_setting_value(mem_ctx, domain, allow_key, + ret = parse_policy_setting_value(mem_ctx, host_domain, allow_key, &allow_sids, &allow_size); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "parse_policy_setting_value failed for key %s: [%d](%s)\n", allow_key, ret, sss_strerror(ret)); ret = EINVAL; goto done; } - ret = parse_policy_setting_value(mem_ctx, domain, deny_key, + ret = parse_policy_setting_value(mem_ctx, host_domain, deny_key, &deny_sids, &deny_size); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "parse_policy_setting_value failed for key %s: [%d](%s)\n", deny_key, ret, sss_strerror(ret)); ret = EINVAL; goto done; } /* perform access check with the final resultant allow_sids and deny_sids */ - ret = ad_gpo_access_check(mem_ctx, gpo_mode, gpo_map_type, user, domain, - allow_sids, allow_size, deny_sids, deny_size); + ret = ad_gpo_access_check(mem_ctx, gpo_mode, gpo_map_type, user, + user_domain, allow_sids, allow_size, deny_sids, + deny_size); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "GPO access check failed: [%d](%s)\n", ret, strerror(ret)); @@ -1461,11 +1463,12 @@ struct ad_gpo_access_state { struct sdap_id_conn_ctx *conn; struct sdap_id_op *sdap_op; char *server_hostname; struct sdap_options *opts; int timeout; - struct sss_domain_info *domain; + struct sss_domain_info *user_domain; + struct sss_domain_info *host_domain; const char *user; int gpo_timeout_option; const char *ad_hostname; const char *target_dn; struct gp_gpo **dacl_filtered_gpos; @@ -1554,26 +1557,31 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, ret = EINVAL; goto immediately; } } + /* GPO Operations all happen against the enrolled domain, + * not the user's domain (which may be a trusted realm) + */ + state->user_domain = domain; + state->host_domain = get_domains_head(domain); + state->gpo_map_type = gpo_map_type; - state->domain = domain; state->dacl_filtered_gpos = NULL; state->num_dacl_filtered_gpos = 0; state->cse_filtered_gpos = NULL; state->num_cse_filtered_gpos = 0; state->cse_gpo_index = 0; state->ev = ev; state->user = user; - state->ldb_ctx = sysdb_ctx_get_ldb(domain->sysdb); + state->ldb_ctx = sysdb_ctx_get_ldb(state->host_domain->sysdb); state->gpo_mode = ctx->gpo_access_control_mode; state->gpo_timeout_option = ctx->gpo_cache_timeout; state->ad_hostname = dp_opt_get_string(ctx->ad_options, AD_HOSTNAME); state->opts = ctx->sdap_access_ctx->id_ctx->opts; state->timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT); - state->conn = ad_get_dom_ldap_conn(ctx->ad_id_ctx, domain); + state->conn = ad_get_dom_ldap_conn(ctx->ad_id_ctx, state->host_domain); state->sdap_op = sdap_id_op_create(state, state->conn->conn_cache); if (state->sdap_op == NULL) { DEBUG(SSSDBG_OP_FAILURE, "sdap_id_op_create failed.\n"); ret = ENOMEM; goto immediately; @@ -1604,21 +1612,23 @@ immediately: static errno_t process_offline_gpos(TALLOC_CTX *mem_ctx, const char *user, enum gpo_access_control_mode gpo_mode, - struct sss_domain_info *domain, + struct sss_domain_info *user_domain, + struct sss_domain_info *host_domain, enum gpo_map_type gpo_map_type) { errno_t ret; ret = ad_gpo_perform_hbac_processing(mem_ctx, gpo_mode, gpo_map_type, user, - domain); + user_domain, + host_domain); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "HBAC processing failed: [%d](%s}\n", ret, sss_strerror(ret)); goto done; } @@ -1660,11 +1670,12 @@ ad_gpo_connect_done(struct tevent_req *subreq) } else { DEBUG(SSSDBG_TRACE_FUNC, "Preparing for offline operation.\n"); ret = process_offline_gpos(state, state->user, state->gpo_mode, - state->domain, + state->user_domain, + state->host_domain, state->gpo_map_type); if (ret == EOK) { DEBUG(SSSDBG_TRACE_FUNC, "process_offline_gpos succeeded\n"); tevent_req_done(req); @@ -1712,15 +1723,15 @@ ad_gpo_connect_done(struct tevent_req *subreq) } DEBUG(SSSDBG_TRACE_FUNC, "sam_account_name is %s\n", sam_account_name); /* Convert the domain name into domain DN */ - ret = domain_to_basedn(state, state->domain->name, &domain_dn); + ret = domain_to_basedn(state, state->host_domain->name, &domain_dn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot convert domain name [%s] to base DN [%d]: %s\n", - state->domain->name, ret, sss_strerror(ret)); + state->host_domain->name, ret, sss_strerror(ret)); goto done; } /* SDAP_OC_USER objectclass covers both users and computers */ filter = talloc_asprintf(state, @@ -1835,11 +1846,11 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) state->ldb_ctx, state->sdap_op, state->opts, state->timeout, state->target_dn, - state->domain->name); + state->host_domain->name); if (subreq == NULL) { ret = ENOMEM; goto done; } @@ -1937,11 +1948,11 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) ret, sss_strerror(ret)); ret = ENOENT; goto done; } - ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->domain, + ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->user_domain, state->opts->idmap_ctx->map, candidate_gpos, num_candidate_gpos, &state->dacl_filtered_gpos, &state->num_dacl_filtered_gpos); if (ret != EOK) { @@ -2012,11 +2023,11 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) * before we start processing each gpo, we delete the GPO Result object * from the sysdb cache so that any previous policy settings are cleared; * subsequent functions will add the GPO Result object (and populate it * with resultant policy settings) for this policy application */ - ret = sysdb_gpo_delete_gpo_result_object(state, state->domain); + ret = sysdb_gpo_delete_gpo_result_object(state, state->host_domain); if (ret != EOK) { switch (ret) { case ENOENT: DEBUG(SSSDBG_TRACE_FUNC, "No GPO Result available in cache\n"); break; @@ -2083,11 +2094,11 @@ ad_gpo_cse_step(struct tevent_req *req) /* retrieve gpo cache entry; set cached_gpt_version to -1 if unavailable */ DEBUG(SSSDBG_TRACE_FUNC, "retrieving GPO from cache [%s]\n", cse_filtered_gpo->gpo_guid); ret = sysdb_gpo_get_gpo_by_guid(state, - state->domain, + state->host_domain, cse_filtered_gpo->gpo_guid, &res); if (ret == EOK) { /* * Note: if the timeout is valid, then we can later avoid downloading @@ -2125,11 +2136,11 @@ ad_gpo_cse_step(struct tevent_req *req) cse_filtered_gpo->send_to_child = send_to_child; subreq = ad_gpo_process_cse_send(state, state->ev, send_to_child, - state->domain, + state->host_domain, cse_filtered_gpo->gpo_guid, cse_filtered_gpo->smb_server, cse_filtered_gpo->smb_share, cse_filtered_gpo->smb_path, GP_EXT_GUID_SECURITY_SUFFIX, @@ -2178,11 +2189,11 @@ ad_gpo_cse_done(struct tevent_req *subreq) /* * now that the policy file for this gpo have been downloaded to the * GPO CACHE, we store all of the supported keys present in the file * (as part of the GPO Result object in the sysdb cache). */ - ret = ad_gpo_store_policy_settings(state->domain, + ret = ad_gpo_store_policy_settings(state->host_domain, cse_filtered_gpo->policy_filename); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "ad_gpo_store_policy_settings failed: [%d](%s)\n", ret, sss_strerror(ret)); @@ -2196,11 +2207,12 @@ ad_gpo_cse_done(struct tevent_req *subreq) /* ret is EOK only after all GPO policy files have been downloaded */ ret = ad_gpo_perform_hbac_processing(state, state->gpo_mode, state->gpo_map_type, state->user, - state->domain); + state->user_domain, + state->host_domain); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "HBAC processing failed: [%d](%s}\n", ret, sss_strerror(ret)); goto done; } -- 2.3.5
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
