URL: https://github.com/SSSD/sssd/pull/267
Author: jhrozek
 Title: #267: IFP: Resolve group names from GIDs if required
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/267/head:pr267
git checkout pr267
From 7c3d03e866f7c4a3df72da60cf84eda42c4092f3 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Wed, 24 May 2017 17:34:55 +0200
Subject: [PATCH 1/3] RESP: Provide a reusable request to fully resolve
 incomplete groups

After initgroups, the group objects might not be complete, but just
stubs that contain the SID and the GID. If the caller needs to know the
group names as well, this request allows them to iterate over the list
of the groups and resolve them one-by-one.
---
 src/responder/common/responder.h       |  14 +++
 src/responder/common/responder_utils.c | 206 +++++++++++++++++++++++++++++++++
 2 files changed, 220 insertions(+)

diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index dfe1ec455..c09ecd493 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -414,4 +414,18 @@ int sized_domain_name(TALLOC_CTX *mem_ctx,
                       const char *member_name,
                       struct sized_string **_name);
 
+/* Given a ldb_result structure that contains a result of sysdb_initgroups
+ * where some groups might be just 'stubs' that don't have a name, but only
+ * a SID and a GID, resolve those incomplete groups into full group objects
+ */
+struct tevent_req *resp_resolve_group_names_send(TALLOC_CTX *mem_ctx,
+                                                 struct tevent_context *ev,
+                                                 struct resp_ctx *rctx,
+                                                 struct sss_domain_info *dom,
+                                                 struct ldb_result *initgr_res);
+
+int resp_resolve_group_names_recv(TALLOC_CTX *mem_ctx,
+                                  struct tevent_req *req,
+                                  struct ldb_result **_initgr_named_res);
+
 #endif /* __SSS_RESPONDER_H__ */
diff --git a/src/responder/common/responder_utils.c b/src/responder/common/responder_utils.c
index b02212dfd..92f78a5bc 100644
--- a/src/responder/common/responder_utils.c
+++ b/src/responder/common/responder_utils.c
@@ -23,6 +23,7 @@
 #include <talloc.h>
 
 #include "responder/common/responder.h"
+#include "responder/common/cache_req/cache_req.h"
 #include "util/util.h"
 
 static inline bool
@@ -193,3 +194,208 @@ char *sss_resp_create_fqname(TALLOC_CTX *mem_ctx,
     talloc_free(tmp_ctx);
     return name;
 }
