URL: https://github.com/SSSD/sssd/pull/905 Author: dmulder Title: #905: WIP: Don't ignore host entries in Group Policy security filters Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/905/head:pr905 git checkout pr905
From 2671122722aaf7ca9d7a2eb4952a469a33f55f5a Mon Sep 17 00:00:00 2001 From: David Mulder <dmul...@suse.com> Date: Fri, 4 Oct 2019 13:04:01 -0600 Subject: [PATCH 1/2] WIP: SSSD should accept host entries from GPO's security filter --- src/providers/ad/ad_gpo.c | 276 ++++++++++++++++++++++++++++------ src/providers/ad/ad_gpo_ndr.c | 2 +- 2 files changed, 233 insertions(+), 45 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 7442f27cca..96306adf7b 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -65,6 +65,7 @@ #define AD_AT_MACHINE_EXT_NAMES "gPCMachineExtensionNames" #define AD_AT_FUNC_VERSION "gPCFunctionalityVersion" #define AD_AT_FLAGS "flags" +#define AD_AT_SID "objectSid" #define UAC_WORKSTATION_TRUST_ACCOUNT 0x00001000 #define UAC_SERVER_TRUST_ACCOUNT 0x00002000 @@ -654,6 +655,7 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, */ static errno_t ad_gpo_ace_includes_client_sid(const char *user_sid, + struct dom_sid *host_dom_sid, const char **group_sids, int group_size, struct dom_sid ace_dom_sid, @@ -679,6 +681,12 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, return EOK; } + included = ad_gpo_dom_sid_equal(&ace_dom_sid, host_dom_sid); + if (included) { + *_included = true; + return EOK; + } + for (i = 0; i < group_size; i++) { err = sss_idmap_sid_to_smb_sid(idmap_ctx, group_sids[i], &group_dom_sid); if (err != IDMAP_SUCCESS) { @@ -728,6 +736,7 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace, struct sss_idmap_ctx *idmap_ctx, const char *user_sid, + struct dom_sid *host_sid, const char **group_sids, int group_size) { @@ -741,8 +750,9 @@ static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace, return AD_GPO_ACE_NEUTRAL; } - ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, - ace->trustee, idmap_ctx, &included); + ret = ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, + group_size, ace->trustee, idmap_ctx, + &included); if (ret != EOK) { return AD_GPO_ACE_DENIED; @@ -786,6 +796,7 @@ static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace, static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, struct sss_idmap_ctx *idmap_ctx, const char *user_sid, + struct dom_sid *host_sid, const char **group_sids, int group_size, bool *_dacl_access_allowed) @@ -810,7 +821,7 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, for (i = 0; i < dacl->num_aces; i ++) { ace = &dacl->aces[i]; - ace_status = ad_gpo_evaluate_ace(ace, idmap_ctx, user_sid, + ace_status = ad_gpo_evaluate_ace(ace, idmap_ctx, user_sid, host_sid, group_sids, group_size); switch (ace_status) { @@ -829,6 +840,207 @@ static errno_t ad_gpo_evaluate_dacl(struct security_acl *dacl, return EOK; } +struct ad_gpo_access_state { + struct tevent_context *ev; + struct ldb_context *ldb_ctx; + struct ad_access_ctx *access_ctx; + enum gpo_access_control_mode gpo_mode; + bool gpo_implicit_deny; + enum gpo_map_type gpo_map_type; + 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 *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; + int num_dacl_filtered_gpos; + struct gp_gpo **cse_filtered_gpos; + int num_cse_filtered_gpos; + int cse_gpo_index; +}; + +struct ad_gpo_get_host_sid_state { + struct tevent_context *ev; + struct sss_domain_info *domain; + struct sdap_id_conn_ctx *conn; + struct sdap_id_op *sdap_op; + struct sdap_options *opts; + struct sss_domain_info *host_domain; + const char *ad_hostname; + int timeout; +}; + +struct ad_gpo_get_host_sid_callback_data { + struct dom_sid host_sid; +}; + +struct tevent_req * +ad_gpo_get_host_sid_send(TALLOC_CTX *mem_ctx, + struct ad_gpo_access_state *access_state) +{ + struct tevent_req *req; + struct ad_gpo_get_host_sid_state *state; + + req = tevent_req_create(mem_ctx, &state, struct ad_gpo_get_host_sid_state); + if (req == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create() failed\n"); + return NULL; + } + + state->ev = access_state->ev; + state->domain = access_state->user_domain; + state->conn = access_state->conn; + state->sdap_op = access_state->sdap_op; + state->opts = access_state->opts; + state->host_domain = access_state->host_domain; + state->timeout = access_state->timeout; + state->ad_hostname = access_state->ad_hostname; + + return req; +} + +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 ad_gpo_get_host_sid_state *state; + struct ad_gpo_get_host_sid_callback_data *data; + int ret; + size_t reply_count; + struct sysdb_attrs **reply; + struct ldb_message_element *el = NULL; + enum ndr_err_code ndr_err; + + state = tevent_req_data(subreq, struct ad_gpo_get_host_sid_state); + data = tevent_req_callback_data(subreq, struct ad_gpo_get_host_sid_callback_data); + ret = sdap_get_generic_recv(subreq, state, + &reply_count, &reply); + + /* 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(&(el->values[0]), + subreq, &(data->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; + } + +done: + if (ret == EOK) { + tevent_req_done(subreq); + } else { + tevent_req_error(subreq, ret); + } +} + +static void +ad_gpo_get_host_sid_done(struct tevent_req *req) +{ + struct ad_gpo_get_host_sid_state *state; + struct ad_gpo_get_host_sid_callback_data *data; + struct tevent_req *subreq; + int ret; + char *filter = NULL; + char *domain_dn; + const char *attrs[] = {AD_AT_SID, NULL}; + + state = tevent_req_data(req, struct ad_gpo_get_host_sid_state); + data = tevent_req_callback_data(req, struct ad_gpo_get_host_sid_callback_data); + + /* Convert the domain name into 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->host_domain->name, ret, sss_strerror(ret)); + goto done; + } + + filter = talloc_asprintf(req, + "(&(objectCategory=computer)(name=%s))", + state->ad_hostname); + if (!filter) { + 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"); + ret = EIO; + goto done; + } + + tevent_req_set_callback(subreq, ad_gpo_get_host_sid_retrieval_done, data); + +done: + if (ret == EOK) { + tevent_req_done(req); + } else { + tevent_req_error(req, ret); + } +} + +static errno_t +ad_gpo_get_host_sid(TALLOC_CTX *mem_ctx, + struct ad_gpo_access_state *state, + struct dom_sid **host_sid) +{ + struct tevent_req *req; + struct ad_gpo_get_host_sid_callback_data data; + + req = ad_gpo_get_host_sid_send(mem_ctx, state); + if (req == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "ad_gpo_get_host_sid_send() failed\n"); + return EIO; + } + + tevent_req_set_callback(req, ad_gpo_get_host_sid_done, &data); + + *host_sid = talloc_size(mem_ctx, sizeof(struct dom_sid)); + if (*host_sid) { + memcpy(*host_sid, &data.host_sid, sizeof(struct dom_sid)); + } else { + return ENOMEM; + } + + return EOK; +} + /* * This function takes candidate_gpos as input, filters out any gpo that is * not applicable to the policy target and assigns the result to the @@ -837,13 +1049,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, - struct sss_domain_info *domain, - struct sss_idmap_ctx *idmap_ctx, + struct ad_gpo_access_state *state, struct gp_gpo **candidate_gpos, - int num_candidate_gpos, - struct gp_gpo ***_dacl_filtered_gpos, - int *_num_dacl_filtered_gpos) + int num_candidate_gpos) { TALLOC_CTX *tmp_ctx = NULL; int i = 0; @@ -852,6 +1060,7 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, struct security_descriptor *sd = NULL; struct security_acl *dacl = NULL; const char *user_sid = NULL; + struct dom_sid *host_sid = NULL; const char **group_sids = NULL; int group_size = 0; int gpo_dn_idx = 0; @@ -864,7 +1073,7 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, goto done; } - ret = ad_gpo_get_sids(tmp_ctx, user, domain, &user_sid, + ret = ad_gpo_get_sids(tmp_ctx, state->user, state->user_domain, &user_sid, &group_sids, &group_size); if (ret != EOK) { ret = ERR_NO_SIDS; @@ -873,6 +1082,13 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, goto done; } + ret = ad_gpo_get_host_sid(tmp_ctx, state, &host_sid); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Unable to retrieve host SID: [%d](%s)\n", ret, sss_strerror(ret)); + goto done; + } + dacl_filtered_gpos = talloc_array(tmp_ctx, struct gp_gpo *, num_candidate_gpos + 1); @@ -927,8 +1143,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, break; } - ret = ad_gpo_evaluate_dacl(dacl, idmap_ctx, user_sid, group_sids, - group_size, &access_allowed); + ret = ad_gpo_evaluate_dacl(dacl, state->opts->idmap_ctx->map, user_sid, + host_sid, group_sids, group_size, &access_allowed); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not determine if GPO is applicable\n"); continue; @@ -950,8 +1166,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, dacl_filtered_gpos[gpo_dn_idx] = NULL; - *_dacl_filtered_gpos = talloc_steal(mem_ctx, dacl_filtered_gpos); - *_num_dacl_filtered_gpos = gpo_dn_idx; + state->dacl_filtered_gpos = talloc_steal(mem_ctx, dacl_filtered_gpos); + state->num_dacl_filtered_gpos = gpo_dn_idx; ret = EOK; @@ -1585,31 +1801,6 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx, /* == ad_gpo_access_send/recv implementation ================================*/ -struct ad_gpo_access_state { - struct tevent_context *ev; - struct ldb_context *ldb_ctx; - struct ad_access_ctx *access_ctx; - enum gpo_access_control_mode gpo_mode; - bool gpo_implicit_deny; - enum gpo_map_type gpo_map_type; - 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 *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; - int num_dacl_filtered_gpos; - struct gp_gpo **cse_filtered_gpos; - int num_cse_filtered_gpos; - int cse_gpo_index; -}; - static void ad_gpo_connect_done(struct tevent_req *subreq); static void ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq); static void ad_gpo_process_som_done(struct tevent_req *subreq); @@ -2143,11 +2334,8 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) goto done; } - 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); + ret = ad_gpo_filter_gpos_by_dacl(state, state, candidate_gpos, + num_candidate_gpos); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Unable to filter GPO list by DACL: [%d](%s)\n", diff --git a/src/providers/ad/ad_gpo_ndr.c b/src/providers/ad/ad_gpo_ndr.c index 101701cd52..d573033494 100644 --- a/src/providers/ad/ad_gpo_ndr.c +++ b/src/providers/ad/ad_gpo_ndr.c @@ -248,7 +248,7 @@ ndr_pull_security_ace_object_ctr(struct ndr_pull *ndr, return NDR_ERR_SUCCESS; } -static enum ndr_err_code +enum ndr_err_code ndr_pull_dom_sid(struct ndr_pull *ndr, int ndr_flags, struct dom_sid *r) From 044f7e3f513b3708ee5d504cdcb456b87e06fdb7 Mon Sep 17 00:00:00 2001 From: David Mulder <dmul...@suse.com> Date: Fri, 4 Oct 2019 21:14:39 +0000 Subject: [PATCH 2/2] WIP: Test the host sid checking --- src/tests/cmocka/test_ad_gpo.c | 38 ++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/tests/cmocka/test_ad_gpo.c b/src/tests/cmocka/test_ad_gpo.c index 0589adcc3d..5e4a4acc6f 100644 --- a/src/tests/cmocka/test_ad_gpo.c +++ b/src/tests/cmocka/test_ad_gpo.c @@ -267,6 +267,7 @@ void test_populate_gplink_list_malformed(void **state) * Test SID-matching logic */ static void test_ad_gpo_ace_includes_client_sid(const char *user_sid, + const char *host_sid, const char **group_sids, int group_size, struct dom_sid ace_dom_sid, @@ -277,6 +278,7 @@ static void test_ad_gpo_ace_includes_client_sid(const char *user_sid, struct sss_idmap_ctx *idmap_ctx; bool includes_client_sid; TALLOC_CTX *tmp_ctx; + struct dom_sid *host_dom_sid; tmp_ctx = talloc_new(global_talloc_context); assert_non_null(tmp_ctx); @@ -286,9 +288,13 @@ static void test_ad_gpo_ace_includes_client_sid(const char *user_sid, &idmap_ctx); assert_int_equal(err, IDMAP_SUCCESS); - ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, - ace_dom_sid, idmap_ctx, + err = sss_idmap_sid_to_smb_sid(idmap_ctx, host_sid, &host_dom_sid); + assert_int_equal(err, IDMAP_SUCCESS); + + ret = ad_gpo_ace_includes_client_sid(user_sid, host_dom_sid, group_sids, + group_size, ace_dom_sid, idmap_ctx, &includes_client_sid); + talloc_free(host_dom_sid); talloc_free(idmap_ctx); assert_int_equal(ret, EOK); @@ -305,13 +311,14 @@ void test_ad_gpo_ace_includes_client_sid_true(void **state) struct dom_sid ace_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5}, {21, 2, 3, 4}}; const char *user_sid = "S-1-5-21-1175337206-4250576914-2321192831-1103"; + const char *host_sid = "S-1-5-21-1898687337-2196588786-2775055786-2102"; int group_size = 2; const char *group_sids[] = {"S-1-5-21-2-3-4", "S-1-5-21-2-3-5"}; - test_ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, - ace_dom_sid, true); + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, + group_size, ace_dom_sid, true); } void test_ad_gpo_ace_includes_client_sid_false(void **state) @@ -320,13 +327,29 @@ void test_ad_gpo_ace_includes_client_sid_false(void **state) struct dom_sid ace_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5}, {21, 2, 3, 4}}; const char *user_sid = "S-1-5-21-1175337206-4250576914-2321192831-1103"; + const char *host_sid = "S-1-5-21-1898687337-2196588786-2775055786-2102"; int group_size = 2; const char *group_sids[] = {"S-1-5-21-2-3-5", "S-1-5-21-2-3-6"}; - test_ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, - ace_dom_sid, false); + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, + group_size, ace_dom_sid, false); +} + +void test_ad_gpo_ace_includes_host_sid_true(void **state) +{ + /* ace_dom_sid represents "S-1-5-21-1898687337-2196588786-2775055786-2102" */ + struct dom_sid ace_dom_sid = {1, 5, {0, 0, 0, 0, 0, 5}, {21, 1898687337, 2196588786, 2775055786, 2102}}; + + const char *user_sid = "S-1-5-21-1175337206-4250576914-2321192831-1103"; + const char *host_sid = "S-1-5-21-1898687337-2196588786-2775055786-2102"; + + int group_size = 0; + const char *group_sids[] = {}; + + test_ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, + group_size, ace_dom_sid, true); } int main(int argc, const char *argv[]) @@ -364,6 +387,9 @@ int main(int argc, const char *argv[]) cmocka_unit_test_setup_teardown(test_ad_gpo_ace_includes_client_sid_false, ad_gpo_test_setup, ad_gpo_test_teardown), + cmocka_unit_test_setup_teardown(test_ad_gpo_ace_includes_host_sid_true, + ad_gpo_test_setup, + ad_gpo_test_teardown), }; /* Set debug level to invalid value so we can decide if -d 0 was used. */
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org