[SSSD] [PATCHES] IPA: allow initgroups by UUID for FreeIPA users

Sumit Bose sbose at redhat.com
Thu Apr 30 10:40:00 UTC 2015


Hi,

those two patches should fix https://fedorahosted.org/sssd/ticket/2642 .
The first patch fixes another issue as well which I found during testing
which is described in the commit message. The second patch is the UUID
version of 'IPA: allow initgroups by SID for AD users' sent recently. I
took the opportunity to make the handling in sdap_get_initgr_send() a
bit more clean.

bye,
Sumit
-------------- next part --------------
From 9624ecdcae0451a910428f04e64a366e6746f9a0 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 29 Apr 2015 15:21:17 +0200
Subject: [PATCH 1/2] NSS: check for overrides before calling backend

Currently the flag that the input data in a user or group lookup request
might be an override value is only set if no cached entry was found. If
the cached entry of an object with overrides is expired and a request
with the override value as input is processed the flag is not set and
the backend might not be able to find the right entry on the server.
Typically this should not happen because of mid-point refreshes. To
reproduce this create a FreeIPA user and override the login name for a
specific view. On a client which has this view applied call

getent passwd overridename
sss_cache -E
getent passwd overridename

The second getent command will still show the right output but in the
logs a

[sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error
 code: 3 errno: 0 error message: Account info lookup failed

message can be found for the second request.

Related to https://fedorahosted.org/sssd/ticket/2642
---
 src/responder/nss/nsssrv_cmd.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 45d65717312f90cadb88f3a7334aca0629e33d58..41586e9fc0fd05b4a485f6c7541f0fef6bfc2d50 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -960,7 +960,10 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx)
 
             if (cmdctx->name_is_upn) {
                 extra_flag = EXTRA_NAME_IS_UPN;
-            } else if (DOM_HAS_VIEWS(dom) && dctx->res->count == 0) {
+            } else if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
+                    || ldb_msg_find_attr_as_string(dctx->res->msgs[0],
+                                                   OVERRIDE_PREFIX SYSDB_NAME,
+                                                   NULL) != NULL)) {
                 extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
             } else {
                 extra_flag = NULL;
@@ -1626,7 +1629,10 @@ static int nss_cmd_getpwuid_search(struct nss_dom_ctx *dctx)
          * yet) then verify that the cache is uptodate */
         if (dctx->check_provider) {
 
-            if (DOM_HAS_VIEWS(dom) && dctx->res->count == 0) {
+            if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
+                    || ldb_msg_find_attr_as_uint64(dctx->res->msgs[0],
+                                                   OVERRIDE_PREFIX SYSDB_UIDNUM,
+                                                   0) != 0)) {
                 extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
             } else {
                 extra_flag = NULL;
@@ -3073,7 +3079,10 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx)
          * yet) then verify that the cache is uptodate */
         if (dctx->check_provider) {
 
-            if (DOM_HAS_VIEWS(dom) && dctx->res->count == 0) {
+            if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
+                    || ldb_msg_find_attr_as_string(dctx->res->msgs[0],
+                                                   OVERRIDE_PREFIX SYSDB_NAME,
+                                                   NULL) != NULL)) {
                 extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
             } else {
                 extra_flag = NULL;
@@ -3197,7 +3206,10 @@ static int nss_cmd_getgrgid_search(struct nss_dom_ctx *dctx)
          * yet) then verify that the cache is uptodate */
         if (dctx->check_provider) {
 
-            if (DOM_HAS_VIEWS(dom) && dctx->res->count == 0) {
+            if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
+                    || ldb_msg_find_attr_as_uint64(dctx->res->msgs[0],
+                                                   OVERRIDE_PREFIX SYSDB_GIDNUM,
+                                                   0) != 0)) {
                 extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
             } else {
                 extra_flag = NULL;
@@ -4155,7 +4167,10 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
 
             if (cmdctx->name_is_upn) {
                 extra_flag = EXTRA_NAME_IS_UPN;
-            } else if (DOM_HAS_VIEWS(dom) && dctx->res->count == 0) {
+            } else if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
+                    || ldb_msg_find_attr_as_string(dctx->res->msgs[0],
+                                                   OVERRIDE_PREFIX SYSDB_NAME,
+                                                   NULL) != NULL)) {
                 extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
             } else {
                 extra_flag = NULL;
-- 
2.1.0

-------------- next part --------------
From bab74590459ab51f4fdbb14ac7df22e2d69c07f1 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 29 Apr 2015 16:46:14 +0200
Subject: [PATCH 2/2] IPA: allow initgroups by UUID for FreeIPA users

If a FreeIPA user is searched with the help of an override name the UUID
from the override anchor is used to search the user. Currently the
initgroups request only allows searches by SID or name. With this patch
a UUID can be used as well.

Related to https://fedorahosted.org/sssd/ticket/2642
---
 src/db/sysdb_search.c                      | 32 ++++++++++++++++++++----------
 src/providers/data_provider.h              |  1 -
 src/providers/ipa/ipa_id.c                 | 15 +++++++++++++-
 src/providers/ldap/ldap_id.c               | 20 ++++++++-----------
 src/providers/ldap/sdap_async.h            |  1 +
 src/providers/ldap/sdap_async_initgroups.c | 14 ++++++++++---
 src/tests/sysdb-tests.c                    |  9 +++++++++
 7 files changed, 64 insertions(+), 28 deletions(-)

diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index 39b3abb557ef7d2ce153c4e2765e06e3253f5800..a8dcc9f8d6617be8e8fb82a1c6360c6df9726a37 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -1604,20 +1604,30 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
     if (res->count == 0) {
         ret = sysdb_search_user_by_upn(tmp_ctx, domain, name_or_upn_or_sid,
                                        NULL, &msg);
-        if (ret != EOK) {
+        if (ret == ENOENT) {
+            ret = sysdb_search_user_by_sid_str(tmp_ctx, domain,
+                                               name_or_upn_or_sid, NULL, &msg);
             if (ret == ENOENT) {
-                ret = sysdb_search_user_by_sid_str(tmp_ctx, domain,
-                                                   name_or_upn_or_sid, NULL,
-                                                   &msg);
-            }
-
-            if (ret != EOK) {
-                /* User cannot be found in cache */
-                DEBUG(SSSDBG_OP_FAILURE, "Cannot find user [%s] in cache\n",
-                                         name_or_upn_or_sid);
-                goto done;
+                ret = sysdb_search_object_by_uuid(tmp_ctx, domain,
+                                                  name_or_upn_or_sid, NULL,
+                                                  &res);
+                if (ret == EOK && res->count == 1) {
+                    msg = res->msgs[0];
+                } else {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "sysdb_search_object_by_uuid did not return a " \
+                          "single result.\n");
+                    ret = ENOENT;
+                    goto done;
+                }
             }
         }
+        if (ret != EOK) {
+            /* User cannot be found in cache */
+            DEBUG(SSSDBG_OP_FAILURE, "Cannot find user [%s] in cache\n",
+                                     name_or_upn_or_sid);
+            goto done;
+        }
     } else if (res->count == 1) {
         msg = res->msgs[0];
     } else {
diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h
index 89fb06a0d6f791a8ae50f9d8b4b69d6176912c6c..5df493e9d1ae21ada6f5fd6198a6d9c36680d044 100644
--- a/src/providers/data_provider.h
+++ b/src/providers/data_provider.h
@@ -150,7 +150,6 @@
 #define DP_SEC_ID_LEN (sizeof(DP_SEC_ID) - 1)
 
 #define EXTRA_NAME_IS_UPN "U"
-#define EXTRA_NAME_IS_SID "S"
 #define EXTRA_INPUT_MAYBE_WITH_VIEW "V"
 
 /* AUTH related common data and functions */
diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c
index 764943479a9f42e1389636baf0364eb40bd5b662..2bae97cd959af6d5ebceb697a9c8d7af33c358a6 100644
--- a/src/providers/ipa/ipa_id.c
+++ b/src/providers/ipa/ipa_id.c
@@ -554,6 +554,7 @@ struct ipa_id_get_account_info_state {
     struct sss_domain_info *domain;
     struct be_req *be_req;
     struct be_acct_req *ar;
+    struct be_acct_req *orig_ar;
     const char *realm;
 
     struct sysdb_attrs *override_attrs;
@@ -732,13 +733,25 @@ static void ipa_id_get_account_info_got_override(struct tevent_req *subreq)
 
         if (strcmp(state->ar->domain, anchor_domain) == 0) {
 
+            state->orig_ar = state->ar;
+
             ret = get_be_acct_req_for_uuid(state, ipa_uuid,
                                            state->ar->domain,
                                            &state->ar);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "get_be_acct_req_for_sid failed.\n");
+                DEBUG(SSSDBG_OP_FAILURE, "get_be_acct_req_for_uuid failed.\n");
                 goto fail;
             }
+
+            if ((state->orig_ar->entry_type & BE_REQ_TYPE_MASK)
+                                                         == BE_REQ_INITGROUPS) {
+                DEBUG(SSSDBG_TRACE_ALL,
+                      "Switching back to BE_REQ_INITGROUPS.\n");
+                state->ar->entry_type = BE_REQ_INITGROUPS;
+                state->ar->filter_type = BE_FILTER_UUID;
+                state->ar->attr_type = BE_ATTR_CORE;
+            }
+
         } else {
             DEBUG(SSSDBG_MINOR_FAILURE,
                   "Anchor from a different domain [%s], expected [%s]. " \
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index d65bd5f6ac929ecce2e6e5bbbba5edb2fce01f14..997313bec051c3ccb04f50eb094b0d0cda0af173 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -965,6 +965,7 @@ struct groups_by_user_state {
     struct sss_domain_info *domain;
 
     const char *name;
+    int name_type;
     const char *extra_value;
     const char **attrs;
 
@@ -983,6 +984,7 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
                                               struct sdap_domain *sdom,
                                               struct sdap_id_conn_ctx *conn,
                                               const char *name,
+                                              int name_type,
                                               const char *extra_value,
                                               bool noexist_delete)
 {
@@ -1008,6 +1010,7 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
     }
 
     state->name = name;
+    state->name_type = name_type;
     state->extra_value = extra_value;
     state->domain = sdom->dom;
     state->sysdb = sdom->dom->sysdb;
@@ -1070,6 +1073,7 @@ static void groups_by_user_connect_done(struct tevent_req *subreq)
                                   state->ctx,
                                   state->conn,
                                   state->name,
+                                  state->name_type,
                                   state->extra_value,
                                   state->attrs);
     if (!subreq) {
@@ -1393,7 +1397,8 @@ sdap_handle_acct_req_send(TALLOC_CTX *mem_ctx,
 
     case BE_REQ_INITGROUPS: /* init groups for user */
         if (ar->filter_type != BE_FILTER_NAME
-                && ar->filter_type != BE_FILTER_SECID) {
+                && ar->filter_type != BE_FILTER_SECID
+                && ar->filter_type != BE_FILTER_UUID) {
             ret = EINVAL;
             state->err = "Invalid filter type";
             goto done;
@@ -1403,21 +1408,12 @@ sdap_handle_acct_req_send(TALLOC_CTX *mem_ctx,
             state->err = "Invalid attr type";
             goto done;
         }
-        if (ar->filter_type == BE_FILTER_SECID && ar->extra_value != NULL
-                && strcmp(ar->extra_value, EXTRA_NAME_IS_SID) != 0) {
-            DEBUG(SSSDBG_OP_FAILURE,
-                  "Unexpected extra value [%s] for BE_FILTER_SECID.\n",
-                  ar->extra_value);
-            ret = EINVAL;
-            state->err = "Invalid extra value";
-            goto done;
-        }
 
         subreq = groups_by_user_send(state, be_ctx->ev, id_ctx,
                                      sdom, conn,
                                      ar->filter_value,
-                                     (ar->filter_type == BE_FILTER_SECID)
-                                        ? EXTRA_NAME_IS_SID : ar->extra_value,
+                                     ar->filter_type,
+                                     ar->extra_value,
                                      noexist_delete);
         break;
 
diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h
index 29afd8e1ad8cceae66fbe3cd5c6b1ffb69e93e07..f2ea9bf2ecdaeefa897d414df051532c6219cb97 100644
--- a/src/providers/ldap/sdap_async.h
+++ b/src/providers/ldap/sdap_async.h
@@ -136,6 +136,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,
                                         struct sdap_id_ctx *id_ctx,
                                         struct sdap_id_conn_ctx *conn,
                                         const char *name,
+                                        int name_type,
                                         const char *extra_value,
                                         const char **grp_attrs);
 int sdap_get_initgr_recv(struct tevent_req *req);
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 5c5be5eabd7006b457291062519cdad9626f13fa..4f775d76b77a311c3394beec4546c4f6c7dc5f6f 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -2667,6 +2667,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,
                                         struct sdap_id_ctx *id_ctx,
                                         struct sdap_id_conn_ctx *conn,
                                         const char *name,
+                                        int name_type,
                                         const char *extra_value,
                                         const char **grp_attrs)
 {
@@ -2716,10 +2717,17 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX *memctx,
 
     if (extra_value && strcmp(extra_value, EXTRA_NAME_IS_UPN) == 0) {
         search_attr =  state->opts->user_map[SDAP_AT_USER_PRINC].name;
-    } else if (extra_value && strcmp(extra_value, EXTRA_NAME_IS_SID) == 0) {
-        search_attr =  state->opts->user_map[SDAP_AT_USER_OBJECTSID].name;
     } else {
-        search_attr =  state->opts->user_map[SDAP_AT_USER_NAME].name;
+        switch (name_type) {
+        case BE_FILTER_SECID:
+            search_attr =  state->opts->user_map[SDAP_AT_USER_OBJECTSID].name;
+            break;
+        case BE_FILTER_UUID:
+            search_attr =  state->opts->user_map[SDAP_AT_USER_UUID].name;
+            break;
+        default:
+            search_attr =  state->opts->user_map[SDAP_AT_USER_NAME].name;
+        }
     }
 
     state->user_base_filter =
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index e41fb0504ae6b01e586a54ec1d903a07b65c22df..1623ae9f3b58b12858ed04b0bdc7b8b18822dc87 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -3584,6 +3584,10 @@ START_TEST(test_sysdb_get_real_name)
                                  "S-1-5-21-123-456-789-111");
     fail_unless(ret == EOK, "sysdb_attrs_add_string failed.");
 
+    ret = sysdb_attrs_add_string(user_attrs, SYSDB_UUID,
+                                 "12345678-9012-3456-7890-123456789012");
+    fail_unless(ret == EOK, "sysdb_attrs_add_string failed.");
+
     ret = sysdb_store_user(test_ctx->domain, "RealName",
                            NULL, 22345, 0, "gecos",
                            "/home/realname", "/bin/bash",
@@ -3607,6 +3611,11 @@ START_TEST(test_sysdb_get_real_name)
     fail_unless(strcmp(str, "RealName") == 0, "Expected [%s], got [%s].",
                                               "RealName", str);
 
+    ret = sysdb_get_real_name(test_ctx, test_ctx->domain,
+                              "12345678-9012-3456-7890-123456789012", &str);
+    fail_unless(ret == EOK, "sysdb_get_real_name failed.");
+    fail_unless(strcmp(str, "RealName") == 0, "Expected [%s], got [%s].",
+                                              "RealName", str);
 }
 END_TEST
 
-- 
2.1.0



More information about the sssd-devel mailing list