sdap: properly handle binary objectGuid attribute

Although in the initial processing SSSD treats the binary value right at
some point it mainly assumes that it is a string. Depending on the value
this might end up with the correct binary value stored in the cache but
in most cases there will be only a broken entry in the cache.

This patch converts the binary value into a string representation which
is described in [MS-DTYP] and stores the result in the cache.

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

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

11 files changed, 307 insertions, 34 deletions

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index cf6028acb..ee5757130 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1113,4 +1113,10 @@ errno_t sysdb_get_sids_of_members(TALLOC_CTX *mem_ctx,
                                   const char ***_sids,
                                   const char ***_dns,
                                   size_t *_n);
+
+errno_t sysdb_handle_original_uuid(const char *orig_name,
+                                   struct sysdb_attrs *src_attrs,
+                                   const char *src_name,
+                                   struct sysdb_attrs *dest_attrs,
+                                   const char *dest_name);
 #endif /* __SYS_DB_H__ */
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 6085762dc..7e1c1d976 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3670,3 +3670,55 @@ done:
     talloc_free(tmp_ctx);
     return ret;
 }
+
+errno_t sysdb_handle_original_uuid(const char *orig_name,
+                                   struct sysdb_attrs *src_attrs,
+                                   const char *src_name,
+                                   struct sysdb_attrs *dest_attrs,
+                                   const char *dest_name)
+{
+    int ret;
+    struct ldb_message_element *el;
+    char guid_str_buf[GUID_STR_BUF_SIZE];
+
+    if (orig_name == NULL || src_attrs == NULL || src_name == NULL
+            || dest_attrs == NULL || dest_name == NULL) {
+        return EINVAL;
+    }
+
+    ret = sysdb_attrs_get_el_ext(src_attrs, src_name, false, &el);
+    if (ret != EOK) {
+        if (ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_el failed.\n");
+        }
+        return ret;
+    }
+
+    if (el->num_values != 1) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              "Found more than one UUID value, using the first.\n");
+    }
+
+    /* Check if we got a binary AD objectGUID */
+    if (el->values[0].length == GUID_BIN_LENGTH
+            && strcasecmp(orig_name, "objectGUID") == 0) {
+        ret = guid_blob_to_string_buf(el->values[0].data, guid_str_buf,
+                                      GUID_STR_BUF_SIZE);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "guid_blob_to_string_buf failed.\n");
+            return ret;
+        }
+
+        ret = sysdb_attrs_add_string(dest_attrs, dest_name, guid_str_buf);
+    } else {
+        ret = sysdb_attrs_add_string(dest_attrs, dest_name,
+                                     (const char *)el->values[0].data);
+    }
+
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n");
+        return ret;;
+    }
+
+    return EOK;
+}
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 00a676372..1714188be 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -512,7 +512,6 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     bool use_id_mapping;
     bool need_filter;
     char *sid_str;
-    const char *uuid;
     struct sss_domain_info *subdomain;
 
     tmpctx = talloc_new(NULL);
@@ -549,22 +548,14 @@ static int sdap_save_group(TALLOC_CTX *memctx,
     }
 
     /* Always store UUID if available */
