[SSSD] [PATCH] LDAP: Try all attributes when saving an entry

Jakub Hrozek jhrozek at redhat.com
Mon Jul 7 20:12:56 UTC 2014


On Wed, Oct 23, 2013 at 06:58:16PM +0200, Jakub Hrozek wrote:
> On Wed, Oct 23, 2013 at 01:21:48PM +0200, Jakub Hrozek wrote:
> > Hi,
> > 
> > this bug was reported on #sssd by a user. He was running some flavor of
> > IBM Tivoli where the entries only had an "ID", not separate "UID" and
> > "GID". But due to a bug in sssd he couldn't use the same value for both,
> > this configuration:
> > 
> > ldap_user_uid_number = idAttribute
> > ldap_user_gid_number = idAttribute
> > 
> > only saved the ID into UID and left GID empty. It appears we have a long
> > standing bug in sdap_parse where we only consider first match. If this
> > patch is accepted, I would also like to refactor sdap_parse in master
> > because currently it is a 250-lines long function with multiple
> > branches..
> 
> self-nack, this patch breaks parsing of rootDSE. I will prepare a new
> version.

After a slight delay I'm attaching a revised patchset. I know we're getting
close to the release of 1.12.0 and this patch changes a critical function,
so I also prepared a unit test. I think only patches #1 to #3 are important
for 1.12.0, so I'm fine with postponing the other patches.

[PATCH 1/7] PROVIDERS: Add ldap_common.h to opts.h of each provider
Trivial header file amendment, please see the commit message for details.

[PATCH 2/7] TESTS: Add a unit test for the sdap.c module
As said earlier, a unit test was added. I haven't covered only the range
extensions.

[PATCH 3/7] LDAP: Try all attributes when saving an entry
https://fedorahosted.org/sssd/ticket/2184

I tested by setting the 'telephoneNumber' attribute to a numeric value
and ensuring that SSSD was able to use the value for both UID and GID.

And now the less important patches, mostly just changes I did while I
was changing that area of code:
[PATCH 4/7] SDAP: Fix DEBUG message priorities in sdap_parse_entry
Amends log levels of DEBUG messages.

[PATCH 5/7] LDAP: Remove unused output parameter _dn from sdap_parse_entry
Does what the commit message suggests.

[PATCH 6/7] SDAP: Remove unused function sdap_get_msg_dn
This function was unused since 2009

[PATCH 7/7] SDAP: Free bervals on failure in sdap_parse_entry
I wasn't able to test this patch, so review would only be with visual
inspection, I think..
-------------- next part --------------
>From 92b7e15854267c6844e7d75bf15ca610121f3248 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:15:12 +0200
Subject: [PATCH 1/7] PROVIDERS: Add ldap_common.h to opts.h of each provider

the opts.h files were consuming some #defines from ldap_common.h (such
as SSS_LDAP_SRV_NAME) without including ldap_common.h. That's bad
practice and break programs that wish to just include the opts.h header.
---
 src/providers/ad/ad_opts.h     | 1 +
 src/providers/ipa/ipa_opts.h   | 1 +
 src/providers/ldap/ldap_opts.h | 1 +
 3 files changed, 3 insertions(+)

diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index 8f6ce7e1e9cc3906044c6d8eaa8b4c42e038b57b..b80aaddf29b7fb236d89bfb4e148bba7aeeecd14 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -26,6 +26,7 @@
 #include "src/providers/data_provider.h"
 #include "db/sysdb_services.h"
 #include "db/sysdb_autofs.h"
