From e935d41ec6c187c61e0a6f8c353276fbf69780a5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 28 May 2019 14:56:05 +0200
Subject: [PATCH 45/64] SYSDB: Add sysdb_search_with_ts_attr
Adds a new public sysdb call sysdb_search_with_ts_attr() that allows to
search on the timestamp cache attributes, but merge back persistent
cache attributes. The converse also works, when searching the persistent
cache the timestamp attributes or even entries matches only in the
timestamp cache are merged.
What does not work is AND-ed complex filter that contains both
attributes from the timestamp cache and the persistent cache because
the searches use the same filter, which doesn't match. We would need to
decompose the filter ourselves.
Because matching and merging the results can be time-consuming, two
flags are provided:
SYSDB_SEARCH_WITH_TS_ONLY_TS_FILTER that only searches the timestamp
cache, but merges back the corresponding entries from the persistent
cache
SYSDB_SEARCH_WITH_TS_ONLY_SYSDB_FILTER that only searches the
persistent cache but merges back the attributes from the timestamp
cache
Related:
https://pagure.io/SSSD/sssd/issue/4012
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit db99504a5295ae1f9bc5166133c8f21e4510c676)
Reviewed-by: Sumit Bose <sbose@redhat.com>
---
src/db/sysdb.h | 12 ++
src/db/sysdb_ops.c | 16 +-
src/db/sysdb_private.h | 10 ++
src/db/sysdb_search.c | 231 +++++++++++++++++++++++--
src/tests/cmocka/test_sysdb_ts_cache.c | 198 +++++++++++++++++++++
5 files changed, 446 insertions(+), 21 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 56468a169..15df1a726 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1190,6 +1190,18 @@ int sysdb_search_users(TALLOC_CTX *mem_ctx,
size_t *msgs_count,
struct ldb_message ***msgs);
+#define SYSDB_SEARCH_WITH_TS_ONLY_TS_FILTER 0x0001
+#define SYSDB_SEARCH_WITH_TS_ONLY_SYSDB_FILTER 0x0002
+
+errno_t sysdb_search_with_ts_attr(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ struct ldb_dn *base_dn,
+ enum ldb_scope scope,
+ int optflags,
+ const char *filter,
+ const char *attrs[],
+ struct ldb_result **_result);
+
int sysdb_search_users_by_timestamp(TALLOC_CTX *mem_ctx,
struct sss_domain_info *domain,
const char *sub_filter,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index d05950732..340062f6f 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -261,14 +261,14 @@ done:
/* =Search-Entry========================================================== */
-static int sysdb_cache_search_entry(TALLOC_CTX *mem_ctx,
- struct ldb_context *ldb,
- struct ldb_dn *base_dn,
- enum ldb_scope scope,
- const char *filter,
- const char **attrs,
- size_t *_msgs_count,
- struct ldb_message ***_msgs)
+int sysdb_cache_search_entry(TALLOC_CTX *mem_ctx,
+ struct ldb_context *ldb,
+ struct ldb_dn *base_dn,
+ enum ldb_scope scope,
+ const char *filter,
+ const char **attrs,
+ size_t *_msgs_count,
+ struct ldb_message ***_msgs)
{
TALLOC_CTX *tmp_ctx;
struct ldb_result *res;
diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h
index f3d34dd6f..7063d4594 100644
--- a/src/db/sysdb_private.h
+++ b/src/db/sysdb_private.h
@@ -253,6 +253,16 @@ errno_t sysdb_merge_msg_list_ts_attrs(struct sysdb_ctx *ctx,
struct ldb_result *sss_merge_ldb_results(struct ldb_result *res,
struct ldb_result *subres);
+/* Search Entry in an ldb cache */
+int sysdb_cache_search_entry(TALLOC_CTX *mem_ctx,
+ struct ldb_context *ldb,
+ struct ldb_dn *base_dn,
+ enum ldb_scope scope,
+ const char *filter,
+ const char **attrs,
+ size_t *_msgs_count,
+ struct ldb_message ***_msgs);
+
/* Search Entry in the timestamp cache */
int sysdb_search_ts_entry(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb,
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index f0918bf9a..a71c43112 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -68,6 +68,29 @@ static errno_t merge_ts_attr(struct ldb_message *ts_msg,
return EOK;
}
+static errno_t merge_all_ts_attrs(struct ldb_message *ts_msg,
+ struct ldb_message *sysdb_msg,
+ const char *want_attrs[])
+{
+ int ret;
+
+ /* Deliberately start from 2 in order to not merge
+ * objectclass/objectcategory and avoid breaking MPGs where the OC might
+ * be made up
+ */
+ for (size_t c = 2; sysdb_ts_cache_attrs[c]; c++) {
+ ret = merge_ts_attr(ts_msg, sysdb_msg,
+ sysdb_ts_cache_attrs[c], want_attrs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cannot merge ts attr %s\n", sysdb_ts_cache_attrs[c]);
+ return ret;
+ }
+ }
+
+ return EOK;
+}
+
static errno_t merge_msg_ts_attrs(struct sysdb_ctx *sysdb,
struct ldb_message *sysdb_msg,
const char *attrs[])
@@ -114,21 +137,46 @@ static errno_t merge_msg_ts_attrs(struct sysdb_ctx *sysdb,
return EIO;
}
- /* Deliberately start from 2 in order to not merge
- * objectclass/objectcategory and avoid breaking MPGs where the OC might
- * be made up
- */
- for (size_t c = 2; sysdb_ts_cache_attrs[c]; c++) {
- ret = merge_ts_attr(ts_msgs[0], sysdb_msg,
- sysdb_ts_cache_attrs[c], attrs);
- if (ret != EOK) {
- DEBUG(SSSDBG_MINOR_FAILURE,
- "Cannot merge ts attr %s\n", sysdb_ts_cache_attrs[c]);
- goto done;
- }
+ ret = merge_all_ts_attrs(ts_msgs[0], sysdb_msg, attrs);
+done:
+ talloc_zfree(tmp_ctx);
+ return ret;
+}
+
+static errno_t merge_msg_sysdb_attrs(TALLOC_CTX *mem_ctx,
+ struct sysdb_ctx *sysdb,
+ struct ldb_message *ts_msg,
+ struct ldb_message **_sysdb_msg,
+ const char *attrs[])
+{
+ errno_t ret;
+ TALLOC_CTX *tmp_ctx;
+ size_t msgs_count;
+ struct ldb_message **sysdb_msgs;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
}
- ret = EOK;
+ ret = sysdb_cache_search_entry(tmp_ctx, sysdb->ldb, ts_msg->dn, LDB_SCOPE_BASE,
+ NULL, attrs, &msgs_count, &sysdb_msgs);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ if (msgs_count != 1) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Expected 1 result for base search, got %zu\n", msgs_count);
+ goto done;
+ }
+
+ ret = merge_all_ts_attrs(ts_msg, sysdb_msgs[0], attrs);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ *_sysdb_msg = talloc_steal(mem_ctx, sysdb_msgs[0]);
done:
talloc_zfree(tmp_ctx);
return ret;
@@ -166,6 +214,50 @@ errno_t sysdb_merge_res_ts_attrs(struct sysdb_ctx *ctx,
return EOK;
}
+static errno_t merge_res_sysdb_attrs(TALLOC_CTX *mem_ctx,
+ struct sysdb_ctx *ctx,
+ struct ldb_result *ts_res,
+ struct ldb_result **_ts_cache_res,
+ const char *attrs[])
+{
+ errno_t ret;
+ struct ldb_result *ts_cache_res = NULL;
+
+ if (ts_res == NULL || ctx->ldb_ts == NULL) {
+ return EOK;
+ }
+
+ ts_cache_res = talloc_zero(mem_ctx, struct ldb_result);
+ if (ts_cache_res == NULL) {
+ return ENOMEM;
+ }
+ ts_cache_res->count = ts_res->count;
+ ts_cache_res->msgs = talloc_zero_array(ts_cache_res,
+ struct ldb_message *,
+ ts_res->count);
+ if (ts_cache_res->msgs == NULL) {
+ talloc_free(ts_cache_res);
+ return ENOMEM;
+ }
+
+ for (size_t c = 0; c < ts_res->count; c++) {
+ ret = merge_msg_sysdb_attrs(ts_cache_res->msgs,
+ ctx,
+ ts_res->msgs[c],
+ &ts_cache_res->msgs[c], attrs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cannot merge sysdb cache values for %s\n",
+ ldb_dn_get_linearized(ts_res->msgs[c]->dn));
+ /* non-fatal, we just get only the non-timestamp attrs */
+ continue;
+ }
+ }
+
+ *_ts_cache_res = ts_cache_res;
+ return EOK;
+}
+
errno_t sysdb_merge_msg_list_ts_attrs(struct sysdb_ctx *ctx,
size_t msgs_count,
struct ldb_message **msgs,
@@ -543,6 +635,119 @@ done:
return ret;
}
+errno_t sysdb_search_with_ts_attr(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ struct ldb_dn *base_dn,
+ enum ldb_scope scope,
+ int optflags,
+ const char *filter,
+ const char *attrs[],
+ struct ldb_result **_res)
+{
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct ldb_result *res;
+ errno_t ret;
+ struct ldb_message **ts_msgs = NULL;
+ struct ldb_result *ts_cache_res = NULL;
+ size_t ts_count;
+
+ if (filter == NULL) {
+ return EINVAL;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ res = talloc_zero(tmp_ctx, struct ldb_result);
+ if (res == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ if (optflags & SYSDB_SEARCH_WITH_TS_ONLY_SYSDB_FILTER) {
+ /* We only care about searching the persistent db */
+ ts_cache_res = talloc_zero(tmp_ctx, struct ldb_result);
+ if (ts_cache_res == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ ts_cache_res->count = 0;
+ ts_cache_res->msgs = NULL;
+ } else {
+ /* Because the timestamp database does not contain all the
+ * attributes, we need to search the persistent db for each
+ * of the entries found and merge the results
+ */
+ struct ldb_result ts_res;
+
+ /* We assume that some of the attributes are more up-to-date in
+ * timestamps db and we're supposed to search by them, so let's
+ * first search the timestamp db
+ */
+ ret = sysdb_search_ts_entry(tmp_ctx, domain->sysdb, base_dn,
+ scope, filter, attrs,
+ &ts_count, &ts_msgs);
+ if (ret == ENOENT) {
+ ts_count = 0;
+ } else if (ret != EOK) {
+ goto done;
+ }
+
+ memset(&ts_res, 0, sizeof(struct ldb_result));
+ ts_res.count = ts_count;
+ ts_res.msgs = ts_msgs;
+
+ /* Overlay the results from the main cache with the ts attrs */
+ ret = merge_res_sysdb_attrs(tmp_ctx,
+ domain->sysdb,
+ &ts_res,
+ &ts_cache_res,
+ attrs);
+ if (ret != EOK) {
+ goto done;
+ }
+ }
+
+ if (optflags & SYSDB_SEARCH_WITH_TS_ONLY_TS_FILTER) {
+ /* The filter only contains timestamp attrs, no need to search the
+ * persistent db
+ */
+ if (ts_cache_res) {
+ res->count = ts_cache_res->count;
+ res->msgs = talloc_steal(res, ts_cache_res->msgs);
+ }
+ } else {
+ /* Because some of the attributes being searched might exist in the persistent
+ * database only, we also search the persistent db
+ */
+ size_t count;
+
+ ret = sysdb_search_entry(res, domain->sysdb, base_dn, scope,
+ filter, attrs, &count, &res->msgs);
+ if (ret == ENOENT) {
+ res->count = 0;
+ } else if (ret != EOK) {
+ goto done;
+ }
+ res->count = count; /* Just to cleanly assign size_t to unsigned */
+
+ res = sss_merge_ldb_results(res, ts_cache_res);
+ if (res == NULL) {
+ ret = ENOMEM;
+ goto done;
+ }
+ }
+
+ *_res = talloc_steal(mem_ctx, res);
+ ret = EOK;
+
+done:
+ talloc_zfree(tmp_ctx);
+ return ret;
+}
+
static errno_t sysdb_enum_dn_filter(TALLOC_CTX *mem_ctx,
struct ldb_result *ts_res,
const char *name_filter,
diff --git a/src/tests/cmocka/test_sysdb_ts_cache.c b/src/tests/cmocka/test_sysdb_ts_cache.c
index fdf9935da..d2296d1b8 100644
--- a/src/tests/cmocka/test_sysdb_ts_cache.c
+++ b/src/tests/cmocka/test_sysdb_ts_cache.c
@@ -1411,6 +1411,201 @@ static void test_sysdb_zero_now(void **state)
assert_true(cache_expire_ts > TEST_CACHE_TIMEOUT);
}
+static void test_sysdb_search_with_ts(void **state)
+{
+ int ret;
+ struct sysdb_ts_test_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct sysdb_ts_test_ctx);
+ struct ldb_result *res = NULL;
+ struct ldb_dn *base_dn;
+ const char *attrs[] = { SYSDB_NAME,
+ SYSDB_OBJECTCATEGORY,
+ SYSDB_GIDNUM,
+ SYSDB_CACHE_EXPIRE,
+ NULL };
+ struct sysdb_attrs *group_attrs = NULL;
+ char *filter;
+ uint64_t cache_expire_sysdb;
+ uint64_t cache_expire_ts;
+ size_t count;
+ struct ldb_message **msgs;
+
+ base_dn = sysdb_base_dn(test_ctx->tctx->dom->sysdb, test_ctx);
+ assert_non_null(base_dn);
+
+ /* Nothing must be stored in either cache at the beginning of the test */
+ ret = sysdb_search_with_ts_attr(test_ctx,
+ test_ctx->tctx->dom,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ 0,
+ SYSDB_NAME"=*",
+ attrs,
+ &res);
+ assert_int_equal(ret, EOK);
+ assert_int_equal(res->count, 0);
+ talloc_free(res);
+
+ group_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+ assert_non_null(group_attrs);
+
+ ret = sysdb_store_group(test_ctx->tctx->dom,
+ TEST_GROUP_NAME,
+ TEST_GROUP_GID,
+ group_attrs,
+ TEST_CACHE_TIMEOUT,
+ TEST_NOW_1);
+ assert_int_equal(ret, EOK);
+ talloc_zfree(group_attrs);
+
+ group_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+ assert_non_null(group_attrs);
+
+ ret = sysdb_store_group(test_ctx->tctx->dom,
+ TEST_GROUP_NAME_2,
+ TEST_GROUP_GID_2,
+ group_attrs,
+ TEST_CACHE_TIMEOUT,
+ TEST_NOW_2);
+ assert_int_equal(ret, EOK);
+ talloc_zfree(group_attrs);
+
+ /* Bump the timestamps in the cache so that the ts cache
+ * and sysdb differ
+ */
+
+ group_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+ assert_non_null(group_attrs);
+
+ ret = sysdb_store_group(test_ctx->tctx->dom,
+ TEST_GROUP_NAME,
+ TEST_GROUP_GID,
+ group_attrs,
+ TEST_CACHE_TIMEOUT,
+ TEST_NOW_3);
+ assert_int_equal(ret, EOK);
+
+ talloc_zfree(group_attrs);
+
+
+ group_attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+ assert_non_null(group_attrs);
+
+ ret = sysdb_store_group(test_ctx->tctx->dom,
+ TEST_GROUP_NAME_2,
+ TEST_GROUP_GID_2,
+ group_attrs,
+ TEST_CACHE_TIMEOUT,
+ TEST_NOW_4);
+ assert_int_equal(ret, EOK);
+
+ talloc_zfree(group_attrs);
+
+ get_gr_timestamp_attrs(test_ctx, TEST_GROUP_NAME,
+ &cache_expire_sysdb, &cache_expire_ts);
+ assert_int_equal(cache_expire_sysdb, TEST_CACHE_TIMEOUT + TEST_NOW_1);
+ assert_int_equal(cache_expire_ts, TEST_CACHE_TIMEOUT + TEST_NOW_3);
+
+ get_gr_timestamp_attrs(test_ctx, TEST_GROUP_NAME_2,
+ &cache_expire_sysdb, &cache_expire_ts);
+ assert_int_equal(cache_expire_sysdb, TEST_CACHE_TIMEOUT + TEST_NOW_2);
+ assert_int_equal(cache_expire_ts, TEST_CACHE_TIMEOUT + TEST_NOW_4);
+
+ /* Search for groups that don't expire until TEST_NOW_4 */
+ filter = talloc_asprintf(test_ctx, SYSDB_CACHE_EXPIRE">=%d", TEST_NOW_4);
+ assert_non_null(filter);
+
+ /* This search should yield only one group (so, it needs to search the ts
+ * cache to hit the TEST_NOW_4), but should return attributes merged from
+ * both caches
+ */
+ ret = sysdb_search_with_ts_attr(test_ctx,
+ test_ctx->tctx->dom,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ 0,
+ filter,
+ attrs,
+ &res);
+ assert_int_equal(ret, EOK);
+ assert_int_equal(res->count, 1);
+ assert_int_equal(TEST_GROUP_GID_2, ldb_msg_find_attr_as_uint64(res->msgs[0],
+ SYSDB_GIDNUM, 0));
+ talloc_free(res);
+
+ /*
+ * In contrast, sysdb_search_entry merges the timestamp attributes, but does
+ * not search the timestamp cache
+ */
+ ret = sysdb_search_entry(test_ctx,
+ test_ctx->tctx->dom->sysdb,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ filter,
+ attrs,
+ &count,
+ &msgs);
+ assert_int_equal(ret, ENOENT);
+
+ /* Should get the same result when searching by ts attrs only */
+ ret = sysdb_search_with_ts_attr(test_ctx,
+ test_ctx->tctx->dom,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ SYSDB_SEARCH_WITH_TS_ONLY_TS_FILTER,
+ filter,
+ attrs,
+ &res);
+ talloc_zfree(filter);
+ assert_int_equal(ret, EOK);
+ assert_int_equal(res->count, 1);
+ assert_int_equal(TEST_GROUP_GID_2, ldb_msg_find_attr_as_uint64(res->msgs[0],
+ SYSDB_GIDNUM, 0));
+ talloc_free(res);
+
+ /* We can also search in sysdb only as well, we should get back ts attrs */
+ filter = talloc_asprintf(test_ctx, SYSDB_GIDNUM"=%d", TEST_GROUP_GID);
+ assert_non_null(filter);
+
+ ret = sysdb_search_with_ts_attr(test_ctx,
+ test_ctx->tctx->dom,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ SYSDB_SEARCH_WITH_TS_ONLY_SYSDB_FILTER,
+ filter,
+ attrs,
+ &res);
+ talloc_zfree(filter);
+ assert_int_equal(ret, EOK);
+ assert_int_equal(res->count, 1);
+ assert_int_equal(TEST_GROUP_GID, ldb_msg_find_attr_as_uint64(res->msgs[0],
+ SYSDB_GIDNUM, 0));
+ assert_int_equal(TEST_CACHE_TIMEOUT + TEST_NOW_3,
+ ldb_msg_find_attr_as_uint64(res->msgs[0], SYSDB_CACHE_EXPIRE, 0));
+ talloc_free(res);
+
+ /* We can also search in both using an OR-filter. Note that an AND-filter is not possible
+ * unless we deconstruct the filter..
+ */
+ filter = talloc_asprintf(test_ctx, "(|("SYSDB_GIDNUM"=%d)"
+ "("SYSDB_CACHE_EXPIRE">=%d))",
+ TEST_GROUP_GID, TEST_NOW_4);
+ assert_non_null(filter);
+
+ ret = sysdb_search_with_ts_attr(test_ctx,
+ test_ctx->tctx->dom,
+ base_dn,
+ LDB_SCOPE_SUBTREE,
+ 0,
+ filter,
+ attrs,
+ &res);
+ talloc_zfree(filter);
+ assert_int_equal(ret, EOK);
+ assert_int_equal(res->count, 2);
+ talloc_free(res);
+}
+
int main(int argc, const char *argv[])
{
int rv;
@@ -1462,6 +1657,9 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_sysdb_zero_now,
test_sysdb_ts_setup,
test_sysdb_ts_teardown),
+ cmocka_unit_test_setup_teardown(test_sysdb_search_with_ts,
+ test_sysdb_ts_setup,
+ test_sysdb_ts_teardown),
};
/* Set debug level to invalid value so we can decide if -d 0 was used. */
--
2.20.1