-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/10/2010 06:34 AM, Sumit Bose wrote: > On Thu, Dec 09, 2010 at 10:20:09AM -0500, Stephen Gallagher wrote: > 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. > > Fixes https://fedorahosted.org/sssd/ticket/440
+
+ /* Construct a list of the user's groups */
+ for (i = 0; i < msg->num_elements; i++) {
+ if (strcasecmp(msg->elements[i].name, SYSDB_MEMBEROF) == 0) {
> If there is no SYSDB_MEMBEROF groups and j might be undefined later on.
> Why do you run the loop yourself instead of using ldb_msg_find_element() ?
Because I glanced at all of the other ldb_msg_find_* functions and saw
that they said "Assumes that elements are single valued", so I
reimplemented it. I've switched to using ldb_msg_find_element() now.
Good catch, by the way. I didn't think about the case where the user has
no groups but the primary.
New patch attached.
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0CfcMACgkQeiVVYja6o6O6EACgocA2AF6kJJ2nNvpBXC1a7VPp
6M4AoIENoVqW6A40JGJwDfX4mcTe+D3n
=ohMT
-----END PGP SIGNATURE-----
From 2c05ba396bc65992464a67e324e698375b23a17e Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Thu, 9 Dec 2010 10:14:04 -0500 Subject: [PATCH] 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 --- src/config/etc/sssd.api.d/sssd-simple.conf | 2 + src/man/sssd-simple.5.xml | 56 ++++++-- src/providers/simple/simple_access.c | 226 ++++++++++++++++++++++++++-- src/providers/simple/simple_access.h | 5 + src/tests/simple_access-tests.c | 4 +- 5 files changed, 263 insertions(+), 30 deletions(-) diff --git a/src/config/etc/sssd.api.d/sssd-simple.conf b/src/config/etc/sssd.api.d/sssd-simple.conf index 13fbeb9e976f2c77c479130e23edabffcfe3845d..e14ea45d9e2b46efa6073d8650a1bddc55213a73 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 260d15ab8dfe54b9e2ed19f3400619b38eb7df3b..fd3b8b0e29aa440e2b22ab5627ee94673404f751 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 4d6135fa4592c806eb03e660714f0f97953bdf26..a54bad0001c75cf22cbb01874a8549718e50e931 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; - return EOK; } } - } else { + } 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; - 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; + } + + /* 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 { + /* 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 0aac42a5f043d4067a0a481910485696dea13fcb..abcf61ac29f31b3e424dd12340759ee6cc8c9489 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 c9bf4ea54e353eaddaf334c8918540051c891f2b..fbbc8361838ccbd7d4861f1e34cac788b2d81690 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."); -- 1.7.3.2
0001-Add-group-support-to-the-simple-access-provider.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
