[SSSD] [PATCHES] PAC responder improvements

Sumit Bose sbose at redhat.com
Fri Aug 16 16:25:02 UTC 2013


Hi,

this series of patches contains improvements for the PAC responder
related to the support of UIDs and GIDs managed by AD.

The first patch is a fix for https://fedorahosted.org/sssd/ticket/1996.
The original idea in the ticket was to modify an existing user entry
instead of deleting and recreating it. But since the PAC does not
contain any useful information which would improve the entry I decided to
not touch existing user entries at all and only update the group
memberships.

Please find details about the other patches in the commit messages.

bye,
Sumit
-------------- next part --------------
From 1e54d2061d72d2f72c30d3fced9b43d4ec369a28 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 1 Aug 2013 12:40:24 +0200
Subject: [PATCH 1/6] PAC: if user entry already exists keep it

Currently the PAC responder deletes a user entry and recreates it if
some attributes seems to be different.

Two of the attributes where the home directory and the shell of the
user. Those two attributes are not available from the PAC but where
generates by the PAC responder. The corresponding ID provider might have
better means to determine those attributes, e.g. read them from LDAP, so
we shouldn't change them here.

The third attribute is the user name. Since the PAC responder does
lookups only based on the UID we can wait until the ID provider updates
the entry.

Fixes https://fedorahosted.org/sssd/ticket/1996
---
 src/responder/pac/pacsrv.h       |    2 -
 src/responder/pac/pacsrv_cmd.c   |   55 +++++++-------------------------------
 src/responder/pac/pacsrv_utils.c |   39 ---------------------------
 3 files changed, 10 insertions(+), 86 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 6477e44..126ec7d 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -81,6 +81,4 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
                          struct PAC_LOGON_INFO *logon_info,
                          struct passwd **_pwd,
                          struct sysdb_attrs **_attrs);
-
-bool new_and_cached_user_differs(struct passwd *pwd, struct ldb_message *msg);
 #endif /* __PACSRV_H__ */
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 2b11aca..e515200 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -549,7 +549,6 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
     struct passwd *pwd = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
     struct sysdb_attrs *user_attrs = NULL;
-    const char *tmp_str;
 
     sysdb = pr_ctx->dom->sysdb;
     if (sysdb == NULL) {
@@ -575,53 +574,19 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
 
     ret = sysdb_search_user_by_uid(tmp_ctx, sysdb, pr_ctx->dom,
                                    pwd->pw_uid, attrs, &msg);
-    if (ret == EOK) {
-        if (new_and_cached_user_differs(pwd, msg)) {
-            ret = sysdb_delete_user(sysdb, pr_ctx->dom, NULL, pwd->pw_uid);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_delete_user failed.\n"));
-                goto done;
-            }
-
-            /* If the entry is delete we might loose the information about the
-             * original DN of e.g. an IPA user or a chache password. */
-            tmp_str = ldb_msg_find_attr_as_string(msg, SYSDB_ORIG_DN, NULL);
-            if (tmp_str != NULL) {
-                ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_DN,
-                                             tmp_str);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_OP_FAILURE,
-                          ("sysdb_attrs_add_string failed.\n"));
-                    goto done;
-                }
-            }
-
-            tmp_str = ldb_msg_find_attr_as_string(msg, SYSDB_CACHEDPWD, NULL);
-            if (tmp_str != NULL) {
-                ret = sysdb_attrs_add_string(user_attrs, SYSDB_CACHEDPWD,
-                                             tmp_str);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_OP_FAILURE,
-                          ("sysdb_attrs_add_string failed.\n"));
-                    goto done;
-                }
-            }
-        } else {
+    if (ret == ENOENT) {
+        ret = sysdb_store_user(sysdb, pr_ctx->dom, pwd->pw_name, NULL,
+                               pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
+                               pwd->pw_dir,
+                               pwd->pw_shell, NULL, user_attrs, NULL,
+                               pr_ctx->dom->user_timeout, 0);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n",
+                                      ret, strerror(ret)));
             goto done;
         }
     } else if (ret != EOK && ret != ENOENT) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_search_user_by_name failed.\n"));