+#include "providers/ldap/ldap_common.h"
 
 struct dp_option ad_basic_opts[] = {
     { "ad_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING },
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h
index 0b39b8c5adb61617c90c1b79a45d350afc3084c8..d7c2b189fad57cb60c29b56c5e351a46070349e2 100644
--- a/src/providers/ipa/ipa_opts.h
+++ b/src/providers/ipa/ipa_opts.h
@@ -29,6 +29,7 @@
 #include "db/sysdb_autofs.h"
 #include "db/sysdb_services.h"
 #include "db/sysdb_selinux.h"
+#include "providers/ldap/ldap_common.h"
 
 struct dp_option ipa_basic_opts[] = {
     { "ipa_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING },
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h
index f4e18d94942ded72cd5dade585cc4200b1d136da..adf200caa90b51e3e459e01251340c85ec2f518b 100644
--- a/src/providers/ldap/ldap_opts.h
+++ b/src/providers/ldap/ldap_opts.h
@@ -28,6 +28,7 @@
 #include "db/sysdb_sudo.h"
 #include "db/sysdb_autofs.h"
 #include "db/sysdb_services.h"
+#include "providers/ldap/ldap_common.h"
 
 struct dp_option default_basic_opts[] = {
     { "ldap_uri", DP_OPT_STRING, NULL_STRING, NULL_STRING },
-- 
1.9.3

-------------- next part --------------
>From b1ca11678e3f5c149f2a0041aaacef7de3ec835f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:16:01 +0200
Subject: [PATCH 2/7] TESTS: Add a unit test for the sdap.c module

Covers the sdap_parse_entry function with unit tests so that we know
that modifying the function in a later patch will not result in a
regression.
---
 Makefile.am                  |  33 ++-
 src/tests/cmocka/test_sdap.c | 530 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 562 insertions(+), 1 deletion(-)
 create mode 100644 src/tests/cmocka/test_sdap.c

diff --git a/Makefile.am b/Makefile.am
index 1c423fc6d40754852a9775dc72016703e398f17c..c05d8cb5ec0bdd79847c6fb44e714c93ebc3e50c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -191,7 +191,9 @@ if HAVE_CMOCKA
         responder-get-domains-tests \
         sbus-internal-tests \
         sss_sifp-tests \
-        test_search_bases
+        test_search_bases \
+        sdap-tests \
+        $(NULL)
 
 if BUILD_IFP
 non_interactive_cmocka_based_tests += ifp_tests
@@ -1838,6 +1840,35 @@ dp_opt_tests_LDADD = \
     $(SSSD_INTERNAL_LTLIBS) \
     libsss_test_common.la
 
+sdap_tests_SOURCES = \
+    src/providers/data_provider_opts.c \
+    src/providers/ldap/sdap.c \
+    src/providers/ldap/sdap_range.c \
+    src/util/sss_ldap.c \
+    src/tests/cmocka/test_sdap.c \
+    $(NULL)
+sdap_tests_CFLAGS = \
+    $(AM_CFLAGS) \
+    $(SSS_CRYPT_CFLAGS) \
+    $(NULL)
+sdap_tests_LDFLAGS = \
+    -Wl,-wrap,ldap_set_option \
+    -Wl,-wrap,ldap_get_dn \
+    -Wl,-wrap,ldap_memfree \
+    -Wl,-wrap,ldap_get_values_len \
+    -Wl,-wrap,ldap_value_free_len \
+    -Wl,-wrap,ldap_first_attribute \
+    -Wl,-wrap,ldap_next_attribute \
+    $(NULL)
+sdap_tests_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(TALLOC_LIBS) \
+    $(POPT_LIBS) \
+    $(SSSD_INTERNAL_LTLIBS) \
+    $(SSS_CRYPT_LIBS) \
+    libsss_test_common.la \
+    $(NULL)
+
 if BUILD_IFP
 ifp_tests_SOURCES = \
      $(TEST_MOCK_RESP_OBJ) \
diff --git a/src/tests/cmocka/test_sdap.c b/src/tests/cmocka/test_sdap.c
new file mode 100644
index 0000000000000000000000000000000000000000..3990d7a3ec7124e77a17b683de0113618083d78b
--- /dev/null
+++ b/src/tests/cmocka/test_sdap.c
@@ -0,0 +1,530 @@
+/*
+    Authors:
+        Jakub Hrozek <jhrozek at redhat.com>
+
+    Copyright (C) 2014 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 <talloc.h>
+#include <tevent.h>
+#include <errno.h>
+#include <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+#include "providers/ldap/ldap_opts.h"
+#include "providers/ipa/ipa_opts.h"
+#include "util/crypto/sss_crypto.h"
+
+/* mock an LDAP entry */
+struct mock_ldap_attr {
+    const char *name;
+    const char **values;
+};
+
+struct mock_ldap_entry {
+    const char *dn;
+    struct mock_ldap_attr *attrs;
+};
+
+struct mock_ldap_entry *global_ldap_entry;
+
+static int mock_ldap_entry_iter(void)
+{
+    return sss_mock_type(int);
+}
+
+static struct mock_ldap_entry *mock_ldap_entry_get(void)
+{
+    return sss_mock_ptr_type(struct mock_ldap_entry *);
+}
+
+void set_entry_parse(struct mock_ldap_entry *entry)
+{
+    will_return_always(mock_ldap_entry_get, entry);
+}
+
+/* libldap wrappers */
+int __wrap_ldap_set_option(LDAP *ld,
+                           int option,
+                           void *invalue)
+{
+    return LDAP_OPT_SUCCESS;
+}
+
+char *__wrap_ldap_get_dn(LDAP *ld, LDAPMessage *entry)
+{
+    struct mock_ldap_entry *ldap_entry = mock_ldap_entry_get();
+    return discard_const(ldap_entry->dn);
+}
+
+void __wrap_ldap_memfree(void *p)
+{
+    return;
+}
+
+struct berval **__wrap_ldap_get_values_len(LDAP *ld,
+                                           LDAPMessage *entry,
+                                           LDAP_CONST char *target)
+{
+    size_t count, i;
+    struct berval **vals;
+    const char **attrvals;
+    struct mock_ldap_entry *ldap_entry = mock_ldap_entry_get();
+
+    if (target == NULL) return NULL;
+    if (ldap_entry == NULL) return NULL;
+    /* Should we return empty array here? */
+    if (ldap_entry->attrs == NULL) return NULL;
+
+    attrvals = NULL;
+    for (i = 0; ldap_entry->attrs[i].name != NULL; i++) {
+        if (strcmp(ldap_entry->attrs[i].name, target) == 0) {
+            attrvals = ldap_entry->attrs[i].values;
+            break;
+        }
+    }
+
+    if (attrvals == NULL) {
+        return NULL;
+    }
+
+    count = 0;
+    for (i = 0; attrvals[i]; i++) {
+        count++;
+    }
+
+    vals = talloc_zero_array(global_talloc_context,
+                             struct berval *,
+                             count + 1);
+    assert_non_null(vals);
+
+    for (i = 0; attrvals[i]; i++) {
+        vals[i] = talloc_zero(vals, struct berval);
+        assert_non_null(vals[i]);
+
+        vals[i]->bv_val = talloc_strdup(vals[i], attrvals[i]);
+        if (vals[i]->bv_val == NULL) {
+            talloc_free(vals);
+            return NULL;
+        }
+        vals[i]->bv_len = strlen(attrvals[i]);
+        assert_non_null(vals[i]->bv_len);
+    }
+
+    return vals;
+}
+
+void __wrap_ldap_value_free_len(struct berval **vals)
+{
+    talloc_free(vals);  /* Allocated on global_talloc_context */
+}
+
+char *__wrap_ldap_first_attribute(LDAP *ld,
+                                  LDAPMessage *entry,
+                                  BerElement **berout)
+{
+    struct mock_ldap_entry *ldap_entry = mock_ldap_entry_get();
+
+    if (ldap_entry == NULL) return NULL;
+    if (ldap_entry->attrs == NULL) return NULL;
+
+    will_return(mock_ldap_entry_iter, 1);
+    return discard_const(ldap_entry->attrs[0].name);
+}
+
+char *__wrap_ldap_next_attribute(LDAP *ld,
+                                 LDAPMessage *entry,
+                                 BerElement *ber)
+{
+    struct mock_ldap_entry *ldap_entry = mock_ldap_entry_get();
+
+    int index = mock_ldap_entry_iter();
+    char *val;
+
+    val = discard_const(ldap_entry->attrs[index].name);
+    if (val != NULL) {
+        will_return(mock_ldap_entry_iter, index+1);
+    }
+    return val;
+}
+
+/* Mock parsing search base without overlinking the test */
+errno_t sdap_parse_search_base(TALLOC_CTX *mem_ctx,
+                               struct dp_option *opts, int class,
+                               struct sdap_search_base ***_search_bases)
+{
+    return EOK;
+}
+
+/* Utility function */
+void assert_entry_has_attr(struct sysdb_attrs *attrs,
+                           const char *attr,
+                           const char *value)
+{
+    const char *v;
+    int ret;
+
+    ret = sysdb_attrs_get_string(attrs, attr, &v);
+    assert_int_equal(ret, ERR_OK);
+    assert_non_null(v);
+    assert_string_equal(v, value);
+}
+
+void assert_entry_has_no_attr(struct sysdb_attrs *attrs,
+                              const char *attr)
+{
+    int ret;
+    const char *v;
+    ret = sysdb_attrs_get_string(attrs, attr, &v);
+    assert_int_equal(ret, ENOENT);
+}
+
+struct parse_test_ctx {
+    struct sdap_handle sh;
+    struct sdap_msg sm;
+};
+
+void parse_entry_test_setup(void **state)
+{
+    struct parse_test_ctx *test_ctx;
+
+    assert_true(leak_check_setup());
+
+    test_ctx = talloc_zero(global_talloc_context, struct parse_test_ctx);
+    assert_non_null(test_ctx);
+
+    check_leaks_push(test_ctx);
+    *state = test_ctx;
+}
+
+void parse_entry_test_teardown(void **state)
+{
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+
+    assert_true(check_leaks_pop(test_ctx) == true);
+    talloc_free(test_ctx);
+    assert_true(leak_check_teardown());
+}
+
+void test_parse_with_map(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_ipa_user;
+    struct sdap_attr_map *map;
+    struct ldb_message_element *el;
+    uint8_t *decoded_key;
+    size_t key_len;
+
+    const char *oc_values[] = { "posixAccount", NULL };
+    const char *uid_values[] = { "tuser1", NULL };
+    const char *extra_values[] = { "extra", NULL };
+    const char *multi_values[] = { "svc1", "svc2", NULL };
+    const char *ssh_values[] = { "1234", NULL };
+    struct mock_ldap_attr test_ipa_user_attrs[] = {
+        { .name = "objectClass", .values = oc_values },
+        { .name = "uid", .values = uid_values },
+        { .name = "extra", .values = extra_values },
+        { .name = "authorizedService", .values = multi_values },
+        { .name = "ipaSshPubKey", .values = ssh_values },
+        { NULL, NULL }
+    };
+
+    test_ipa_user.dn = "cn=testuser,dc=example,dc=com";
+    test_ipa_user.attrs = test_ipa_user_attrs;
+    set_entry_parse(&test_ipa_user);
+
+    ret = sdap_copy_map(test_ctx, ipa_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_equal(ret, ERR_OK);
+
+    assert_int_equal(attrs->num, 4);
+
+    /* Every entry has a DN */
+    assert_entry_has_attr(attrs, SYSDB_ORIG_DN,
+                          "cn=testuser,dc=example,dc=com");
+    /* Test the single-valued attribute */
+    assert_entry_has_attr(attrs, SYSDB_NAME, "tuser1");
+
+    /* Multivalued attributes must return all values */
+    ret = sysdb_attrs_get_el_ext(attrs, SYSDB_AUTHORIZED_SERVICE, false, &el);
+    assert_int_equal(ret, ERR_OK);
+    assert_int_equal(el->num_values, 2);
+    assert_true((strcmp((const char *) el->values[0].data, "svc1") == 0 &&
+                    strcmp((const char *) el->values[1].data, "svc2") == 0) ||
+                (strcmp((const char *) el->values[1].data, "svc1") == 0 &&
+                    strcmp((const char *) el->values[0].data, "svc2") == 0));
+
+    /* The SSH attribute must be base64 encoded */
+    ret = sysdb_attrs_get_el_ext(attrs, SYSDB_SSH_PUBKEY, false, &el);
+    assert_int_equal(ret, ERR_OK);
+    assert_int_equal(el->num_values, 1);
+    decoded_key = sss_base64_decode(test_ctx,
+                                    (const char *)el->values[0].data,
+                                    &key_len);
+    assert_string_equal(decoded_key, "1234");
+
+    /* The extra attribute must not be downloaded, it's not present in map */
+    assert_entry_has_no_attr(attrs, "extra");
+
+    talloc_free(decoded_key);
+    talloc_free(map);
+    talloc_free(attrs);
+}
+
+/* Some searches, like rootDSE search do not use any map */
+void test_parse_no_map(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_nomap_entry;
+    struct ldb_message_element *el;
+
+    const char *foo_values[] = { "fooval1", "fooval2", NULL };
+    const char *bar_values[] = { "barval1", NULL };
+    struct mock_ldap_attr test_nomap_entry_attrs[] = {
+        { .name = "foo", .values = foo_values },
+        { .name = "bar", .values = bar_values },
+        { NULL, NULL }
+    };
+
+    test_nomap_entry.dn = "cn=testentry,dc=example,dc=com";
+    test_nomap_entry.attrs = test_nomap_entry_attrs;
+    set_entry_parse(&test_nomap_entry);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           NULL, 0,
+                           &attrs, NULL, false);
+    assert_int_equal(ret, ERR_OK);
+
+    assert_int_equal(attrs->num, 3);
+    assert_entry_has_attr(attrs, SYSDB_ORIG_DN,
+                          "cn=testentry,dc=example,dc=com");
+    assert_entry_has_attr(attrs, "bar", "barval1");
+    /* Multivalued attributes must return all values */
+    ret = sysdb_attrs_get_el_ext(attrs, "foo", false, &el);
+    assert_int_equal(ret, ERR_OK);
+    assert_int_equal(el->num_values, 2);
+    assert_true((strcmp((const char *) el->values[0].data, "fooval1") == 0 &&
+                    strcmp((const char *) el->values[1].data, "fooval2") == 0) ||
+                (strcmp((const char *) el->values[1].data, "fooval1") == 0 &&
+                    strcmp((const char *) el->values[0].data, "fooval2") == 0));
+
+
+    talloc_free(attrs);
+}
+
+/* Only DN and OC, no real attributes */
+void test_parse_no_attrs(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_rfc2307_user;
+    struct sdap_attr_map *map;
+
+    const char *oc_values[] = { "posixAccount", NULL };
+    struct mock_ldap_attr test_rfc2307_user_attrs[] = {
+        { .name = "objectClass", .values = oc_values },
+        { NULL, NULL }
+    };
+
+    test_rfc2307_user.dn = "cn=testuser,dc=example,dc=com";
+    test_rfc2307_user.attrs = test_rfc2307_user_attrs;
+    set_entry_parse(&test_rfc2307_user);
+
+    ret = sdap_copy_map(test_ctx, rfc2307_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_equal(ret, ERR_OK);
+
+    assert_int_equal(attrs->num, 1);
+    assert_entry_has_attr(attrs, SYSDB_ORIG_DN,
+                          "cn=testuser,dc=example,dc=com");
+
+    talloc_free(map);
+    talloc_free(attrs);
+}
+
+/* Negative test - objectclass doesn't match the map */
+void test_parse_bad_oc(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_rfc2307_user;
+    struct sdap_attr_map *map;
+
+    const char *oc_values[] = { "someRandomValueWhoCaresItsAUnitTest", NULL };
+    const char *uid_values[] = { "tuser1", NULL };
+    struct mock_ldap_attr test_rfc2307_user_attrs[] = {
+        { .name = "objectClass", .values = oc_values },
+        { .name = "uid", .values = uid_values },
+        { NULL, NULL }
+    };
+
+    test_rfc2307_user.dn = "cn=testuser,dc=example,dc=com";
+    test_rfc2307_user.attrs = test_rfc2307_user_attrs;
+    set_entry_parse(&test_rfc2307_user);
+
+    ret = sdap_copy_map(test_ctx, rfc2307_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_not_equal(ret, ERR_OK);
+
+    talloc_free(map);
+}
+
+/* Negative test - the entry has no objectClass. Just make sure
+ * we don't crash
+ */
+void test_parse_no_oc(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_rfc2307_user;
+    struct sdap_attr_map *map;
+
+    const char *uid_values[] = { "tuser1", NULL };
+    struct mock_ldap_attr test_rfc2307_user_attrs[] = {
+        { .name = "uid", .values = uid_values },
+        { NULL, NULL }
+    };
+
+    test_rfc2307_user.dn = "cn=testuser,dc=example,dc=com";
+    test_rfc2307_user.attrs = test_rfc2307_user_attrs;
+    set_entry_parse(&test_rfc2307_user);
+
+    ret = sdap_copy_map(test_ctx, rfc2307_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_not_equal(ret, ERR_OK);
+
+    talloc_free(map);
+}
+
+/* Negative test - the entry has no DN. Just make sure
+ * we don't crash and detect the failure.
+ */
+void test_parse_no_dn(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_rfc2307_user;
+    struct sdap_attr_map *map;
+
+    const char *oc_values[] = { "posixAccount", NULL };
+    const char *uid_values[] = { "tuser1", NULL };
+    struct mock_ldap_attr test_rfc2307_user_attrs[] = {
+        { .name = "objectClass", .values = oc_values },
+        { .name = "uid", .values = uid_values },
+        { NULL, NULL }
+    };
+
+    test_rfc2307_user.dn = NULL;        /* Test */
+    test_rfc2307_user.attrs = test_rfc2307_user_attrs;
+    set_entry_parse(&test_rfc2307_user);
+
+    ret = sdap_copy_map(test_ctx, rfc2307_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_not_equal(ret, ERR_OK);
+
+    talloc_free(map);
+}
+
+int main(int argc, const char *argv[])
+{
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test_setup_teardown(test_parse_with_map,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+        unit_test_setup_teardown(test_parse_no_map,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+        unit_test_setup_teardown(test_parse_no_attrs,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+        /* Negative tests */
+        unit_test_setup_teardown(test_parse_no_oc,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+        unit_test_setup_teardown(test_parse_bad_oc,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+        unit_test_setup_teardown(test_parse_no_dn,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
+    };
+
+    /* 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_INIT(debug_level);
+
+    /* Even though normally the tests should clean up after themselves
+     * they might not after a failed run. Remove the old db to be sure */
+    tests_set_cwd();
+
+    return run_tests(tests);
+}
-- 
1.9.3

-------------- next part --------------
>From 29ba0737a125be9b62359fdd3d609c8231b34f47 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 4 Jul 2014 16:58:11 +0200
Subject: [PATCH 3/7] LDAP: Try all attributes when saving an entry

https://fedorahosted.org/sssd/ticket/2184

The same LDAP attribute might be used several times for the same user or
group attribute. For instance, some servers have a global "ID" number
that should be used for both UID and GID. However, our
sdap_parse_entry() function only copied the LDAP attribute to the first
matching sysdb attribute.

This patch adds a second nested loop that checks if any of the other
LDAP attributes are eligible.
---
 src/providers/ldap/sdap.c    | 27 +++++++++++++++++++---
 src/tests/cmocka/test_sdap.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index e8d23c9dc5eac34fc5182305dc4be9180f8be176..133e6dcf31109b69edb56437796e34ebd22e17fc 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -302,7 +302,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     struct ldb_val v;
     char *str;
     int lerrno;
-    int a, i, ret;
+    int a, i, ret, ai;
     const char *name;
     bool store;
     bool base64;
@@ -480,8 +480,29 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                         v.length = vals[i]->bv_len;
                     }
 
-                    ret = sysdb_attrs_add_val(attrs, name, &v);
-                    if (ret) goto done;
+                    if (map) {
+                        /* The same LDAP attr might be used for more sysdb
+                         * attrs in case there is a map. Find all that match
+                         * and copy the value
+                         */
+                        for (ai = a; ai < attrs_num; ai++) {
+                            /* check if this attr is valid with the chosen
+                             * schema */
+                            if (!map[ai].name) continue;
+
+                            /* check if it is an attr we are interested in */
+                            if (strcasecmp(base_attr, map[ai].name) == 0) {
+                                ret = sysdb_attrs_add_val(attrs,
+                                                          map[ai].sys_name,
+                                                          &v);
+                                if (ret) goto done;
+                            }
+                        }
+                    } else {
+                        /* No map, just store the attribute */
+                        ret = sysdb_attrs_add_val(attrs, name, &v);
+                        if (ret) goto done;
+                    }
                 }
                 ldap_value_free_len(vals);
             }
diff --git a/src/tests/cmocka/test_sdap.c b/src/tests/cmocka/test_sdap.c
index 3990d7a3ec7124e77a17b683de0113618083d78b..8fdf1a4a21f7f3f436662d6ef2aee94d85be7d91 100644
--- a/src/tests/cmocka/test_sdap.c
+++ b/src/tests/cmocka/test_sdap.c
@@ -372,6 +372,58 @@ void test_parse_no_attrs(void **state)
     talloc_free(attrs);
 }
 
+void test_parse_dups(void **state)
+{
+    int ret;
+    struct sysdb_attrs *attrs;
+    struct parse_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                      struct parse_test_ctx);
+    struct mock_ldap_entry test_dupattr_user;
+    struct sdap_attr_map *map;
+    int i;
+
+    const char *oc_values[] = { "posixAccount", NULL };
+    const char *uid_values[] = { "1234", NULL };
+    struct mock_ldap_attr test_dupattr_attrs[] = {
+        { .name = "objectClass", .values = oc_values },
+        { .name = "idNumber", .values = uid_values },
+        { NULL, NULL }
+    };
+
+    test_dupattr_user.dn = "cn=dupuser,dc=example,dc=com";
+    test_dupattr_user.attrs = test_dupattr_attrs;
+    set_entry_parse(&test_dupattr_user);
+
+    ret = sdap_copy_map(test_ctx, rfc2307_user_map, SDAP_OPTS_USER, &map);
+    assert_int_equal(ret, ERR_OK);
+    /* Set both uidNumber and gidNumber to idNumber */
+    for (i = 0; i < SDAP_OPTS_USER; i++) {
+        if (map[i].name == NULL) continue;
+
+        if (strcmp(map[i].name, "uidNumber") == 0
+             || strcmp(map[i].name, "gidNumber") == 0) {
+            map[i].name = discard_const("idNumber");
+        }
+    }
+
+    ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
+                           map, SDAP_OPTS_USER,
+                           &attrs, NULL, false);
+    assert_int_equal(ret, ERR_OK);
+
+    assert_int_equal(attrs->num, 3);
+
+    /* Every entry has a DN */
+    assert_entry_has_attr(attrs, SYSDB_ORIG_DN,
+                          "cn=dupuser,dc=example,dc=com");
+    /* Test the single-valued attribute */
+    assert_entry_has_attr(attrs, SYSDB_UIDNUM, "1234");
+    assert_entry_has_attr(attrs, SYSDB_GIDNUM, "1234");
+
+    talloc_free(map);
+    talloc_free(attrs);
+}
+
 /* Negative test - objectclass doesn't match the map */
 void test_parse_bad_oc(void **state)
 {
@@ -493,6 +545,9 @@ int main(int argc, const char *argv[])
         unit_test_setup_teardown(test_parse_no_attrs,
                                  parse_entry_test_setup,
                                  parse_entry_test_teardown),
+        unit_test_setup_teardown(test_parse_dups,
+                                 parse_entry_test_setup,
+                                 parse_entry_test_teardown),
         /* Negative tests */
         unit_test_setup_teardown(test_parse_no_oc,
                                  parse_entry_test_setup,
-- 
1.9.3

-------------- next part --------------
>From a9b9c187ecefaec051b1400b09d895bf737c74e4 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:23:23 +0200
Subject: [PATCH 4/7] SDAP: Fix DEBUG message priorities in sdap_parse_entry

While I was changing the sdap_parse_entry function, I also realized that
some of the DEBUG messages were converted to the #defines, but their
level was still not accurate. This patch fixes the DEBUG levels and
indentation around them.
---
 src/providers/ldap/sdap.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 133e6dcf31109b69edb56437796e34ebd22e17fc..4c364df73ff16c6d4975fc075a09aaf163576daa 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -315,8 +315,8 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     lerrno = 0;
     ret = ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
     if (ret != LDAP_OPT_SUCCESS) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "ldap_set_option failed [%s], ignored.\n",
-                  sss_ldap_err2string(ret));
+        DEBUG(SSSDBG_MINOR_FAILURE, "ldap_set_option failed [%s], ignored.\n",
+              sss_ldap_err2string(ret));
     }
 
     attrs = sysdb_new_attrs(tmp_ctx);
@@ -329,12 +329,12 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     if (!str) {
         ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
         DEBUG(SSSDBG_CRIT_FAILURE, "ldap_get_dn failed: %d(%s)\n",
-                  lerrno, sss_ldap_err2string(lerrno));
+              lerrno, sss_ldap_err2string(lerrno));
         ret = EIO;
         goto done;
     }
 
-    DEBUG(SSSDBG_TRACE_ALL, "OriginalDN: [%s].\n", str);
+    DEBUG(SSSDBG_TRACE_LIBS, "OriginalDN: [%s].\n", str);
     ret = sysdb_attrs_add_string(attrs, SYSDB_ORIG_DN, str);
     if (ret) goto done;
     if (_dn) {
@@ -366,7 +366,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
         }
         if (!vals[i]) {
             DEBUG(SSSDBG_CRIT_FAILURE, "objectClass not matching: %s\n",
-                      map[0].name);
+                  map[0].name);
             ldap_value_free_len(vals);
             ret = EINVAL;
             goto done;
@@ -378,7 +378,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     if (!str) {
         ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
         DEBUG(lerrno == LDAP_SUCCESS
-              ? SSSDBG_TRACE_INTERNAL
+              ? SSSDBG_TRACE_LIBS
               : SSSDBG_MINOR_FAILURE,
               "Entry has no attributes [%d(%s)]!?\n",
                lerrno, sss_ldap_err2string(lerrno));
@@ -406,7 +406,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
         case EOK:
             break;
         default:
-            DEBUG(SSSDBG_MINOR_FAILURE,
+            DEBUG(SSSDBG_CRIT_FAILURE,
                   "Could not determine if attribute [%s] was ranged\n", str);
             goto done;
         }
@@ -445,12 +445,12 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                 ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
                 if (lerrno != LDAP_SUCCESS) {
                     DEBUG(SSSDBG_CRIT_FAILURE, "LDAP Library error: %d(%s)",
-                              lerrno, sss_ldap_err2string(lerrno));
+                          lerrno, sss_ldap_err2string(lerrno));
                     ret = EIO;
                     goto done;
                 }
 
-                DEBUG(SSSDBG_FUNC_DATA,
+                DEBUG(SSSDBG_TRACE_LIBS,
                       "Attribute [%s] has no values, skipping.\n", str);
 
             } else {
@@ -462,14 +462,14 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                 }
                 for (i = 0; vals[i]; i++) {
                     if (vals[i]->bv_len == 0) {
-                        DEBUG(SSSDBG_MINOR_FAILURE,
+                        DEBUG(SSSDBG_TRACE_LIBS,
                               "Value of attribute [%s] is empty. "
                                "Skipping this value.\n", str);
                         continue;
                     }
                     if (base64) {
-                        v.data = (uint8_t *)sss_base64_encode(attrs,
-                                (uint8_t *)vals[i]->bv_val, vals[i]->bv_len);
+                        v.data = (uint8_t *) sss_base64_encode(attrs,
+                                 (uint8_t *) vals[i]->bv_val, vals[i]->bv_len);
                         if (!v.data) {
                             ret = ENOMEM;
                             goto done;
@@ -517,7 +517,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
     if (lerrno) {
         DEBUG(SSSDBG_CRIT_FAILURE, "LDAP Library error: %d(%s)",
-                  lerrno, sss_ldap_err2string(lerrno));
+              lerrno, sss_ldap_err2string(lerrno));
         ret = EIO;
         goto done;
     }
-- 
1.9.3

-------------- next part --------------
>From 6907ac861ef1c7e17b18b5ebb0b37b0d6eb27d7d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:29:05 +0200
Subject: [PATCH 5/7] LDAP: Remove unused output parameter _dn from
 sdap_parse_entry

No caller directly accessed this parameter. Moreover, it seemed useless
since the same data is available as SYSDB_ORIGINAL_DN in the attributes.
---
 src/providers/ldap/sdap.c       | 14 ++------------
 src/providers/ldap/sdap.h       |  2 +-
 src/providers/ldap/sdap_async.c |  6 +++---
 src/tests/cmocka/test_sdap.c    | 15 +++++++--------
 4 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 4c364df73ff16c6d4975fc075a09aaf163576daa..4bb701187c385933a167f4f85d77f523808ce994 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -293,7 +293,7 @@ int sdap_get_map(TALLOC_CTX *memctx,
 int sdap_parse_entry(TALLOC_CTX *memctx,
                      struct sdap_handle *sh, struct sdap_msg *sm,
                      struct sdap_attr_map *map, int attrs_num,
-                     struct sysdb_attrs **_attrs, char **_dn,
+                     struct sysdb_attrs **_attrs,
                      bool disable_range_retrieval)
 {
     struct sysdb_attrs *attrs;
@@ -307,7 +307,6 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     bool store;
     bool base64;
     char *base_attr;
-    char *dn = NULL;
     uint32_t range_offset;
     TALLOC_CTX *tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
@@ -336,16 +335,8 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
 
     DEBUG(SSSDBG_TRACE_LIBS, "OriginalDN: [%s].\n", str);
     ret = sysdb_attrs_add_string(attrs, SYSDB_ORIG_DN, str);
+    ldap_memfree(str);
     if (ret) goto done;
-    if (_dn) {
-        dn = talloc_strdup(tmp_ctx, str);
-        if (!dn) {
-            ret = ENOMEM;
-            ldap_memfree(str);
-            goto done;
-        }
-    }
-    ldap_memfree(str);
 
     if (map) {
         vals = ldap_get_values_len(sh->ldap, sm->msg, "objectClass");
@@ -523,7 +514,6 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     }
 
     *_attrs = talloc_steal(memctx, attrs);
-    if (_dn) *_dn = talloc_steal(memctx, dn);
     ret = EOK;
 
 done:
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 960054f3621ac3dfb42c6928c8b4870348883eb7..d5366ad8948619c33e0a5fa3878ef1e1900823ef 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -495,7 +495,7 @@ int sdap_get_map(TALLOC_CTX *memctx,
 int sdap_parse_entry(TALLOC_CTX *memctx,
                      struct sdap_handle *sh, struct sdap_msg *sm,
                      struct sdap_attr_map *map, int attrs_num,
-                     struct sysdb_attrs **_attrs, char **_dn,
+                     struct sysdb_attrs **_attrs,
                      bool disable_range_retrieval);
 
 errno_t sdap_parse_deref(TALLOC_CTX *mem_ctx,
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index b4645738b78b7471d3e095fba490f077429432a1..590cbe100bdf5f3d6df8d9d7b356fd1d560cd729 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1543,7 +1543,7 @@ static errno_t sdap_get_generic_parse_entry(struct sdap_handle *sh,
 
     ret = sdap_parse_entry(state, sh, msg,
                            state->map, state->map_num_attrs,
-                           &attrs, NULL, disable_range_rtrvl);
+                           &attrs, disable_range_rtrvl);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
               "sdap_parse_entry failed [%d]: %s\n", ret, strerror(ret));
@@ -1948,7 +1948,7 @@ static errno_t sdap_sd_search_parse_entry(struct sdap_handle *sh,
 
     ret = sdap_parse_entry(state, sh, msg,
                            NULL, 0,
-                           &attrs, NULL, disable_range_rtrvl);
+                           &attrs, disable_range_rtrvl);
     if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE,
               "sdap_parse_entry failed [%d]: %s\n", ret, strerror(ret));
@@ -2201,7 +2201,7 @@ static errno_t sdap_asq_search_parse_entry(struct sdap_handle *sh,
 
         ret = sdap_parse_entry(res[mi], sh, msg,
                                map, num_attrs,
-                               &res[mi]->attrs, NULL, disable_range_rtrvl);
+                               &res[mi]->attrs, disable_range_rtrvl);
         if (ret != EOK) {
             DEBUG(SSSDBG_MINOR_FAILURE,
                   "sdap_parse_entry failed [%d]: %s\n", ret, strerror(ret));
diff --git a/src/tests/cmocka/test_sdap.c b/src/tests/cmocka/test_sdap.c
index 8fdf1a4a21f7f3f436662d6ef2aee94d85be7d91..33cefcc88b3a1c662751c463cc44c19526d77169 100644
--- a/src/tests/cmocka/test_sdap.c
+++ b/src/tests/cmocka/test_sdap.c
@@ -255,7 +255,7 @@ void test_parse_with_map(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_equal(ret, ERR_OK);
 
     assert_int_equal(attrs->num, 4);
@@ -315,8 +315,7 @@ void test_parse_no_map(void **state)
     set_entry_parse(&test_nomap_entry);
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
-                           NULL, 0,
-                           &attrs, NULL, false);
+                           NULL, 0, &attrs, false);
     assert_int_equal(ret, ERR_OK);
 
     assert_int_equal(attrs->num, 3);
@@ -361,7 +360,7 @@ void test_parse_no_attrs(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_equal(ret, ERR_OK);
 
     assert_int_equal(attrs->num, 1);
@@ -408,7 +407,7 @@ void test_parse_dups(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_equal(ret, ERR_OK);
 
     assert_int_equal(attrs->num, 3);
@@ -451,7 +450,7 @@ void test_parse_bad_oc(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_not_equal(ret, ERR_OK);
 
     talloc_free(map);
@@ -484,7 +483,7 @@ void test_parse_no_oc(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_not_equal(ret, ERR_OK);
 
     talloc_free(map);
@@ -519,7 +518,7 @@ void test_parse_no_dn(void **state)
 
     ret = sdap_parse_entry(test_ctx, &test_ctx->sh, &test_ctx->sm,
                            map, SDAP_OPTS_USER,
-                           &attrs, NULL, false);
+                           &attrs, false);
     assert_int_not_equal(ret, ERR_OK);
 
     talloc_free(map);
-- 
1.9.3

-------------- next part --------------
>From fbbccfac1c1f9265f899ac53425950de47c3fee9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:33:32 +0200
Subject: [PATCH 6/7] SDAP: Remove unused function sdap_get_msg_dn

This function was not used since 2009. Unused and untested function
would just rot, better to remove it completely.
---
 src/providers/ldap/sdap.c | 29 -----------------------------
 src/providers/ldap/sdap.h |  3 ---
 2 files changed, 32 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 4bb701187c385933a167f4f85d77f523808ce994..ac90685a74814316ae90235b3b639af7fc366980 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -688,35 +688,6 @@ done:
 
 /* =Get-DN-from-message=================================================== */
 
-int sdap_get_msg_dn(TALLOC_CTX *memctx, struct sdap_handle *sh,
-                    struct sdap_msg *sm, char **_dn)
-{
-    char *str;
-    int lerrno;
-    int ret;
-
-    lerrno = 0;
-    ret = ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
-    if (ret != LDAP_OPT_SUCCESS) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "ldap_set_option failed [%s], ignored.\n",
-                  sss_ldap_err2string(ret));
-    }
-
-    str = ldap_get_dn(sh->ldap, sm->msg);
-    if (!str) {
-        ldap_get_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
-        DEBUG(SSSDBG_CRIT_FAILURE, "ldap_get_dn failed: %d(%s)\n",
-                  lerrno, sss_ldap_err2string(lerrno));
-        return EIO;
-    }
-
-    *_dn = talloc_strdup(memctx, str);
-    ldap_memfree(str);
-    if (!*_dn) return ENOMEM;
-
-    return EOK;
-}
-
 errno_t setup_tls_config(struct dp_option *basic_opts)
 {
     int ret;
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index d5366ad8948619c33e0a5fa3878ef1e1900823ef..87f2131ae6e8eea68e4db81b7de6e70a4c0636a7 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -504,9 +504,6 @@ errno_t sdap_parse_deref(TALLOC_CTX *mem_ctx,
                          LDAPDerefRes *dref,
                          struct sdap_deref_attrs ***_res);
 
-int sdap_get_msg_dn(TALLOC_CTX *memctx, struct sdap_handle *sh,
-                    struct sdap_msg *sm, char **_dn);
-
 errno_t setup_tls_config(struct dp_option *basic_opts);
 
 int sdap_set_rootdse_supported_lists(struct sysdb_attrs *rootdse,
-- 
1.9.3

-------------- next part --------------
>From 854200f2bde1db0737e7d2f5eab2ab9305fdbb6f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 7 Jul 2014 21:55:14 +0200
Subject: [PATCH 7/7] SDAP: Free bervals on failure in sdap_parse_entry

---
 src/providers/ldap/sdap.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index ac90685a74814316ae90235b3b639af7fc366980..6d2fe6254ea706ddbb178060dbb7117aa2cad48d 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -462,6 +462,7 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                         v.data = (uint8_t *) sss_base64_encode(attrs,
                                  (uint8_t *) vals[i]->bv_val, vals[i]->bv_len);
                         if (!v.data) {
+                            ldap_value_free_len(vals);
                             ret = ENOMEM;
                             goto done;
                         }
@@ -486,13 +487,19 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
                                 ret = sysdb_attrs_add_val(attrs,
                                                           map[ai].sys_name,
                                                           &v);
-                                if (ret) goto done;
+                                if (ret) {
+                                    ldap_value_free_len(vals);
+                                    goto done;
+                                }
                             }
                         }
                     } else {
                         /* No map, just store the attribute */
                         ret = sysdb_attrs_add_val(attrs, name, &v);
-                        if (ret) goto done;
+                        if (ret) {
+                            ldap_value_free_len(vals);
+                            goto done;
+                        }
                     }
                 }
                 ldap_value_free_len(vals);
-- 
1.9.3



More information about the sssd-devel mailing list