From 4ba4b2d96b59386f3fd4d8bb0c4ada4798db48b0 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Mon, 1 Jul 2019 14:15:29 +0200
Subject: [PATCH 44/48] DP/SYSDB: Move the code to set initgrExpireTimestamp to
a reusable function
Related: https://pagure.io/SSSD/sssd/issue/4012
Because the initgroups request can, especially in the case of IPA provider
with trusts, contain several sub-requests that run some provider-specific
initgroups internally and then run post-processing AND because at the same
time concurrent requests in the responder need to be sure that the
initgrExpireTimestamp is only increased when the initgroups request is
really done, we only set the initgrExpireTimestamp in the DP when the
request finishes.
This means, the background refresh task needs to also set the
initgrExpireTimestamp attribute on its own as well. This patch so far
splits the helper function into a reusable one so it can later be used
by the background refresh.
For examples of the bugs caused by the initgrTimestamp being set before
the whole multi-step operation finishes, please see tickets #3744
or #2634.
Reviewed-by: Sumit Bose <sbose@redhat.com>
---
src/db/sysdb.h | 11 ++++
src/db/sysdb_ops.c | 70 ++++++++++++++++++++++
src/providers/data_provider/dp_target_id.c | 55 ++---------------
3 files changed, 85 insertions(+), 51 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 28801e030..56fd770e4 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1113,6 +1113,17 @@ errno_t sysdb_store_override(struct sss_domain_info *domain,
enum sysdb_member_type type,
struct sysdb_attrs *attrs, struct ldb_dn *obj_dn);
+/*
+ * Cache the time of last initgroups invocation. Typically this is not done when
+ * the provider-specific request itself finishes, because currently the request
+ * might hand over to other requests from a different provider (e.g. an AD user
+ * from a trusted domain might need to also call an IPA request to fetch the
+ * external groups). Instead, the caller of the initgroups request, typically
+ * the DP or the periodical refresh task sets the timestamp.
+ */
+errno_t sysdb_set_initgr_expire_timestamp(struct sss_domain_info *domain,
+ const char *name_or_upn_or_sid);
+
/* Password caching function.
* If you are in a transaction ignore sysdb and pass in the handle.
* If you are not in a transaction pass NULL in handle and provide sysdb,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 55ba62140..c57a13be1 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3277,6 +3277,76 @@ int sysdb_cache_password(struct sss_domain_info *domain,
SSS_AUTHTOK_TYPE_PASSWORD, 0);
}
+static errno_t set_initgroups_expire_attribute(struct sss_domain_info *domain,
+ const char *name)
+{
+ errno_t ret;
+ time_t cache_timeout;
+ struct sysdb_attrs *attrs;
+
+ attrs = sysdb_new_attrs(NULL);
+ if (attrs == NULL) {
+ return ENOMEM;
+ }
+
+ cache_timeout = domain->user_timeout
+ ? time(NULL) + domain->user_timeout
+ : 0;
+
+ ret = sysdb_attrs_add_time_t(attrs, SYSDB_INITGR_EXPIRE, cache_timeout);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up attrs\n");
+ goto done;
+ }
+
+ ret = sysdb_set_user_attr(domain, name, attrs, SYSDB_MOD_REP);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set initgroups expire attribute\n");
+ goto done;
+ }
+
+done:
+ talloc_zfree(attrs);
+ return ret;
+}
+
+errno_t sysdb_set_initgr_expire_timestamp(struct sss_domain_info *domain,
+ const char *name_or_upn_or_sid)
+{
+ const char *cname;
+ errno_t ret;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ return ENOMEM;
+ }
+
+ ret = sysdb_get_real_name(tmp_ctx, domain, name_or_upn_or_sid, &cname);
+ if (ret == ENOENT) {
+ /* No point trying to bump timestamp of an entry that does not exist..*/
+ ret = EOK;
+ goto done;
+ } else if (ret != EOK) {
+ cname = name_or_upn_or_sid;
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Failed to canonicalize name, using [%s]\n", name_or_upn_or_sid);
+ }
+
+ ret = set_initgroups_expire_attribute(domain, cname);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cannot set the initgroups expire attribute [%d]: %s\n",
+ ret, sss_strerror(ret));
+ }
+
+ ret = EOK;
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
/* =Custom Search================== */
int sysdb_search_custom(TALLOC_CTX *mem_ctx,
diff --git a/src/providers/data_provider/dp_target_id.c b/src/providers/data_provider/dp_target_id.c
index 748d88674..d5b3823ac 100644
--- a/src/providers/data_provider/dp_target_id.c
+++ b/src/providers/data_provider/dp_target_id.c
@@ -390,69 +390,22 @@ done:
return ret;
}
-static errno_t set_initgroups_expire_attribute(struct sss_domain_info *domain,
- const char *name)
-{
- errno_t ret;
- time_t cache_timeout;
- struct sysdb_attrs *attrs;
-
- attrs = sysdb_new_attrs(NULL);
- if (attrs == NULL) {
- return ENOMEM;
- }
-
- cache_timeout = domain->user_timeout
- ? time(NULL) + domain->user_timeout
- : 0;
-
- ret = sysdb_attrs_add_time_t(attrs, SYSDB_INITGR_EXPIRE, cache_timeout);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up attrs\n");
- goto done;
- }
-
- ret = sysdb_set_user_attr(domain, name, attrs, SYSDB_MOD_REP);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "Failed to set initgroups expire attribute\n");
- goto done;
- }
-
-done:
- talloc_zfree(attrs);
- return ret;
-}
-
static void dp_req_initgr_pp_set_initgr_timestamp(struct dp_initgr_ctx *ctx,
struct dp_reply_std *reply)
{
errno_t ret;
- const char *cname;
if (reply->dp_error != DP_ERR_OK || reply->error != EOK) {
/* Only bump the timestamp on successful lookups */
return;
}
- ret = sysdb_get_real_name(ctx,
- ctx->domain_info,
- ctx->filter_value,
- &cname);
- if (ret == ENOENT) {
- /* No point trying to bump timestamp of an entry that does not exist..*/
- return;
- } else if (ret != EOK) {
- cname = ctx->filter_value;
- DEBUG(SSSDBG_MINOR_FAILURE,
- "Failed to canonicalize name, using [%s]\n", cname);
- }
-
- ret = set_initgroups_expire_attribute(ctx->domain_info, cname);
+ ret = sysdb_set_initgr_expire_timestamp(ctx->domain_info,
+ ctx->filter_value);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
- "Cannot set the initgroups expire attribute [%d]: %s\n",
- ret, sss_strerror(ret));
+ "Failed to set initgroups expiration for [%s]\n",
+ ctx->filter_value);
}
}
--
2.20.1