-        goto done;
-    }
-
-    ret = sysdb_store_user(sysdb, pr_ctx->dom, pwd->pw_name, NULL,
-                           pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
-                           pwd->pw_dir,
-                           pwd->pw_shell, NULL, user_attrs, NULL,
-                           pr_ctx->dom->user_timeout, 0);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_store_user failed [%d][%s].\n",
-                                  ret, strerror(ret)));
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_search_user_by_id failed.\n"));
         goto done;
     }
 
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index f70438b..a52282c 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -506,42 +506,3 @@ done:
 
     return ret;
 }
-
-static bool compare_string_with_attr(const char *val, struct ldb_message *msg,
-                                     const char *attr)
-{
-    const char *str;
-
-    str = ldb_msg_find_attr_as_string(msg, attr, NULL);
-    if ((str == NULL && val == NULL) ||
-        (str != NULL && val != NULL && strcmp(str, val) == 0)) {
-        return true;
-    }
-
-    return false;
-}
-
-bool new_and_cached_user_differs(struct passwd *pwd, struct ldb_message *msg)
-{
-    if (pwd == NULL || msg == NULL) {
-        return true;
-    }
-
-    if (!compare_string_with_attr(pwd->pw_name, msg, SYSDB_NAME) &&
-        !compare_string_with_attr(pwd->pw_name, msg, SYSDB_NAME_ALIAS)) {
-        DEBUG(SSSDBG_TRACE_FUNC, ("Names differ."));
-        return true;
-    }
-
-    if (!compare_string_with_attr(pwd->pw_dir, msg, SYSDB_HOMEDIR)) {
-        DEBUG(SSSDBG_TRACE_FUNC, ("Home directories differ."));
-        return true;
-    }
-
-    if (!compare_string_with_attr(pwd->pw_shell, msg, SYSDB_SHELL)) {
-        DEBUG(SSSDBG_TRACE_FUNC, ("Shells differ."));
-        return true;
-    }
-
-    return false;
-}
-- 
1.7.7.6

-------------- next part --------------
From c9a57d100b10356c24b39383eac618e35874f741 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 6 Aug 2013 11:10:10 +0200
Subject: [PATCH 2/6] PAC: do not create users with missing GID