+
+struct resp_resolve_group_names_state {
+    struct tevent_context *ev;
+    struct resp_ctx *rctx;
+    struct sss_domain_info *dom;
+    struct ldb_result *initgr_res;
+
+    bool needs_refresh;
+    unsigned int group_iter;
+
+    struct ldb_result *initgr_named_res;
+};
+
+static void resp_resolve_group_done(struct tevent_req *subreq);
+static errno_t resp_resolve_group_next(struct tevent_req *req);
+static errno_t resp_resolve_group_reread_names(struct resp_resolve_group_names_state *state);
+
+struct tevent_req *resp_resolve_group_names_send(TALLOC_CTX *mem_ctx,
+                                                 struct tevent_context *ev,
+                                                 struct resp_ctx *rctx,
+                                                 struct sss_domain_info *dom,
+                                                 struct ldb_result *initgr_res)
+{
+    struct resp_resolve_group_names_state *state;
+    struct tevent_req *req;
+    errno_t ret;
+
+    req = tevent_req_create(mem_ctx, &state, struct resp_resolve_group_names_state);
+    if (req == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create() failed\n");
+        return NULL;
+    }
+    state->ev = ev;
+    state->rctx = rctx;
+    state->dom = dom;
+    state->initgr_res = initgr_res;
+
+    ret = resp_resolve_group_next(req);
+    if (ret == EOK) {
+        goto immediate;
+    } else if (ret != EAGAIN) {
+        goto immediate;
+    }
+
+    return req;
+
+immediate:
+    if (ret == EOK) {
+        tevent_req_done(req);
+    } else {
+        tevent_req_error(req, ret);
+    }
+    tevent_req_post(req, ev);
+    return req;
+}
+
+static bool
+resp_resolve_group_needs_refresh(struct resp_resolve_group_names_state *state)
+{
+    /* Refresh groups that have a non-zero GID,
+     * but are marked as non-POSIX
+     */
+    bool is_posix;
+    uint64_t gid;
+    struct ldb_message *group_msg;
+
+    group_msg = state->initgr_res->msgs[state->group_iter];
+
+    is_posix = ldb_msg_find_attr_as_bool(group_msg, SYSDB_POSIX, false);
+    gid = ldb_msg_find_attr_as_uint64(group_msg, SYSDB_GIDNUM, 0);
+
+    if (is_posix == false && gid != 0) {
+        return true;
+    }
+
+    return false;
+}
+
+static errno_t resp_resolve_group_next(struct tevent_req *req)
+{
+    struct cache_req_data *data;
+    uint64_t gid;
+    struct tevent_req *subreq;
+    struct resp_resolve_group_names_state *state;
+
+    state = tevent_req_data(req, struct resp_resolve_group_names_state);
+
+    while (state->group_iter < state->initgr_res->count
+            && !resp_resolve_group_needs_refresh(state)) {
+        state->group_iter++;
+    }
+
+    if (state->group_iter >= state->initgr_res->count) {
+        /* All groups were refreshed */
+        return EOK;
+    }
+
+    /* Fire a request */
+    gid = ldb_msg_find_attr_as_uint64(state->initgr_res->msgs[state->group_iter],
+                                      SYSDB_GIDNUM, 0);
+    if (gid == 0) {
+        return EINVAL;
+    }
+
+    data = cache_req_data_id_attrs(state, CACHE_REQ_GROUP_BY_ID, gid, NULL);
+    if (data == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to set cache request data!\n");
+        return ENOMEM;
+    }
+
+    subreq = cache_req_send(state,
+                            state->ev,
+                            state->rctx,
+                            state->rctx->ncache,
+                            0,
+                            CACHE_REQ_ANY_DOM,
+                            NULL,
+                            data);
+    if (subreq == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to send cache request!\n");
+        return ENOMEM;
+    }
+
+    tevent_req_set_callback(subreq, resp_resolve_group_done, req);
+    return EAGAIN;
+}
+
+static void resp_resolve_group_done(struct tevent_req *subreq)
+{
+    struct resp_resolve_group_names_state *state;
+    struct tevent_req *req;
+    errno_t ret;
+
+    req = tevent_req_callback_data(subreq, struct tevent_req);
+    state = tevent_req_data(req, struct resp_resolve_group_names_state);
+
+    ret = cache_req_single_domain_recv(state, subreq, NULL);
+    talloc_zfree(subreq);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "Failed to refresh group\n");
+        /* Try to refresh the others on error */
+    }
+
+    state->group_iter++;
+    state->needs_refresh = true;
+
+    ret = resp_resolve_group_next(req);
+    if (ret == EOK) {
+        ret = resp_resolve_group_reread_names(state);
+        if (ret != EOK) {
+            tevent_req_error(req, ret);
+            return;
+        }
+        DEBUG(SSSDBG_TRACE_FUNC, "All groups are refreshed, done\n");
+        tevent_req_done(req);
+        return;
+    } else if (ret != EAGAIN) {
+        tevent_req_error(req, ret);
+        return;
+    }
+
+    /* Continue refreshing.. */
+}
+
+static errno_t
+resp_resolve_group_reread_names(struct resp_resolve_group_names_state *state)
+{
+    errno_t ret;
+    const char *username;
+
+    /* re-read reply in case any groups were renamed */
+    /* msgs[0] is the user entry */
+    username = sss_view_ldb_msg_find_attr_as_string(state->dom,
+                                                    state->initgr_res->msgs[0],
+                                                    SYSDB_NAME,
+                                                    NULL);
+    if (username == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "A user with no name?\n");
+        return EINVAL;
+    }
+
+    ret = sysdb_initgroups_with_views(state,
+                                      state->dom,
+                                      username,
+                                      &state->initgr_named_res);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "Cannot re-read the group names\n");
+        return ret;
+    }
+
+    return EOK;
+}
+
+int resp_resolve_group_names_recv(TALLOC_CTX *mem_ctx,
+                                  struct tevent_req *req,
+                                  struct ldb_result **_initgr_named_res)
+{
+    struct resp_resolve_group_names_state *state = NULL;
+    state = tevent_req_data(req, struct resp_resolve_group_names_state);
+
+    TEVENT_REQ_RETURN_ON_ERROR(req);
+
+    *_initgr_named_res = talloc_steal(mem_ctx, state->initgr_named_res);
+    return EOK;
+}

