From 6f25f357e3d000f6ad750bc336d24f8402e896af Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Thu, 19 Nov 2015 11:42:39 +0100

Subject: [PATCH] IPA: fix override with the same name

If the user name of a AD user is overridden with the name itself in an

IPA override object SSSD adds this name twice to the alias list causing

an ldb error when trying to write the user object to the cache. As a

result the user is not available.

This patch makes sure that there are no duplicated alias names.

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

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

(cherry picked from commit aedc71fe8360a51785933523f14bb5c4e7e2c38b)

---

 src/db/sysdb.c                   | 18 ++++++++--

 src/db/sysdb.h                   |  4 ++-

 src/providers/ipa/ipa_s2n_exop.c | 13 +++----

 src/tests/sysdb-tests.c          | 78 ++++++++++++++++++++++++++++++++++++++++

 4 files changed, 103 insertions(+), 10 deletions(-)

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

index 07a83a8a8e30df1b8e461a8d04866f2dbc53baf8..a71364d7c4b600eafd10fafa6641eac7b2292764 100644

--- a/src/db/sysdb.c

+++ b/src/db/sysdb.c

@@ -598,7 +598,7 @@ int sysdb_attrs_add_string(struct sysdb_attrs *attrs,

     return sysdb_attrs_add_val(attrs, name, &v);

 }

-int sysdb_attrs_add_lower_case_string(struct sysdb_attrs *attrs,

+int sysdb_attrs_add_lower_case_string(struct sysdb_attrs *attrs, bool safe,

                                       const char *name, const char *str)

 {

     char *lc_str;

@@ -614,7 +614,11 @@ int sysdb_attrs_add_lower_case_string(struct sysdb_attrs *attrs,

         return ENOMEM;

     }

-    ret = sysdb_attrs_add_string(attrs, name, lc_str);

+    if (safe) {

+        ret = sysdb_attrs_add_string_safe(attrs, name, lc_str);

+    } else {

+        ret = sysdb_attrs_add_string(attrs, name, lc_str);

+    }

     talloc_free(lc_str);

     return ret;

@@ -729,7 +733,15 @@ int sysdb_attrs_add_time_t(struct sysdb_attrs *attrs,

 int sysdb_attrs_add_lc_name_alias(struct sysdb_attrs *attrs,

                                   const char *value)

 {

-    return sysdb_attrs_add_lower_case_string(attrs, SYSDB_NAME_ALIAS, value);

+    return sysdb_attrs_add_lower_case_string(attrs, false, SYSDB_NAME_ALIAS,

+                                             value);

+}

+

+int sysdb_attrs_add_lc_name_alias_safe(struct sysdb_attrs *attrs,

+                                       const char *value)

+{

+    return sysdb_attrs_add_lower_case_string(attrs, true, SYSDB_NAME_ALIAS,

+                                             value);

 }

 int sysdb_attrs_copy_values(struct sysdb_attrs *src,

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

index 9e28b5c6691f3710e3051d9746ac5fa47aff8424..3fa3f040708a4984158206d66a1d28a079091cf7 100644

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

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

@@ -315,7 +315,7 @@ int sysdb_attrs_add_string_safe(struct sysdb_attrs *attrs,

                                 const char *name, const char *str);

 int sysdb_attrs_add_string(struct sysdb_attrs *attrs,

                            const char *name, const char *str);

-int sysdb_attrs_add_lower_case_string(struct sysdb_attrs *attrs,

+int sysdb_attrs_add_lower_case_string(struct sysdb_attrs *attrs, bool safe,

                                       const char *name, const char *str);

 int sysdb_attrs_add_mem(struct sysdb_attrs *attrs, const char *name,

                         const void *mem, size_t size);

@@ -329,6 +329,8 @@ int sysdb_attrs_add_time_t(struct sysdb_attrs *attrs,

                            const char *name, time_t value);

 int sysdb_attrs_add_lc_name_alias(struct sysdb_attrs *attrs,

                                   const char *value);

+int sysdb_attrs_add_lc_name_alias_safe(struct sysdb_attrs *attrs,

+                                       const char *value);

 int sysdb_attrs_copy_values(struct sysdb_attrs *src,

                             struct sysdb_attrs *dst,

                             const char *name);

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c

index 1e6368dc7ef1a6f60b541409f7f6740d602f0d43..bcd11749fbde4cae2a47b9b2182138ae04f2d6bc 100644

--- a/src/providers/ipa/ipa_s2n_exop.c

+++ b/src/providers/ipa/ipa_s2n_exop.c

@@ -1804,10 +1804,11 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,

         ret = sysdb_attrs_get_string(attrs->sysdb_attrs,

                                      SYSDB_DEFAULT_OVERRIDE_NAME, &tmp_str);

         if (ret == EOK) {

-            ret = sysdb_attrs_add_lc_name_alias(attrs->sysdb_attrs, tmp_str);

+            ret = sysdb_attrs_add_lc_name_alias_safe(attrs->sysdb_attrs,

+                                                     tmp_str);

             if (ret != EOK) {

                 DEBUG(SSSDBG_OP_FAILURE,

-                      "sysdb_attrs_add_lc_name_alias failed.\n");

+                      "sysdb_attrs_add_lc_name_alias_safe failed.\n");

                 goto done;

             }

         } else if (ret != ENOENT) {

@@ -1876,10 +1877,10 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,

                 }

             }

-            ret = sysdb_attrs_add_lc_name_alias(attrs->sysdb_attrs, name);

+            ret = sysdb_attrs_add_lc_name_alias_safe(attrs->sysdb_attrs, name);

             if (ret != EOK) {

                 DEBUG(SSSDBG_OP_FAILURE,

-                      "sysdb_attrs_add_lc_name_alias failed.\n");

+                      "sysdb_attrs_add_lc_name_alias_safe failed.\n");

                 goto done;

             }

@@ -2133,10 +2134,10 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,

             }

             DEBUG(SSSDBG_TRACE_FUNC, "Processing group %s\n", name);

-            ret = sysdb_attrs_add_lc_name_alias(attrs->sysdb_attrs, name);

+            ret = sysdb_attrs_add_lc_name_alias_safe(attrs->sysdb_attrs, name);

             if (ret != EOK) {

                 DEBUG(SSSDBG_OP_FAILURE,

-                      "sysdb_attrs_add_lc_name_alias failed.\n");

+                      "sysdb_attrs_add_lc_name_alias_safe failed.\n");

                 goto done;

             }

diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c

index 522a44aa4d5c0da6d10bba10d960fff9426200c1..0b091f741ce158713ed383ad3d98dfea25f388ed 100644

--- a/src/tests/sysdb-tests.c

+++ b/src/tests/sysdb-tests.c

@@ -4690,6 +4690,7 @@ START_TEST(test_sysdb_attrs_add_lc_name_alias)

     int ret;

     struct sysdb_attrs *attrs;

     const char *str;

+    char **list = NULL;

     ret = sysdb_attrs_add_lc_name_alias(NULL, NULL);

     fail_unless(ret == EINVAL, "EINVAL not returned for NULL input");

@@ -4706,6 +4707,82 @@ START_TEST(test_sysdb_attrs_add_lc_name_alias)

                 "Unexpected value, expected [%s], got [%s]",

                 LC_NAME_ALIAS_CHECK_VAL, str);

+    /* Add the same value a second time, it is not recommended to do this on

+     * purpose but the test should illustrate the different to

+     * sysdb_attrs_add_lc_name_alias_safe(). */

+    ret = sysdb_attrs_add_lc_name_alias(attrs, LC_NAME_ALIAS_TEST_VAL);

+    fail_unless(ret == EOK, "sysdb_attrs_add_lc_name_alias failed");

+

+    ret = sysdb_attrs_get_string_array(attrs, SYSDB_NAME_ALIAS, attrs, &list);

+    fail_unless(ret == EOK, "sysdb_attrs_get_string_array failed");

+    fail_unless(list != NULL, "No list returned");

+    fail_unless(list[0] != NULL, "Missing first list element");