If the user entry does not exist in the cache and a primary GID cannot
be found it does not make sense to create a user entry.
---
 src/responder/pac/pacsrv_cmd.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index e515200..bcdcdc4 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -575,6 +575,20 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
     ret = sysdb_search_user_by_uid(tmp_ctx, sysdb, pr_ctx->dom,
                                    pwd->pw_uid, attrs, &msg);
     if (ret == ENOENT) {
+        if (pwd->pw_gid == 0 && !pr_ctx->dom->mpg) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Primary group RID from the PAC " \
+                                        "cannot be translated into a GID for " \
+                                        "user [%s]. Typically this happens " \
+                                        "when UIDs and GIDs are read from AD " \
+                                        "and the primary AD group does not " \
+                                        "have a GID assigned. Make sure the " \
+                                        "user is created by the ID provider " \
+                                        "before GSSAPI based authentication " \
+                                        "is used in this case.", pwd->pw_name));
+            ret = EINVAL;
+            goto done;
+        }
+
         ret = sysdb_store_user(sysdb, pr_ctx->dom, pwd->pw_name, NULL,
                                pwd->pw_uid, pwd->pw_gid, pwd->pw_gecos,
                                pwd->pw_dir,
-- 
1.7.7.6

-------------- next part --------------
From d5383508cf0c1c632c79200598bb00e6a0809498 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 8 Aug 2013 12:35:12 +0200
Subject: [PATCH 3/6] PAC: handle non-POSIX groups in cache

Since the DN of the group is used to remove a membership it is not
necessary to check if the GID is valid.
---
 src/responder/pac/pacsrv.h     |    1 -
 src/responder/pac/pacsrv_cmd.c |   19 +++++++++----------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/responder/pac/pacsrv.h b/src/responder/pac/pacsrv.h
index 126ec7d..f90b40c 100644
--- a/src/responder/pac/pacsrv.h
+++ b/src/responder/pac/pacsrv.h
@@ -55,7 +55,6 @@ struct pac_ctx {
 };
 
 struct grp_info {
-    gid_t gid;
     char *orig_dn;
     struct ldb_dn *dn;
 };
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index bcdcdc4..79841b5 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -454,7 +454,10 @@ static errno_t pac_user_get_grp_info(TALLOC_CTX *mem_ctx,
             key.str = discard_const(cur_sid);
             ret = hash_lookup(pr_ctx->sid_table, &key, &value);
             if (ret == HASH_SUCCESS) {
-                /* user is already member of the group */
+                DEBUG(SSSDBG_TRACE_ALL, ("User [%s] already member of group " \
+                                         "with SID [%s].\n",
+                                         pr_ctx->user_name, cur_sid));
+
                 ret = hash_delete(pr_ctx->sid_table, &key);
                 if (ret != HASH_SUCCESS) {
                     DEBUG(SSSDBG_OP_FAILURE, ("Failed to remove hash entry.\n"));
@@ -462,15 +465,9 @@ static errno_t pac_user_get_grp_info(TALLOC_CTX *mem_ctx,
                     goto done;
                 }
             } else if (ret == HASH_ERROR_KEY_NOT_FOUND) {
-                /* group is not in the PAC anymore, membership must be removed */
-                del_grp_list[del_idx].gid =
-                                  ldb_msg_find_attr_as_uint64(res->msgs[c + 1],
-                                                              SYSDB_GIDNUM, 0);
-                if (del_grp_list[del_idx].gid == 0) {
-                    DEBUG(SSSDBG_OP_FAILURE, ("Missing GID.\n"));
-                    ret = EINVAL;
-                    goto done;
-                }
+                DEBUG(SSSDBG_TRACE_INTERNAL, ("Group with SID [%s] is not in " \
+                                              "the PAC anymore, membership " \
+                                              "must be removed.\n", cur_sid));
 
                 tmp_str = ldb_msg_find_attr_as_string(res->msgs[c + 1],
                                                       SYSDB_ORIG_DN, NULL);
@@ -517,6 +514,8 @@ static errno_t pac_user_get_grp_info(TALLOC_CTX *mem_ctx,
                     ret = ENOMEM;
                     goto done;
                 }
+                DEBUG(SSSDBG_TRACE_ALL, ("SID [%s] added to add_sids " \
+                                         "list.\n", entry->key.str));
                 c++;
             }
         }
-- 
1.7.7.6

-------------- next part --------------
From 49775fa334b61d387f9138bfb53807cbf787dbb8 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 8 Aug 2013 14:09:42 +0200
Subject: [PATCH 4/6] PAC: read user DN instead of constructing it

To avoid issues with case-sensitivity it is more reliable to search the
user entry in the cache and use the returned DN instead of constructing
it.
---
 src/responder/pac/pacsrv_cmd.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 79841b5..bf3ea75 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -627,6 +627,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
     struct tevent_req *req;
     errno_t ret;
     char *dom_name = NULL;
+    struct ldb_message *msg;
 
     req = tevent_req_create(pr_ctx, &state, struct pac_save_memberships_state);
     if (req == NULL) {
@@ -642,11 +643,15 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
-    state->user_dn = sysdb_user_dn(dom->sysdb, state, dom, dom_name);
-    if (state->user_dn == NULL) {
-        ret = ENOMEM;
+    ret = sysdb_search_user_by_name(state, dom->sysdb, dom, dom_name, NULL,
+                                    &msg);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_search_user_by_name failed " \
+                                  "[%d][%s].\n", ret, strerror(ret)));
         goto done;
     }
+
+    state->user_dn = msg->dn;
     state->pr_ctx = pr_ctx;
 
     ret = pac_save_memberships_delete(state);
@@ -718,7 +723,11 @@ pac_save_memberships_delete(struct pac_save_memberships_state *state)
                                      pr_ctx->del_grp_list[c].dn,
                                      LDB_FLAG_MOD_DELETE);
         if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed.\n"));
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed for " \
+                                      "user [%s] and group[%s].\n",
+                                      ldb_dn_get_linearized(state->user_dn),
+                                      ldb_dn_get_linearized(
+                                                  pr_ctx->del_grp_list[c].dn)));
             goto done;
         }
 
@@ -921,7 +930,10 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     ret = sysdb_mod_group_member(grp_dom->sysdb, user_dn, group->dn,
                                  LDB_FLAG_MOD_ADD);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed.\n"));
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed user [%s] " \
+                                  "group [%s].\n",
+                                  ldb_dn_get_linearized(user_dn),
+                                  ldb_dn_get_linearized(group->dn)));
         goto done;
     }
 
-- 
1.7.7.6