From eac99fa4a173009b36c8458d06ec3a8169e93e55 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 9 May 2017 12:21:32 +0200
Subject: [PATCH 2/3] IFP: Only format the output name to the short version
 before output

The ifp_user_get_attr_done() request handler was reused for both
GetUserGroups and GetUserAttrs requests. Yet, it performed output
formatting of name and nameAlias.

This is bad, because the output formatting should really be done only
during output. Also, it broke any post-processing of the returned
message which the request might do later.
---
 src/responder/ifp/ifpsrv_cmd.c | 64 ++++++++++++------------------------------
 1 file changed, 18 insertions(+), 46 deletions(-)

diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c
index e4d6c42ef..915f77e38 100644
--- a/src/responder/ifp/ifpsrv_cmd.c
+++ b/src/responder/ifp/ifpsrv_cmd.c
@@ -181,26 +181,6 @@ static void ifp_user_get_attr_process(struct tevent_req *req)
 }
 
 static errno_t
-ifp_user_get_attr_replace_space(TALLOC_CTX *mem_ctx,
-                                struct ldb_message_element *el,
-                                const char sub)
-{
-    int i;
-
-    for (i = 0; i < el->num_values; i++) {
-        el->values[i].data = (uint8_t *) sss_replace_space(mem_ctx,
-                                             (const char *) el->values[i].data,
-                                             sub);
-        if (el->values[i].data == NULL) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "sss_replace_space failed, skipping\n");
-            return ENOMEM;
-        }
-    }
-
-    return EOK;
-}
-
-static errno_t
 ifp_user_get_attr_handle_reply(struct sss_domain_info *domain,
                                struct ifp_req *ireq,
                                const char **attrs,
@@ -234,6 +214,24 @@ ifp_user_get_attr_handle_reply(struct sss_domain_info *domain,
     }
 
     if (res->count > 0) {
+        ret = ifp_ldb_el_output_name(ireq->ifp_ctx->rctx, res->msgs[0],
+                                     SYSDB_NAME, domain);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Cannot convert SYSDB_NAME to output format [%d]: %s\n",
+                  ret, sss_strerror(ret));
+            return sbus_request_finish(ireq->dbus_req, NULL);
+        }
+
+        ret = ifp_ldb_el_output_name(ireq->ifp_ctx->rctx, res->msgs[0],
+                                     SYSDB_NAME_ALIAS, domain);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "Cannot convert SYSDB_NAME_ALIAS to output format [%d]: %s\n",
+                  ret, sss_strerror(ret));
+            return sbus_request_finish(ireq->dbus_req, NULL);
+        }
+
         for (ai = 0; attrs[ai]; ai++) {
             el = sss_view_ldb_msg_find_element(domain, res->msgs[0], attrs[ai]);
             if (el == NULL || el->num_values == 0) {
@@ -243,18 +241,6 @@ ifp_user_get_attr_handle_reply(struct sss_domain_info *domain,
                 continue;
             }
 
-            /* Normalize white space in user names */
-            if (ireq->ifp_ctx->rctx->override_space != '\0' &&
-                    strcmp(attrs[ai], SYSDB_NAME) == 0) {
-                ret = ifp_user_get_attr_replace_space(ireq, el,
-                                        ireq->ifp_ctx->rctx->override_space);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_MINOR_FAILURE, "Cannot normalize %s\n",
-                          attrs[ai]);
-                    continue;
-                }
-            }
-
             ret = ifp_add_ldb_el_to_dict(&iter_dict, el);
             if (ret != EOK) {
                 DEBUG(SSSDBG_MINOR_FAILURE,
@@ -575,20 +561,6 @@ static void ifp_user_get_attr_done(struct tevent_req *subreq)
         }
     }
 
-    ret = ifp_ldb_el_output_name(state->rctx, state->res->msgs[0],
-                                 SYSDB_NAME, state->dom);
-    if (ret != EOK) {
-        tevent_req_error(req, ret);
-        return;
-    }
-
-    ret = ifp_ldb_el_output_name(state->rctx, state->res->msgs[0],
-                                 SYSDB_NAME_ALIAS, state->dom);
-    if (ret != EOK) {
-        tevent_req_error(req, ret);
-        return;
-    }
-
     tevent_req_done(req);
 }
 

