From 745cf4cc7f4e8f7cdc6ea74b5c39a70f0201a883 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Tue, 17 Feb 2015 04:41:21 +0100

Subject: [PATCH 201/207] 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>

---

 src/db/sysdb.h                             |   6 ++

 src/db/sysdb_ops.c                         |  52 +++++++++++

 src/providers/ldap/sdap_async_groups.c     |  25 ++----

 src/providers/ldap/sdap_async_initgroups.c |   7 +-

 src/providers/ldap/sdap_async_users.c      |  23 ++---

 src/tests/cmocka/test_string_utils.c       |  59 +++++++++++++

 src/tests/cmocka/test_sysdb_utils.c        | 134 +++++++++++++++++++++++++++++

 src/tests/cmocka/test_utils.h              |   1 +

 src/tests/cwrap/Makefile.am                |   2 +

 src/util/string_utils.c                    |  25 ++++++

 src/util/util.h                            |   7 ++

 11 files changed, 307 insertions(+), 34 deletions(-)

 create mode 100644 src/tests/cmocka/test_sysdb_utils.c

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

index cf6028acb806d5d4eedf4cf0680cf4ac9fd6368d..ee5757130ec24a4ddfef854af5f59fc3ccc5b8ae 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 6085762dcc5585114dd3049dd3a365856cb6b190..7e1c1d9763a04cd33374770f4ea5d51286bcfee2 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 00a676372fa042dfc2d57e5799261f9a45ed4a73..1714188bee681ff70a03db741cf50058f145abbe 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 43b72fe2051b452c6ea755c8842117cceafa143a..416d2a9594e456b159f24c224fdd8bf8617377d7 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 367e3d795ddd0db5c1c2f8e57d700419f371cd15..82b4df4793f5f0679046f259c251f5897af831cf 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 e446387d6c429515360b23b428555befa915b49a..5d3fcf4fe454a0be3a4c72b778003481f66910bb 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 0000000000000000000000000000000000000000..d217314ccb9234f8d0d329d87c5dc9e847acbcf0

--- /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 f85ac2f2b3c50a60099970752b06adbad38b9fd1..61ef7e43a82649d775d9b932def9e957b0761bed 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 46abab5ae32189b0561d1901407d2bb38a1ec4c0..7e603fda15024da71cf57912acc69bddcc882357 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 a39b950e852de7ed43d6e8a32de3e7fb08a0dc56..71b2a092018076fd9c20ef9ac39a11964876cfc3 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 bf3a9a057aed77e93949370f8651af2631d91432..1530b550bb85c121cbc33c8c6353b7ecae9edaae 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;

-- 

2.1.0