-    ret = sysdb_attrs_get_string(attrs,
-                                 opts->group_map[SDAP_AT_GROUP_UUID].sys_name,
-                                 &uuid);
-    if (ret == EOK) {
-        ret = sysdb_attrs_add_string(group_attrs, SYSDB_UUID, uuid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE, "Could not add UUID string: [%s]\n",
-                                         sss_strerror(ret));
-            goto done;
-        }
-    } else if (ret == ENOENT) {
-        DEBUG(SSSDBG_TRACE_ALL, "UUID not available for group [%s].\n",
-                                 group_name);
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE, "Could not identify UUID [%s]\n",
-                                     sss_strerror(ret));
+    ret = sysdb_handle_original_uuid(
+                                   opts->group_map[SDAP_AT_GROUP_UUID].def_name,
+                                   attrs,
+                                   opts->group_map[SDAP_AT_GROUP_UUID].sys_name,
+                                   group_attrs, SYSDB_UUID);
+    if (ret != EOK) {
+        DEBUG((ret == ENOENT) ? SSSDBG_TRACE_ALL : SSSDBG_MINOR_FAILURE,
+              "Failed to retrieve UUID [%d][%s].\n", ret, sss_strerror(ret));
     }
 
     /* If this object has a SID available, we will determine the correct
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 43b72fe20..416d2a959 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -197,8 +197,13 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                     original_dn = NULL;
                 }
 
+                ret = sysdb_handle_original_uuid(
+                                   opts->group_map[SDAP_AT_GROUP_UUID].def_name,
+                                   ldap_groups[ai],
+                                   opts->group_map[SDAP_AT_GROUP_UUID].sys_name,
+                                   ldap_groups[ai], "uniqueIDstr");
                 ret = sysdb_attrs_get_string(ldap_groups[ai],
-                                             SYSDB_UUID,
+                                             "uniqueIDstr",
                                              &uuid);
                 if (ret) {
                     DEBUG(SSSDBG_FUNC_DATA,
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index 367e3d795..82b4df479 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -140,7 +140,6 @@ int sdap_save_user(TALLOC_CTX *memctx,
     TALLOC_CTX *tmpctx = NULL;
     bool use_id_mapping;
     char *sid_str;
-    const char *uuid;
     char *dom_sid_str = NULL;
     struct sss_domain_info *subdomain;
 
@@ -179,21 +178,13 @@ int sdap_save_user(TALLOC_CTX *memctx,
     }
 
     /* Always store UUID if available */
