URL: https://github.com/SSSD/sssd/pull/905 Author: dmulder Title: #905: 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 81c27ef33f3a79bc809b3293ef369556db33459f 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/6] SSSD should accept host entries from GPO's security filter Not accepting host entries in the security filter creates the need for sub-OU's, each with its own GPO, otherwise one OU with an assigned GPO would be sufficient. --- Makefile.am | 2 + src/db/sysdb_computer.c | 164 ++++++++++++++++++++++++++++ src/db/sysdb_computer.h | 49 +++++++++ src/providers/ad/ad_gpo.c | 196 ++++++++++++++++++++++++++++++++-- src/providers/ad/ad_gpo_ndr.c | 2 +- 5 files changed, 403 insertions(+), 10 deletions(-) create mode 100644 src/db/sysdb_computer.c create mode 100644 src/db/sysdb_computer.h diff --git a/Makefile.am b/Makefile.am index f6ae4f8de3..66319f29f1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -781,6 +781,7 @@ dist_noinst_HEADERS = \ src/db/sysdb_services.h \ src/db/sysdb_ssh.h \ src/db/sysdb_domain_resolution_order.h \ + src/db/sysdb_computer.h \ src/confdb/confdb.h \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ @@ -1245,6 +1246,7 @@ libsss_util_la_SOURCES = \ src/db/sysdb_certmap.c \ src/db/sysdb_domain_resolution_order.c \ src/util/sss_pam_data.c \ + src/db/sysdb_computer.c \ src/util/util.c \ src/util/util_ext.c \ src/util/util_preauth.c \ diff --git a/src/db/sysdb_computer.c b/src/db/sysdb_computer.c new file mode 100644 index 0000000000..e3db01ac63 --- /dev/null +++ b/src/db/sysdb_computer.c @@ -0,0 +1,164 @@ +/* + SSSD + + Authors: + Samuel Cabrero <scabr...@suse.com> + David Mulder <dmul...@suse.com> + + Copyright (C) 2019 SUSE LINUX GmbH, Nuernberg, Germany. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <arpa/inet.h> + +#include "db/sysdb.h" +#include "db/sysdb_private.h" +#include "db/sysdb_computer.h" + +static errno_t +sysdb_search_computer(TALLOC_CTX *mem_ctx, + struct sss_domain_info *domain, + const char *filter, + const char **attrs, + size_t *_num_hosts, + struct ldb_message ***_hosts) +{ + errno_t ret; + TALLOC_CTX *tmp_ctx; + struct ldb_message **results; + size_t num_results; + + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return ENOMEM; + } + + ret = sysdb_search_custom(tmp_ctx, domain, filter, + COMPUTERS_SUBDIR, attrs, + &num_results, &results); + if (ret != EOK && ret != ENOENT) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Error looking up host [%d]: %s\n", + ret, strerror(ret)); + goto done; + } else if (ret == ENOENT) { + DEBUG(SSSDBG_TRACE_FUNC, "No such host\n"); + *_hosts = NULL; + *_num_hosts = 0; + goto done; + } + + *_hosts = talloc_steal(mem_ctx, results); + *_num_hosts = num_results; + ret = EOK; + +done: + talloc_free(tmp_ctx); + + return ret; +} + +int +sysdb_get_computer(TALLOC_CTX *mem_ctx, + struct sss_domain_info *domain, + const char *computer_name, + const char **attrs, + struct ldb_message **_computer) +{ + TALLOC_CTX *tmp_ctx; + errno_t ret; + const char *filter; + struct ldb_message **hosts; + size_t num_hosts; + + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return ENOMEM; + } + + filter = talloc_asprintf(tmp_ctx, SYSDB_COMP_FILTER, computer_name); + if (!filter) { + ret = ENOMEM; + goto done; + } + + ret = sysdb_search_computer(tmp_ctx, domain, filter, attrs, + &num_hosts, &hosts); + if (ret != EOK) { + goto done; + } + + if (num_hosts != 1) { + ret = EINVAL; + DEBUG(SSSDBG_CRIT_FAILURE, + "Did not find a single host with name %s\n", computer_name); + goto done; + } + + *_computer = talloc_steal(mem_ctx, hosts[0]); + ret = EOK; + +done: + talloc_free(tmp_ctx); + + return ret; +} + +int +sysdb_set_computer(TALLOC_CTX *mem_ctx, + struct sss_domain_info *domain, + const char *computer_name, + const char *sid_str) +{ + TALLOC_CTX *tmp_ctx; + int ret; + struct sysdb_attrs *attrs; + + tmp_ctx = talloc_new(NULL); + if (!tmp_ctx) { + return ENOMEM; + } + + attrs = sysdb_new_attrs(tmp_ctx); + if (!attrs) { + ret = ENOMEM; + goto done; + } + + ret = sysdb_attrs_add_string(attrs, SYSDB_SID_STR, sid_str); + if (ret) goto done; + + ret = sysdb_attrs_add_string(attrs, SYSDB_OBJECTCLASS, SYSDB_COMPUTER_CLASS); + if (ret) goto done; + + ret = sysdb_attrs_add_string(attrs, SYSDB_NAME, computer_name); + if (ret) goto done; + + /* creation time */ + ret = sysdb_attrs_add_time_t(attrs, SYSDB_CREATE_TIME, time(NULL)); + if (ret) goto done; + + ret = sysdb_store_custom(domain, computer_name, COMPUTERS_SUBDIR, attrs); + if (ret) goto done; + + +done: + if (ret) { + DEBUG(SSSDBG_TRACE_FUNC, "Error: %d (%s)\n", ret, strerror(ret)); + } + talloc_zfree(tmp_ctx); + + return ret; +} diff --git a/src/db/sysdb_computer.h b/src/db/sysdb_computer.h new file mode 100644 index 0000000000..7c937003d1 --- /dev/null +++ b/src/db/sysdb_computer.h @@ -0,0 +1,49 @@ +/* + SSSD + + Authors: + Samuel Cabrero <scabr...@suse.com> + David Mulder <dmul...@suse.com> + + Copyright (C) 2019 SUSE LINUX GmbH, Nuernberg, Germany. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef SYSDB_COMPUTERS_H_ +#define SYSDB_COMPUTERS_H_ + +#include "db/sysdb.h" + +#define COMPUTERS_SUBDIR "computers" +#define SYSDB_COMPUTER_CLASS "computer" +#define SYSDB_COMPUTERS_CONTAINER "cn="COMPUTERS_SUBDIR +#define SYSDB_TMPL_COMPUTER_BASE SYSDB_COMPUTERS_CONTAINER","SYSDB_DOM_BASE +#define SYSDB_TMPL_COMPUTER SYSDB_NAME"=%s,"SYSDB_TMPL_COMPUTER_BASE +#define SYSDB_COMP_FILTER "(&("SYSDB_NAME"=%s)("SYSDB_OBJECTCLASS"="SYSDB_COMPUTER_CLASS"))" + +int +sysdb_get_computer(TALLOC_CTX *mem_ctx, + struct sss_domain_info *domain, + const char *computer_name, + const char **attrs, + struct ldb_message **computer); + +int +sysdb_set_computer(TALLOC_CTX *mem_ctx, + struct sss_domain_info *domain, + const char *computer_name, + const char *sid_str); + +#endif /* SYSDB_COMPUTERS_H_ */ diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 7442f27cca..224841db3c 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -51,6 +51,7 @@ #include "util/util_sss_idmap.h" #include <ndr.h> #include <gen_ndr/security.h> +#include <db/sysdb_computer.h> /* == gpo-ldap constants =================================================== */ @@ -65,6 +66,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 @@ -573,8 +575,10 @@ ad_gpo_dom_sid_equal(const struct dom_sid *sid1, const struct dom_sid *sid2) static errno_t ad_gpo_get_sids(TALLOC_CTX *mem_ctx, const char *user, + const char *ad_hostname, struct sss_domain_info *domain, const char **_user_sid, + const char **_host_sid, const char ***_group_sids, int *_group_size) { @@ -584,6 +588,7 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, int i = 0; int num_group_sids = 0; const char *user_sid = NULL; + const char *host_sid = NULL; const char *group_sid = NULL; const char **group_sids = NULL; @@ -641,6 +646,24 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, *_group_size = num_group_sids + 1; *_group_sids = talloc_steal(mem_ctx, group_sids); *_user_sid = talloc_steal(mem_ctx, user_sid); + + /* Get the cached computer object by computer name */ + if (ad_hostname != NULL) { + static const char *host_attrs[] = { SYSDB_SID_STR, NULL }; + struct ldb_message *msg; + ret = sysdb_get_computer(tmp_ctx, domain, ad_hostname, host_attrs, &msg); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "sysdb_get_computer failed: [%d](%s)\n", + ret, sss_strerror(ret)); + goto done; + } + + /* Get the computer SID from the cached entry */ + host_sid = ldb_msg_find_attr_as_string(msg, SYSDB_SID_STR, NULL); + *_host_sid = talloc_steal(mem_ctx, host_sid); + } + ret = EOK; done: @@ -654,6 +677,7 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, */ static errno_t 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, @@ -662,6 +686,7 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, { int i = 0; struct dom_sid *user_dom_sid; + struct dom_sid *host_dom_sid; struct dom_sid *group_dom_sid; enum idmap_error_code err; bool included = false; @@ -679,6 +704,19 @@ ad_gpo_ace_includes_client_sid(const char *user_sid, return EOK; } + err = sss_idmap_sid_to_smb_sid(idmap_ctx, host_sid, &host_dom_sid); + if (err != IDMAP_SUCCESS) { + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to initialize idmap context.\n"); + return EFAULT; + } + + included = ad_gpo_dom_sid_equal(&ace_dom_sid, host_dom_sid); + sss_idmap_free_smb_sid(idmap_ctx, 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 +766,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, + const char *host_sid, const char **group_sids, int group_size) { @@ -741,8 +780,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 +826,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, + const char *host_sid, const char **group_sids, int group_size, bool *_dacl_access_allowed) @@ -810,7 +851,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) { @@ -838,6 +879,7 @@ 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 *ad_hostname, struct sss_domain_info *domain, struct sss_idmap_ctx *idmap_ctx, struct gp_gpo **candidate_gpos, @@ -852,6 +894,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; + const char *host_sid = NULL; const char **group_sids = NULL; int group_size = 0; int gpo_dn_idx = 0; @@ -864,8 +907,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, goto done; } - ret = ad_gpo_get_sids(tmp_ctx, user, domain, &user_sid, - &group_sids, &group_size); + ret = ad_gpo_get_sids(tmp_ctx, user, ad_hostname, domain, &user_sid, + &host_sid, &group_sids, &group_size); if (ret != EOK) { ret = ERR_NO_SIDS; DEBUG(SSSDBG_OP_FAILURE, @@ -927,8 +970,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, idmap_ctx, 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; @@ -1388,7 +1431,7 @@ ad_gpo_access_check(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, " denied_sids[%d] = %s\n", j, denied_sids[j]); } - ret = ad_gpo_get_sids(mem_ctx, user, domain, &user_sid, + ret = ad_gpo_get_sids(mem_ctx, user, NULL, domain, &user_sid, NULL, &group_sids, &group_size); if (ret != EOK) { ret = ERR_NO_SIDS; @@ -1617,6 +1660,7 @@ 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, @@ -1924,6 +1968,9 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) struct sysdb_attrs **reply; const char *target_dn = NULL; uint32_t uac; + char *filter = NULL; + char *domain_dn; + const char *attrs[] = {AD_AT_SID, NULL}; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct ad_gpo_access_state); @@ -2008,6 +2055,136 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } + /* 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; + } + + /* Query the computer sid from LDAP, if computer does not exist in cache */ + filter = talloc_asprintf(subreq, SYSDB_COMP_FILTER, 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, req); + ret = EOK; + + done: + + if (ret != EOK) { + tevent_req_error(req, ret); + } + +} + +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; + } + + /* Put the sid string in the sysdb */ + ret = sysdb_set_computer(subreq, state->user_domain, + state->ad_hostname, sid_str); + 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, @@ -2143,7 +2320,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, + ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->ad_hostname, + state->user_domain, state->opts->idmap_ctx->map, candidate_gpos, num_candidate_gpos, &state->dacl_filtered_gpos, 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 44f632ddcc10c0727ec44e0c73e176017f502652 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/6] Test the host sid checking --- src/tests/cmocka/test_ad_gpo.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/tests/cmocka/test_ad_gpo.c b/src/tests/cmocka/test_ad_gpo.c index 0589adcc3d..97f70408a1 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, @@ -286,8 +287,8 @@ 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, + ret = ad_gpo_ace_includes_client_sid(user_sid, host_sid, group_sids, + group_size, ace_dom_sid, idmap_ctx, &includes_client_sid); talloc_free(idmap_ctx); @@ -305,13 +306,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 +322,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 +382,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. */ From 17181afedc225a99dd836feaccad29cf052ed03d Mon Sep 17 00:00:00 2001 From: Samuel Cabrero <scabr...@suse.de> Date: Tue, 5 Nov 2019 19:05:36 +0100 Subject: [PATCH 3/6] AD: Improve host SID retrieval Set the entry expire time for cached computers and avoid querying twice the cache by passing the host SID in the processing state if it is found the first time. Signed-off-by: Samuel Cabrero <scabr...@suse.de> --- src/db/sysdb_computer.c | 27 ++++++++- src/db/sysdb_computer.h | 4 +- src/providers/ad/ad_gpo.c | 119 ++++++++++++++++++++++---------------- 3 files changed, 97 insertions(+), 53 deletions(-) diff --git a/src/db/sysdb_computer.c b/src/db/sysdb_computer.c index e3db01ac63..9fcaf5a7c3 100644 --- a/src/db/sysdb_computer.c +++ b/src/db/sysdb_computer.c @@ -120,7 +120,9 @@ int sysdb_set_computer(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *computer_name, - const char *sid_str) + const char *sid_str, + int cache_timeout, + time_t now) { TALLOC_CTX *tmp_ctx; int ret; @@ -147,13 +149,32 @@ sysdb_set_computer(TALLOC_CTX *mem_ctx, if (ret) goto done; /* creation time */ - ret = sysdb_attrs_add_time_t(attrs, SYSDB_CREATE_TIME, time(NULL)); + ret = sysdb_attrs_add_time_t(attrs, SYSDB_CREATE_TIME, now); if (ret) goto done; + /* Set a cache expire time. There is a periodic task that cleans up + * expired entries from the cache even when enumeration is disabled */ + ret = sysdb_attrs_add_time_t(attrs, SYSDB_CACHE_EXPIRE, + cache_timeout ? (now + cache_timeout) : 0); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Could not set sysdb cache expire [%d]: %s\n", + ret, strerror(ret)); + goto done; + } + ret = sysdb_store_custom(domain, computer_name, COMPUTERS_SUBDIR, attrs); if (ret) goto done; - + /* FIXME As a future improvement we have to extend domain enumeration. + * When 'enumerate = true' for a domain, sssd starts a periodic task + * that brings all users and groups to the cache, cleaning up + * stale objects after each run. If enumeration is disabled, the cleanup + * task for expired entries is started instead. + * + * We have to extend the enumeration task to fetch 'computer' + * objects as well (see ad_id_enumeration_send, the entry point of the + * enumeration task for the id provider). + */ done: if (ret) { DEBUG(SSSDBG_TRACE_FUNC, "Error: %d (%s)\n", ret, strerror(ret)); diff --git a/src/db/sysdb_computer.h b/src/db/sysdb_computer.h index 7c937003d1..4be67fdf51 100644 --- a/src/db/sysdb_computer.h +++ b/src/db/sysdb_computer.h @@ -44,6 +44,8 @@ int sysdb_set_computer(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *computer_name, - const char *sid_str); + const char *sid_str, + int cache_timeout, + time_t now); #endif /* SYSDB_COMPUTERS_H_ */ diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 224841db3c..6441e69a68 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -575,10 +575,8 @@ ad_gpo_dom_sid_equal(const struct dom_sid *sid1, const struct dom_sid *sid2) static errno_t ad_gpo_get_sids(TALLOC_CTX *mem_ctx, const char *user, - const char *ad_hostname, struct sss_domain_info *domain, const char **_user_sid, - const char **_host_sid, const char ***_group_sids, int *_group_size) { @@ -588,7 +586,6 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, int i = 0; int num_group_sids = 0; const char *user_sid = NULL; - const char *host_sid = NULL; const char *group_sid = NULL; const char **group_sids = NULL; @@ -646,24 +643,6 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx, *_group_size = num_group_sids + 1; *_group_sids = talloc_steal(mem_ctx, group_sids); *_user_sid = talloc_steal(mem_ctx, user_sid); - - /* Get the cached computer object by computer name */ - if (ad_hostname != NULL) { - static const char *host_attrs[] = { SYSDB_SID_STR, NULL }; - struct ldb_message *msg; - ret = sysdb_get_computer(tmp_ctx, domain, ad_hostname, host_attrs, &msg); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "sysdb_get_computer failed: [%d](%s)\n", - ret, sss_strerror(ret)); - goto done; - } - - /* Get the computer SID from the cached entry */ - host_sid = ldb_msg_find_attr_as_string(msg, SYSDB_SID_STR, NULL); - *_host_sid = talloc_steal(mem_ctx, host_sid); - } - ret = EOK; done: @@ -879,7 +858,7 @@ 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 *ad_hostname, + const char *host_sid, struct sss_domain_info *domain, struct sss_idmap_ctx *idmap_ctx, struct gp_gpo **candidate_gpos, @@ -894,7 +873,6 @@ 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; - const char *host_sid = NULL; const char **group_sids = NULL; int group_size = 0; int gpo_dn_idx = 0; @@ -907,8 +885,8 @@ ad_gpo_filter_gpos_by_dacl(TALLOC_CTX *mem_ctx, goto done; } - ret = ad_gpo_get_sids(tmp_ctx, user, ad_hostname, domain, &user_sid, - &host_sid, &group_sids, &group_size); + ret = ad_gpo_get_sids(tmp_ctx, user, domain, &user_sid, + &group_sids, &group_size); if (ret != EOK) { ret = ERR_NO_SIDS; DEBUG(SSSDBG_OP_FAILURE, @@ -1431,7 +1409,7 @@ ad_gpo_access_check(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, " denied_sids[%d] = %s\n", j, denied_sids[j]); } - ret = ad_gpo_get_sids(mem_ctx, user, NULL, domain, &user_sid, NULL, + ret = ad_gpo_get_sids(mem_ctx, user, domain, &user_sid, &group_sids, &group_size); if (ret != EOK) { ret = ERR_NO_SIDS; @@ -1645,6 +1623,7 @@ struct ad_gpo_access_state { const char *user; int gpo_timeout_option; const char *ad_hostname; + const char *host_sid; const char *target_dn; struct gp_gpo **dacl_filtered_gpos; int num_dacl_filtered_gpos; @@ -1971,6 +1950,8 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) char *filter = NULL; char *domain_dn; const char *attrs[] = {AD_AT_SID, NULL}; + struct ldb_message *msg; + static const char *host_attrs[] = { SYSDB_SID_STR, NULL }; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct ad_gpo_access_state); @@ -2055,36 +2036,70 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } - /* 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; - } + /* 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 */ + /* 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; + } - /* Query the computer sid from LDAP, if computer does not exist in cache */ - filter = talloc_asprintf(subreq, SYSDB_COMP_FILTER, state->ad_hostname); - if (!filter) { - ret = ENOMEM; + filter = talloc_asprintf(subreq, SYSDB_COMP_FILTER, 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 = 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; } - 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); + /* 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); + talloc_steal(state, state->host_sid); + 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->host_domain->name); if (subreq == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "sdap_get_generic_send failed.\n"); - ret = EIO; + ret = ENOMEM; goto done; } - tevent_req_set_callback(subreq, ad_gpo_get_host_sid_retrieval_done, req); + tevent_req_set_callback(subreq, ad_gpo_process_som_done, req); + ret = EOK; done: @@ -2174,10 +2189,16 @@ static void ad_gpo_get_host_sid_retrieval_done(struct tevent_req *subreq) ret, sss_strerror(ret)); goto done; } + state->host_sid = talloc_steal(state, sid_str); /* Put the sid string in the sysdb */ + /* FIXME Using the same timeout as user cache objects. We should create + * a specific setting, check autofsmap_timeout or ssh_host_timeout for + * example */ ret = sysdb_set_computer(subreq, state->user_domain, - state->ad_hostname, sid_str); + state->ad_hostname, state->host_sid, + state->user_domain->user_timeout, + time(NULL)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_set_computer failed: [%d](%s)\n", @@ -2320,7 +2341,7 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) goto done; } - ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->ad_hostname, + ret = ad_gpo_filter_gpos_by_dacl(state, state->user, state->host_sid, state->user_domain, state->opts->idmap_ctx->map, candidate_gpos, num_candidate_gpos, From 3eb30b6ce3a6b9de859fbf6ba1cef5ee652febe5 Mon Sep 17 00:00:00 2001 From: David Mulder <dmul...@suse.com> Date: Wed, 4 Dec 2019 17:54:13 +0000 Subject: [PATCH 4/6] Remove sssd Security Filtering host comment from man Remove the sssd-ad man page comment explaining that host entries in GPO Security Filtering is not supported. --- src/man/sssd-ad.5.xml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml index fdcb4e4b90..53d945408c 100644 --- a/src/man/sssd-ad.5.xml +++ b/src/man/sssd-ad.5.xml @@ -406,13 +406,6 @@ DOM:dom1:(memberOf:1.2.840.113556.1.4.1941:=cn=nestedgroup,ou=groups,dc=example, the Authenticated Users group permissions on the GPO always apply also to the user. </para> - <para> - NOTE: The current version of SSSD does not support - host (computer) entries in the GPO 'Security - Filtering' list. Only user and group entries are - supported. Host entries in the list have no - effect. - </para> <para> NOTE: If the operation mode is set to enforcing, it is possible that users that were previously allowed From de07821e1bf37614ce5f510414dd4ec10e04be24 Mon Sep 17 00:00:00 2001 From: David Mulder <dmul...@suse.com> Date: Thu, 5 Dec 2019 16:01:56 +0000 Subject: [PATCH 5/6] Create a computer_timeout for caching GPO security filter --- src/confdb/confdb.c | 11 +++++++++++ src/confdb/confdb.h | 2 ++ src/config/cfg_rules.ini | 1 + src/man/sssd.conf.5.xml | 13 +++++++++++++ src/providers/ad/ad_gpo.c | 5 +---- 5 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index be65310dcc..6ec09116dc 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -1228,6 +1228,17 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, goto done; } + /* Override the computer timeout, if specified */ + ret = get_entry_as_uint32(res->msgs[0], &domain->computer_timeout, + CONFDB_DOMAIN_COMPUTER_CACHE_TIMEOUT, + entry_cache_timeout); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Invalid value for [%s]\n", + CONFDB_DOMAIN_COMPUTER_CACHE_TIMEOUT); + goto done; + } + /* Set refresh_expired_interval, if specified */ ret = get_entry_as_uint32(res->msgs[0], &domain->refresh_expired_interval, CONFDB_DOMAIN_REFRESH_EXPIRED_INTERVAL, diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 2560efc767..25995998af 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -226,6 +226,7 @@ #define CONFDB_DOMAIN_AUTOFS_CACHE_TIMEOUT "entry_cache_autofs_timeout" #define CONFDB_DOMAIN_SUDO_CACHE_TIMEOUT "entry_cache_sudo_timeout" #define CONFDB_DOMAIN_SSH_HOST_CACHE_TIMEOUT "entry_cache_ssh_host_timeout" +#define CONFDB_DOMAIN_COMPUTER_CACHE_TIMEOUT "entry_cache_computer_timeout" #define CONFDB_DOMAIN_PWD_EXPIRATION_WARNING "pwd_expiration_warning" #define CONFDB_DOMAIN_REFRESH_EXPIRED_INTERVAL "refresh_expired_interval" #define CONFDB_DOMAIN_OFFLINE_TIMEOUT "offline_timeout" @@ -369,6 +370,7 @@ struct sss_domain_info { uint32_t autofsmap_timeout; uint32_t sudo_timeout; uint32_t ssh_host_timeout; + uint32_t computer_timeout; uint32_t refresh_expired_interval; uint32_t subdomain_refresh_interval; diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 0f3d2045d0..4463592aa4 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -401,6 +401,7 @@ option = entry_cache_service_timeout option = entry_cache_autofs_timeout option = entry_cache_sudo_timeout option = entry_cache_ssh_host_timeout +option = entry_cache_computer_timeout option = refresh_expired_interval # Dynamic DNS updates diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 1a8914ded5..4573bed7d5 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -2191,6 +2191,19 @@ p11_uri = library-description=OpenSC%20smartcard%20framework;slot-id=2 </listitem> </varlistentry> + <varlistentry> + <term>entry_cache_computer_timeout (integer)</term> + <listitem> + <para> + How many seconds to keep the local computer + entry before asking the backend again + </para> + <para> + Default: entry_cache_timeout + </para> + </listitem> + </varlistentry> + <varlistentry> <term>refresh_expired_interval (integer)</term> <listitem> diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 6441e69a68..90e1909f83 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -2192,12 +2192,9 @@ static void ad_gpo_get_host_sid_retrieval_done(struct tevent_req *subreq) state->host_sid = talloc_steal(state, sid_str); /* Put the sid string in the sysdb */ - /* FIXME Using the same timeout as user cache objects. We should create - * a specific setting, check autofsmap_timeout or ssh_host_timeout for - * example */ ret = sysdb_set_computer(subreq, state->user_domain, state->ad_hostname, state->host_sid, - state->user_domain->user_timeout, + state->user_domain->computer_timeout, time(NULL)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, From 0ea7661eeb7783c45d7f0ec9d19d99ce9fe407cf Mon Sep 17 00:00:00 2001 From: David Mulder <dmul...@suse.com> Date: Fri, 10 Jan 2020 18:21:05 +0000 Subject: [PATCH 6/6] Resolve computer lookup failure when sam!=cn --- src/providers/ad/ad_gpo.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 90e1909f83..6dd850cc97 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1947,7 +1947,6 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) struct sysdb_attrs **reply; const char *target_dn = NULL; uint32_t uac; - char *filter = NULL; char *domain_dn; const char *attrs[] = {AD_AT_SID, NULL}; struct ldb_message *msg; @@ -2050,16 +2049,10 @@ ad_gpo_target_dn_retrieval_done(struct tevent_req *subreq) goto done; } - filter = talloc_asprintf(subreq, SYSDB_COMP_FILTER, 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->target_dn, LDAP_SCOPE_BASE, + "(&)", attrs, NULL, 0, state->timeout, false);
_______________________________________________ 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