Add group support to the simple access provider

This patch adds simple_allow_groups and simple_deny_groups options
to the simple access provider. It makes it possible to grant or
deny access based on a user's group memberships within the domain.

This patch makes one minor change to previous functionality: now
all deny rules will supersede allow rules. Previously, if both
simple_allow_users and simple_deny_users were set with the same
value, the allow would win.

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

5 files changed, 262 insertions, 29 deletions

diff --git a/src/config/etc/sssd.api.d/sssd-simple.conf b/src/config/etc/sssd.api.d/sssd-simple.conf
index 13fbeb9e..e14ea45d 100644
--- a/src/config/etc/sssd.api.d/sssd-simple.conf
+++ b/src/config/etc/sssd.api.d/sssd-simple.conf
@@ -3,3 +3,5 @@
 [provider/simple/access]
 simple_allow_users = str, None, false
 simple_deny_users = str, None, false
+simple_allow_groups = str, None, false
+simple_deny_groups = str, None, false
diff --git a/src/man/sssd-simple.5.xml b/src/man/sssd-simple.5.xml
index 260d15ab..fd3b8b0e 100644
--- a/src/man/sssd-simple.5.xml
+++ b/src/man/sssd-simple.5.xml
@@ -36,21 +36,30 @@
         </para>
         <para>
             The simple access provider grants or denies access based on an
-            access or deny list of user names. Here to following rules apply:
+            access or deny list of user or group names. The following rules
+            apply:
             <itemizedlist>
                 <listitem>
-                    <para>If both lists are empty, access is granted</para>
+                    <para>If all lists are empty, access is granted</para>
                 </listitem>
                 <listitem>
-                    <para>If simple_allow_users is set, only users from this
-                        list are allowed access.</para>
-                    <para>This setting supersedes the simple_deny_users list
-                        (which would be redundant).</para>
+                    <para>
+                        If any list is provided, the order of evaluation is
+                        allow,deny. This means that any matching deny rule
+                        will supersede any matched allow rule.
+                    </para>
                 </listitem>
                 <listitem>
-                    <para>If the simple_allow_users list is empty, users are
-                        allowed access unless they appear in the
-                        simple_deny_users list</para>
+                    <para>
+                        If either or both "allow" lists are provided, all
+                        users are denied unless they appear in the list.
+                    </para>
+                </listitem>
+                <listitem>
+                    <para>
+                        If only "deny" lists are provided, all users are
+                        granted access unless they appear in the list.
+                    </para>
                 </listitem>
             </itemizedlist>
         </para>
@@ -69,8 +78,8 @@
                     <term>simple_allow_users (string)</term>
                     <listitem>
                         <para>
-                            Comma separated list of users who are allowed to log
-                            in.
+                            Comma separated list of users who are allowed to
+                            log in.
                         </para>
                     </listitem>
                 </varlistentry>
@@ -79,8 +88,29 @@
                     <term>simple_deny_users (string)</term>
                     <listitem>
                         <para>
-                            Comma separated list of users who are rejected if
-                            simple_allow_users is not set.
+                            Comma separated list of users who are explicitly
+                            denied access.
+                        </para>
+                    </listitem>
+                </varlistentry>
+                <varlistentry>
+                    <term>simple_allow_groups (string)</term>
+                    <listitem>
+                        <para>
+                            Comma separated list of groups that are allowed to
+                            log in. This applies only to groups within this
+                            SSSD domain. Local groups are not evaluated.
+                        </para>
+                    </listitem>
+                </varlistentry>
+
+                <varlistentry>
+                    <term>simple_deny_groups (string)</term>
+                    <listitem>
+                        <para>
+                            Comma separated list of groups that are explicitly
+                            denied access. This applies only to groups within
+                            this SSSD domain. Local groups are not evaluated.
                         </para>
                     </listitem>
                 </varlistentry>
diff --git a/src/providers/simple/simple_access.c b/src/providers/simple/simple_access.c
index 4d6135fa..a54bad00 100644
--- a/src/providers/simple/simple_access.c
+++ b/src/providers/simple/simple_access.c
@@ -31,36 +31,200 @@
 #define CONFDB_SIMPLE_ALLOW_USERS "simple_allow_users"
 #define CONFDB_SIMPLE_DENY_USERS "simple_deny_users"
 