From 6eab62ffdf9824a7e282e1b94b9934e4f579de67 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Wed, 24 May 2017 21:32:28 +0200
Subject: [PATCH 3/3] IFP: Resolve group names from GIDs if required

Resolves:
    https://pagure.io/SSSD/sssd/issue/3392

The AD provider only converts SIDs to GIDs during initgroups
to improve performance. But this is not sufficient for the
org.freedesktop.sssd.infopipe.GetUserGroups method, which needs to return
names.

We need to resolve the GIDs to names ourselves in that method.
---
 src/responder/ifp/ifpsrv_cmd.c | 115 +++++++++++++++++++++++++++++++----------
 1 file changed, 89 insertions(+), 26 deletions(-)

diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c
index 915f77e38..70728e1bb 100644
--- a/src/responder/ifp/ifpsrv_cmd.c
+++ b/src/responder/ifp/ifpsrv_cmd.c
@@ -259,7 +259,18 @@ ifp_user_get_attr_handle_reply(struct sss_domain_info *domain,
     return sbus_request_finish(ireq->dbus_req, reply);
 }
 
+struct ifp_user_get_groups_state {
+    struct resp_ctx *rctx;
+
+    struct ifp_attr_req *group_attr_req;
+
+    struct ldb_result *res;
+    struct ldb_result *res_names;
+    struct sss_domain_info *dom;
+};
+
 static void ifp_user_get_groups_process(struct tevent_req *req);
+static void ifp_user_get_groups_names_resolved(struct tevent_req *req);
 static errno_t ifp_user_get_groups_reply(struct sss_domain_info *domain,
                                          struct ifp_req *ireq,
                                          struct ldb_result *res);