-------------- next part --------------
From 966390e5436d2eed565774adddf33f6ce3f85b24 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 8 Aug 2013 16:56:06 +0200
Subject: [PATCH 5/6] PAC: do not fail if a single group cannot be
 added/removed

When processing a list of groups we try to process as much as possible
only not stop on the first error.
---
 src/responder/pac/pacsrv_cmd.c |   49 +++++++++++++++++++++++++--------------
 1 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index bf3ea75..620b3c0 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -704,13 +704,6 @@ pac_save_memberships_delete(struct pac_save_memberships_state *state)
         return ENOMEM;
     }
 
-    user_attrs = sysdb_new_attrs(tmp_ctx);
-    if (user_attrs == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_new_attrs failed.\n"));
-        ret = ENOMEM;
-        goto done;
-    }
-
     ret = sysdb_transaction_start(pr_ctx->dom->sysdb);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_transaction_start failed.\n"));
@@ -719,6 +712,8 @@ pac_save_memberships_delete(struct pac_save_memberships_state *state)
     in_transaction = true;
 
     for (c = 0; c < pr_ctx->del_grp_count; c++) {
+        /* If there is a failure for one group we still try to remove the
+         * remaining groups. */
         ret = sysdb_mod_group_member(pr_ctx->dom->sysdb, state->user_dn,
                                      pr_ctx->del_grp_list[c].dn,
                                      LDB_FLAG_MOD_DELETE);
@@ -728,24 +723,31 @@ pac_save_memberships_delete(struct pac_save_memberships_state *state)
                                       ldb_dn_get_linearized(state->user_dn),
                                       ldb_dn_get_linearized(
                                                   pr_ctx->del_grp_list[c].dn)));
-            goto done;
+            continue;
         }
 
         if (pr_ctx->del_grp_list[c].orig_dn != NULL) {
+            user_attrs = sysdb_new_attrs(tmp_ctx);
+            if (user_attrs == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_new_attrs failed.\n"));
+                continue;
+            }
+
             ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF,
                                          pr_ctx->del_grp_list[c].orig_dn);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
-                goto done;
+                continue;
             }
-        }
-    }
 
-    ret = sysdb_set_entry_attr(pr_ctx->dom->sysdb, state->user_dn, user_attrs,
-                               LDB_FLAG_MOD_DELETE);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
-        goto done;
+            ret = sysdb_set_entry_attr(pr_ctx->dom->sysdb, state->user_dn, user_attrs,
+                                       LDB_FLAG_MOD_DELETE);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
+                continue;
+            }
+            talloc_free(user_attrs);
+        }
     }
 
     ret = sysdb_transaction_commit(pr_ctx->dom->sysdb);
@@ -827,7 +829,10 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
 
             return EAGAIN;
         } else  {
-            goto done;
+            DEBUG(SSSDBG_OP_FAILURE, ("pac_store_membership failed, "
+                                      "trying next group.\n"));
+            state->sid_iter++;
+            continue;
         }
     }
 
@@ -884,7 +889,8 @@ static void pac_get_group_done(struct tevent_req *subreq)
 
     ret = pac_store_membership(state->pr_ctx, state->user_dn, gid, grp_dom);
     if (ret != EOK) {
-        goto error;
+        DEBUG(SSSDBG_OP_FAILURE, ("pac_store_membership failed, "
+                                  "trying next group.\n"));
     }
     state->sid_iter++;
 
@@ -927,6 +933,9 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         goto done;
     }
 
+    DEBUG(SSSDBG_TRACE_ALL, ("Adding user [%s] to group [%d][%s].\n",
+                             ldb_dn_get_linearized(user_dn), gid,
+                             ldb_dn_get_linearized(group->dn)));
     ret = sysdb_mod_group_member(grp_dom->sysdb, user_dn, group->dn,
                                  LDB_FLAG_MOD_ADD);
     if (ret != EOK) {
@@ -959,6 +968,10 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
             goto done;
         }
+    } else {
+        DEBUG(SSSDBG_MINOR_FAILURE, ("Original DN not available for group " \
+                                     "[%d][%s].\n", gid,
+                                     ldb_dn_get_linearized(group->dn)));
     }
 
 done:
-- 
1.7.7.6

-------------- next part --------------
From c5f9cb4f803c06e167d4ab4f3b06bd554ae15f07 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 8 Aug 2013 18:29:48 +0200
Subject: [PATCH 6/6] PAC: use SID instead of GID to search for groups

