----- Original Message -----
> On Thu, Jul 24, 2014 at 06:53:24AM -0400, Yassir Elley wrote:
> > Hi,
> >
> > The attached patch adds support for an "entry_cache_gpo_timeout" option,
> > which allows an admin to specify how many seconds the backend should
> > consider locally stored gpo policy files to be valid before asking the
> > backend again. This is an additional performance enhancement (along with
> > comparing gpo versions to avoid unnecessary downloads). The timeout is set
> > when a gpo entry is stored in the cache (which only happens after policy
> > files have been downloaded).
> >
> > During the processing of a gpo, if the gpo's cache timeout has not elapsed,
> > the backend does not interact with the gpo_child at all (and therefore
> > doesn't download any files from the smb server). Rather, it re-uses the
> > policy files that are already stored locally. Obviously, no gpo version
> > comparisons need to be made in this case. If the timeout has elapsed (or
> > there is no gpo entry in the cache), then the backend does interact with
> > the gpo child in the usual manner (including comparing gpo versions to
> > avoid unnecessary downloads). Note that the entry_cache_gpo_timeout option
> > is *not* an expiration for the data in the gpo cache entry (which includes
> > the cached_gpt_version). Indeed, the cached_gpt_version is never
> > considered to be "expired".
> >
> > Note that this patch does not add support for offline mode, which will be
> > implemented in a subsequent patch.
> >
> > Regards,
> > Yassir.
>
> Hi,
>
> I was wondering if it's a good idea to introduce the option as a global
> one, present in sssd.conf or a sssd-ad option? See for instance
> ipa_hbac_refresh in man sssd-ipa, I think that's quite natural
> counterpart.
>
> Also, the default timeout is way too big, for access control, I think we
> should go with a lower timeout, the one used is for identity lookups
> only where we care more about speed than precision.
>
> Some more comments inline:
>
> > From 8aff4445df5cf42e09cd8b521cbd5ff4d6e1a666 Mon Sep 17 00:00:00 2001
> > From: Yassir Elley <[email protected]>
> > Date: Tue, 22 Jul 2014 14:19:35 -0400
> > Subject: [PATCH] AD-GPO: add entry_cache_gpo_timeout option
> >
> > ---
> > src/confdb/confdb.c | 11 ++
> > src/confdb/confdb.h | 2 +
> > src/config/SSSDConfig/__init__.py.in | 1 +
> > src/config/SSSDConfigTest.py | 2 +
> > src/config/etc/sssd.api.conf | 1 +
> > src/db/sysdb.h | 14 +-
> > src/db/sysdb_gpo.c | 46 +++++-
> > src/man/sssd.conf.5.xml | 13 ++
>
> [...]
>
> > @@ -1305,6 +1311,12 @@ ad_gpo_cse_step(struct tevent_req *req)
> > struct tevent_req *subreq;
> > struct ad_gpo_access_state *state;
> > int i = 0;
> > + struct ldb_result *res;
> > + errno_t ret;
> > + bool found_in_cache = false;
> > + bool send_to_child = true;
> > + int cached_gpt_version = 0;
> > + time_t policy_file_timeout = 0;
> >
> > state = tevent_req_data(req, struct ad_gpo_access_state);
> >
> > @@ -1327,14 +1339,69 @@ ad_gpo_cse_step(struct tevent_req *req)
> > DEBUG(SSSDBG_TRACE_FUNC, "smb_path: %s\n",
> > cse_filtered_gpo->smb_path);
> > DEBUG(SSSDBG_TRACE_FUNC, "gpo_guid: %s\n",
> > cse_filtered_gpo->gpo_guid);
> >
> > + /* retrieve gpo cache entry; set cached_gpt_version to -1 if
> > unavailable */
> > + DEBUG(SSSDBG_TRACE_FUNC, "retrieving GPO from cache [%s]\n",
> > + cse_filtered_gpo->gpo_guid);
> > + ret = sysdb_gpo_get_gpo_by_guid(state,
> > + state->domain,
> > + cse_filtered_gpo->gpo_guid,
> > + &res);
> > + if (ret == EOK) {
> > + found_in_cache = true;
> > + } else {
> > + switch (ret) {
> > + case ENOENT:
> > + DEBUG(SSSDBG_TRACE_FUNC, "ENOENT\n");
> > + cached_gpt_version = -1;
> > + break;
> > + default:
> > + DEBUG(SSSDBG_FATAL_FAILURE, "Could not read GPO from cache:
> > [%s]\n",
> > + strerror(ret));
> > + return ret;
> > + }
> > + }
>
> I think the above is too complex, what about:
>
> if (ret == EOK) {
> /*
> * Note: if the timeout is valid, then we can later avoid
> downloading
> * the GPT.INI file, as well as any policy files (i.e. we don't
> need
> * to interact with the gpo_child at all). However, even if the
> timeout
> * is not valid, while we will have to interact with the gpo child
> to
> * download the GPT.INI file, we may still be able to avoid
> downloading
> * the policy files (if the cached_gpt_version is the same as the
> * GPT.INI version). In other words, the timeout is *not* an
> expiration
> * for the entire cache entry; the cached_gpt_version never
> expires.
> _ */
> cached_gpt_version = ldb_msg_find_attr_as_int(res->msgs[0],
>
> SYSDB_GPO_VERSION_ATTR,
> 0);
>
>
>
> policy_file_timeout = ldb_msg_find_attr_as_uint64
> (res->msgs[0], SYSDB_GPO_TIMEOUT_ATTR, 0);
>
>
>
> if (policy_file_timeout >= time(NULL)) {
> send_to_child = false;
> }
> } else if (ret == ENOENT) {
> DEBUG(SSSDBG_TRACE_FUNC, "ENOENT\n");
> cached_gpt_version = -1;
> } else {
> DEBUG(SSSDBG_FATAL_FAILURE, "Could not read GPO from cache: [%s]\n",
> strerror(ret));
> return ret;
> }
>
> That way you don't need the found_in_cache variable at all.
>
> > +
> > + DEBUG(SSSDBG_TRACE_FUNC, "found_in_cache: %d\n", found_in_cache);
> > + if (found_in_cache) {
> > +
> > + /*
> > + * Note: if the timeout is valid, then we can later avoid
> > downloading
> > + * the GPT.INI file, as well as any policy files (i.e. we don't
> > need
> > + * to interact with the gpo_child at all). However, even if the
> > timeout
> > + * is not valid, while we will have to interact with the gpo child
> > to
> > + * download the GPT.INI file, we may still be able to avoid
> > downloading
> > + * the policy files (if the cached_gpt_version is the same as the
> > + * GPT.INI version). In other words, the timeout is *not* an
> > expiration
> > + * for the entire cache entry; the cached_gpt_version never
> > expires.
> > + */
> > +
> > + cached_gpt_version = ldb_msg_find_attr_as_int(res->msgs[0],
> > +
> > SYSDB_GPO_VERSION_ATTR,
> > + 0);
> > +
> > + policy_file_timeout = ldb_msg_find_attr_as_uint64
> > + (res->msgs[0], SYSDB_GPO_TIMEOUT_ATTR, 0);
> > +
> > + if (policy_file_timeout >= time(NULL)) {
> > + send_to_child = false;
> > + }
> > + }
> > +
> > + DEBUG(SSSDBG_TRACE_FUNC, "send_to_child: %d\n", send_to_child);
> > + DEBUG(SSSDBG_TRACE_FUNC, "cached_gpt_version: %d\n",
> > cached_gpt_version);
> > +
> > + cse_filtered_gpo->send_to_child = send_to_child;
> > +
> > subreq = ad_gpo_process_cse_send(state,
> > state->ev,
> > - state->domain,
> > + send_to_child,
> > + state->domain->name,
> > + cse_filtered_gpo->gpo_guid,
> > cse_filtered_gpo->smb_server,
> > cse_filtered_gpo->smb_share,
> > cse_filtered_gpo->smb_path,
> > GP_EXT_GUID_SECURITY_SUFFIX,
> > - cse_filtered_gpo->gpo_guid);
> > + cached_gpt_version);
> >
> > tevent_req_set_callback(subreq, ad_gpo_cse_done, req);
> > return EAGAIN;
> > @@ -1356,10 +1423,12 @@ ad_gpo_cse_done(struct tevent_req *subreq)
> > struct ad_gpo_access_state *state;
> > int ret;
> > int sysvol_gpt_version;
> > + const char *policy_filename = NULL;
> > char **allowed_sids;
> > int allowed_size;
> > char **denied_sids;
> > int denied_size;
> > + time_t now;
> >
> > req = tevent_req_callback_data(subreq, struct tevent_req);
> > state = tevent_req_data(req, struct ad_gpo_access_state);
> > @@ -1372,8 +1441,7 @@ ad_gpo_cse_done(struct tevent_req *subreq)
> > DEBUG(SSSDBG_TRACE_FUNC, "gpo_guid: %s\n", gpo_guid);
> >
> > ret = ad_gpo_process_cse_recv(subreq, state, &sysvol_gpt_version,
> > - &allowed_size, &allowed_sids,
> > - &denied_size, &denied_sids);
> > + &policy_filename);
> >
> > talloc_zfree(subreq);
> >
> > @@ -1383,17 +1451,35 @@ ad_gpo_cse_done(struct tevent_req *subreq)
> > goto done;
> > }
> >
> > - DEBUG(SSSDBG_TRACE_FUNC, "sysvol_gpt_version: %d\n",
> > sysvol_gpt_version);
> > + /* only store to cache (and refresh timeout) if we interacted with
> > child */
> > + if (cse_filtered_gpo->send_to_child) {
> >
> > - ret = sysdb_gpo_store_gpo(state->domain, gpo_guid,
> > sysvol_gpt_version);
> > + DEBUG(SSSDBG_TRACE_FUNC, "sysvol_gpt_version: %d\n",
> > sysvol_gpt_version);
> > +
> > + now = time(NULL);
> > + ret = sysdb_gpo_store_gpo(state->domain, gpo_guid,
> > sysvol_gpt_version,
> > + state->domain->gpo_timeout, now);
> > + if (ret != EOK) {
> > + DEBUG(SSSDBG_OP_FAILURE, "Unable to store gpo cache entry:
> > [%d](%s}\n",
> > + ret, sss_strerror(ret));
> > + goto done;
> > + }
> > + }
>
> What about moving the above to the ad_gpo_process_cse* request? That way
> you could always store the GPO data after the child has finished and could
> avoid branching in the caller.
>
> The rest looks good to me and the patches seem to work fine.
I agree with all your comments and have attached a revised patch. With regard
to the timeout option, I have changed this to be an sssd-ad option. I have
changed the name to ad_gpo_cache_timeout and have modelled it after
ipa_hbac_refresh (including the 5 second default timeout).
Regards,
Yassir.
From 7e38602a15bb5d430cc3d5efcc91ef76d3a58a2e Mon Sep 17 00:00:00 2001
From: Yassir Elley <[email protected]>
Date: Tue, 22 Jul 2014 14:19:35 -0400
Subject: [PATCH] AD-GPO: add ad_gpo_cache_timeout option
---
src/config/SSSDConfig/__init__.py.in | 1 +
src/config/etc/sssd.api.d/sssd-ad.conf | 1 +
src/db/sysdb.h | 14 +-
src/db/sysdb_gpo.c | 46 +++++-
src/man/sssd-ad.5.xml | 16 ++
src/providers/ad/ad_access.h | 1 +
src/providers/ad/ad_common.h | 1 +
src/providers/ad/ad_gpo.c | 281 +++++++++++++++++++++------------
src/providers/ad/ad_init.c | 6 +
src/providers/ad/ad_opts.h | 1 +
10 files changed, 260 insertions(+), 108 deletions(-)
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 91f94b524b8185be96d701271443ee7a9ab67ee4..c3482b3a4df6d0fd39b2fa93c19ee5316dde0d94 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -169,6 +169,7 @@ option_strings = {
'ad_access_filter' : _('LDAP filter to determine access privileges'),
'ad_enable_gc' : _('Whether to use the Global Catalog for lookups'),
'ad_gpo_access_control' : _('Operation mode for GPO-based access control'),
+ 'ad_gpo_cache_timeout' : _("The amount of time between lookups of the GPO policy files against the AD server"),
# [provider/krb5]
'krb5_kdcip' : _('Kerberos server address'),
diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf
index 7055c41be9ba3616459fa6683d310fa24c073a18..93d869c67f96afbea2e39d061daf578602b2f9a9 100644
--- a/src/config/etc/sssd.api.d/sssd-ad.conf
+++ b/src/config/etc/sssd.api.d/sssd-ad.conf
@@ -7,6 +7,7 @@ ad_enable_dns_sites = bool, None, false
ad_access_filter = str, None, false
ad_enable_gc = bool, None, false
ad_gpo_access_control = str, None, false
+ad_gpo_cache_timeout = int, None, false
ldap_uri = str, None, false
ldap_backup_uri = str, None, false
ldap_search_base = str, None, false
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 63f596007cdf53c5ebbdccf37f3c4bcc7830a205..addf13196352825009a4a4a0ebaed7e343c72a14 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -873,6 +873,7 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
#define SYSDB_GPO_FILTER "(&(objectClass="SYSDB_GPO_OC")("SYSDB_GPO_GUID_ATTR"=%s))"
#define SYSDB_GPO_GUID_ATTR "gpoGUID"
#define SYSDB_GPO_VERSION_ATTR "gpoVersion"
+#define SYSDB_GPO_TIMEOUT_ATTR "gpoPolicyFileTimeout"
#define SYSDB_TMPL_GPO_BASE SYSDB_GPO_CONTAINER","SYSDB_DOM_BASE
#define SYSDB_TMPL_GPO SYSDB_GPO_GUID_ATTR"=%s,"SYSDB_TMPL_GPO_BASE
@@ -881,15 +882,18 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
SYSDB_NAME, \
SYSDB_GPO_GUID_ATTR, \
SYSDB_GPO_VERSION_ATTR, \
+ SYSDB_GPO_TIMEOUT_ATTR, \
NULL }
errno_t sysdb_gpo_store_gpo(struct sss_domain_info *domain,
const char *gpo_guid,
- int gpo_version);
+ int gpo_version,
+ int cache_timeout,
+ time_t now);
-errno_t sysdb_gpo_get_gpo(TALLOC_CTX *mem_ctx,
- struct sss_domain_info *domain,
- const char *gpo_guid,
- struct ldb_result **_result);
+errno_t sysdb_gpo_get_gpo_by_guid(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ const char *gpo_guid,
+ struct ldb_result **_result);
#endif /* __SYS_DB_H__ */
diff --git a/src/db/sysdb_gpo.c b/src/db/sysdb_gpo.c
index 3c23c5b8fae6aaa41168b1466f4b3e520231e7e2..228f131ace761909c98e0c02fffc889599f86c07 100644
--- a/src/db/sysdb_gpo.c
+++ b/src/db/sysdb_gpo.c
@@ -49,7 +49,9 @@ sysdb_gpo_dn(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain,
errno_t
sysdb_gpo_store_gpo(struct sss_domain_info *domain,
const char *gpo_guid,
- int gpo_version)
+ int gpo_version,
+ int cache_timeout,
+ time_t now)
{
errno_t ret, sret;
int lret;
@@ -81,6 +83,10 @@ sysdb_gpo_store_gpo(struct sss_domain_info *domain,
goto done;
}
+ if (!now) {
+ now = time(NULL);
+ }
+
in_transaction = true;
/* Check for an existing gpo_guid entry */
@@ -140,6 +146,21 @@ sysdb_gpo_store_gpo(struct sss_domain_info *domain,
goto done;
}
+ /* Add the Policy File Timeout */
+ lret = ldb_msg_add_empty(update_msg, SYSDB_GPO_TIMEOUT_ATTR,
+ LDB_FLAG_MOD_ADD, NULL);
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+
+ lret = ldb_msg_add_fmt(update_msg, SYSDB_GPO_TIMEOUT_ATTR, "%lu",
+ ((cache_timeout) ? (now + cache_timeout) : 0));
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+
lret = ldb_add(domain->sysdb->ldb, update_msg);
if (lret != LDB_SUCCESS) {
DEBUG(SSSDBG_MINOR_FAILURE,
@@ -170,6 +191,21 @@ sysdb_gpo_store_gpo(struct sss_domain_info *domain,
goto done;
}
+ /* Add the Policy File Timeout */
+ lret = ldb_msg_add_empty(update_msg, SYSDB_GPO_TIMEOUT_ATTR,
+ LDB_FLAG_MOD_REPLACE, NULL);
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+
+ lret = ldb_msg_add_fmt(update_msg, SYSDB_GPO_TIMEOUT_ATTR, "%lu",
+ ((cache_timeout) ? (now + cache_timeout) : 0));
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+
lret = ldb_modify(domain->sysdb->ldb, update_msg);
if (lret != LDB_SUCCESS) {
DEBUG(SSSDBG_MINOR_FAILURE,
@@ -202,10 +238,10 @@ done:
}
errno_t
-sysdb_gpo_get_gpo(TALLOC_CTX *mem_ctx,
- struct sss_domain_info *domain,
- const char *gpo_guid,
- struct ldb_result **_result)
+sysdb_gpo_get_gpo_by_guid(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ const char *gpo_guid,
+ struct ldb_result **_result)
{
errno_t ret;
int lret;
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index d8c423541311922bb6d6eeec32df475a930fd1ba..8d90bc13359b025833b4badde50ec811241fe623 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -317,6 +317,22 @@ FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com)
</varlistentry>
<varlistentry>
+ <term>ad_gpo_cache_timeout (integer)</term>
+ <listitem>
+ <para>
+ The amount of time between lookups of GPO policy
+ files against the AD server. This will reduce the
+ latency and load on the AD server if there are
+ many access-control requests made in a short
+ period.
+ </para>
+ <para>
+ Default: 5 (seconds)
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>dyndns_update (boolean)</term>
<listitem>
<para>
diff --git a/src/providers/ad/ad_access.h b/src/providers/ad/ad_access.h
index 4a9bd40a09472844fe9fa42f7453366264e627fd..60ff5db66c9663e736338cdf7694eb75584c3674 100644
--- a/src/providers/ad/ad_access.h
+++ b/src/providers/ad/ad_access.h
@@ -33,6 +33,7 @@ struct ad_access_ctx {
GPO_ACCESS_CONTROL_PERMISSIVE,
GPO_ACCESS_CONTROL_ENFORCING
} gpo_access_control_mode;
+ int gpo_cache_timeout;
};
void
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index b2949c37fe68add7c44c25258aed45ce6d68c836..c3dc6c1a7362a312262d130ef6188f169bf9b832 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -51,6 +51,7 @@ enum ad_basic_opt {
AD_ACCESS_FILTER,
AD_ENABLE_GC,
AD_GPO_ACCESS_CONTROL,
+ AD_GPO_CACHE_TIMEOUT,
AD_OPTS_BASIC /* opts counter */
};
diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index a650fa13867e40bf8275a646885e229fa44bc57b..c26011d672886faadaf2bb60c4537c2de8bf6532 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -123,6 +123,7 @@ struct gp_gpo {
int num_gpo_cse_guids;
int gpo_func_version;
int gpo_flags;
+ bool send_to_child;
};
enum ace_eval_status {
@@ -157,19 +158,19 @@ int ad_gpo_process_gpo_recv(struct tevent_req *req,
int *num_candidate_gpos);
struct tevent_req *ad_gpo_process_cse_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
+ bool send_to_child,
struct sss_domain_info *domain,
+ const char *gpo_guid,
const char *smb_server,
const char *smb_share,
const char *smb_path,
const char *smb_cse_suffix,
- const char *gpo_guid);
+ int cached_gpt_version,
+ int gpo_timeout_option);
int ad_gpo_process_cse_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
int *_sysvol_gpt_version,
- int *_allowed_size,
- char ***_allowed_sids,
- int *_denied_size,
- char ***_denied_sids);
+ const char **_policy_filename);
/* == ad_gpo_access_send/recv helpers =======================================*/
@@ -871,6 +872,7 @@ struct ad_gpo_access_state {
struct sss_domain_info *domain;
const char *user;
enum gpo_access_control_mode gpo_mode;
+ int gpo_timeout_option;
const char *ad_hostname;
const char *target_dn;
struct gp_gpo **dacl_filtered_gpos;
@@ -886,6 +888,12 @@ static void ad_gpo_process_som_done(struct tevent_req *subreq);
static void ad_gpo_process_gpo_done(struct tevent_req *subreq);
static errno_t ad_gpo_cse_step(struct tevent_req *req);
+static errno_t ad_gpo_parse_policy_file(TALLOC_CTX *mem_ctx,
+ const char *filename,
+ char ***allowed_sids,
+ int *allowed_size,
+ char ***denied_sids,
+ int *denied_size);
static void ad_gpo_cse_done(struct tevent_req *subreq);
struct tevent_req *
@@ -921,6 +929,7 @@ ad_gpo_access_send(TALLOC_CTX *mem_ctx,
state->user = user;
state->ldb_ctx = sysdb_ctx_get_ldb(domain->sysdb);
state->gpo_mode = ctx->gpo_access_control_mode;
+ state->gpo_timeout_option = ctx->gpo_cache_timeout;
state->ad_hostname = dp_opt_get_string(ctx->ad_options, AD_HOSTNAME);
state->opts = ctx->sdap_access_ctx->id_ctx->opts;
state->timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
@@ -1310,6 +1319,11 @@ ad_gpo_cse_step(struct tevent_req *req)
struct tevent_req *subreq;
struct ad_gpo_access_state *state;
int i = 0;
+ struct ldb_result *res;
+ errno_t ret;
+ bool send_to_child = true;
+ int cached_gpt_version = 0;
+ time_t policy_file_timeout = 0;
state = tevent_req_data(req, struct ad_gpo_access_state);
@@ -1332,14 +1346,60 @@ ad_gpo_cse_step(struct tevent_req *req)
DEBUG(SSSDBG_TRACE_FUNC, "smb_path: %s\n", cse_filtered_gpo->smb_path);
DEBUG(SSSDBG_TRACE_FUNC, "gpo_guid: %s\n", cse_filtered_gpo->gpo_guid);
+ /* retrieve gpo cache entry; set cached_gpt_version to -1 if unavailable */
+ DEBUG(SSSDBG_TRACE_FUNC, "retrieving GPO from cache [%s]\n",
+ cse_filtered_gpo->gpo_guid);
+ ret = sysdb_gpo_get_gpo_by_guid(state,
+ state->domain,
+ cse_filtered_gpo->gpo_guid,
+ &res);
+ if (ret == EOK) {
+ /*
+ * Note: if the timeout is valid, then we can later avoid downloading
+ * the GPT.INI file, as well as any policy files (i.e. we don't need
+ * to interact with the gpo_child at all). However, even if the timeout
+ * is not valid, while we will have to interact with the gpo child to
+ * download the GPT.INI file, we may still be able to avoid downloading
+ * the policy files (if the cached_gpt_version is the same as the
+ * GPT.INI version). In other words, the timeout is *not* an expiration
+ * for the entire cache entry; the cached_gpt_version never expires.
+ */
+
+ cached_gpt_version = ldb_msg_find_attr_as_int(res->msgs[0],
+ SYSDB_GPO_VERSION_ATTR,
+ 0);
+
+ policy_file_timeout = ldb_msg_find_attr_as_uint64
+ (res->msgs[0], SYSDB_GPO_TIMEOUT_ATTR, 0);
+
+ if (policy_file_timeout >= time(NULL)) {
+ send_to_child = false;
+ }
+ } else if (ret == ENOENT) {
+ DEBUG(SSSDBG_TRACE_FUNC, "ENOENT\n");
+ cached_gpt_version = -1;
+ } else {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Could not read GPO from cache: [%s]\n",
+ strerror(ret));
+ return ret;
+ }
+
+ DEBUG(SSSDBG_TRACE_FUNC, "send_to_child: %d\n", send_to_child);
+ DEBUG(SSSDBG_TRACE_FUNC, "cached_gpt_version: %d\n", cached_gpt_version);
+
+ cse_filtered_gpo->send_to_child = send_to_child;
+
subreq = ad_gpo_process_cse_send(state,
state->ev,
+ send_to_child,
state->domain,
+ cse_filtered_gpo->gpo_guid,
cse_filtered_gpo->smb_server,
cse_filtered_gpo->smb_share,
cse_filtered_gpo->smb_path,
GP_EXT_GUID_SECURITY_SUFFIX,
- cse_filtered_gpo->gpo_guid);
+ cached_gpt_version,
+ state->gpo_timeout_option);
tevent_req_set_callback(subreq, ad_gpo_cse_done, req);
return EAGAIN;
@@ -1361,6 +1421,7 @@ ad_gpo_cse_done(struct tevent_req *subreq)
struct ad_gpo_access_state *state;
int ret;
int sysvol_gpt_version;
+ const char *policy_filename = NULL;
char **allowed_sids;
int allowed_size;
char **denied_sids;
@@ -1377,8 +1438,7 @@ ad_gpo_cse_done(struct tevent_req *subreq)
DEBUG(SSSDBG_TRACE_FUNC, "gpo_guid: %s\n", gpo_guid);
ret = ad_gpo_process_cse_recv(subreq, state, &sysvol_gpt_version,
- &allowed_size, &allowed_sids,
- &denied_size, &denied_sids);
+ &policy_filename);
talloc_zfree(subreq);
@@ -1388,17 +1448,20 @@ ad_gpo_cse_done(struct tevent_req *subreq)
goto done;
}
- DEBUG(SSSDBG_TRACE_FUNC, "sysvol_gpt_version: %d\n", sysvol_gpt_version);
-
- ret = sysdb_gpo_store_gpo(state->domain, gpo_guid, sysvol_gpt_version);
+ ret = ad_gpo_parse_policy_file(state,
+ policy_filename,
+ &allowed_sids,
+ &allowed_size,
+ &denied_sids,
+ &denied_size);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "Unable to store gpo cache entry: [%d](%s}\n",
- ret, sss_strerror(ret));
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Cannot parse policy file: [%s][%d][%s]\n",
+ policy_filename, ret, strerror(ret));
goto done;
}
- /* TBD: allowed/denied_sids/size, should be retrieved from cache */
ret = ad_gpo_access_check
(state, state->gpo_mode, state->user, state->domain,
allowed_sids, allowed_size, denied_sids, denied_size);
@@ -2387,8 +2450,22 @@ ad_gpo_extract_smb_components(TALLOC_CTX *mem_ctx,
*_smb_server = talloc_asprintf(mem_ctx, "%s%s",
SMB_STANDARD_URI,
server_hostname);
+ if (*_smb_server == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
*_smb_share = talloc_asprintf(mem_ctx, "/%s", smb_share);
+ if (*_smb_share == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
*_smb_path = talloc_asprintf(mem_ctx, "/%s", smb_path);
+ if (*_smb_path == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
ret = EOK;
@@ -3141,8 +3218,13 @@ ad_gpo_parse_gpo_child_response(TALLOC_CTX *mem_ctx,
struct ad_gpo_process_cse_state {
struct tevent_context *ev;
+ struct sss_domain_info *domain;
+ int gpo_timeout_option;
+ const char *gpo_guid;
const char *smb_path;
const char *smb_cse_suffix;
+ int sysvol_gpt_version;
+ const char *policy_filename;
pid_t child_pid;
uint8_t *buf;
ssize_t len;
@@ -3198,19 +3280,20 @@ static void gpo_cse_done(struct tevent_req *subreq);
struct tevent_req *
ad_gpo_process_cse_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
+ bool send_to_child,
struct sss_domain_info *domain,
+ const char *gpo_guid,
const char *smb_server,
const char *smb_share,
const char *smb_path,
const char *smb_cse_suffix,
- const char *gpo_guid)
+ int cached_gpt_version,
+ int gpo_timeout_option)
{
struct tevent_req *req;
struct tevent_req *subreq;
struct ad_gpo_process_cse_state *state;
- struct ldb_result *res;
struct io_buffer *buf = NULL;
- int cached_gpt_version = 0;
errno_t ret;
req = tevent_req_create(mem_ctx, &state, struct ad_gpo_process_cse_state);
@@ -3219,73 +3302,82 @@ ad_gpo_process_cse_send(TALLOC_CTX *mem_ctx,
return NULL;
}
+ state->sysvol_gpt_version = -1;
+
+ if (!send_to_child) {
+ /*
+ * if we don't need to talk to child (b/c cache timeout is valid),
+ * we simply set the policy_filename and complete the request
+ */
+ state->policy_filename =
+ talloc_asprintf(state,
+ GPO_CACHE_PATH"/%s/Policies/%s%s",
+ domain->name,
+ gpo_guid,
+ GP_EXT_GUID_SECURITY_SUFFIX);
+ if (state->policy_filename == NULL) {
+ ret = ENOMEM;
+ goto immediately;
+ }
+
+ DEBUG(SSSDBG_TRACE_FUNC, "policy_filename:%s\n", state->policy_filename);
+ ret = EOK;
+ goto immediately;
+ }
+
state->ev = ev;
state->buf = NULL;
state->len = 0;
+ state->domain = domain;
+ state->gpo_timeout_option = gpo_timeout_option;
+ state->gpo_guid = gpo_guid;
state->smb_path = smb_path;
state->smb_cse_suffix = smb_cse_suffix;
state->io = talloc(state, struct io);
if (state->io == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, "talloc failed.\n");
ret = ENOMEM;
- goto fail;
+ goto immediately;
}
state->io->write_to_child_fd = -1;
state->io->read_from_child_fd = -1;
talloc_set_destructor((void *) state->io, gpo_child_io_destructor);
- /* retrieve cached gpt version (or set it to -1 if unavailable) */
- DEBUG(SSSDBG_TRACE_FUNC, "retrieving GPO from cache [%s]\n", gpo_guid);
- ret = sysdb_gpo_get_gpo(state, domain, gpo_guid, &res);
- if (ret != EOK) {
- switch (ret) {
- case ENOENT:
- DEBUG(SSSDBG_TRACE_FUNC, "ENOENT\n");
- cached_gpt_version = -1;
- break;
- default:
- DEBUG(SSSDBG_FATAL_FAILURE, "Could not read GPO from cache: [%s]\n",
- strerror(ret));
- goto fail;
- }
- }
-
- if (cached_gpt_version != -1) {
- /* cached_gpt_version is in cache, so retrieve it from cache */
- cached_gpt_version = ldb_msg_find_attr_as_int(res->msgs[0],
- SYSDB_GPO_VERSION_ATTR,
- 0);
- }
- DEBUG(SSSDBG_TRACE_FUNC, "cached_gpt_version: %d\n", cached_gpt_version);
-
/* prepare the data to pass to child */
ret = create_cse_send_buffer(state, smb_server, smb_share, smb_path,
smb_cse_suffix, cached_gpt_version, &buf);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "create_cse_send_buffer failed.\n");
- goto fail;
+ goto immediately;
}
ret = gpo_fork_child(req);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "gpo_fork_child failed.\n");
- goto fail;
+ goto immediately;
}
subreq = write_pipe_send(state, ev, buf->data, buf->size,
state->io->write_to_child_fd);
if (subreq == NULL) {
ret = ENOMEM;
- goto fail;
+ goto immediately;
}
tevent_req_set_callback(subreq, gpo_cse_step, req);
return req;
-fail:
- tevent_req_error(req, ret);
- tevent_req_post(req, ev);
+immediately:
+
+ if (ret == EOK) {
+ tevent_req_done(req);
+ tevent_req_post(req, ev);
+ } else {
+ tevent_req_error(req, ret);
+ tevent_req_post(req, ev);
+ }
+
return req;
}
@@ -3321,6 +3413,9 @@ static void gpo_cse_done(struct tevent_req *subreq)
{
struct tevent_req *req;
struct ad_gpo_process_cse_state *state;
+ uint32_t sysvol_gpt_version = -1;
+ uint32_t child_result;
+ time_t now;
req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct ad_gpo_process_cse_state);
@@ -3336,69 +3431,59 @@ static void gpo_cse_done(struct tevent_req *subreq)
close(state->io->read_from_child_fd);
state->io->read_from_child_fd = -1;
- tevent_req_done(req);
- return;
-}
-
-int ad_gpo_process_cse_recv(struct tevent_req *req,
- TALLOC_CTX *mem_ctx,
- int *_sysvol_gpt_version,
- int *_allowed_size,
- char ***_allowed_sids,
- int *_denied_size,
- char ***_denied_sids)
-{
- int ret;
- uint32_t sysvol_gpt_version;
- uint32_t result;
- char **allowed_sids;
- int allowed_size;
- char **denied_sids;
- int denied_size;
- struct ad_gpo_process_cse_state *state;
- const char *policy_filename = NULL;
-
- state = tevent_req_data(req, struct ad_gpo_process_cse_state);
-
- TEVENT_REQ_RETURN_ON_ERROR(req);
-
ret = ad_gpo_parse_gpo_child_response(state, state->buf, state->len,
- &sysvol_gpt_version, &result);
+ &sysvol_gpt_version, &child_result);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"ad_gpo_parse_gpo_child_response failed: [%d][%s]\n",
ret, strerror(ret));
- return ret;
- } else if (result != 0){
+ tevent_req_error(req, ret);
+ } else if (child_result != 0){
DEBUG(SSSDBG_CRIT_FAILURE,
- "Error in gpo_child: [%d][%s]\n", result, strerror(result));
- return result;
+ "Error in gpo_child: [%d][%s]\n",
+ child_result, strerror(child_result));
+ tevent_req_error(req, child_result);
}
- policy_filename = talloc_asprintf(mem_ctx, GPO_CACHE_PATH"%s%s",
- state->smb_path, state->smb_cse_suffix);
-
- DEBUG(SSSDBG_TRACE_FUNC, "policy_filename:%s\n", policy_filename);
+ state->sysvol_gpt_version = sysvol_gpt_version;
+ state->policy_filename = talloc_asprintf(state,
+ GPO_CACHE_PATH"%s%s",
+ state->smb_path,
+ state->smb_cse_suffix);
+ if (state->policy_filename == NULL) {
+ tevent_req_error(req, ENOMEM);
+ }
- ret = ad_gpo_parse_policy_file(state,
- policy_filename,
- &allowed_sids,
- &allowed_size,
- &denied_sids,
- &denied_size);
+ DEBUG(SSSDBG_TRACE_FUNC, "policy_filename:%s\n", state->policy_filename);
+ now = time(NULL);
+ DEBUG(SSSDBG_TRACE_FUNC, "sysvol_gpt_version: %d\n", sysvol_gpt_version);
+ ret = sysdb_gpo_store_gpo(state->domain, state->gpo_guid, sysvol_gpt_version,
+ state->gpo_timeout_option, now);
if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Cannot parse policy file: [%s][%d][%s]\n",
- policy_filename, ret, strerror(ret));
- return ret;
+ DEBUG(SSSDBG_OP_FAILURE, "Unable to store gpo cache entry: [%d](%s}\n",
+ ret, sss_strerror(ret));
+ tevent_req_error(req, ret);
+ return;
}
- *_sysvol_gpt_version = sysvol_gpt_version;
- *_allowed_size = allowed_size;
- *_allowed_sids = talloc_steal(mem_ctx, allowed_sids);
- *_denied_size = denied_size;
- *_denied_sids = talloc_steal(mem_ctx, denied_sids);
+ tevent_req_done(req);
+ return;
+}
+
+int ad_gpo_process_cse_recv(struct tevent_req *req,
+ TALLOC_CTX *mem_ctx,
+ int *_sysvol_gpt_version,
+ const char **_policy_filename)
+{
+ struct ad_gpo_process_cse_state *state;
+
+ state = tevent_req_data(req, struct ad_gpo_process_cse_state);
+
+ TEVENT_REQ_RETURN_ON_ERROR(req);
+
+ *_sysvol_gpt_version = state->sysvol_gpt_version;
+ *_policy_filename = talloc_steal(mem_ctx, state->policy_filename);
return EOK;
}
diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index 0a54d39709a61b39f74e2db04665c0911ab320ed..da3f6c4247f4599908c846d7be4e14b5c0e3d44d 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -371,6 +371,7 @@ sssm_ad_access_init(struct be_ctx *bectx,
struct ad_id_ctx *ad_id_ctx;
const char *filter;
const char *gpo_access_control_mode;
+ int gpo_cache_timeout;
access_ctx = talloc_zero(bectx, struct ad_access_ctx);
if (!access_ctx) return ENOMEM;
@@ -439,6 +440,11 @@ sssm_ad_access_init(struct be_ctx *bectx,
goto fail;
}
+ /* GPO cache timeout */
+ gpo_cache_timeout =
+ dp_opt_get_int(access_ctx->ad_options, AD_GPO_CACHE_TIMEOUT);
+ access_ctx->gpo_cache_timeout = gpo_cache_timeout;
+
*ops = &ad_access_ops;
*pvt_data = access_ctx;
diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index 1f49f9c62247ae1cbed6ec5ec4c52ddb406ee1b6..a3ade012a7c1efb705a011697af36c28e39b1a9b 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -39,6 +39,7 @@ struct dp_option ad_basic_opts[] = {
{ "ad_access_filter", DP_OPT_STRING, NULL_STRING, NULL_STRING},
{ "ad_enable_gc", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
{ "ad_gpo_access_control", DP_OPT_STRING, { "permissive" }, NULL_STRING },
+ { "ad_gpo_cache_timeout", DP_OPT_NUMBER, { .number = 5 }, NULL_NUMBER },
DP_OPTION_TERMINATOR
};
--
1.8.5.3
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
