From 11f6fcedb0ac04528dd319fcf95d1fbaa4ea8bd1 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Thu, 7 Jul 2016 18:54:02 +0200

Subject: [PATCH 48/62] views: properly override group member names

Resolves https://fedorahosted.org/sssd/ticket/2948

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

(cherry picked from commit 1594701fbdc341069e11cff9a85e7a795e52db3d)

---

 src/db/sysdb.h                  |   3 +-

 src/db/sysdb_search.c           |  99 ++++++++++++++++-------------

 src/db/sysdb_views.c            | 136 ++++++++++++++++++----------------------

 src/responder/nss/nsssrv_cmd.c  |   7 ++-

 src/tests/cmocka/test_nss_srv.c |  18 +++---

 5 files changed, 134 insertions(+), 129 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h

index 405f89e2f1ac6fabc06e77c345de8693845f9d92..a27552224bb40bd07c7dee4dfe35bfb7a0b4f2c3 100644

--- a/src/db/sysdb.h

+++ b/src/db/sysdb.h

@@ -572,7 +572,8 @@ errno_t sysdb_add_overrides_to_object(struct sss_domain_info *domain,

                                       const char **req_attrs);

 errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

-                                         struct ldb_message *obj);

+                                         struct ldb_message *obj,

+                                         bool expect_override_dn);

 errno_t sysdb_getpwnam_with_views(TALLOC_CTX *mem_ctx,

                                   struct sss_domain_info *domain,

diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c

index e40b36c38e28992e185447497d1bf69cabc09821..cfee5784dbadd692f30d0758e7e5c3c9fb2814cb 100644

--- a/src/db/sysdb_search.c

+++ b/src/db/sysdb_search.c

@@ -771,28 +771,33 @@ int sysdb_getgrnam_with_views(TALLOC_CTX *mem_ctx,

     /* If there are views we have to check if override values must be added to

      * the original object. */

-    if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) {

-        if (!is_local_view(domain->view_name)) {

-            el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);

-            if (el != NULL && el->num_values != 0) {

-                DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "

-                      "entries which must be resolved before overrides can be "

-                      "applied.\n",

-                      ldb_dn_get_linearized(orig_obj->msgs[0]->dn));

-                ret = ENOENT;

+    if (orig_obj->count == 1) {

+        if (DOM_HAS_VIEWS(domain)) {

+            if (!is_local_view(domain->view_name)) {

+                el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);

+                if (el != NULL && el->num_values != 0) {

+                    DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "

+                          "entries which must be resolved before overrides can be "

+                          "applied.\n",

+                          ldb_dn_get_linearized(orig_obj->msgs[0]->dn));

+                    ret = ENOENT;

+                    goto done;

+                }

+            }

+

+            ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],

+                           override_obj == NULL ? NULL : override_obj ->msgs[0],

+                           NULL);

+            if (ret != EOK) {

+                DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n");

                 goto done;

             }

         }

-        ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],

-                          override_obj == NULL ? NULL : override_obj ->msgs[0],

-                          NULL);

-        if (ret != EOK) {

-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n");

-            goto done;

-        }

-

-        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]);

+        /* Must be called even without views to check to

+         * SYSDB_DEFAULT_OVERRIDE_NAME */

+        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0],

+                                               DOM_HAS_VIEWS(domain));

         if (ret != EOK) {

             DEBUG(SSSDBG_OP_FAILURE,

                   "sysdb_add_group_member_overrides failed.\n");

@@ -922,28 +927,33 @@ int sysdb_getgrgid_with_views(TALLOC_CTX *mem_ctx,

     /* If there are views we have to check if override values must be added to

      * the original object. */

-    if (DOM_HAS_VIEWS(domain) && orig_obj->count == 1) {

-        if (!is_local_view(domain->view_name)) {

-            el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);

-            if (el != NULL && el->num_values != 0) {

-                DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "

-                      "entries which must be resolved before overrides can be "

-                      "applied.\n",

-                      ldb_dn_get_linearized(orig_obj->msgs[0]->dn));

-                ret = ENOENT;

+    if (orig_obj->count == 1) {

+        if (DOM_HAS_VIEWS(domain)) {

+            if (!is_local_view(domain->view_name)) {

+                el = ldb_msg_find_element(orig_obj->msgs[0], SYSDB_GHOST);

+                if (el != NULL && el->num_values != 0) {

+                    DEBUG(SSSDBG_TRACE_ALL, "Group object [%s], contains ghost "

+                          "entries which must be resolved before overrides can be "

+                          "applied.\n",

+                          ldb_dn_get_linearized(orig_obj->msgs[0]->dn));

+                    ret = ENOENT;

+                    goto done;

+                }

+            }

+

+            ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],

+                              override_obj == NULL ? NULL : override_obj ->msgs[0],

+                              NULL);

+            if (ret != EOK) {

+                DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n");

                 goto done;

             }

         }

-        ret = sysdb_add_overrides_to_object(domain, orig_obj->msgs[0],

-                          override_obj == NULL ? NULL : override_obj ->msgs[0],

-                          NULL);

-        if (ret != EOK) {

-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n");

-            goto done;

-        }

-

-        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0]);

