[SSSD] [PATCH] GPO: Ignore ENOENT result from sysdb_gpo_get_gpo_result_setting()

Jakub Hrozek jhrozek at redhat.com
Wed Jan 7 10:50:16 UTC 2015


Fixes offline GPO processing in case no cached GPO result was stored in
sysdb. Includes a unit test.
-------------- next part --------------
>From 50c13e18f43c7215ff3ebca0702e552688da25f8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 6 Jan 2015 13:14:35 +0100
Subject: [PATCH 1/2] GPO: Ignore ENOENT result from
 sysdb_gpo_get_gpo_result_setting()

https://fedorahosted.org/sssd/ticket/2542

If the GPO result object was missing completely, we would error out with
a fatal error code. It's more user-friendly to treat the missing object
as if the requested attribute was missing on the provider level.
---
 src/providers/ad/ad_gpo.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 01dfa1c07953e88477dbd1f30b65ea81531c4f68..6dbe0595f9079426a661e6e5816a79b77ac5023a 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -1339,7 +1339,10 @@ parse_policy_setting_value(TALLOC_CTX *mem_ctx,
     char **sids_list = NULL;
 
     ret = sysdb_gpo_get_gpo_result_setting(mem_ctx, domain, key, &value);
-    if (ret != EOK) {
+    if (ret == ENOENT) {
+        DEBUG(SSSDBG_TRACE_FUNC, "No previous GPO result\n");
+        value = NULL;
+    } else if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
               "Cannot retrieve settings from sysdb for key: '%s' [%d][%s].\n",
               key, ret, sss_strerror(ret));
-- 
1.9.3

-------------- next part --------------
>From 73600f7bd666361f19268f691285779f882d04df Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 6 Jan 2015 14:15:47 +0100
Subject: [PATCH 2/2] TESTS: Cover sysdb_gpo.c with unit tests

Untested code is risky to change.
---
 src/db/sysdb_gpo.c      |  2 ++
 src/tests/sysdb-tests.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/src/db/sysdb_gpo.c b/src/db/sysdb_gpo.c
index 92559d41e341b00a8031deb6209284a9ee397a42..2fb9b748d750b92dc990b8175869262b666306db 100644
--- a/src/db/sysdb_gpo.c
+++ b/src/db/sysdb_gpo.c
@@ -670,6 +670,8 @@ errno_t sysdb_gpo_delete_gpo_result_object(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_gpo_get_gpo_result_object(mem_ctx, domain, &res);
     if (ret != EOK && ret != ENOENT) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "Could not delete GPO result object: %d\n", ret);
         goto done;
     } else if (ret != ENOENT) {
         DEBUG(SSSDBG_TRACE_FUNC, "Deleting GPO Result object\n");
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 92b41e90d08c2d50775289ba8c922f39350ce625..07daf94e3cbc172d94f20baacce016d2ae93fa78 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -6015,6 +6015,83 @@ START_TEST(test_gpo_replace)
 }
 END_TEST
 
+START_TEST(test_gpo_result)
+{
+    errno_t ret;
+    struct sysdb_test_ctx *test_ctx;
+    const char *allow_key = "SeRemoteInteractiveLogonRight";
+    const char *deny_key = "SeDenyRemoteInteractiveLogonRight";
+    const char *value = NULL;
+
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_if(ret != EOK, "Could not setup the test");
+
+    /* No result in cache */
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           allow_key, &value);
+    ck_assert_int_eq(ret, ENOENT);
+
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           deny_key, &value);
+    ck_assert_int_eq(ret, ENOENT);
+
+    /* Delete with no result object is a noop */
+    ret = sysdb_gpo_delete_gpo_result_object(test_ctx, test_ctx->domain);
+    ck_assert_int_eq(ret, EOK);
+
+    /* Store an allow value, triggering a new result object */
+    ret = sysdb_gpo_store_gpo_result_setting(test_ctx->domain,
+                                             allow_key, "allow_val1");
+    ck_assert_int_eq(ret, EOK);
+
+    /* Now both searches should succeed, but only allow_key should return
+     * a valid value
+     */
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           allow_key, &value);
+    ck_assert_int_eq(ret, EOK);
+    ck_assert_str_eq(value, "allow_val1");
+
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           deny_key, &value);
+    ck_assert_int_eq(ret, EOK);
+    fail_unless(value == NULL);
+
+    /* Updating replaces the original value */
+    ret = sysdb_gpo_store_gpo_result_setting(test_ctx->domain,
+                                             allow_key, "allow_val2");
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           allow_key, &value);
+    ck_assert_int_eq(ret, EOK);
+    ck_assert_str_eq(value, "allow_val2");
+
+    /* NULL removes the value completely */
+    ret = sysdb_gpo_store_gpo_result_setting(test_ctx->domain,
+                                             allow_key, NULL);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           allow_key, &value);
+    ck_assert_int_eq(ret, EOK);
+    fail_unless(value == NULL);
+
+    /* Delete the result */
+    ret = sysdb_gpo_delete_gpo_result_object(test_ctx, test_ctx->domain);
+    ck_assert_int_eq(ret, EOK);
+
+    /* No result in cache */
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           allow_key, &value);
+    ck_assert_int_eq(ret, ENOENT);
+
+    ret = sysdb_gpo_get_gpo_result_setting(test_ctx, test_ctx->domain,
+                                           deny_key, &value);
+    ck_assert_int_eq(ret, ENOENT);
+}
+END_TEST
+
 START_TEST(test_confdb_list_all_domain_names_multi_dom)
 {
     int ret;
@@ -6446,6 +6523,7 @@ Suite *create_sysdb_suite(void)
     TCase *tc_gpo = tcase_create("SYSDB GPO tests");
     tcase_add_test(tc_gpo, test_gpo_store_retrieve);
     tcase_add_test(tc_gpo, test_gpo_replace);
+    tcase_add_test(tc_gpo, test_gpo_result);
     suite_add_tcase(s, tc_gpo);
 
     /* ConfDB tests -- modify confdb, must always be last!! */
-- 
1.9.3



More information about the sssd-devel mailing list