From 3543c929ea2264ae83cc5de195a33b48da5dfd16 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Mon, 1 Jul 2019 14:15:29 +0200
Subject: [PATCH 63/64] 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>
(cherry picked from commit 7a08d1dea8cb9148ba1afe13f4d4567229c9b381)
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 15df1a726..dfd91f6b8 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1122,6 +1122,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 340062f6f..fa3842d8f 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3259,6 +3259,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 d123638eb..591ddd32d 100644
--- a/src/providers/data_provider/dp_target_id.c
+++ b/src/providers/data_provider/dp_target_id.c
@@ -372,69 +372,22 @@ done:
talloc_free(tmp_ctx);
}
-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