From 1d93029624d708119bbf803e6647a2cbb271f001 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mar 20 2015 11:26:47 +0000 Subject: 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 --- diff --git a/Makefile.am b/Makefile.am index ba24343..0f7725b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -214,6 +214,7 @@ if HAVE_CMOCKA test_search_bases \ sdap-tests \ test_sysdb_views \ + test_sysdb_utils \ test_be_ptask \ test_copy_ccache \ test_copy_keytab \ @@ -2129,6 +2130,21 @@ test_sysdb_views_LDADD = \ libsss_test_common.la \ $(NULL) +test_sysdb_utils_SOURCES = \ + src/tests/cmocka/test_sysdb_utils.c \ + $(NULL) +test_sysdb_utils_CFLAGS = \ + $(AM_CFLAGS) \ + $(NULL) +test_sysdb_utils_LDADD = \ + $(CMOCKA_LIBS) \ + $(LDB_LIBS) \ + $(POPT_LIBS) \ + $(TALLOC_LIBS) \ + $(SSSD_INTERNAL_LTLIBS) \ + libsss_test_common.la \ + $(NULL) + test_be_ptask_SOURCES = \ src/tests/cmocka/test_be_ptask.c \ src/providers/dp_ptask.c \ diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 84c84a4..2a3a2df 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 183406e..8895b62 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3691,3 +3691,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 818f30b..4783252 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -511,7 +511,6 @@ static int sdap_save_group(TALLOC_CTX *memctx, bool posix_group; bool use_id_mapping; char *sid_str; - const char *uuid; struct sss_domain_info *subdomain; int32_t ad_group_type; @@ -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 2fd235f..96617ae 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -196,8 +196,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 367e3d7..82b4df4 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 e446387..5d3fcf4 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 '' ... + * + * (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 0000000..d217314 --- /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 + + 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 . +*/ + +#include +#include +#include +#include +#include + +#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.c b/src/tests/cmocka/test_utils.c index 848317a..6802c56 100644 --- a/src/tests/cmocka/test_utils.c +++ b/src/tests/cmocka/test_utils.c @@ -1127,6 +1127,7 @@ int main(int argc, const char *argv[]) cmocka_unit_test(test_textual_public_key), cmocka_unit_test(test_replace_whitespaces), cmocka_unit_test(test_reverse_replace_whitespaces), + cmocka_unit_test(test_guid_blob_to_string_buf), cmocka_unit_test_setup_teardown(test_add_strings_lists, setup_add_strings_lists, teardown_add_strings_lists), diff --git a/src/tests/cmocka/test_utils.h b/src/tests/cmocka/test_utils.h index f85ac2f..61ef7e4 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 c1991a1..b805e83 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 a39b950..71b2a09 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 829cf56..704e10d 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -630,6 +630,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;