-    ret = sysdb_attrs_get_string(attrs,
-                                 opts->user_map[SDAP_AT_USER_UUID].sys_name,
-                                 &uuid);
-    if (ret == EOK) {
-        ret = sysdb_attrs_add_string(user_attrs, SYSDB_UUID, uuid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE, "Could not add UUID string: [%s]\n",
-                                         sss_strerror(ret));
-            goto done;
-        }
-    } else if (ret == ENOENT) {
-        DEBUG(SSSDBG_TRACE_ALL, "UUID not available for user.\n");
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE, "Could not identify UUID [%s]\n",
-                                     sss_strerror(ret));
+    ret = sysdb_handle_original_uuid(opts->user_map[SDAP_AT_USER_UUID].def_name,
+                                     attrs,
+                                     opts->user_map[SDAP_AT_USER_UUID].sys_name,
+                                     user_attrs, SYSDB_UUID);
+    if (ret != EOK) {
+        DEBUG((ret == ENOENT) ? SSSDBG_TRACE_ALL : SSSDBG_MINOR_FAILURE,
+              "Failed to retrieve UUID [%d][%s].\n", ret, sss_strerror(ret));
     }
 
     /* If this object has a SID available, we will determine the correct
diff --git a/src/tests/cmocka/test_string_utils.c b/src/tests/cmocka/test_string_utils.c
index e446387d6..5d3fcf4fe 100644
--- a/src/tests/cmocka/test_string_utils.c
+++ b/src/tests/cmocka/test_string_utils.c
@@ -133,3 +133,62 @@ void test_reverse_replace_whitespaces(void **state)
     assert_true(check_leaks_pop(mem_ctx) == true);
     talloc_free(mem_ctx);
 }
+
+void test_guid_blob_to_string_buf(void **state)
+{
+    int ret;
+    char str_buf[GUID_STR_BUF_SIZE];
+    size_t c;
+
+    /* How to get test data:
+     * The objectGUID attribute contains a 16byte long binary value
+     * representing the GUID of the object. This data can be converted
+     * manually to the string representation but it might be easier to use
+     * LDAP_SERVER_EXTENDED_DN_OID as described in [MS-ADST] section
+     * 3.1.1.3.4.1.5. This is an LDAP extended control which adds the GUID and
+     * the SID to the DN of an object. This can be activate with the -E
+     * ldapsearch option like:
+     *
+     *  ldapsearch -E 1.2.840.113556.1.4.529=::MAMCAQE= ....
+     *
+     * where 'MAMCAQE=' is the base64 encoded BER sequence with the integer
+     * value 1 (see [MS-ADTS] for details about possible values).
+     *
+     * Btw, if you want to use the string representation of a GUID to search
+     * for an object in AD you have to use the GUID as the search base in the
+     * following form:
+     *
+     *  ldapsearch b '<GUID=fea80d8d-dbd5-4f84-8574-7db0477f962e>' ...
+     *
+     * (please note that the '<' and '>' are really needed).
+     */
+    struct test_data {
+        uint8_t blob[16];
+        const char *guid_str;
+    } test_data[] = {
+        {{0x8d, 0x0d, 0xa8, 0xfe, 0xd5, 0xdb, 0x84, 0x4f,
+          0x85, 0x74, 0x7d, 0xb0, 0x47, 0x7f, 0x96, 0x2e},
+        "fea80d8d-dbd5-4f84-8574-7db0477f962e"},
+        {{0x91, 0x7e, 0x2e, 0xf8, 0x4e, 0x44, 0xfa, 0x4e,
+         0xb1, 0x13, 0x08, 0x98, 0x63, 0x49, 0x6c, 0xc6},
+        "f82e7e91-444e-4efa-b113-089863496cc6"},
+        {{0}, NULL}
+    };
+
+    ret = guid_blob_to_string_buf(NULL, str_buf, GUID_STR_BUF_SIZE);
+    assert_int_equal(ret, EINVAL);
+
+    ret = guid_blob_to_string_buf((const uint8_t *) "1234567812345678", NULL,
+                                  GUID_STR_BUF_SIZE);
+    assert_int_equal(ret, EINVAL);
+
+    ret = guid_blob_to_string_buf((const uint8_t *) "1234567812345678", str_buf, 0);
+    assert_int_equal(ret, EINVAL);
+
+    for (c = 0; test_data[c].guid_str != NULL; c++) {
+        ret = guid_blob_to_string_buf(test_data[c].blob, str_buf,
+                                      sizeof(str_buf));
+        assert_int_equal(ret, EOK);
+        assert_string_equal(test_data[c].guid_str, str_buf);
+    }
+}
diff --git a/src/tests/cmocka/test_sysdb_utils.c b/src/tests/cmocka/test_sysdb_utils.c
new file mode 100644
index 000000000..d217314cc
--- /dev/null
+++ b/src/tests/cmocka/test_sysdb_utils.c
@@ -0,0 +1,134 @@
+/*
+    SSSD
+
+    sysdb_utils - Tests for various sysdb calls
+
+    Authors:
+        Sumit Bose <sbose@redhat.com>
+
+    Copyright (C) 2015 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+
+#define IPA_UUID "bcae7c40-97eb-11e4-88ca-525400e96a6b"
+
+#define AD_GUID_BIN {0x8d, 0x0d, 0xa8, 0xfe, 0xd5, 0xdb, 0x84, 0x4f, \
+                     0x85, 0x74, 0x7d, 0xb0, 0x47, 0x7f, 0x96, 0x2e};
+#define AD_GUID "fea80d8d-dbd5-4f84-8574-7db0477f962e"
+static void test_sysdb_handle_original_uuid(void **state)
+{
+    int ret;
+    struct sysdb_attrs *src_attrs;
+    struct sysdb_attrs *dest_attrs;
+    const char *guid;
+    uint8_t bin_guid[] = AD_GUID_BIN;
+    struct ldb_val guid_val = {bin_guid, 16};
+
+    ret = sysdb_handle_original_uuid(NULL, NULL, NULL, NULL, NULL);
+    assert_int_equal(ret, EINVAL);
+
+    src_attrs = sysdb_new_attrs(NULL);
+    assert_non_null(src_attrs);
+
+    dest_attrs = sysdb_new_attrs(NULL);
+    assert_non_null(dest_attrs);
+
+    ret = sysdb_handle_original_uuid("xyz", src_attrs, "abc", dest_attrs,
+                                     "def");
+    assert_int_equal(ret, ENOENT);
+
+    ret = sysdb_attrs_add_val(src_attrs, "GUID", &guid_val);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(src_attrs, "UUID", IPA_UUID);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_handle_original_uuid("objectGUID", src_attrs, "GUID",
+                                     dest_attrs, "def");
+    assert_int_equal(ret, EOK);
+    ret = sysdb_attrs_get_string(dest_attrs, "def", &guid);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(guid, AD_GUID);
+
+    ret = sysdb_handle_original_uuid("ipaUniqueID", src_attrs, "UUID",
+                                     dest_attrs, "ghi");
+    assert_int_equal(ret, EOK);
+    ret = sysdb_attrs_get_string(dest_attrs, "ghi", &guid);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(guid, IPA_UUID);
+
+    talloc_free(src_attrs);
+    src_attrs = sysdb_new_attrs(NULL);
+    assert_non_null(src_attrs);
+
+    /* check objectGUID with length other than 16 */
+    ret = sysdb_attrs_add_string(src_attrs, "GUID", IPA_UUID);
+    assert_int_equal(ret, EOK);
+    ret = sysdb_handle_original_uuid("objectGUID", src_attrs, "GUID",
+                                     dest_attrs, "jkl");
+    assert_int_equal(ret, EOK);
+    ret = sysdb_attrs_get_string(dest_attrs, "jkl", &guid);
+    assert_int_equal(ret, EOK);
+    assert_string_equal(guid, IPA_UUID);
+
+    talloc_free(src_attrs);
+    talloc_free(dest_attrs);
+}
+
+int main(int argc, const char *argv[])
+{
+    int rv;
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test(test_sysdb_handle_original_uuid),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_CLI_INIT(debug_level);
+
+    tests_set_cwd();
+    rv = run_tests(tests);
+
+    return rv;
+}
diff --git a/src/tests/cmocka/test_utils.h b/src/tests/cmocka/test_utils.h
index f85ac2f2b..61ef7e43a 100644
--- a/src/tests/cmocka/test_utils.h
+++ b/src/tests/cmocka/test_utils.h
@@ -29,5 +29,6 @@ void test_textual_public_key(void **state);
 /* from src/tests/cmocka/test_string_utils.c */
 void test_replace_whitespaces(void **state);
 void test_reverse_replace_whitespaces(void **state);