+        /* Must be called even without views to check to

+         * SYSDB_DEFAULT_OVERRIDE_NAME */

+        ret = sysdb_add_group_member_overrides(domain, orig_obj->msgs[0],

+                                               DOM_HAS_VIEWS(domain));

         if (ret != EOK) {

             DEBUG(SSSDBG_OP_FAILURE,

                   "sysdb_add_group_member_overrides failed.\n");

@@ -1157,8 +1167,8 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX *mem_ctx,

         goto done;

     }

-    if (DOM_HAS_VIEWS(domain)) {

-        for (c = 0; c < res->count; c++) {

+    for (c = 0; c < res->count; c++) {

+        if (DOM_HAS_VIEWS(domain)) {

             ret = sysdb_add_overrides_to_object(domain, res->msgs[c], NULL,

                                                 NULL);

             /* enumeration assumes that the cache is up-to-date, hence we do not

@@ -1167,13 +1177,14 @@ int sysdb_enumgrent_filter_with_views(TALLOC_CTX *mem_ctx,

                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_add_overrides_to_object failed.\n");

                 goto done;

             }

+        }

-            ret = sysdb_add_group_member_overrides(domain, res->msgs[c]);

-            if (ret != EOK) {

-                DEBUG(SSSDBG_OP_FAILURE,

-                      "sysdb_add_group_member_overrides failed.\n");

-                goto done;

-            }

+        ret = sysdb_add_group_member_overrides(domain, res->msgs[c],

+                                               DOM_HAS_VIEWS(domain));

+        if (ret != EOK) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "sysdb_add_group_member_overrides failed.\n");

+            goto done;

         }

     }

diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c

index 2b89e5ca41f719e1217ef3b9e0fd683656e05d42..79f513d13ba41212a6cd84e1d9e609df6acba29c 100644

--- a/src/db/sysdb_views.c

+++ b/src/db/sysdb_views.c

@@ -1348,14 +1348,13 @@ done:

 }

 errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

-                                         struct ldb_message *obj)

+                                         struct ldb_message *obj,

+                                         bool expect_override_dn)

 {

     int ret;

     size_t c;

-    struct ldb_message_element *members;

+    struct ldb_result *res_members;

     TALLOC_CTX *tmp_ctx;

-    struct ldb_dn *member_dn;

-    struct ldb_result *member_obj;

     struct ldb_result *override_obj;

     static const char *member_attrs[] = SYSDB_PW_ATTRS;

     const char *override_dn_str;

@@ -1366,12 +1365,6 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

     char *val;

     struct sss_domain_info *orig_dom;

-    members = ldb_msg_find_element(obj, SYSDB_MEMBER);

-    if (members == NULL || members->num_values == 0) {

-        DEBUG(SSSDBG_TRACE_ALL, "Group has no members.\n");

-        return EOK;

-    }

-

     tmp_ctx = talloc_new(NULL);

     if (tmp_ctx == NULL) {

         DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");

@@ -1379,38 +1372,30 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

         goto done;

     }

-    for (c = 0; c < members->num_values; c++) {

-        member_dn = ldb_dn_from_ldb_val(tmp_ctx, domain->sysdb->ldb,

-                                        &members->values[c]);

-        if (member_dn == NULL) {

-            DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_from_ldb_val failed.\n");

-            ret = ENOMEM;

-            goto done;

-        }

+    ret = sysdb_get_user_members_recursively(tmp_ctx, domain, obj->dn,

+                                             &res_members);

+    if (ret != EOK) {

+        DEBUG(SSSDBG_OP_FAILURE,

+              "sysdb_get_user_members_recursively failed.\n");

+        goto done;

+    }

-        ret = ldb_search(domain->sysdb->ldb, member_dn, &member_obj, member_dn,

-                         LDB_SCOPE_BASE, member_attrs, NULL);

-        if (ret != LDB_SUCCESS) {

-            ret = sysdb_error_to_errno(ret);

-            goto done;

-        }

+    for (c = 0; c < res_members->count; c++) {

-        if (member_obj->count != 1) {

-            DEBUG(SSSDBG_CRIT_FAILURE,

-                  "Base search for member object returned [%d] results.\n",

-                  member_obj->count);

-            ret = EINVAL;

-            goto done;

-        }

-

-        if (ldb_msg_find_attr_as_uint64(member_obj->msgs[0],

+        if (ldb_msg_find_attr_as_uint64(res_members->msgs[c],

                                         SYSDB_UIDNUM, 0) == 0) {

             /* Skip non-POSIX-user members i.e. groups and non-POSIX users */

             continue;

         }

-        override_dn_str = ldb_msg_find_attr_as_string(member_obj->msgs[0],

-                                                      SYSDB_OVERRIDE_DN, NULL);

+        if (expect_override_dn) {

+            override_dn_str = ldb_msg_find_attr_as_string(res_members->msgs[c],

+                                                          SYSDB_OVERRIDE_DN,

+                                                          NULL);

+        } else {

+            override_dn_str = ldb_dn_get_linearized(res_members->msgs[c]->dn);

+        }

+

         if (override_dn_str == NULL) {

             if (is_local_view(domain->view_name)) {

                 /* LOCAL view doesn't have to have overrideDN specified. */

@@ -1420,12 +1405,12 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

             DEBUG(SSSDBG_CRIT_FAILURE,

                   "Missing override DN for object [%s].\n",

-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));

+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));

             ret = ENOENT;

             goto done;

         }

-        override_dn = ldb_dn_new(member_obj, domain->sysdb->ldb,

+        override_dn = ldb_dn_new(res_members, domain->sysdb->ldb,

                                  override_dn_str);

         if (override_dn == NULL) {

             DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new failed.\n");

@@ -1433,22 +1418,27 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

             goto done;

         }

-        orig_name = ldb_msg_find_attr_as_string(member_obj->msgs[0],

+        orig_name = ldb_msg_find_attr_as_string(res_members->msgs[c],

                                                 SYSDB_NAME,

                                                 NULL);

         if (orig_name == NULL) {

             DEBUG(SSSDBG_CRIT_FAILURE, "Object [%s] has no name.\n",

-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));

+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));

             ret = EINVAL;

             goto done;

         }

