When I have the following in a domain in sssd.conf:

access_provider = simple
simple_allow_users =

... any user is allowed to log in, despite the list being empty. The documentation states:

  ·   If either or both "allow" lists are provided, all users are denied
      unless they appear in the list.

The list is provided, albeit empty. The simple access provider however treats it as if it is not provided.

Since sssd.conf is often machine driven, this sort of unexpected behavior leads to security problems like: removing a user from the simple_allow_users acl leads to any user being allowed.

I've worked around this behavior in realmd, by using a comma:

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=56027
Patch: https://bugs.freedesktop.org/attachment.cgi?id=68615

Attached is a rough patch to sssd which fixes the problem. If you think it's worth fixing, I'll do more testing on it.

Cheers,

Stef
>From fbbfa251feb4a219d250f9c9b8f5373422f82ab8 Mon Sep 17 00:00:00 2001
From: Stef Walter <st...@gnome.org>
Date: Tue, 16 Oct 2012 11:43:05 +0200
Subject: [PATCH] Recognize empty string lists in the 'simple' access provider

 * The simple access provider treats empty string lists as
   having the rules not specified.
 * This causes the simple access provider to fail unsafe/open, when
   all items are removed from one of the allow lists.
 * Fix by recognizing that an empty list, is different from a
   list that is not present.
---
 src/providers/simple/simple_access.c | 54 +++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/providers/simple/simple_access.c b/src/providers/simple/simple_access.c
index 70d1f07..ebff0fc 100644
--- a/src/providers/simple/simple_access.c
+++ b/src/providers/simple/simple_access.c
@@ -276,6 +276,28 @@ struct bet_ops simple_access_ops = {
     .finalize = NULL
 };
 
+int get_conf_list_or_empty(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
+                           const char *section, const char *attribute,
+                           char ***result)
+{
+    int ret;
+    int r;
+
+    ret = confdb_get_string_as_list(cdb, ctx, section, attribute, result);
+
+    if (ret == ENOENT) {
+        /*
+         * If config has an empty string value, then we want to return
+         * an empty string list, not ENOENT.
+         */
+        ret = confdb_get_param (cdb, ctx, section, attribute, result);
+        if (*result == NULL)
+            ret = ENOENT;
+    }
+
+    return ret;
+}
+
 int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
                             void **pvt_data)
 {
@@ -292,12 +314,12 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
     ctx->domain = bectx->domain;
 
     /* Users */
-    ret = confdb_get_string_as_list(bectx->cdb, ctx, bectx->conf_path,
-                                    CONFDB_SIMPLE_ALLOW_USERS,
-                                    &ctx->allow_users);
+    ret = get_conf_list_or_empty(bectx->cdb, ctx, bectx->conf_path,
+                                 CONFDB_SIMPLE_ALLOW_USERS,
+                                 &ctx->allow_users);
     if (ret != EOK) {
         if (ret == ENOENT) {
-            DEBUG(9, ("Allow user list is empty.\n"));
+            DEBUG(9, ("Allow user list is not present.\n"));
             ctx->allow_users = NULL;
         } else {
             DEBUG(1, ("confdb_get_string_as_list failed.\n"));
@@ -305,12 +327,12 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
         }
     }
 
-    ret = confdb_get_string_as_list(bectx->cdb, ctx, bectx->conf_path,
-                                    CONFDB_SIMPLE_DENY_USERS,
-                                    &ctx->deny_users);
+    ret = get_conf_list_or_empty(bectx->cdb, ctx, bectx->conf_path,
+                                 CONFDB_SIMPLE_DENY_USERS,
+                                 &ctx->deny_users);
     if (ret != EOK) {
         if (ret == ENOENT) {
-            DEBUG(9, ("Deny user list is empty.\n"));
+            DEBUG(9, ("Deny user list is not present.\n"));
             ctx->deny_users = NULL;
         } else {
             DEBUG(1, ("confdb_get_string_as_list failed.\n"));
@@ -319,12 +341,12 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
     }
 
     /* Groups */
-    ret = confdb_get_string_as_list(bectx->cdb, ctx, bectx->conf_path,
-                                    CONFDB_SIMPLE_ALLOW_GROUPS,
-                                    &ctx->allow_groups);
+    ret = get_conf_list_or_empty(bectx->cdb, ctx, bectx->conf_path,
+                                 CONFDB_SIMPLE_ALLOW_GROUPS,
+                                 &ctx->allow_groups);
     if (ret != EOK) {
         if (ret == ENOENT) {
-            DEBUG(9, ("Allow group list is empty.\n"));
+            DEBUG(9, ("Allow group list is not present.\n"));
             ctx->allow_groups = NULL;
         } else {
             DEBUG(1, ("confdb_get_string_as_list failed.\n"));
@@ -332,12 +354,12 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
         }
     }
 
-    ret = confdb_get_string_as_list(bectx->cdb, ctx, bectx->conf_path,
-                                    CONFDB_SIMPLE_DENY_GROUPS,
-                                    &ctx->deny_groups);
+    ret = get_conf_list_or_empty(bectx->cdb, ctx, bectx->conf_path,
+                                 CONFDB_SIMPLE_DENY_GROUPS,
+                                 &ctx->deny_groups);
     if (ret != EOK) {
         if (ret == ENOENT) {
-            DEBUG(9, ("Deny user list is empty.\n"));
+            DEBUG(9, ("Deny user list is not present.\n"));
             ctx->deny_groups = NULL;
         } else {
             DEBUG(1, ("confdb_get_string_as_list failed.\n"));
-- 
1.7.12.1

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to