SYSDB: Use entry DN from previous search when storing groups

The sysdb_store_group() function searches the group by name including
lowercased aliases. If a matching entry is found, use its DN to perform
the subsequent entry modification instead of building a new entry DN
based on the name which may differ in case.

It may happen the group is stored in uppercase, but later some server
returns a memberOf attribute in lowercase. When updating the group to
add the memberships the first search will find the entry, but the modify
operation will fail as the group name in the built DN will differ in case.

Just use the entry DN from the first search instead of building a new
one.

Signed-off-by: Samuel Cabrero <[email protected]>

src/db/sysdb.h

@@ -1086,7 +1086,7 @@ errno_t sysdb_update_user_shadow_last_change(struct sss_domain_info *domain,
 
 /* Replace group attrs */
 int sysdb_set_group_attr(struct sss_domain_info *domain,
-                         const char *name,
+                         struct ldb_dn *dn,
                          struct sysdb_attrs *attrs,
                          int mod_op);
 

src/db/sysdb_ops.c

@@ -1514,35 +1514,11 @@ errno_t sysdb_update_user_shadow_last_change(struct sss_domain_info *domain,
 /* =Replace-Attributes-On-Group=========================================== */
 
 int sysdb_set_group_attr(struct sss_domain_info *domain,
-                         const char *name,
+                         struct ldb_dn *dn,
                          struct sysdb_attrs *attrs,
                          int mod_op)
 {
-    struct ldb_dn *dn;
-    TALLOC_CTX *tmp_ctx;
-    errno_t ret;
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    dn = sysdb_group_dn(tmp_ctx, domain, name);
-    if (!dn) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    ret = sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
-    if (ret) {
-        goto done;
-    }
-
-    ret = EOK;
-done:
-    talloc_free(tmp_ctx);
-    return ret;
+    return sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
 }
 
 /* =Replace-Attributes-On-Netgroup=========================================== */
@@ -2085,6 +2061,7 @@ int sysdb_add_group(struct sss_domain_info *domain,
                     time_t now)
 {
     TALLOC_CTX *tmp_ctx;
+    struct ldb_dn *dn;
     struct ldb_message *basic_grp_msg;
     struct ldb_message *msg;
     int ret;
@@ -2224,7 +2201,13 @@ int sysdb_add_group(struct sss_domain_info *domain,
         goto done;
     }
 
-    ret = sysdb_set_group_attr(domain, name, attrs, SYSDB_MOD_REP);
+    dn = sysdb_group_dn(tmp_ctx, domain, name);
+    if (!dn) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = sysdb_set_group_attr(domain, basic_grp_msg->dn, attrs, SYSDB_MOD_REP);
     if (ret) {
         DEBUG(SSSDBG_TRACE_LIBS, "sysdb_set_group_attr failed.\n");
         goto done;
@@ -2340,7 +2323,7 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain,
         if (ret) goto done;
     }
 
-    ret = sysdb_set_group_attr(domain, name, attrs, SYSDB_MOD_REP);
+    ret = sysdb_set_group_attr(domain, basic_grp_msg->dn, attrs, SYSDB_MOD_REP);
 
 done:
     if (ret != EOK) {
@@ -2785,7 +2768,7 @@ static errno_t sysdb_store_new_group(struct sss_domain_info *domain,
                                      time_t now);
 
 static errno_t sysdb_store_group_attrs(struct sss_domain_info *domain,
-                                       const char *name,
+                                       struct ldb_dn *dn,
                                        gid_t gid,
                                        struct sysdb_attrs *attrs,
                                        uint64_t cache_timeout,
@@ -2800,7 +2783,7 @@ int sysdb_store_group(struct sss_domain_info *domain,
 {
     TALLOC_CTX *tmp_ctx;
     static const char *src_attrs[] = { "*", NULL };
-    struct ldb_message *msg;
+    struct ldb_message *msg = NULL;
     bool new_group = false;
     int ret;
     errno_t sret = EOK;
@@ -2856,14 +2839,13 @@ int sysdb_store_group(struct sss_domain_info *domain,
         ret = sysdb_store_new_group(domain, name, gid, attrs,
                                     cache_timeout, now);
     } else {
-        ret = sysdb_store_group_attrs(domain, name, gid, attrs,
+        ret = sysdb_store_group_attrs(domain, msg->dn, gid, attrs,
                                       cache_timeout, now);
     }
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Cache update failed: %d\n", ret);
         goto done;
     }
-
     sret = sysdb_transaction_commit(domain->sysdb);
     if (sret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to commit transaction\n");
@@ -2936,7 +2918,7 @@ static errno_t sysdb_store_new_group(struct sss_domain_info *domain,
 }
 
 static errno_t sysdb_store_group_attrs(struct sss_domain_info *domain,
-                                       const char *name,
+                                       struct ldb_dn *dn,
                                        gid_t gid,
                                        struct sysdb_attrs *attrs,
                                        uint64_t cache_timeout,
@@ -2967,7 +2949,7 @@ static errno_t sysdb_store_group_attrs(struct sss_domain_info *domain,
         return ret;
     }
 
-    ret = sysdb_set_group_attr(domain, name, attrs, SYSDB_MOD_REP);
+    ret = sysdb_set_group_attr(domain, dn, attrs, SYSDB_MOD_REP);
     if (ret) {
         DEBUG(SSSDBG_TRACE_LIBS, "sysdb_set_group_attr failed.\n");
         return ret;

src/tests/cmocka/test_ldap_id_cleanup.c

@@ -159,7 +159,7 @@ static int test_sysdb_teardown(void **state)
 
 static errno_t invalidate_group(TALLOC_CTX *ctx,
                                 struct sss_domain_info *domain,
-                                const char *name)
+                                struct ldb_dn *grp_dn)
 {
     struct sysdb_attrs *sys_attrs = NULL;
     errno_t ret;
@@ -169,7 +169,7 @@ static errno_t invalidate_group(TALLOC_CTX *ctx,
         ret = sysdb_attrs_add_time_t(sys_attrs,
                                      SYSDB_CACHE_EXPIRE, 1);
         if (ret == EOK) {
-            ret = sysdb_set_group_attr(domain, name, sys_attrs,
+            ret = sysdb_set_group_attr(domain, grp_dn, sys_attrs,
                                        SYSDB_MOD_REP);
         } else {
             DEBUG(SSSDBG_MINOR_FAILURE,
@@ -261,24 +261,26 @@ static void test_id_cleanup_exp_group(void **state)
     ret = sysdb_search_group_by_name(test_ctx, test_ctx->domain,
                                      special_grp, NULL, &msg);
     assert_int_equal(ret, EOK);
+    /* let records to expire */
+    invalidate_group(test_ctx, test_ctx->domain, msg->dn);
 
     ret = sysdb_search_group_by_name(test_ctx, test_ctx->domain,
                                      empty_special_grp, NULL, &msg);
     assert_int_equal(ret, EOK);
+    /* let records to expire */
+    invalidate_group(test_ctx, test_ctx->domain, msg->dn);
 
     ret = sysdb_search_group_by_name(test_ctx, test_ctx->domain,
                                      grp, NULL, &msg);
     assert_int_equal(ret, EOK);
+    /* let records to expire */
+    invalidate_group(test_ctx, test_ctx->domain, msg->dn);
 
     ret = sysdb_search_group_by_name(test_ctx, test_ctx->domain,
                                      empty_grp, NULL, &msg);
     assert_int_equal(ret, EOK);
-
     /* let records to expire */
-    invalidate_group(test_ctx, test_ctx->domain, special_grp);
-    invalidate_group(test_ctx, test_ctx->domain, empty_special_grp);
-    invalidate_group(test_ctx, test_ctx->domain, grp);
-    invalidate_group(test_ctx, test_ctx->domain, empty_grp);
+    invalidate_group(test_ctx, test_ctx->domain, msg->dn);
 
     ret = ldap_id_cleanup(test_ctx->id_ctx, &sdom);
     assert_int_equal(ret, EOK);

src/tests/cmocka/test_sysdb_ts_cache.c

@@ -748,13 +748,13 @@ static void test_sysdb_getgr_merges(void **state)
     assert_int_equal(msgs_count, 1);
     assert_ts_attrs_msgs_list(msgs_count, msgs,
                               TEST_NOW_2 + TEST_CACHE_TIMEOUT, TEST_NOW_2);
-    talloc_free(msgs);
 
     group_attrs = create_ts_attrs(test_ctx, TEST_NOW_3 + TEST_CACHE_TIMEOUT, TEST_NOW_3);
     assert_non_null(group_attrs);
-    ret = sysdb_set_group_attr(test_ctx->tctx->dom, TEST_GROUP_NAME,
+    ret = sysdb_set_group_attr(test_ctx->tctx->dom, msgs[0]->dn,
                                group_attrs, SYSDB_MOD_REP);
     talloc_free(group_attrs);
+    talloc_free(msgs);
 
     ret = sysdb_getgrnam(test_ctx, test_ctx->tctx->dom, TEST_GROUP_NAME, &res);
     assert_int_equal(ret, EOK);

src/tests/sysdb-tests.c

@@ -2646,6 +2646,7 @@ START_TEST (test_sysdb_memberof_mod_add)
     struct ldb_message_element *el;
     struct ldb_val gv, *test_gv;
     gid_t itergid;
+    struct ldb_dn *group_dn;
 
     /* Setup */
     ret = setup_sysdb_tests(&test_ctx);
@@ -2692,8 +2693,10 @@ START_TEST (test_sysdb_memberof_mod_add)
     }
 
     /* Perform the add operation */
+    group_dn = sysdb_group_dn(test_ctx, test_ctx->domain, data->groupname);
+    sss_ck_fail_if_msg(group_dn == NULL, "Cannot allocate group DN\n");
     ret = sysdb_set_group_attr(test_ctx->domain,
-                               data->groupname, data->attrs, SYSDB_MOD_ADD);
+                               group_dn, data->attrs, SYSDB_MOD_ADD);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
     /* Before the delete, all groups with gid >= _i have the testuser%_i
@@ -2728,6 +2731,7 @@ START_TEST (test_sysdb_memberof_mod_replace)
     struct ldb_message_element *el;
     struct ldb_val gv, *test_gv;
     gid_t itergid;
+    struct ldb_dn *group_dn;
 
     /* Setup */
     ret = setup_sysdb_tests(&test_ctx);
@@ -2776,8 +2780,10 @@ START_TEST (test_sysdb_memberof_mod_replace)
     }
 
     /* Perform the replace operation */
+    group_dn = sysdb_group_dn(test_ctx, test_ctx->domain, data->groupname);
+    sss_ck_fail_if_msg(group_dn == NULL, "Cannot allocate group DN\n");
     ret =  sysdb_set_group_attr(test_ctx->domain,
-                                data->groupname, data->attrs, SYSDB_MOD_REP);
+                                group_dn, data->attrs, SYSDB_MOD_REP);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
     /* After the replace, all groups with gid >= _i have the testghost%_i
@@ -2815,6 +2821,7 @@ START_TEST (test_sysdb_memberof_mod_replace_keep)
     struct ldb_val gv, *test_gv;
     gid_t itergid;
     uid_t iteruid;
+    struct ldb_dn *group_dn;
 
     /* Setup */
     ret = setup_sysdb_tests(&test_ctx);
@@ -2899,8 +2906,10 @@ START_TEST (test_sysdb_memberof_mod_replace_keep)
     }
 
     /* Perform the replace operation */
+    group_dn = sysdb_group_dn(test_ctx, test_ctx->domain, data->groupname);
+    sss_ck_fail_if_msg(group_dn == NULL, "Cannot allocate group DN\n");
     ret = sysdb_set_group_attr(test_ctx->domain,
-                               data->groupname, data->attrs, SYSDB_MOD_REP);
+                               group_dn, data->attrs, SYSDB_MOD_REP);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
     /* After the replace, testusera should still be there, but we also need
@@ -3356,6 +3365,7 @@ START_TEST (test_sysdb_memberof_mod_del)
     struct ldb_message_element *el;
     struct ldb_val gv, *test_gv;
     gid_t itergid;
+    struct ldb_dn *group_dn;
 
     /* Setup */
     ret = setup_sysdb_tests(&test_ctx);
@@ -3398,8 +3408,11 @@ START_TEST (test_sysdb_memberof_mod_del)
 
     /* Delete the attribute */
     null_ctx_get_size(test_ctx);
+    group_dn = sysdb_group_dn(test_ctx, test_ctx->domain, data->groupname);
+    sss_ck_fail_if_msg(group_dn == NULL, "Cannot allocate group DN\n");
     ret = sysdb_set_group_attr(test_ctx->domain,
-                               data->groupname, data->attrs, SYSDB_MOD_DEL);
+                               group_dn, data->attrs, SYSDB_MOD_DEL);
+    talloc_zfree(group_dn);
     fail_if_null_ctx_leaks(test_ctx);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
@@ -3633,7 +3646,7 @@ START_TEST (test_sysdb_memberof_ghost_replace)
 
     /* Perform the replace operation */
     ret =  sysdb_set_group_attr(test_ctx->domain,
-                                data->groupname, data->attrs, SYSDB_MOD_REP);
+                                data->msg->dn, data->attrs, SYSDB_MOD_REP);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
     /* After the replace, the group has the testghost%_i as a member */
@@ -3707,7 +3720,7 @@ START_TEST (test_sysdb_memberof_ghost_replace_noop)
 
     /* Perform the replace operation */
     ret =  sysdb_set_group_attr(test_ctx->domain,
-                                data->groupname, data->attrs, SYSDB_MOD_REP);
+                                data->msg->dn, data->attrs, SYSDB_MOD_REP);
     ck_assert_msg(ret == EOK, "Cannot set group attrs\n");
 
     /* After the replace, the group has the testghost%_i as a member */

src/tools/sss_cache.c

@@ -554,6 +554,7 @@ static errno_t invalidate_entry(TALLOC_CTX *ctx,
                                 const char *name, int entry_type)
 {
     struct sysdb_attrs *sys_attrs = NULL;
+    struct ldb_dn *dn = NULL;
     errno_t ret;
 
     sys_attrs = sysdb_new_attrs(ctx);
@@ -590,8 +591,13 @@ static errno_t invalidate_entry(TALLOC_CTX *ctx,
                             SYSDB_USN, 1);
                     if (ret != EOK) return ret;
 
-                    ret = sysdb_set_group_attr(domain, name, sys_attrs,
+                    dn = sysdb_group_dn(ctx, domain, name);
+                    if (dn == NULL) {
+                        return ENOMEM;
+                    }
+                    ret = sysdb_set_group_attr(domain, dn, sys_attrs,
                                                SYSDB_MOD_REP);
+                    talloc_zfree(dn);
                     if (ret != EOK) break;
 
                     /* WARNING: Direct writing to persistent cache!! */