-        memberuid = NULL;

-        if (ldb_dn_compare(member_obj->msgs[0]->dn, override_dn) != 0) {

+        /* start with default view name, if it exists or use NULL */

+        memberuid = ldb_msg_find_attr_as_string(res_members->msgs[c],

+                                                SYSDB_DEFAULT_OVERRIDE_NAME,

+                                                NULL);

+

+        /* If there is an override object, check if the name is overridden */

+        if (ldb_dn_compare(res_members->msgs[c]->dn, override_dn) != 0) {

             DEBUG(SSSDBG_TRACE_ALL, "Checking override for object [%s].\n",

-                  ldb_dn_get_linearized(member_obj->msgs[0]->dn));

+                  ldb_dn_get_linearized(res_members->msgs[c]->dn));

-            ret = ldb_search(domain->sysdb->ldb, member_obj, &override_obj,

+            ret = ldb_search(domain->sysdb->ldb, res_members, &override_obj,

                              override_dn, LDB_SCOPE_BASE, member_attrs, NULL);

             if (ret != LDB_SUCCESS) {

                 ret = sysdb_error_to_errno(ret);

@@ -1458,43 +1448,44 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

             if (override_obj->count != 1) {

                 DEBUG(SSSDBG_CRIT_FAILURE,

                      "Base search for override object returned [%d] results.\n",

-                     member_obj->count);

+                    override_obj->count);

                 ret = EINVAL;

                 goto done;

             }

             memberuid = ldb_msg_find_attr_as_string(override_obj->msgs[0],

                                                     SYSDB_NAME,

-                                                    NULL);

+                                                    memberuid);

+        }

