[SSSD] [PATCHES] simple access provider: support subdomain users and groups

Jakub Hrozek jhrozek at redhat.com
Mon Sep 16 15:30:38 UTC 2013


On Thu, Sep 12, 2013 at 02:01:43PM +0200, Pavel Březina wrote:
> On 09/10/2013 03:46 PM, Jakub Hrozek wrote:
> >On Tue, Sep 03, 2013 at 12:42:21PM +0200, Pavel Březina wrote:
> >>Patch 0001: I haven't used this one after all, but I still think it
> >>is nice to have. It reduces amount of code duplication.
> >>
> >>Patch 0005:
> >>>+    /* Try to get group SID and assign it a domain */ +
> >>>group_sid = ldb_msg_find_attr_as_string(group, SYSDB_SID_STR,
> >>>NULL); +    if (group_sid == NULL) {
> >>
> >>I'm not sure if this branch isn't a dead code, but I kept it there
> >>because it mimics current behaviour.
> >>
> >>>+        /* We will look it up in main domain. */ +        domain
> >>>= state->ctx->domain; +    } else { +        domain =
> >>>find_subdomain_by_sid(state->ctx->domain, group_sid); +        if
> >>>(domain == NULL) { +            DEBUG(SSSDBG_CRIT_FAILURE,
> >>>("There is no domain information for " +
> >>>"SID %s\n", group_sid)); +            return ENOENT; +        } +
> >>>}
> >
> >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com> Date:
> >>Mon, 2 Sep 2013 13:36:25 +0200 Subject: [PATCH 1/6] util: add
> >>sss_idmap_talloc[_free]
> >>
> >>Remove code duplication.
> >
> >Removing code duplication added code :-) 76 insertions(+), 62
> >deletions(-)
> >
> >But it's mostly the comment blobs, so ack.
> >
> >>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com> Date:
> >>Tue, 27 Aug 2013 14:53:03 +0200 Subject: [PATCH 2/6] simple access
> >>tests: fix typos
> >
> >ACK
> >
> >>From 5648c71b1a5a6bad5ce8e3361c7b4baf7d972c0b Mon Sep 17 00:00:00
> >>2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >>Date: Tue, 27 Aug 2013 14:02:42 +0200 Subject: [PATCH 3/6] simple
> >>provider: support subdomain users
> >>
> >>Resolves: https://fedorahosted.org/sssd/ticket/2034
> >
> >There is one thing that still needs fixing -- the case sensitivity
> >flag is still read from the master domain through be_ctx->domain. In
> >general, the providers should not access be_ctx->domain directly, but
> >rather the handler should assign a sss_domain_info structure to some
> >per-provider or per-request state and the code should operate on
> >that.
> >
> >>From c1b31c3c2eefa71f5c17539fd615a785d7e1ccd0 Mon Sep 17 00:00:00
> >>2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >>Date: Mon, 2 Sep 2013 15:15:59 +0200 Subject: [PATCH 4/6] util: add
> >>find_subdomain_by_sid()
> >
> >ACK, but please file a ticket to mass-namespace the functions in
> >this module.
> 
> https://fedorahosted.org/sssd/ticket/2086
> 
> >>From 78fff868aa9f1b5a8392328973e4ce3ba000a9e6 Mon Sep 17 00:00:00
> >>2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >>Date: Wed, 28 Aug 2013 11:35:27 +0200 Subject: [PATCH 5/6] simple
> >>provider: support subdomain groups
> >>
> >
> >Same comment about case sensitivity.
> >
> >>From 844b9cc30ad694116772f519791b3725a72d178a Mon Sep 17 00:00:00
> >>2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >>Date: Tue, 3 Sep 2013 12:34:07 +0200 Subject: [PATCH 6/6] simple
> >>access test: initialize be_ctx for all tests
> >>
> >
> >ACK
> 
> Thanks. New patches are attached. I'm sending the case sensitivity
> change in a separate patch, since I think it can stand on its own as
> improvement of subdomains support in simple access provider.
> 
> I also included patch that contains find_subdomain_by_object_name().
> 

Hi,

as part of review, I resolved conflict between your and Lukas' patches.
Can you check if it's OK?
-------------- next part --------------
>From 048ea6c47d142a4689baa956fab8e86ec8d07466 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Wed, 28 Aug 2013 11:35:27 +0200
Subject: [PATCH] simple provider: support subdomain groups

