Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP5:GA
sssd.33609
0058-ad-gpo-use-hash-to-store-intermediate-resu...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0058-ad-gpo-use-hash-to-store-intermediate-results.patch of Package sssd.33609
From 1fdb83a832bac5c5d24fd79b6c8b548c52d4a0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek@redhat.com> Date: Wed, 11 Apr 2018 18:56:53 +0200 Subject: [PATCH 1/3] GPO: Fix bug with empty GPO rules When two or more GPO rules were defined on the server and one of them contained no SIDs (no users or groups were specified), then SSSD failed to store such rule and users were denied access (system error). This patch changes the behavior so that in case there are no SIDs in the rule a special value is stored with the rule to indicate that the rule was actually specified, but this value will not match any real SID (because the rule should be empty). Resolves: https://pagure.io/SSSD/sssd/issue/3680 Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit e6e5fe349aa6ed85eb9acb3273007fa90ee99450) --- src/providers/ad/ad_gpo.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index ed519c486..a2f9ce9cb 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1155,6 +1155,7 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, int i; char *allow_value = NULL; char *deny_value = NULL; + const char *empty_val = "NO_SID"; const char *allow_key = NULL; const char *deny_key = NULL; TALLOC_CTX *tmp_ctx = NULL; @@ -1259,7 +1260,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, } for (i = 0; i < GPO_MAP_NUM_OPTS; i++) { - + /* The NO_SID val is used as special SID value for the case when + * no SIDs are found in the rule, but we need to store some + * value (SID) with the key (rule name) so that it is clear + * that the rule is defined on the server. */ struct gpo_map_option_entry entry = gpo_map_option_entries[i]; allow_key = entry.allow_key; @@ -1275,9 +1279,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, allow_key, ret, sss_strerror(ret)); goto done; } else if (ret != ENOENT) { + const char *value = allow_value ? allow_value : empty_val; ret = sysdb_gpo_store_gpo_result_setting(domain, allow_key, - allow_value); + value); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_gpo_store_gpo_result_setting failed for key:" @@ -1301,9 +1306,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, deny_key, ret, sss_strerror(ret)); goto done; } else if (ret != ENOENT) { + const char *value = deny_value ? deny_value : empty_val; ret = sysdb_gpo_store_gpo_result_setting(domain, deny_key, - deny_value); + value); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_gpo_store_gpo_result_setting failed for key:" -- 2.44.0 From 1e87eb9f95ff4c9973054c0d4dd6e7df5052d19d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com> Date: Thu, 9 Jul 2020 12:49:24 +0200 Subject: [PATCH 2/3] gpo: use correct base dn Domain name in SSSD configuration does not have to be the same as the AD domain. GPO did not work in this case. Steps to reproduce: 1. Join SSSD to an AD domain (ad.vm) 2. Create GPO that is applicable to the host/user 3. Name the SSSD domain differently ([domain/AD]) 4. Try to authenticate as AD user Resolves: https://github.com/SSSD/sssd/issues/4840 Reviewed-by: Pawel Polawski <ppolawsk@redhat.com> (cherry picked from commit a0792b32fce7a029768be8e0fc78741218564cd8) --- src/providers/ad/ad_gpo.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index a2f9ce9cb..bc889fb25 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1608,6 +1608,7 @@ struct ad_gpo_access_state { struct gp_gpo **cse_filtered_gpos; int num_cse_filtered_gpos; int cse_gpo_index; + const char *ad_domain; }; static void ad_gpo_connect_done(struct tevent_req *subreq); @@ -1697,6 +1698,8 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, */ state->user_domain = domain; state->host_domain = get_domains_head(domain); + state->ad_domain = dp_opt_get_string(ctx->ad_id_ctx->ad_options->basic, + AD_DOMAIN); state->gpo_map_type = gpo_map_type; state->dacl_filtered_gpos = NULL; @@ -1721,6 +1724,7 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, goto immediately; } + subreq = sdap_id_op_connect_send(state->sdap_op, state, &ret); if (subreq == NULL) { DEBUG(SSSDBG_OP_FAILURE, @@ -1861,11 +1865,11 @@ 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->host_domain->name, &domain_dn); + ret = domain_to_basedn(state, state->ad_domain, &domain_dn); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot convert domain name [%s] to base DN [%d]: %s\n", - state->host_domain->name, ret, sss_strerror(ret)); + state->ad_domain, ret, sss_strerror(ret)); goto done; } @@ -2052,7 +2056,7 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) state->opts, state->timeout, state->target_dn, - state->host_domain->name); + state->ad_domain); if (subreq == NULL) { ret = ENOMEM; goto done; @@ -2171,7 +2175,7 @@ static void ad_gpo_get_host_sid_retrieval_done(struct tevent_req *subreq) state->opts, state->timeout, state->target_dn, - state->host_domain->name); + state->ad_domain); if (subreq == NULL) { ret = ENOMEM; goto done; -- 2.44.0 From 585566839804cf5e02dcc0651cfc26ef64acca1f Mon Sep 17 00:00:00 2001 From: Sumit Bose <sbose@redhat.com> Date: Wed, 8 Nov 2023 14:50:24 +0100 Subject: [PATCH 3/3] ad-gpo: use hash to store intermediate results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently after the evaluation of a single GPO file the intermediate results are stored in the cache and this cache entry is updated until all applicable GPO files are evaluated. Finally the data in the cache is used to make the decision of access is granted or rejected. If there are two or more access-control request running in parallel one request might overwrite the cache object with intermediate data while another request reads the cached data for the access decision and as a result will do this decision based on intermediate data. To avoid this the intermediate results are not stored in the cache anymore but in hash tables which are specific to the request. Only the final result is written to the cache to have it available for offline authentication. Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com> (cherry picked from commit d7db7971682da2dbf7642ac94940d6b0577ec35a) (cherry picked from commit f4ebe1408e0bc67abfbfb5f0ca2ea13803b36726) --- src/providers/ad/ad_gpo.c | 116 +++++++++++++++++++++++++++++++++----- 1 file changed, 102 insertions(+), 14 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index bc889fb25..fc16b47da 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1140,6 +1140,33 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, return ret; } +static errno_t +add_result_to_hash(hash_table_t *hash, const char *key, char *value) +{ + int hret; + hash_key_t k; + hash_value_t v; + + if (hash == NULL || key == NULL || value == NULL) { + return EINVAL; + } + + k.type = HASH_KEY_CONST_STRING; + k.c_str = key; + + v.type = HASH_VALUE_PTR; + v.ptr = value; + + hret = hash_enter(hash, &k, &v); + if (hret != HASH_SUCCESS) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to add [%s][%s] to hash: [%s].\n", + key, value, hash_error_string(hret)); + return EIO; + } + + return EOK; +} + /* * This function parses the cse-specific (GP_EXT_GUID_SECURITY) filename, * and stores the allow_key and deny_key of all of the gpo_map_types present @@ -1147,6 +1174,7 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, */ static errno_t ad_gpo_store_policy_settings(struct sss_domain_info *domain, + hash_table_t *allow_maps, hash_table_t *deny_maps, const char *filename) { struct ini_cfgfile *file_ctx = NULL; @@ -1280,14 +1308,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, goto done; } else if (ret != ENOENT) { const char *value = allow_value ? allow_value : empty_val; - ret = sysdb_gpo_store_gpo_result_setting(domain, - allow_key, - value); + ret = add_result_to_hash(allow_maps, allow_key, + talloc_strdup(allow_maps, value)); if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "sysdb_gpo_store_gpo_result_setting failed for key:" - "'%s' value:'%s' [%d][%s]\n", allow_key, allow_value, - ret, sss_strerror(ret)); + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " + "value: [%s] to allow maps " + "[%d][%s].\n", + allow_key, value, ret, + sss_strerror(ret)); goto done; } } @@ -1307,14 +1335,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, goto done; } else if (ret != ENOENT) { const char *value = deny_value ? deny_value : empty_val; - ret = sysdb_gpo_store_gpo_result_setting(domain, - deny_key, - value); + ret = add_result_to_hash(deny_maps, deny_key, + talloc_strdup(deny_maps, value)); if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "sysdb_gpo_store_gpo_result_setting failed for key:" - "'%s' value:'%s' [%d][%s]\n", deny_key, deny_value, - ret, sss_strerror(ret)); + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " + "value: [%s] to deny maps " + "[%d][%s].\n", + deny_key, value, ret, + sss_strerror(ret)); goto done; } } @@ -1609,6 +1637,8 @@ struct ad_gpo_access_state { int num_cse_filtered_gpos; int cse_gpo_index; const char *ad_domain; + hash_table_t *allow_maps; + hash_table_t *deny_maps; }; static void ad_gpo_connect_done(struct tevent_req *subreq); @@ -1724,6 +1754,19 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, goto immediately; } + ret = sss_hash_create(state, 0, &state->allow_maps); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Could not create allow maps " + "hash table [%d]: %s\n", ret, sss_strerror(ret)); + goto immediately; + } + + ret = sss_hash_create(state, 0, &state->deny_maps); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Could not create deny maps " + "hash table [%d]: %s\n", ret, sss_strerror(ret)); + goto immediately; + } subreq = sdap_id_op_connect_send(state->sdap_op, state, &ret); if (subreq == NULL) { @@ -2522,6 +2565,43 @@ ad_gpo_cse_step(struct tevent_req *req) return EAGAIN; } +static errno_t +store_hash_maps_in_cache(struct sss_domain_info *domain, + hash_table_t *allow_maps, hash_table_t *deny_maps) +{ + int ret; + struct hash_iter_context_t *iter; + hash_entry_t *entry; + size_t c; + hash_table_t *hash_list[] = { allow_maps, deny_maps, NULL}; + + + for (c = 0; hash_list[c] != NULL; c++) { + iter = new_hash_iter_context(hash_list[c]); + if (iter == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to create hash iterator.\n"); + return EINVAL; + } + + while ((entry = iter->next(iter)) != NULL) { + ret = sysdb_gpo_store_gpo_result_setting(domain, + entry->key.c_str, + entry->value.ptr); + if (ret != EOK) { + free(iter); + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_gpo_store_gpo_result_setting failed for key:" + "[%s] value:[%s] [%d][%s]\n", entry->key.c_str, + (char *) entry->value.ptr, ret, sss_strerror(ret)); + return ret; + } + } + talloc_free(iter); + } + + return EOK; +} + /* * This cse-specific function (GP_EXT_GUID_SECURITY) increments the * cse_gpo_index until the policy settings for all applicable GPOs have been @@ -2563,6 +2643,7 @@ ad_gpo_cse_done(struct tevent_req *subreq) * (as part of the GPO Result object in the sysdb cache). */ ret = ad_gpo_store_policy_settings(state->host_domain, + state->allow_maps, state->deny_maps, cse_filtered_gpo->policy_filename); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, @@ -2576,6 +2657,13 @@ ad_gpo_cse_done(struct tevent_req *subreq) if (ret == EOK) { /* ret is EOK only after all GPO policy files have been downloaded */ + ret = store_hash_maps_in_cache(state->host_domain, + state->allow_maps, state->deny_maps); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to store evaluated GPO maps " + "[%d][%s].\n", ret, sss_strerror(ret)); + goto done; + } ret = ad_gpo_perform_hbac_processing(state, state->gpo_mode, state->gpo_map_type, -- 2.44.0
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