@@ -269,7 +280,7 @@ int ifp_user_get_groups(struct sbus_request *dbus_req,
 {
     struct ifp_req *ireq;
     struct ifp_ctx *ifp_ctx;
-    struct ifp_attr_req *group_req;
+    struct ifp_user_get_groups_state *state;
     struct tevent_req *req;
     errno_t ret;
 
@@ -284,68 +295,120 @@ int ifp_user_get_groups(struct sbus_request *dbus_req,
         return ifp_req_create_handle_failure(dbus_req, ret);
     }
 
-    group_req = talloc_zero(ireq, struct ifp_attr_req);
-    if (group_req == NULL) {
+    state = talloc_zero(ireq, struct ifp_user_get_groups_state);
+    if (state == NULL) {
         return sbus_request_finish(dbus_req, NULL);
     }
-    group_req->ireq = ireq;
-    group_req->name = arg_user;
+    state->rctx = ifp_ctx->rctx;
 
-    group_req->attrs = talloc_zero_array(group_req, const char *, 2);
-    if (group_req->attrs == NULL) {
+    state->group_attr_req = talloc_zero(state, struct ifp_attr_req);
+    if (state->group_attr_req == NULL) {
         return sbus_request_finish(dbus_req, NULL);
     }
+    state->group_attr_req->ireq = ireq;
+    state->group_attr_req->name = arg_user;
 
-    group_req->attrs[0] = talloc_strdup(group_req->attrs, SYSDB_MEMBEROF);
-    if (group_req->attrs[0] == NULL) {
+    state->group_attr_req->attrs = talloc_zero_array(state->group_attr_req,
+                                                     const char *, 2);
+    if (state->group_attr_req->attrs == NULL) {
+        return sbus_request_finish(dbus_req, NULL);
+    }
+
+    state->group_attr_req->attrs[0] = talloc_strdup(state->group_attr_req->attrs,
+                                                    SYSDB_MEMBEROF);
+    if (state->group_attr_req->attrs[0] == NULL) {
         return sbus_request_finish(dbus_req, NULL);
     }
 
     DEBUG(SSSDBG_FUNC_DATA,
           "Looking up groups of user [%s] on behalf of %"PRIi64"\n",
-          group_req->name, group_req->ireq->dbus_req->client);
+          state->group_attr_req->name,
+          state->group_attr_req->ireq->dbus_req->client);
 
     req = ifp_user_get_attr_send(ireq, ifp_ctx->rctx,
                                  ifp_ctx->rctx->ncache, SSS_DP_INITGROUPS,
-                                 group_req->name, group_req->attrs);
+                                 state->group_attr_req->name,
+                                 state->group_attr_req->attrs);
     if (req == NULL) {
         return sbus_request_finish(dbus_req, NULL);
     }
-    tevent_req_set_callback(req, ifp_user_get_groups_process, group_req);
+    tevent_req_set_callback(req,
+                            ifp_user_get_groups_process,
+                            state);
     return EOK;
 }
 
 static void ifp_user_get_groups_process(struct tevent_req *req)
 {
-    struct ifp_attr_req *group_req;
+    struct ifp_user_get_groups_state *state;
+    struct ifp_attr_req *group_attr_req;
     errno_t ret;
-    struct ldb_result *res;
-    struct sss_domain_info *dom;
 
-    group_req = tevent_req_callback_data(req, struct ifp_attr_req);
+    state = tevent_req_callback_data(req, struct ifp_user_get_groups_state);
+    group_attr_req = state->group_attr_req;
 
-    ret = ifp_user_get_attr_recv(group_req, req, &res, &dom);
+    ret = ifp_user_get_attr_recv(group_attr_req, req, &state->res, &state->dom);
     talloc_zfree(req);
     if (ret == ENOENT) {
-        sbus_request_fail_and_finish(group_req->ireq->dbus_req,
-                               sbus_error_new(group_req->ireq->dbus_req,
+        sbus_request_fail_and_finish(group_attr_req->ireq->dbus_req,
+                               sbus_error_new(group_attr_req->ireq->dbus_req,
                                               DBUS_ERROR_FAILED,
                                               "No such user\n"));
         return;
     } else if (ret != EOK) {
-        sbus_request_fail_and_finish(group_req->ireq->dbus_req,
-                               sbus_error_new(group_req->ireq->dbus_req,
+        sbus_request_fail_and_finish(group_attr_req->ireq->dbus_req,
+                               sbus_error_new(group_attr_req->ireq->dbus_req,
                                               DBUS_ERROR_FAILED,
                                               "Failed to read attribute\n"));
         return;
     }
 
-    ret = ifp_user_get_groups_reply(dom, group_req->ireq, res);
+    req = resp_resolve_group_names_send(state,
+                                        state->rctx->ev,
+                                        state->rctx,
+                                        state->dom,
+                                        state->res);
+    if (req == NULL) {
+        sbus_request_finish(group_attr_req->ireq->dbus_req, NULL);
+        return;
+    }
+    tevent_req_set_callback(req,
+                            ifp_user_get_groups_names_resolved,
+                            state);
+}
+
+static void ifp_user_get_groups_names_resolved(struct tevent_req *req)
+{
+    struct ifp_user_get_groups_state *state;
+    struct ifp_attr_req *group_attr_req;
+    errno_t ret;
+
+    state = tevent_req_callback_data(req, struct ifp_user_get_groups_state);
+    group_attr_req = state->group_attr_req;
+
+    ret = resp_resolve_group_names_recv(state, req, &state->res_names);
+    talloc_zfree(req);
     if (ret != EOK) {
-        sbus_request_fail_and_finish(group_req->ireq->dbus_req,
-                               sbus_error_new(group_req->ireq->dbus_req,
-                                              DBUS_ERROR_FAILED,
-                                              "Failed to build a reply\n"));
+        sbus_request_fail_and_finish(group_attr_req->ireq->dbus_req,
+                            sbus_error_new(group_attr_req->ireq->dbus_req,
+                                           DBUS_ERROR_FAILED,
+                                           "Failed to resolve groupnames\n"));
+        return;
+    }
+
+    if (state->res_names == NULL) {
+        state->res_names = state->res;
+    }
+
+    ret = ifp_user_get_groups_reply(state->dom,
+                                    group_attr_req->ireq,
+                                    state->res_names);
+    if (ret != EOK) {
+        sbus_request_fail_and_finish(group_attr_req->ireq->dbus_req,
+                                     sbus_error_new(
+                                            group_attr_req->ireq->dbus_req,
+                                            DBUS_ERROR_FAILED,
+                                            "Failed to build a reply\n"));
         return;
     }
 }
_______________________________________________
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