With the support of POSIX IDs managed on the AD side we may find
non-POSIX groups, i.e. groups which do not have a GID assigned in AD, in
the PAC. Since in this case all cached groups have a SDI attribute it is
more reliable to search the groups by SID instead of GID.
---
 src/responder/pac/pacsrv_cmd.c |   89 ++++++++++++++++++---------------------
 1 files changed, 41 insertions(+), 48 deletions(-)

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 620b3c0..0d81258 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -91,7 +91,7 @@ static void pac_get_group_done(struct tevent_req *subreq);
 static errno_t pac_save_memberships_next(struct tevent_req *req);
 static errno_t pac_store_membership(struct pac_req_ctx *pr_ctx,
                                     struct ldb_dn *user_dn,
-                                    gid_t gid,
+                                    const char *grp_sid_str,
                                     struct sss_domain_info *grp_dom);
 struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx);
 static void pac_save_memberships_done(struct tevent_req *req);
@@ -774,16 +774,11 @@ done:
 static errno_t pac_save_memberships_next(struct tevent_req *req)
 {
     errno_t ret;
-    uint32_t gid;
     char *sid;
     struct sss_domain_info *grp_dom;
     struct tevent_req *subreq;
     struct pac_save_memberships_state *state;
     struct pac_req_ctx *pr_ctx;
-    hash_key_t key;
-    hash_value_t value;
-
-    key.type = HASH_KEY_STRING;
 
     state = tevent_req_data(req, struct pac_save_memberships_state);
     pr_ctx = state->pr_ctx;
@@ -804,23 +799,15 @@ static errno_t pac_save_memberships_next(struct tevent_req *req)
             DEBUG(SSSDBG_OP_FAILURE, ("responder_get_domain_by_id failed.\n"));
             return ret;
         }
-        key.str = sid;
-        ret = hash_lookup(pr_ctx->sid_table, &key, &value);
-        if (ret != HASH_SUCCESS) {
-            DEBUG(SSSDBG_OP_FAILURE, ("hash_lookup failed.\n"));
-            return EIO;
-        }
-        gid = value.ul;
 
-        ret = pac_store_membership(state->pr_ctx, state->user_dn, gid, grp_dom);
+        ret = pac_store_membership(state->pr_ctx, state->user_dn, sid, grp_dom);
         if (ret == EOK) {
             state->sid_iter++;
             continue;
         } else if (ret == ENOENT) {
             subreq = sss_dp_get_account_send(state, pr_ctx->cctx->rctx,
                                              grp_dom, true,
-                                             SSS_DP_GROUP, NULL,
-                                             gid, NULL);
+                                             SSS_DP_SECID, sid, 0, NULL);
             if (subreq == NULL) {
                 ret = ENOMEM;
                 goto done;
@@ -853,14 +840,9 @@ static void pac_get_group_done(struct tevent_req *subreq)
     dbus_uint16_t err_maj;
     dbus_uint32_t err_min;
     char *err_msg;
-    gid_t gid;
     char *sid;
     struct sss_domain_info *grp_dom;
     struct pac_req_ctx *pr_ctx = state->pr_ctx;
-    hash_key_t key;
-    hash_value_t value;
-
-    key.type = HASH_KEY_STRING;
 
     ret = sss_dp_get_account_recv(req, subreq,
                                   &err_maj, &err_min,
@@ -878,16 +860,7 @@ static void pac_get_group_done(struct tevent_req *subreq)
         goto error;
     }
 
-    key.str = sid;
-    ret = hash_lookup(pr_ctx->sid_table, &key, &value);
-    if (ret != HASH_SUCCESS) {
-        DEBUG(SSSDBG_OP_FAILURE, ("hash_lookup failed.\n"));
-        ret = EIO;
-        goto error;
-    }
-    gid = value.ul;
-
-    ret = pac_store_membership(state->pr_ctx, state->user_dn, gid, grp_dom);
+    ret = pac_store_membership(state->pr_ctx, state->user_dn, sid, grp_dom);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("pac_store_membership failed, "
                                   "trying next group.\n"));
