URL: https://github.com/SSSD/sssd/pull/551
Author: mzidek-rh
 Title: #551: GPO: Fix bug with empty GPO rules
Action: opened

PR body:

To reproduce:
On AD server create 2 GPO rules:
- Allow log on locally - allow group Administrators and user 
- Deny log on locally - define the rule, but enter no users or groups

On client:
- Try to: su administra...@ad.test

Result without patch: Login fails with "System error"
Result with patch: Login succeeds

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/551/head:pr551
git checkout pr551
From c5efd2fedd8b3eeab769b9b4364ba092bdb90183 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 11 Apr 2018 18:56:53 +0200
Subject: [PATCH] GPO: Fix bug with empty GPO rules

When two or more GPO rules were defined on the server
and one of them contained no SIDs (no users or groups
were specified), then SSSD failed to store such rule
and users were denied access (system error).

This patch changes the behavior so that in case
there are no SIDs in the rule a special value is
stored with the rule to indicate that the rule
was actually specified, but this value will not
match any real SID (because the rule should be

 src/providers/ad/ad_gpo.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index a48f264c7..ae3329b90 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -1132,6 +1132,7 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
     int i;
     char *allow_value = NULL;
     char *deny_value = NULL;
+    const char *empty_val = "NO_SID";
     const char *allow_key = NULL;
     const char *deny_key = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
@@ -1236,7 +1237,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
     for (i = 0; i < GPO_MAP_NUM_OPTS; i++) {
+        /* The NO_SID val is used as special SID value for the case when
+         * no SIDs are found in the rule, but we need to store some
+         * value (SID) with the key (rule name) so that it is clear
+         * that the rule is defined on the server. */
         struct gpo_map_option_entry entry = gpo_map_option_entries[i];
         allow_key = entry.allow_key;
@@ -1252,9 +1256,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
                       allow_key, ret, sss_strerror(ret));
                 goto done;
             } else if (ret != ENOENT) {
+                const char *value = allow_value ? allow_value : empty_val;
                 ret = sysdb_gpo_store_gpo_result_setting(domain,
-                                                         allow_value);
+                                                         value);
                 if (ret != EOK) {
                           "sysdb_gpo_store_gpo_result_setting failed for key:"
@@ -1278,9 +1283,10 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain,
                       deny_key, ret, sss_strerror(ret));
                 goto done;
             } else if (ret != ENOENT) {
+                const char *value = deny_value ? deny_value : empty_val;
                 ret = sysdb_gpo_store_gpo_result_setting(domain,
-                                                         deny_value);
+                                                         value);
                 if (ret != EOK) {
                           "sysdb_gpo_store_gpo_result_setting failed for key:"
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to