-            if (memberuid != NULL) {

-                ret = sss_parse_internal_fqname(tmp_ctx, orig_name,

-                                                NULL, &orig_domain);

-                if (ret != EOK) {

-                    DEBUG(SSSDBG_OP_FAILURE,

-                         "sss_parse_internal_fqname failed to split [%s].\n",

-                         orig_name);

+        /* add domain name if memberuid is a short name */

+        if (memberuid != NULL && strchr(memberuid, '@') == NULL) {

+            ret = sss_parse_internal_fqname(tmp_ctx, orig_name,

+                                            NULL, &orig_domain);

+            if (ret != EOK) {

+                DEBUG(SSSDBG_OP_FAILURE,

+                     "sss_parse_internal_fqname failed to split [%s].\n",

+                     orig_name);

+                goto done;

+            }

+

+            if (orig_domain != NULL) {

+                orig_dom = find_domain_by_name(get_domains_head(domain),

+                                               orig_domain, true);

+                if (orig_dom == NULL) {

+                    DEBUG(SSSDBG_CRIT_FAILURE,

+                          "Cannot find domain with name [%s].\n",

+                          orig_domain);

+                    ret = ERR_DOMAIN_NOT_FOUND;

                     goto done;

                 }

-

-                if (orig_domain != NULL) {

-                    orig_dom = find_domain_by_name(get_domains_head(domain),

-                                                   orig_domain, true);

-                    if (orig_dom == NULL) {

-                        DEBUG(SSSDBG_CRIT_FAILURE,

-                              "Cannot find domain with name [%s].\n",

-                              orig_domain);

-                        ret = ERR_DOMAIN_NOT_FOUND;

-                        goto done;

-                    }

-                    memberuid = sss_create_internal_fqname(tmp_ctx, memberuid,

-                                                           orig_dom->name);

-                    if (memberuid == NULL) {

-                        DEBUG(SSSDBG_OP_FAILURE,

-                              "sss_create_internal_fqname failed.\n");

-                        ret = ENOMEM;

-                        goto done;

-                    }

+                memberuid = sss_create_internal_fqname(tmp_ctx, memberuid,

+                                                       orig_dom->name);

+                if (memberuid == NULL) {

+                    DEBUG(SSSDBG_OP_FAILURE,

+                          "sss_create_internal_fqname failed.\n");

+                    ret = ENOMEM;

+                    goto done;

                 }

             }

         }

@@ -1521,9 +1512,6 @@ errno_t sysdb_add_group_member_overrides(struct sss_domain_info *domain,

         DEBUG(SSSDBG_TRACE_ALL, "Added [%s] to [%s].\n", memberuid,

                                 OVERRIDE_PREFIX SYSDB_MEMBERUID);

-        /* Free all temporary data of the current member to avoid memory usage

-         * spikes. All temporary data should be allocated below member_dn. */

-        talloc_free(member_dn);

     }

     ret = EOK;

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c

index 1ae17969688fa29734ca14fd2b152decef1fdbca..4e84b3202cbf367e70a47a3c7edb06e357657538 100644

--- a/src/responder/nss/nsssrv_cmd.c

+++ b/src/responder/nss/nsssrv_cmd.c

@@ -2976,7 +2976,12 @@ static int fill_grent(struct sss_packet *packet,

         memnum = 0;

         if (!dom->ignore_group_members) {

-            el = sss_view_ldb_msg_find_element(dom, msg, SYSDB_MEMBERUID);

+            /* unconditionally prefer OVERRIDE_PREFIX SYSDB_MEMBERUID, it

+             * might contain override names from the default view */

+            el = ldb_msg_find_element(msg, OVERRIDE_PREFIX SYSDB_MEMBERUID);

+            if (el == NULL) {

+                el = ldb_msg_find_element(msg, SYSDB_MEMBERUID);

+            }

             if (el) {

                 ret = fill_members(packet, nctx->rctx, dom, nctx, el,

                                    &rzero, &rsize, &memnum);

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c

index 82a304feed864b09168d0f3e06a4e1bb120df7e4..41425e76f3b76fafa917f33fcfef0946f2f71c7d 100644

--- a/src/tests/cmocka/test_nss_srv.c

+++ b/src/tests/cmocka/test_nss_srv.c

@@ -1619,11 +1619,11 @@ static int test_nss_getgrnam_check_mix_dom(uint32_t status,

     tmp_ctx = talloc_new(nss_test_ctx);

     assert_non_null(tmp_ctx);

-    exp_members[0] = testmember1.pw_name;

-    exp_members[1] = testmember2.pw_name;

-    exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,

+    exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,

                                    nss_test_ctx->subdom, submember1.pw_name);

-    assert_non_null(exp_members[2]);

+    assert_non_null(exp_members[0]);

+    exp_members[1] = testmember1.pw_name;

+    exp_members[2] = testmember2.pw_name;

     assert_int_equal(status, EOK);

@@ -1682,14 +1682,14 @@ static int test_nss_getgrnam_check_mix_dom_fqdn(uint32_t status,

     tmp_ctx = talloc_new(nss_test_ctx);

     assert_non_null(tmp_ctx);

-    exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,

-                                   nss_test_ctx->tctx->dom, testmember1.pw_name);

+    exp_members[0] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,

+                                   nss_test_ctx->subdom, submember1.pw_name);

     assert_non_null(exp_members[0]);

     exp_members[1] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,

-                                   nss_test_ctx->tctx->dom, testmember2.pw_name);

+                                   nss_test_ctx->tctx->dom, testmember1.pw_name);

     assert_non_null(exp_members[1]);

-    exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->subdom->names,

-                                   nss_test_ctx->subdom, submember1.pw_name);

+    exp_members[2] = sss_tc_fqname(tmp_ctx, nss_test_ctx->tctx->dom->names,

+                                   nss_test_ctx->tctx->dom, testmember2.pw_name);

     assert_non_null(exp_members[2]);

     expected.gr_name = sss_tc_fqname(tmp_ctx,

-- 

2.4.11