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 7a4aa7d4daa919cb823c384f58258f235604ebdb 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/3] SSSD should accept host entries from GPO's security
filter
---
Makefile.am | 2 +
src/db/sysdb_computer.c | 165 +++++++++++++++++++++++++++++
src/db/sysdb_computer.h | 49 +++++++++
src/providers/ad/ad_gpo.c | 189 ++++++++++++++++++++++++++++++++--
src/providers/ad/ad_gpo_ndr.c | 2 +-
5 files changed, 397 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..6a7e4a9cf4
--- /dev/null
+++ b/src/db/sysdb_computer.c
@@ -0,0 +1,165 @@
+/*
+ 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;
+ struct ldb_dn *dn;
+
+ 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..1de2e28bc4 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,129 @@ 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;
+ }
+
+ /* 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 +2313,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 5335980eeafcc9f0f30bff05dd38ae03715eb3e8 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/3] 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 c8f51038b311073e90a597dc53143602999d9bb7 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/3] 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 6a7e4a9cf4..e01ac63683 100644
--- a/src/db/sysdb_computer.c
+++ b/src/db/sysdb_computer.c
@@ -121,7 +121,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;
@@ -148,13 +150,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 1de2e28bc4..d0c0e119ed 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:
@@ -2167,10 +2182,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",
@@ -2313,7 +2334,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,
_______________________________________________
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