+void test_guid_blob_to_string_buf(void **state);
 
 #endif /* __TESTS__CMOCKA__TEST_UTILS_H__ */
diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am
index 46abab5ae..7e603fda1 100644
--- a/src/tests/cwrap/Makefile.am
+++ b/src/tests/cwrap/Makefile.am
@@ -78,6 +78,7 @@ server_tests_SOURCES = \
     ../../../src/util/atomic_io.c \
     ../../../src/util/signal.c \
     ../../../src/util/util.c \
+    ../../../src/util/string_utils.c \
     ../../../src/util/strtonum.c \
     ../../../src/util/util_errors.c \
     ../../../src/util/safe-format-string.c \
@@ -115,6 +116,7 @@ usertools_tests_SOURCES = \
     ../../../src/util/domain_info_utils.c \
     ../../../src/util/safe-format-string.c \
     ../../../src/util/usertools.c \
+    ../../../src/util/string_utils.c \
     ../../../src/util/strtonum.c \
     ../../../src/util/backup_file.c \
     ../../../src/util/atomic_io.c \
diff --git a/src/util/string_utils.c b/src/util/string_utils.c
index a39b950e8..71b2a0920 100644
--- a/src/util/string_utils.c
+++ b/src/util/string_utils.c
@@ -83,3 +83,28 @@ char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
 
     return replace_char(mem_ctx, orig_name, subst, ' ');
 }
+
+errno_t guid_blob_to_string_buf(const uint8_t *blob, char *str_buf,
+                                size_t buf_size)
+{
+    int ret;
+
+    if (blob == NULL || str_buf == NULL || buf_size < GUID_STR_BUF_SIZE) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Buffer too small.\n");
+        return EINVAL;
+    }
+
+    ret = snprintf(str_buf, buf_size,
+         "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+         blob[3], blob[2], blob[1], blob[0],
+         blob[5], blob[4],
+         blob[7], blob[6],
+         blob[8], blob[9],
+         blob[10], blob[11],blob[12], blob[13],blob[14], blob[15]);;
+    if (ret != (GUID_STR_BUF_SIZE -1)) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "snprintf failed.\n");
+        return EIO;
+    }
+
+    return EOK;
+}
diff --git a/src/util/util.h b/src/util/util.h
index bf3a9a057..1530b550b 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -618,6 +618,13 @@ char * sss_reverse_replace_space(TALLOC_CTX *mem_ctx,
                                  const char *orig_name,
                                  const char replace_char);
 
+#define GUID_BIN_LENGTH 16
+/* 16 2-digit hex values + 4 dashes + terminating 0 */
+#define GUID_STR_BUF_SIZE (2 * GUID_BIN_LENGTH + 4 + 1)
+
+errno_t guid_blob_to_string_buf(const uint8_t *blob, char *str_buf,
+                                size_t buf_size);
+
 /* from become_user.c */
 errno_t become_user(uid_t uid, gid_t gid);
 struct sss_creds;