@@ -910,44 +883,64 @@ error:
 static errno_t
 pac_store_membership(struct pac_req_ctx *pr_ctx,
                      struct ldb_dn *user_dn,
-                     gid_t gid, struct sss_domain_info *grp_dom)
+                     const char *grp_sid_str,
+                     struct sss_domain_info *grp_dom)
 {
     TALLOC_CTX *tmp_ctx;
     struct sysdb_attrs *user_attrs;
-    struct ldb_message *group;
+    struct ldb_result *group;
     errno_t ret;
     const char *orig_group_dn;
-    const char *group_attrs[] = { SYSDB_ORIG_DN, NULL };
+    const char *group_attrs[] = { SYSDB_ORIG_DN, SYSDB_OBJECTCLASS, NULL };
+    const char *oc;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
         return ENOMEM;
     }
 
-    ret = sysdb_search_group_by_gid(tmp_ctx, grp_dom->sysdb, grp_dom,
-                                    gid, group_attrs, &group);
+    ret = sysdb_search_object_by_sid(tmp_ctx, grp_dom->sysdb, grp_dom,
+                                     grp_sid_str, group_attrs, &group);
     if (ret != EOK) {
-        DEBUG(SSSDBG_TRACE_INTERNAL, ("sysdb_search_group_by_gid for gid [%d]" \
-                                      "failed [%d][%s].\n",
-                                      gid, ret, strerror(ret)));
+        DEBUG(SSSDBG_TRACE_INTERNAL, ("sysdb_search_object_by_sid " \
+                                      "for SID [%s] failed [%d][%s].\n",
+                                      grp_sid_str, ret, strerror(ret)));
+        goto done;
+    }
+
+    if (group->count != 1) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Unexpected number of groups returned.\n"));
+        ret = EINVAL;
+        goto done;
+    }
+
+    oc = ldb_msg_find_attr_as_string(group->msgs[0], SYSDB_OBJECTCLASS, NULL);
+    if (oc == NULL || strcmp(oc, SYSDB_GROUP_CLASS) != 0) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Return object does not have group " \
+                                  "objectclass.\n"));
+        ret = EINVAL;
         goto done;
     }
 
-    DEBUG(SSSDBG_TRACE_ALL, ("Adding user [%s] to group [%d][%s].\n",
-                             ldb_dn_get_linearized(user_dn), gid,
-                             ldb_dn_get_linearized(group->dn)));
-    ret = sysdb_mod_group_member(grp_dom->sysdb, user_dn, group->dn,
+    DEBUG(SSSDBG_TRACE_ALL, ("Adding user [%s] to group [%s][%s].\n",
+                             ldb_dn_get_linearized(user_dn), grp_sid_str,
+                             ldb_dn_get_linearized(group->msgs[0]->dn)));
+    ret = sysdb_mod_group_member(grp_dom->sysdb, user_dn, group->msgs[0]->dn,
                                  LDB_FLAG_MOD_ADD);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_mod_group_member failed user [%s] " \
                                   "group [%s].\n",
                                   ldb_dn_get_linearized(user_dn),
-                                  ldb_dn_get_linearized(group->dn)));
+                                  ldb_dn_get_linearized(group->msgs[0]->dn)));
         goto done;
     }
 
-    orig_group_dn = ldb_msg_find_attr_as_string(group, SYSDB_ORIG_DN, NULL);
+    orig_group_dn = ldb_msg_find_attr_as_string(group->msgs[0], SYSDB_ORIG_DN,
+                                                NULL);
     if (orig_group_dn != NULL) {
+        DEBUG(SSSDBG_TRACE_ALL, ("Adding original group DN [%s] to user [%s].\n",
+                                 orig_group_dn,
+                                 ldb_dn_get_linearized(user_dn)));
         user_attrs = sysdb_new_attrs(tmp_ctx);
         if (user_attrs == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_new_attrs failed.\n"));
@@ -963,15 +956,15 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
         }
 
         ret = sysdb_set_entry_attr(pr_ctx->dom->sysdb, user_dn, user_attrs,
-                                  LDB_FLAG_MOD_ADD);
+                                   LDB_FLAG_MOD_ADD);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed.\n"));
             goto done;
         }
     } else {
         DEBUG(SSSDBG_MINOR_FAILURE, ("Original DN not available for group " \
-                                     "[%d][%s].\n", gid,
-                                     ldb_dn_get_linearized(group->dn)));
+                                     "[%s][%s].\n", grp_sid_str,
+                                     ldb_dn_get_linearized(group->msgs[0]->dn)));
     }
 
 done:
-- 
1.7.7.6



More information about the sssd-devel mailing list