From c02e09afe9610d872121708893db8a21fb201b12 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 6 Nov 2023 18:17:28 +0100 Subject: [PATCH] ad: gpo evalute host groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this patch the group-memberships of the client running SSSD are included in the evaluation of the security filtering. Similar as in AD the host object is more or less handled as a user object which allows to skip some code dedicated to computers only. Resolves: https://github.com/SSSD/sssd/issues/5708 Reviewed-by: Justin Stephenson Reviewed-by: Tomáš Halman --- src/providers/ad/ad_gpo.c | 332 +++++++++++---------------------- src/tests/cmocka/test_ad_gpo.c | 25 ++- 2 files changed, 134 insertions(+), 223 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index b119aa3776..1c731b222b 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -661,6 +661,8 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, const char *host_sid, const char **group_sids, int group_size, + const char **host_group_sids, + int host_group_size, struct dom_sid ace_dom_sid, struct sss_idmap_ctx *idmap_ctx, bool *_included) @@ -718,6 +720,22 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, } } + for (i = 0; i < host_group_size; i++) { + err = sss_idmap_sid_to_smb_sid(idmap_ctx, host_group_sids[i], &group_dom_sid); + if (err != IDMAP_SUCCESS) { + DEBUG(SSSDBG_CRIT_FAILURE, + "sss_idmap_sid_to_smb_sid() failed for group_sid '%s': %d\n", + group_sids[i], err); + return EFAULT; + } + included = ad_gpo_dom_sid_equal(&ace_dom_sid, group_dom_sid); + sss_idmap_free_smb_sid(idmap_ctx, group_dom_sid); + if (included) { + *_included = true; + return EOK; + } + } + *_included = false; return EOK; } @@ -770,7 +788,9 @@ static enum ace_eval_agp_status ad_gpo_evaluate_ace(struct security_ace *ace, const char *user_sid, const char *host_sid, const char **group_sids, - int group_size) + int group_size, + const char **host_group_sids, + int host_group_size) { bool included = false; int ret = 0; @@ -782,9 +802,9 @@ static enum ace_eval_agp_status ad_gpo_evaluate_ace(struct security_ace *ace, } ret = ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, - group_size, ace->trustee, idmap_ctx, - &included); - + group_size, host_group_sids, + host_group_size, ace->trustee, + idmap_ctx, &included); if (ret != EOK) { return AD_GPO_ACE_DENIED; } @@ -844,6 +864,8 @@ static errno_t ad_gpo_simple_evaluate_ace(struct security_ace *ace, const char *host_sid, const char **group_sids, int group_size, + const char **host_group_sids, + int host_group_size, uint32_t *_gpo_access_granted_status, uint32_t *_gpo_access_denied_status) { @@ -856,6 +878,7 @@ static errno_t ad_gpo_simple_evaluate_ace(struct security_ace *ace, } ret = ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, group_size, + host_group_sids, host_group_size, ace->trustee, idmap_ctx, &included); if (ret != EOK || !included) { @@ -895,6 +918,8 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, const char *host_sid, const char **group_sids, int group_size, + const char **host_group_sids, + int host_group_size, bool *_dacl_access_allowed) { uint32_t num_aces = 0; @@ -931,6 +956,7 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, ret = ad_gpo_simple_evaluate_ace(ace, idmap_ctx, user_sid, host_sid, group_sids, group_size, + host_group_sids, host_group_size, &access_granted_status, &access_denied_status); @@ -963,7 +989,8 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, } ace_status = ad_gpo_evaluate_ace(ace, idmap_ctx, user_sid, host_sid, - group_sids, group_size); + group_sids, group_size, + host_group_sids, host_group_size); switch (ace_status) { case AD_GPO_ACE_NEUTRAL: @@ -1016,8 +1043,9 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, static errno_t ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, const char *user, - const char *host_sid, + const char *host_fqdn, struct sss_domain_info *domain, + struct sss_domain_info *host_domain, struct sss_idmap_ctx *idmap_ctx, struct gp_gpo **candidate_gpos, int num_candidate_gpos, @@ -1033,6 +1061,9 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, const char *user_sid = NULL; const char **group_sids = NULL; int group_size = 0; + const char *host_sid = NULL; + const char **host_group_sids = NULL; + int host_group_size = 0; int gpo_dn_idx = 0; bool access_allowed = false; struct gp_gpo **dacl_filtered_gpos = NULL; @@ -1052,6 +1083,15 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, goto done; } + ret = ad_gpo_get_sids(tmp_ctx, host_fqdn, host_domain, &host_sid, + &host_group_sids, &host_group_size); + if (ret != EOK) { + ret = ERR_NO_SIDS; + DEBUG(SSSDBG_OP_FAILURE, + "Unable to retrieve host SIDs: [%d](%s)\n", ret, sss_strerror(ret)); + goto done; + } + dacl_filtered_gpos = talloc_array(tmp_ctx, struct gp_gpo *, num_candidate_gpos + 1); @@ -1096,7 +1136,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, if ((sd->type & SEC_DESC_DACL_PRESENT) && (dacl != NULL)) { ret = ad_gpo_evaluate_dacl(dacl, idmap_ctx, user_sid, host_sid, - group_sids, group_size, &access_allowed); + group_sids, group_size, host_group_sids, + host_group_size, &access_allowed); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not determine if GPO is applicable\n"); @@ -1773,6 +1814,8 @@ struct ad_gpo_access_state { int timeout; struct sss_domain_info *user_domain; struct sss_domain_info *host_domain; + const char *host_sam_account_name; + char *host_fqdn; const char *user; int gpo_timeout_option; const char *ad_hostname; @@ -1793,7 +1836,6 @@ static void ad_gpo_process_gpo_done(struct tevent_req *subreq); static errno_t ad_gpo_cse_step(struct tevent_req *req); static void ad_gpo_cse_done(struct tevent_req *subreq); -static void ad_gpo_get_host_sid_retrieval_done(struct tevent_req *subreq); struct tevent_req * ad_gpo_access_send(TALLOC_CTX *mem_ctx, @@ -1967,15 +2009,11 @@ ad_gpo_connect_done(struct tevent_req *subreq) { struct tevent_req *req; struct ad_gpo_access_state *state; - char *filter; - const char *sam_account_name; - char *domain_dn; int dp_error; errno_t ret; char *server_uri; LDAPURLDesc *lud; - - const char *attrs[] = {AD_AT_DN, AD_AT_UAC, NULL}; + struct sdap_domain *sdom; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct ad_gpo_access_state); @@ -2041,47 +2079,40 @@ ad_gpo_connect_done(struct tevent_req *subreq) /* SDAP_SASL_AUTHID contains the name used for kinit and SASL bind which * in the AD case is the NetBIOS name. */ - sam_account_name = dp_opt_get_string(state->opts->basic, SDAP_SASL_AUTHID); - if (sam_account_name == NULL) { + state->host_sam_account_name = dp_opt_get_string(state->opts->basic, + SDAP_SASL_AUTHID); + if (state->host_sam_account_name == NULL) { ret = ENOMEM; goto done; } - DEBUG(SSSDBG_TRACE_FUNC, "sam_account_name is %s\n", sam_account_name); + DEBUG(SSSDBG_TRACE_FUNC, "sam_account_name is %s\n", + state->host_sam_account_name); - /* Convert the domain name into domain DN */ - ret = domain_to_basedn(state, state->ad_domain, &domain_dn); - if (ret != EOK) { + state->host_fqdn = sss_create_internal_fqname(state, state->host_sam_account_name, + state->host_domain->name); + if (state->host_fqdn == NULL) { DEBUG(SSSDBG_OP_FAILURE, - "Cannot convert domain name [%s] to base DN [%d]: %s\n", - state->ad_domain, ret, sss_strerror(ret)); - goto done; - } - - /* SDAP_OC_USER objectclass covers both users and computers */ - filter = talloc_asprintf(state, - "(&(objectclass=%s)(%s=%s))", - state->opts->user_map[SDAP_OC_USER].name, - AD_AT_SAMACCOUNTNAME, - sam_account_name); - if (filter == NULL) { + "Failed to create fully-qualified host name.\n"); ret = ENOMEM; goto done; } - subreq = sdap_get_generic_send(state, state->ev, state->opts, - sdap_id_op_handle(state->sdap_op), - domain_dn, LDAP_SCOPE_SUBTREE, - filter, attrs, NULL, 0, - state->timeout, - false); - - if (subreq == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "sdap_get_generic_send failed.\n"); + /* AD handle computers the same as users */ + sdom = sdap_domain_get(state->access_ctx->ad_id_ctx->sdap_id_ctx->opts, + state->host_domain); + if (sdom == NULL) { ret = EIO; goto done; } + subreq = groups_by_user_send(state, state->ev, + state->access_ctx->ad_id_ctx->sdap_id_ctx, + sdom, state->conn, + state->host_fqdn, + BE_FILTER_NAME, + NULL, + true); tevent_req_set_callback(subreq, ad_gpo_target_dn_retrieval_done, req); ret = EOK; @@ -2100,22 +2131,20 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) struct ad_gpo_access_state *state; int ret; int dp_error; - size_t reply_count; - struct sysdb_attrs **reply; + int sdap_ret; const char *target_dn = NULL; uint32_t uac; - const char *attrs[] = {AD_AT_SID, NULL}; - struct ldb_message *msg; - static const char *host_attrs[] = { SYSDB_SID_STR, NULL }; + static const char *host_attrs[] = { SYSDB_ORIG_DN, SYSDB_AD_USER_ACCOUNT_CONTROL, SYSDB_SID_STR, NULL }; + struct ldb_result *res = NULL; + const char *tmp = NULL; + char *endptr; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct ad_gpo_access_state); - ret = sdap_get_generic_recv(subreq, state, - &reply_count, &reply); + ret = groups_by_user_recv(subreq, &dp_error, &sdap_ret); talloc_zfree(subreq); if (ret != EOK) { - ret = sdap_id_op_done(state->sdap_op, ret, &dp_error); - if (ret == EAGAIN && dp_error == DP_ERR_OFFLINE) { + if (sdap_ret == EAGAIN && dp_error == DP_ERR_OFFLINE) { DEBUG(SSSDBG_TRACE_FUNC, "Preparing for offline operation.\n"); ret = process_offline_gpos(state, state->user, @@ -2144,27 +2173,25 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } - /* make sure there is only one non-NULL reply returned */ - - if (reply_count < 1) { - DEBUG(SSSDBG_OP_FAILURE, "No DN retrieved for policy target.\n"); - ret = ENOENT; - goto done; - } else if (reply_count > 1) { - DEBUG(SSSDBG_OP_FAILURE, "Multiple replies for policy target\n"); - ret = ERR_INTERNAL; + ret = sysdb_get_user_attr(state, state->host_domain, + state->host_fqdn, + host_attrs, &res); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to read host attributes.\n"); goto done; - } else if (reply == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "reply_count is 1, but reply is NULL\n"); - ret = ERR_INTERNAL; + } + if (res->count != 1) { + DEBUG(SSSDBG_OP_FAILURE, "Unexpected number [%d] of results searching " + "for [%s], expected 1.\n", res->count, + state->host_sam_account_name); + ret = EINVAL; goto done; } - /* reply[0] holds requested attributes of single reply */ - ret = sysdb_attrs_get_string(reply[0], AD_AT_DN, &target_dn); - if (ret != EOK) { + target_dn = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_ORIG_DN, NULL); + if (target_dn == NULL) { DEBUG(SSSDBG_OP_FAILURE, - "sysdb_attrs_get_string failed: [%d](%s)\n", + "ldb_msg_find_attr_as_string failed: [%d](%s)\n", ret, sss_strerror(ret)); goto done; } @@ -2174,14 +2201,29 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } - ret = sysdb_attrs_get_uint32_t(reply[0], AD_AT_UAC, &uac); - if (ret != EOK) { + tmp = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_AD_USER_ACCOUNT_CONTROL, + NULL); + if (tmp == NULL) { DEBUG(SSSDBG_OP_FAILURE, - "sysdb_attrs_get_uint32_t failed: [%d](%s)\n", + "ldb_msg_find_attr_as_string failed: [%d](%s)\n", ret, sss_strerror(ret)); goto done; } + uac = strtouint32(tmp, &endptr, 10); + if (errno != 0) { + ret = errno; + DEBUG(SSSDBG_OP_FAILURE, "Failed to convert UAC [%s] into uint32_t.\n", + tmp); + goto done; + } + if (*endptr != '\0') { + ret = EINVAL; + DEBUG(SSSDBG_OP_FAILURE, "UAC [%s] is not a pure numerical value.\n", + tmp); + goto done; + } + /* we only support computer policy targets, not users */ if (!(uac & UAC_WORKSTATION_TRUST_ACCOUNT || uac & UAC_SERVER_TRUST_ACCOUNT)) { @@ -2192,36 +2234,8 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } - /* Check if computer exists in cache */ - ret = sysdb_get_computer(state, state->user_domain, state->ad_hostname, - host_attrs, &msg); - if (ret == ENOENT) { - /* The computer is not in cache so query LDAP server */ - subreq = sdap_get_generic_send(state, state->ev, state->opts, - sdap_id_op_handle(state->sdap_op), - state->target_dn, LDAP_SCOPE_BASE, - "(&)", attrs, NULL, 0, - state->timeout, - false); - - if (subreq == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "sdap_get_generic_send failed.\n"); - ret = ENOMEM; - goto done; - } - - tevent_req_set_callback(subreq, ad_gpo_get_host_sid_retrieval_done, req); - return; - } else if (ret != EOK) { - ret = sdap_id_op_done(state->sdap_op, ret, &dp_error); - goto done; - } - - /* The computer exists in the cache, there is no need to query LDAP. - * Store the retrieved host sid from cache in the state to avoid querying - * the cache again in ad_gpo_get_sids. - */ - state->host_sid = ldb_msg_find_attr_as_string(msg, SYSDB_SID_STR, NULL); + state->host_sid = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_SID_STR, + NULL); talloc_steal(state, state->host_sid); subreq = ad_gpo_process_som_send(state, @@ -2251,125 +2265,6 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) } -enum ndr_err_code -ndr_pull_dom_sid(struct ndr_pull *ndr, - int ndr_flags, - struct dom_sid *r); - -static void ad_gpo_get_host_sid_retrieval_done(struct tevent_req *subreq) -{ - struct tevent_req *req; - struct ad_gpo_access_state *state; - int ret; - int dp_error; - size_t reply_count; - struct sysdb_attrs **reply; - struct ldb_message_element *el = NULL; - enum ndr_err_code ndr_err; - struct dom_sid host_sid; - char *sid_str; - - req = tevent_req_callback_data(subreq, struct tevent_req); - state = tevent_req_data(req, struct ad_gpo_access_state); - - ret = sdap_get_generic_recv(subreq, state, - &reply_count, &reply); - talloc_zfree(subreq); - - if (ret != EOK) { - ret = sdap_id_op_done(state->sdap_op, ret, &dp_error); - - DEBUG(SSSDBG_OP_FAILURE, - "sdap_get_generic_recv failed: [%d](%s)\n", - ret, sss_strerror(ret)); - ret = ENOENT; - tevent_req_error(req, ret); - return; - } - - if (reply_count == 0 || !reply) { - DEBUG(SSSDBG_OP_FAILURE, - "sdap_get_generic_recv failed to receive host sid\n"); - ret = EIO; - goto done; - } - - /* reply[0] holds the requested attribute */ - ret = sysdb_attrs_get_el(reply[0], AD_AT_SID, &el); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sysdb_attrs_get_el failed: [%d](%s)\n", - ret, sss_strerror(ret)); - goto done; - } - if (el->num_values != 1) { - DEBUG(SSSDBG_OP_FAILURE, - "ad_gpo_get_host_sid_retrieval_done failed: sid not present\n"); - ret = EIO; - goto done; - } - - /* parse the dom_sid from the ldb blob */ - ndr_err = ndr_pull_struct_blob_all((DATA_BLOB*)&(el->values[0]), - subreq, &host_sid, - (ndr_pull_flags_fn_t)ndr_pull_dom_sid); - if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - DEBUG(SSSDBG_OP_FAILURE, - "ndr_pull_struct_blob_all failed: [%d]\n", - ndr_err); - ret = EIO; - goto done; - } - - /* Convert the dom_sid to a sid string */ - ret = sss_idmap_smb_sid_to_sid(state->opts->idmap_ctx->map, - &host_sid, &sid_str); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sss_idmap_smb_sid_to_sid failed: [%d](%s)\n", - ret, sss_strerror(ret)); - goto done; - } - state->host_sid = talloc_steal(state, sid_str); - - /* Put the sid string in the sysdb */ - ret = sysdb_set_computer(subreq, state->user_domain, - state->ad_hostname, state->host_sid, - state->user_domain->computer_timeout, - time(NULL)); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sysdb_set_computer failed: [%d](%s)\n", - ret, sss_strerror(ret)); - goto done; - } - - subreq = ad_gpo_process_som_send(state, - state->ev, - state->conn, - state->ldb_ctx, - state->sdap_op, - state->opts, - state->access_ctx->ad_options, - state->timeout, - state->target_dn, - state->ad_domain); - if (subreq == NULL) { - ret = ENOMEM; - goto done; - } - - tevent_req_set_callback(subreq, ad_gpo_process_som_done, req); - - ret = EOK; - - done: - - if (ret != EOK) { - tevent_req_error(req, ret); - } -} - static void ad_gpo_process_som_done(struct tevent_req *subreq) { @@ -2487,8 +2382,9 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) goto done; } - ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->host_sid, + ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->host_fqdn, state->user_domain, + state->host_domain, state->opts->idmap_ctx->map, candidate_gpos, num_candidate_gpos, &state->dacl_filtered_gpos, diff --git a/src/tests/cmocka/test_ad_gpo.c b/src/tests/cmocka/test_ad_gpo.c index 8660b05107..d49f6c54c2 100644 --- a/src/tests/cmocka/test_ad_gpo.c +++ b/src/tests/cmocka/test_ad_gpo.c @@ -270,6 +270,8 @@ static void test_ad_gpo_ace_includes_client_sid(const char *user_sid, const char *host_sid, const char **group_sids, int group_size, + const char **host_group_sids, + int host_group_size, struct dom_sid ace_dom_sid, bool expected) { @@ -288,8 +290,9 @@ static void test_ad_gpo_ace_includes_client_sid(const char *user_sid, assert_int_equal(err, IDMAP_SUCCESS); ret = ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, - group_size, ace_dom_sid, idmap_ctx, - &includes_client_sid); + group_size, host_group_sids, + host_group_size, ace_dom_sid, + idmap_ctx, &includes_client_sid); talloc_free(idmap_ctx); assert_int_equal(ret, EOK); @@ -312,8 +315,12 @@ void test_ad_gpo_ace_includes_client_sid_true(void **state) const char *group_sids[] = {"S-1-5-21-2-3-4", "S-1-5-21-2-3-5"}; + int host_group_size = 0; + const char *host_group_sids[] = { NULL }; + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, - group_size, ace_dom_sid, true); + group_size, host_group_sids, + host_group_size, ace_dom_sid, true); } void test_ad_gpo_ace_includes_client_sid_false(void **state) @@ -328,8 +335,12 @@ void test_ad_gpo_ace_includes_client_sid_false(void **state) const char *group_sids[] = {"S-1-5-21-2-3-5", "S-1-5-21-2-3-6"}; + int host_group_size = 0; + const char *host_group_sids[] = { NULL }; + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, - group_size, ace_dom_sid, false); + group_size, host_group_sids, + host_group_size, ace_dom_sid, false); } void test_ad_gpo_ace_includes_host_sid_true(void **state) @@ -343,8 +354,12 @@ void test_ad_gpo_ace_includes_host_sid_true(void **state) int group_size = 0; const char *group_sids[] = {}; + int host_group_size = 0; + const char *host_group_sids[] = { NULL }; + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, - group_size, ace_dom_sid, true); + group_size, host_group_sids, + host_group_size, ace_dom_sid, true); } uint8_t test_sid_data[] = {