+#define CONFDB_SIMPLE_ALLOW_GROUPS "simple_allow_groups"
+#define CONFDB_SIMPLE_DENY_GROUPS "simple_deny_groups"
+
 errno_t simple_access_check(struct simple_ctx *ctx, const char *username,
                             bool *access_granted)
 {
-    int i;
+    int i, j;
+    errno_t ret;
+    TALLOC_CTX *tmp_ctx = NULL;
+    const char *user_attrs[] = { SYSDB_MEMBEROF,
+                                 SYSDB_GIDNUM,
+                                 NULL };
+    const char *group_attrs[] = { SYSDB_NAME,
+                                  NULL };
+    struct ldb_message *msg;
+    struct ldb_message_element *el;
+    char **groups;
+    const char *primary_group;
+    gid_t gid;
+    bool matched;
 
     *access_granted = false;
+
+    /* First, check whether the user is in the allowed users list */
     if (ctx->allow_users != NULL) {
         for(i = 0; ctx->allow_users[i] != NULL; i++) {
             if (strcmp(username, ctx->allow_users[i]) == 0) {
                 DEBUG(9, ("User [%s] found in allow list, access granted.\n",
                       username));
+
+                /* Do not return immediately on explicit allow
+                 * We need to make sure none of the user's groups
+                 * are denied.
+                 */
                 *access_granted = true;
+            }
+        }
+    } else if (!ctx->allow_groups) {
+        /* If neither allow rule is in place, we'll assume allowed
+         * unless a deny rule disables us below.
+         */
+        *access_granted = true;
+    }
+
+    /* Next check whether this user has been specifically denied */
+    if (ctx->deny_users != NULL) {
+        for(i = 0; ctx->deny_users[i] != NULL; i++) {
+            if (strcmp(username, ctx->deny_users[i]) == 0) {
+                DEBUG(9, ("User [%s] found in deny list, access denied.\n",
+                      username));
+
+                /* Return immediately on explicit denial */
+                *access_granted = false;
                 return EOK;
             }
         }
+    }
+
+    if (!ctx->allow_groups && !ctx->deny_groups) {
+        /* There are no group restrictions, so just return
+         * here with whatever we've decided.
+         */
+        return EOK;
+    }
+
+    /* Now get a list of this user's groups and check those against the
+     * simple_allow_groups list.
+     */
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = sysdb_search_user_by_name(tmp_ctx, ctx->sysdb, ctx->domain,
+                                    username, user_attrs, &msg);
+    if (ret != EOK) {
+        DEBUG(1, ("Could not look up username [%s]: [%d][%s]\n",
+                  username, ret, strerror(ret)));
+        goto done;
+    }
+
+    /* Construct a list of the user's groups */
+    el = ldb_msg_find_element(msg, SYSDB_MEMBEROF);
+    if (el && el->num_values) {
+        /* Get the groups from the memberOf entries
+         * Allocate the array with room for both the NULL
+         * terminator and the primary group
+         */
+        groups = talloc_array(tmp_ctx, char *, el->num_values + 2);
+        if (!groups) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        for (j = 0; j < el->num_values; j++) {
+            ret = sysdb_group_dn_name(
+                    ctx->sysdb, tmp_ctx,
+                    (char *)el->values[j].data,
+                    &groups[j]);
+            if (ret != EOK) {
+                goto done;
+            }
+        }
     } else {
-        *access_granted = true;
-        if (ctx->deny_users != NULL) {
-            for(i = 0; ctx->deny_users[i] != NULL; i++) {
-                if (strcmp(username, ctx->deny_users[i]) == 0) {
-                    DEBUG(9, ("User [%s] found in deny list, access denied.\n",
-                          username));
-                    *access_granted = false;
-                    return EOK;
+        /* User is not a member of any groups except primary */
+        groups = talloc_array(tmp_ctx, char *, 2);
+        if (!groups) {
+            ret = ENOMEM;
+            goto done;
+        }
+        j = 0;
+    }
+
+    /* Get the user's primary group */
+    gid = ldb_msg_find_attr_as_uint64(msg, SYSDB_GIDNUM, 0);
+    if (!gid) {
+        ret = EINVAL;
+        goto done;
+    }
+    talloc_zfree(msg);
+
+    ret = sysdb_search_group_by_gid(tmp_ctx, ctx->sysdb, ctx->domain,
+                                    gid, group_attrs, &msg);
+    if (ret != EOK) {
+        DEBUG(1, ("Could not look up primary group [%lu]: [%d][%s]\n",
+                  gid, ret, strerror(ret)));
+        goto done;
+    }
+
+    primary_group = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
+    if (!primary_group) {
+        ret = EINVAL;
+        goto done;
+    }
+
+    groups[j] = talloc_strdup(tmp_ctx, primary_group);
+    if (!groups[j]) {
+        ret = ENOMEM;
+        goto done;
+    }
+    groups[j+1] = NULL;
+    talloc_zfree(msg);
+
+    /* Now process allow and deny group rules
+     * If access was already granted above, we'll skip
+     * this redundant rule check
+     */
+    if (ctx->allow_groups && !*access_granted) {
+        matched = false;
+        for (i = 0; ctx->allow_groups[i]; i++) {
+            for(j = 0; groups[j]; j++) {
+                if (strcmp(groups[j], ctx->allow_groups[i])== 0) {
+                    matched = true;
+                    break;
                 }
             }
+
+            /* If any group has matched, we can skip out on the
+             * processing early
+             */
+            if (matched) {
+                *access_granted = true;
+                break;
+            }
         }
     }
 
-    return EOK;
+    /* Finally, process the deny group rules */
+    if (ctx->deny_groups) {
+        matched = false;
+        for (i = 0; ctx->deny_groups[i]; i++) {
+            for(j = 0; groups[j]; j++) {
+                if (strcmp(groups[j], ctx->deny_groups[i])== 0) {
+                    matched = true;
+                    break;
+                }
+            }
+
+            /* If any group has matched, we can skip out on the
+             * processing early
+             */
+            if (matched) {
+                *access_granted = false;
+                break;
+            }
+        }
+    }
+
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
 }
 
 void simple_access_handler(struct be_req *be_req)
@@ -116,6 +280,10 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
         return ENOMEM;
     }
 
+    ctx->sysdb = bectx->sysdb;
+    ctx->domain = bectx->domain;
+
+    /* Users */
     ret = confdb_get_string_as_list(bectx->cdb, ctx, bectx->conf_path,
                                     CONFDB_SIMPLE_ALLOW_USERS,
                                     &ctx->allow_users);
@@ -142,12 +310,40 @@ int sssm_simple_access_init(struct be_ctx *bectx, struct bet_ops **ops,
         }
     }
 
-    if (ctx->allow_users != NULL && ctx->deny_users != NULL) {
-        DEBUG(1, ("Access and deny list are defined, only one is allowed.\n"));
-        ret = EINVAL;
-        goto failed;
+    /* Groups */
+    ret = confdb_get_string_as_list(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"));
+            ctx->allow_groups = NULL;
+        } else {
+            DEBUG(1, ("confdb_get_string_as_list failed.\n"));
+            goto failed;
+        }
     }
 
+    ret = confdb_get_string_as_list(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"));
+            ctx->deny_groups = NULL;
+        } else {
+            DEBUG(1, ("confdb_get_string_as_list failed.\n"));
+            goto failed;
+        }
+    }
+
+    if (!ctx->allow_users &&
+            !ctx->allow_groups &&
+            !ctx->deny_users &&
+            !ctx->deny_groups) {
+        DEBUG(1, ("No rules supplied for simple access provider. "
+                  "Access will be granted for all users.\n"));
+    }
 
     *ops = &simple_access_ops;
     *pvt_data = ctx;