Resolves:
https://fedorahosted.org/sssd/ticket/2034
---
 src/providers/simple/simple_access_check.c | 82 ++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 21 deletions(-)

diff --git a/src/providers/simple/simple_access_check.c b/src/providers/simple/simple_access_check.c
index 5c2884abe86936e9b69e212518c79cad318deed4..a2a44c8f0b40c1f7eefdcef8bee17eea01ad3f56 100644
--- a/src/providers/simple/simple_access_check.c
+++ b/src/providers/simple/simple_access_check.c
@@ -152,6 +152,7 @@ simple_check_groups(struct simple_ctx *ctx, const char **group_names,
 }
 
 struct simple_resolve_group_state {
+    struct sss_domain_info *domain;
     gid_t gid;
     struct simple_ctx *ctx;
 
@@ -166,6 +167,7 @@ static struct tevent_req *
 simple_resolve_group_send(TALLOC_CTX *mem_ctx,
                           struct tevent_context *ev,
                           struct simple_ctx *ctx,
+                          struct sss_domain_info *domain,
                           gid_t gid)
 {
     errno_t ret;
@@ -178,6 +180,7 @@ simple_resolve_group_send(TALLOC_CTX *mem_ctx,
                             struct simple_resolve_group_state);
     if (!req) return NULL;
 
+    state->domain = domain;
     state->gid = gid;
     state->ctx = ctx;
 
@@ -206,7 +209,7 @@ simple_resolve_group_send(TALLOC_CTX *mem_ctx,
     ar->attr_type = BE_ATTR_CORE;
     ar->filter_type = BE_FILTER_IDNUM;
     ar->filter_value = talloc_asprintf(ar, "%llu", (unsigned long long) gid);
-    ar->domain = talloc_strdup(ar, ctx->domain->name);
+    ar->domain = talloc_strdup(ar, state->domain->name);
     if (!ar->domain || !ar->filter_value) {
         ret = ENOMEM;
         goto done;
@@ -240,10 +243,13 @@ simple_resolve_group_check(struct simple_resolve_group_state *state)
                                   SYSDB_GIDNUM, NULL };
 
     /* Check the cache by GID again and fetch the name */
-    ret = sysdb_search_group_by_gid(state, state->ctx->domain->sysdb,
-                                    state->ctx->domain, state->gid,
+    ret = sysdb_search_group_by_gid(state, state->domain->sysdb,
+                                    state->domain, state->gid,
                                     group_attrs, &group);
-    if (ret != EOK) {
+    if (ret == ENOENT) {
+        /* The group is missing, we will try to update it. */
+        return EAGAIN;
+    } else if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
                ("Could not look up group by gid [%"SPRIgid"]: [%d][%s]\n",
                state->gid, ret, sss_strerror(ret)));
@@ -321,12 +327,18 @@ simple_resolve_group_recv(struct tevent_req *req,
     return EOK;
 }
 
+struct simple_group {
+    struct sss_domain_info *domain;
+    gid_t gid;
+};
+
 struct simple_check_groups_state {
     struct tevent_context *ev;
     struct simple_ctx *ctx;
+    struct sss_domain_info *domain;
 
-    gid_t *lookup_gids;
-    size_t num_gids;
+    struct simple_group *lookup_groups;
+    size_t num_groups;
     size_t giter;
 
     const char **group_names;
@@ -352,7 +364,8 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
     struct tevent_req *req;
     struct tevent_req *subreq;
     struct simple_check_groups_state *state;
-    const char *attrs[] = { SYSDB_NAME, SYSDB_POSIX, SYSDB_GIDNUM, NULL };
+    const char *attrs[] = { SYSDB_NAME, SYSDB_POSIX, SYSDB_GIDNUM,
+                            SYSDB_SID_STR, NULL };
     size_t group_count;
     struct ldb_message *user;
     struct ldb_message **groups;
@@ -368,7 +381,15 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Looking up groups for user %s\n", username));
 
-    ret = sysdb_search_user_by_name(state, ctx->domain->sysdb, ctx->domain,
+    /* get domain from username */
+    state->domain = find_subdomain_by_object_name(ctx->domain, username);
+    if (state->domain == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid user %s!\n", username));
+        ret = EINVAL;
+        goto done;
+    }
+
+    ret = sysdb_search_user_by_name(state, state->domain->sysdb, state->domain,
                                     username, attrs, &user);
     if (ret == ENOENT) {
         DEBUG(SSSDBG_MINOR_FAILURE, ("No such user %s\n", username));
@@ -381,7 +402,7 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sysdb_asq_search(state, ctx->domain->sysdb,
+    ret = sysdb_asq_search(state, state->domain->sysdb,
                            user->dn, NULL, SYSDB_MEMBEROF,
                            attrs, &group_count, &groups);
     if (ret != EOK) {
@@ -394,8 +415,9 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
 
     /* One extra space for terminator, one extra space for private group */
     state->group_names = talloc_zero_array(state, const char *, group_count + 2);
-    state->lookup_gids = talloc_zero_array(state, gid_t, group_count + 2);
-    if (!state->group_names || !state->lookup_gids) {
+    state->lookup_groups = talloc_zero_array(state, struct simple_group,
+                                             group_count + 2);
+    if (!state->group_names || !state->lookup_groups) {
         ret = ENOMEM;
         goto done;
     }
@@ -426,7 +448,7 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    if (state->num_gids == 0) {
+    if (state->num_groups == 0) {
         /* If all groups could have been resolved by name, we are
          * done
          */
@@ -438,7 +460,8 @@ simple_check_get_groups_send(TALLOC_CTX *mem_ctx,
     DEBUG(SSSDBG_TRACE_FUNC, ("Need to resolve %zu groups\n", state->num_gids));
     state->giter = 0;
     subreq = simple_resolve_group_send(req, state->ev, state->ctx,
-                                       state->lookup_gids[state->giter]);
+                                       state->lookup_groups[state->giter].domain,
+                                       state->lookup_groups[state->giter].gid);
     if (!subreq) {
         ret = ENOMEM;
         goto done;
@@ -479,9 +502,10 @@ static void simple_check_get_groups_next(struct tevent_req *subreq)
     state->num_names++;
     state->giter++;
 
-    if (state->giter < state->num_gids) {
+    if (state->giter < state->num_groups) {
         subreq = simple_resolve_group_send(req, state->ev, state->ctx,
-                                           state->lookup_gids[state->giter]);
+                                   state->lookup_groups[state->giter].domain,
+                                   state->lookup_groups[state->giter].gid);
         if (!subreq) {
             tevent_req_error(req, ENOMEM);
             return;
@@ -499,6 +523,8 @@ simple_check_process_group(struct simple_check_groups_state *state,
                            struct ldb_message *group)
 {
     const char *name;
+    const char *group_sid;
+    struct sss_domain_info *domain;
     gid_t gid;
     bool posix;
 
@@ -543,10 +569,25 @@ simple_check_process_group(struct simple_check_groups_state *state,
         return EOK;
     }
 
-    /* Non-posix group with a GID. Needs resolving */
-    state->lookup_gids[state->num_gids] = gid;
+    /* Try to get group SID and assign it a domain */
+    group_sid = ldb_msg_find_attr_as_string(group, SYSDB_SID_STR, NULL);
+    if (group_sid == NULL) {
+        /* We will look it up in main domain. */
+        domain = state->ctx->domain;
+    } else {
+        domain = find_subdomain_by_sid(state->ctx->domain, group_sid);
+        if (domain == NULL) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("There is no domain information for "
+                                        "SID %s\n", group_sid));
+            return ENOENT;
+        }
+    }
+
+    /* It is a non-posix group with a GID. Needs resolving */
+    state->lookup_groups[state->num_groups].domain = domain;
+    state->lookup_groups[state->num_groups].gid = gid;
     DEBUG(SSSDBG_TRACE_INTERNAL, ("Adding GID %"SPRIgid"\n", gid));
-    state->num_gids++;
+    state->num_groups++;
     return EOK;
 }
 
@@ -556,11 +597,10 @@ simple_check_get_groups_primary(struct simple_check_groups_state *state,
 {
     errno_t ret;
     const char *group_attrs[] = { SYSDB_NAME, SYSDB_POSIX,
-                                  SYSDB_GIDNUM, NULL };
+                                  SYSDB_GIDNUM, SYSDB_SID_STR, NULL };
     struct ldb_message *msg;
 
-    ret = sysdb_search_group_by_gid(state, state->ctx->domain->sysdb,
-                                    state->ctx->domain,
+    ret = sysdb_search_group_by_gid(state, state->domain->sysdb, state->domain,
                                     gid, group_attrs, &msg);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
-- 
1.8.3.1



More information about the sssd-devel mailing list