+    fail_unless(strcmp(list[0], LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                LC_NAME_ALIAS_CHECK_VAL, list[0]);

+    fail_unless(list[1] != NULL, "Missing second list element");

+    fail_unless(strcmp(list[1], LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                LC_NAME_ALIAS_CHECK_VAL, list[1]);

+    fail_unless(list[2] == NULL, "Missing list terminator");

+

+    talloc_free(attrs);

+}

+END_TEST

+

+START_TEST(test_sysdb_attrs_add_lc_name_alias_safe)

+{

+    int ret;

+    struct sysdb_attrs *attrs;

+    const char *str;

+    char **list = NULL;

+

+    ret = sysdb_attrs_add_lc_name_alias_safe(NULL, NULL);

+    fail_unless(ret == EINVAL, "EINVAL not returned for NULL input");

+

+    attrs = sysdb_new_attrs(NULL);

+    fail_unless(attrs != NULL, "sysdb_new_attrs failed");

+

+    ret = sysdb_attrs_add_lc_name_alias_safe(attrs, LC_NAME_ALIAS_TEST_VAL);

+    fail_unless(ret == EOK, "sysdb_attrs_add_lc_name_alias failed");

+

+    ret = sysdb_attrs_get_string(attrs, SYSDB_NAME_ALIAS, &str);

+    fail_unless(ret == EOK, "sysdb_attrs_get_string failed");

+    fail_unless(strcmp(str, LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                LC_NAME_ALIAS_CHECK_VAL, str);

+

+    /* Adding the same value a second time should be ignored */

+    ret = sysdb_attrs_add_lc_name_alias_safe(attrs, LC_NAME_ALIAS_TEST_VAL);

+    fail_unless(ret == EOK, "sysdb_attrs_add_lc_name_alias failed");

+

+    ret = sysdb_attrs_get_string_array(attrs, SYSDB_NAME_ALIAS, attrs, &list);

+    fail_unless(ret == EOK, "sysdb_attrs_get_string_array failed");

+    fail_unless(list != NULL, "No list returned");

+    fail_unless(list[0] != NULL, "Missing first list element");

+    fail_unless(strcmp(list[0], LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                LC_NAME_ALIAS_CHECK_VAL, list[0]);

+    fail_unless(list[1] == NULL, "Missing list terminator");

+

+    /* Adding different value */

+    ret = sysdb_attrs_add_lc_name_alias_safe(attrs,

+                                             "2nd_" LC_NAME_ALIAS_TEST_VAL);

+    fail_unless(ret == EOK, "sysdb_attrs_add_lc_name_alias failed");

+

+    ret = sysdb_attrs_get_string_array(attrs, SYSDB_NAME_ALIAS, attrs, &list);

+    fail_unless(ret == EOK, "sysdb_attrs_get_string_array failed");

+    fail_unless(list != NULL, "No list returned");

+    fail_unless(list[0] != NULL, "Missing first list element");

+    fail_unless(strcmp(list[0], LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                LC_NAME_ALIAS_CHECK_VAL, list[0]);

+    fail_unless(list[1] != NULL, "Missing first list element");

+    fail_unless(strcmp(list[1], "2nd_" LC_NAME_ALIAS_CHECK_VAL) == 0,

+                "Unexpected value, expected [%s], got [%s]",

+                "2nd_" LC_NAME_ALIAS_CHECK_VAL, list[1]);

+    fail_unless(list[2] == NULL, "Missing list terminator");

+

     talloc_free(attrs);

 }

 END_TEST

@@ -6412,6 +6489,7 @@ Suite *create_sysdb_suite(void)

     tcase_add_test(tc_sysdb, test_sysdb_svc_remove_alias);

     tcase_add_test(tc_sysdb, test_sysdb_attrs_add_lc_name_alias);

+    tcase_add_test(tc_sysdb, test_sysdb_attrs_add_lc_name_alias_safe);

 /* ===== UTIL TESTS ===== */

     tcase_add_test(tc_sysdb, test_sysdb_attrs_get_string_array);

-- 

2.4.3