diff --git a/src/providers/simple/simple_access.h b/src/providers/simple/simple_access.h
index 0aac42a5..abcf61ac 100644
--- a/src/providers/simple/simple_access.h
+++ b/src/providers/simple/simple_access.h
@@ -27,8 +27,13 @@
 #include "util/util.h"
 
 struct simple_ctx {
+    struct sysdb_ctx *sysdb;
+    struct sss_domain_info *domain;
+
     char **allow_users;
     char **deny_users;
+    char **allow_groups;
+    char **deny_groups;
 };
 
 errno_t simple_access_check(struct simple_ctx *ctx, const char *username,
diff --git a/src/tests/simple_access-tests.c b/src/tests/simple_access-tests.c
index c9bf4ea5..fbbc8361 100644
--- a/src/tests/simple_access-tests.c
+++ b/src/tests/simple_access-tests.c
@@ -113,8 +113,8 @@ START_TEST(test_both_set)
 
     ret = simple_access_check(ctx, "u1", &access_granted);
     fail_unless(ret == EOK, "access_simple_check failed.");
-    fail_unless(access_granted == true, "Access denied "
-                                        "while user is in allow list.");
+    fail_unless(access_granted == false, "Access granted "
+                                         "while user is in deny list.");
 
     ret = simple_access_check(ctx, "u3", &access_granted);
     fail_unless(ret == EOK, "access_simple_check failed.");