1From f4ebe1408e0bc67abfbfb5f0ca2ea13803b36726 Mon Sep 17 00:00:00 2001 2From: Sumit Bose <sbose@redhat.com> 3Date: Wed, 8 Nov 2023 14:50:24 +0100 4Subject: [PATCH] ad-gpo: use hash to store intermediate results 5 6Currently after the evaluation of a single GPO file the intermediate 7results are stored in the cache and this cache entry is updated until 8all applicable GPO files are evaluated. Finally the data in the cache is 9used to make the decision of access is granted or rejected. 10 11If there are two or more access-control request running in parallel one 12request might overwrite the cache object with intermediate data while 13another request reads the cached data for the access decision and as a 14result will do this decision based on intermediate data. 15 16To avoid this the intermediate results are not stored in the cache 17anymore but in hash tables which are specific to the request. Only the 18final result is written to the cache to have it available for offline 19authentication. 20 21Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> 22Reviewed-by: Tomáš Halman <thalman@redhat.com> 23(cherry picked from commit d7db7971682da2dbf7642ac94940d6b0577ec35a) 24 25Upstream-Status: Backport [https://github.com/SSSD/sssd/commit/f4ebe1408e0bc67abfbfb5f0ca2ea13803b36726] 26CVE: CVE-2023-3758 27Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> 28 29--- 30 src/providers/ad/ad_gpo.c | 116 +++++++++++++++++++++++++++++++++----- 31 1 file changed, 102 insertions(+), 14 deletions(-) 32 33diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c 34index 44e9cbb..cec0cb4 100644 35--- a/src/providers/ad/ad_gpo.c 36+++ b/src/providers/ad/ad_gpo.c 37@@ -1317,6 +1317,33 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, 38 return ret; 39 } 40 41+static errno_t 42+add_result_to_hash(hash_table_t *hash, const char *key, char *value) 43+{ 44+ int hret; 45+ hash_key_t k; 46+ hash_value_t v; 47+ 48+ if (hash == NULL || key == NULL || value == NULL) { 49+ return EINVAL; 50+ } 51+ 52+ k.type = HASH_KEY_CONST_STRING; 53+ k.c_str = key; 54+ 55+ v.type = HASH_VALUE_PTR; 56+ v.ptr = value; 57+ 58+ hret = hash_enter(hash, &k, &v); 59+ if (hret != HASH_SUCCESS) { 60+ DEBUG(SSSDBG_OP_FAILURE, "Failed to add [%s][%s] to hash: [%s].\n", 61+ key, value, hash_error_string(hret)); 62+ return EIO; 63+ } 64+ 65+ return EOK; 66+} 67+ 68 /* 69 * This function parses the cse-specific (GP_EXT_GUID_SECURITY) filename, 70 * and stores the allow_key and deny_key of all of the gpo_map_types present 71@@ -1324,6 +1351,7 @@ ad_gpo_extract_policy_setting(TALLOC_CTX *mem_ctx, 72 */ 73 static errno_t 74 ad_gpo_store_policy_settings(struct sss_domain_info *domain, 75+ hash_table_t *allow_maps, hash_table_t *deny_maps, 76 const char *filename) 77 { 78 struct ini_cfgfile *file_ctx = NULL; 79@@ -1457,14 +1485,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, 80 goto done; 81 } else if (ret != ENOENT) { 82 const char *value = allow_value ? allow_value : empty_val; 83- ret = sysdb_gpo_store_gpo_result_setting(domain, 84- allow_key, 85- value); 86+ ret = add_result_to_hash(allow_maps, allow_key, 87+ talloc_strdup(allow_maps, value)); 88 if (ret != EOK) { 89- DEBUG(SSSDBG_CRIT_FAILURE, 90- "sysdb_gpo_store_gpo_result_setting failed for key:" 91- "'%s' value:'%s' [%d][%s]\n", allow_key, allow_value, 92- ret, sss_strerror(ret)); 93+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " 94+ "value: [%s] to allow maps " 95+ "[%d][%s].\n", 96+ allow_key, value, ret, 97+ sss_strerror(ret)); 98 goto done; 99 } 100 } 101@@ -1484,14 +1512,14 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, 102 goto done; 103 } else if (ret != ENOENT) { 104 const char *value = deny_value ? deny_value : empty_val; 105- ret = sysdb_gpo_store_gpo_result_setting(domain, 106- deny_key, 107- value); 108+ ret = add_result_to_hash(deny_maps, deny_key, 109+ talloc_strdup(deny_maps, value)); 110 if (ret != EOK) { 111- DEBUG(SSSDBG_CRIT_FAILURE, 112- "sysdb_gpo_store_gpo_result_setting failed for key:" 113- "'%s' value:'%s' [%d][%s]\n", deny_key, deny_value, 114- ret, sss_strerror(ret)); 115+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to add key: [%s] " 116+ "value: [%s] to deny maps " 117+ "[%d][%s].\n", 118+ deny_key, value, ret, 119+ sss_strerror(ret)); 120 goto done; 121 } 122 } 123@@ -1784,6 +1812,8 @@ struct ad_gpo_access_state { 124 int num_cse_filtered_gpos; 125 int cse_gpo_index; 126 const char *ad_domain; 127+ hash_table_t *allow_maps; 128+ hash_table_t *deny_maps; 129 }; 130 131 static void ad_gpo_connect_done(struct tevent_req *subreq); 132@@ -1906,6 +1936,19 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx, 133 goto immediately; 134 } 135 136+ ret = sss_hash_create(state, 0, &state->allow_maps); 137+ if (ret != EOK) { 138+ DEBUG(SSSDBG_FATAL_FAILURE, "Could not create allow maps " 139+ "hash table [%d]: %s\n", ret, sss_strerror(ret)); 140+ goto immediately; 141+ } 142+ 143+ ret = sss_hash_create(state, 0, &state->deny_maps); 144+ if (ret != EOK) { 145+ DEBUG(SSSDBG_FATAL_FAILURE, "Could not create deny maps " 146+ "hash table [%d]: %s\n", ret, sss_strerror(ret)); 147+ goto immediately; 148+ } 149 150 subreq = sdap_id_op_connect_send(state->sdap_op, state, &ret); 151 if (subreq == NULL) { 152@@ -2725,6 +2768,43 @@ ad_gpo_cse_step(struct tevent_req *req) 153 return EAGAIN; 154 } 155 156+static errno_t 157+store_hash_maps_in_cache(struct sss_domain_info *domain, 158+ hash_table_t *allow_maps, hash_table_t *deny_maps) 159+{ 160+ int ret; 161+ struct hash_iter_context_t *iter; 162+ hash_entry_t *entry; 163+ size_t c; 164+ hash_table_t *hash_list[] = { allow_maps, deny_maps, NULL}; 165+ 166+ 167+ for (c = 0; hash_list[c] != NULL; c++) { 168+ iter = new_hash_iter_context(hash_list[c]); 169+ if (iter == NULL) { 170+ DEBUG(SSSDBG_OP_FAILURE, "Failed to create hash iterator.\n"); 171+ return EINVAL; 172+ } 173+ 174+ while ((entry = iter->next(iter)) != NULL) { 175+ ret = sysdb_gpo_store_gpo_result_setting(domain, 176+ entry->key.c_str, 177+ entry->value.ptr); 178+ if (ret != EOK) { 179+ free(iter); 180+ DEBUG(SSSDBG_OP_FAILURE, 181+ "sysdb_gpo_store_gpo_result_setting failed for key:" 182+ "[%s] value:[%s] [%d][%s]\n", entry->key.c_str, 183+ (char *) entry->value.ptr, ret, sss_strerror(ret)); 184+ return ret; 185+ } 186+ } 187+ talloc_free(iter); 188+ } 189+ 190+ return EOK; 191+} 192+ 193 /* 194 * This cse-specific function (GP_EXT_GUID_SECURITY) increments the 195 * cse_gpo_index until the policy settings for all applicable GPOs have been 196@@ -2766,6 +2846,7 @@ ad_gpo_cse_done(struct tevent_req *subreq) 197 * (as part of the GPO Result object in the sysdb cache). 198 */ 199 ret = ad_gpo_store_policy_settings(state->host_domain, 200+ state->allow_maps, state->deny_maps, 201 cse_filtered_gpo->policy_filename); 202 if (ret != EOK && ret != ENOENT) { 203 DEBUG(SSSDBG_OP_FAILURE, 204@@ -2779,6 +2860,13 @@ ad_gpo_cse_done(struct tevent_req *subreq) 205 206 if (ret == EOK) { 207 /* ret is EOK only after all GPO policy files have been downloaded */ 208+ ret = store_hash_maps_in_cache(state->host_domain, 209+ state->allow_maps, state->deny_maps); 210+ if (ret != EOK) { 211+ DEBUG(SSSDBG_OP_FAILURE, "Failed to store evaluated GPO maps " 212+ "[%d][%s].\n", ret, sss_strerror(ret)); 213+ goto done; 214+ } 215 ret = ad_gpo_perform_hbac_processing(state, 216 state->gpo_mode, 217 state->gpo_map_type, 218-- 2192.25.1 220