[SSSD] [PATCH] Fix LDAP search paths and other fixes for IPA

Sumit Bose sbose at redhat.com
Tue Mar 16 10:55:23 UTC 2010


Hi,

the LDAP search paths in the IPA access modules currently will only work
with single word domain names. This patch fixes this and adds some
tests.

bye,
Sumit
-------------- next part --------------
From c610f72c5b8b28067e24cb72319603e7d906a6d8 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 15 Mar 2010 20:58:06 +0100
Subject: [PATCH] Fix LDAP search paths and other fixes for IPA

- use domain_to_basedn() to construct LDAP search paths for IPA HBAC
- fix the number of Kerberos options and add a test
- add a unit test for domain_to_basedn() and move domain_to_basedn() to
  a seperate file to simplify the build of the test
---
 src/Makefile.am                |    3 ++
 src/providers/ipa/ipa_access.c |   35 ++++++++++++++----------
 src/providers/ipa/ipa_access.h |    1 +
 src/providers/ipa/ipa_common.c |   32 ----------------------
 src/providers/ipa/ipa_common.h |    7 +++-
 src/providers/ipa/ipa_utils.c  |   57 ++++++++++++++++++++++++++++++++++++++++
 src/tests/ipa_ldap_opt-tests.c |   55 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 141 insertions(+), 49 deletions(-)
 create mode 100644 src/providers/ipa/ipa_utils.c

diff --git a/src/Makefile.am b/src/Makefile.am
index 6d46cda..3a156cb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -631,12 +631,14 @@ auth_tests_LDADD = \
     libsss_test_common.la
 
 ipa_ldap_opt_tests_SOURCES = \
+    providers/ipa/ipa_utils.c \
     tests/ipa_ldap_opt-tests.c
 ipa_ldap_opt_tests_CFLAGS = \
     $(AM_CFLAGS) \
     $(CHECK_CFLAGS)
 ipa_ldap_opt_tests_LDADD = \
     $(CHECK_LIBS) \
+    $(TALLOC_LIBS) \
     libsss_test_common.la
 
 simple_access_tests_SOURCES = \
@@ -769,6 +771,7 @@ libsss_ipa_la_SOURCES = \
     providers/child_common.c \
     providers/ipa/ipa_init.c \
     providers/ipa/ipa_common.c \
+    providers/ipa/ipa_utils.c \
     providers/ipa/ipa_auth.c \
     providers/ipa/ipa_access.c \
     providers/ipa/ipa_timerules.c \
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 7dfe1fd..55a7133 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -48,8 +48,8 @@
 #define IPA_HOST_CATEGORY "hostCategory"
 #define IPA_CN "cn"
 
-#define IPA_HOST_BASE_TMPL "cn=computers,cn=accounts,dc=%s"
-#define IPA_HBAC_BASE_TMPL "cn=hbac,dc=%s"
+#define IPA_HOST_BASE_TMPL "cn=computers,cn=accounts,%s"
+#define IPA_HBAC_BASE_TMPL "cn=hbac,%s"
 
 #define SYSDB_HBAC_BASE_TMPL "cn=hbac,"SYSDB_TMPL_CUSTOM_BASE
 
@@ -340,7 +340,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
                                                   bool offline,
                                                   struct sdap_id_ctx *sdap_ctx,
                                                   struct sysdb_ctx *sysdb,
-                                                  const char *ipa_domain,
+                                                  const char *basedn,
                                                   const char **hostnames)
 {
     struct tevent_req *req = NULL;
@@ -349,7 +349,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
     int ret;
     int i;
 
-    if (hostnames == NULL || ipa_domain == NULL) {
+    if (hostnames == NULL || basedn == NULL) {
         DEBUG(1, ("Missing hostnames or domain.\n"));
         return NULL;
     }
@@ -394,7 +394,7 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
     }
 
     state->host_search_base = talloc_asprintf(state, IPA_HOST_BASE_TMPL,
-                                              ipa_domain);
+                                              basedn);
     if (state->host_search_base == NULL) {
         DEBUG(1, ("Failed to create host search base.\n"));
         ret = ENOMEM;
@@ -832,7 +832,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
                                              bool offline,
                                              struct sdap_id_ctx *sdap_ctx,
                                              struct sysdb_ctx *sysdb,
-                                             const char *ipa_domain,
+                                             const char *basedn,
                                              const char *host_dn,
                                              const char **memberof)
 {
@@ -842,7 +842,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
     int ret;
     int i;
 
-    if (host_dn == NULL || ipa_domain == NULL) {
+    if (host_dn == NULL || basedn == NULL) {
         DEBUG(1, ("Missing host_dn or domain.\n"));
         return NULL;
     }
@@ -867,7 +867,7 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
     state->current_item = 0;
 
     state->hbac_search_base = talloc_asprintf(state, IPA_HBAC_BASE_TMPL,
-                                              ipa_domain);
+                                              basedn);
     if (state->hbac_search_base == NULL) {
         DEBUG(1, ("Failed to create HBAC search base.\n"));
         ret = ENOMEM;
@@ -1642,6 +1642,7 @@ void ipa_access_handler(struct be_req *be_req)
     int pam_status = PAM_SYSTEM_ERR;
     struct ipa_access_ctx *ipa_access_ctx;
     const char *hostlist[3];
+    int ret;
 
     pd = talloc_get_type(be_req->req_data, struct pam_data);
 
@@ -1658,6 +1659,13 @@ void ipa_access_handler(struct be_req *be_req)
     hbac_ctx->sdap_ctx = ipa_access_ctx->sdap_ctx;
     hbac_ctx->ipa_options = ipa_access_ctx->ipa_options;
     hbac_ctx->tr_ctx = ipa_access_ctx->tr_ctx;
+    ret = domain_to_basedn(hbac_ctx,
+                           dp_opt_get_string(hbac_ctx->ipa_options, IPA_DOMAIN),
+                           &hbac_ctx->ldap_basedn);
+    if (ret != EOK) {
+        DEBUG(1, ("domain_to_basedn failed.\n"));
+        goto fail;
+    }
     hbac_ctx->offline = be_is_offline(be_req->be_ctx);
 
     DEBUG(9, ("Connection status is [%s].\n", hbac_ctx->offline ? "offline" :
@@ -1682,10 +1690,8 @@ void ipa_access_handler(struct be_req *be_req)
     hostlist[2] = NULL;
 
     req = hbac_get_host_info_send(hbac_ctx, be_req->be_ctx->ev,
-                                  hbac_ctx->offline,
-                                  hbac_ctx->sdap_ctx, be_req->be_ctx->sysdb,
-                                  dp_opt_get_string(hbac_ctx->ipa_options,
-                                                    IPA_DOMAIN),
+                                  hbac_ctx->offline, hbac_ctx->sdap_ctx,
+                                  be_req->be_ctx->sysdb, hbac_ctx->ldap_basedn,
                                   hostlist);
     if (req == NULL) {
         DEBUG(1, ("hbac_get_host_info_send failed.\n"));
@@ -1744,9 +1750,8 @@ static void hbac_get_host_info_done(struct tevent_req *req)
     }
     req = hbac_get_rules_send(hbac_ctx, be_req->be_ctx->ev, hbac_ctx->offline,
                               hbac_ctx->sdap_ctx, be_req->be_ctx->sysdb,
-                              dp_opt_get_string(hbac_ctx->ipa_options,
-                                                IPA_DOMAIN),
-                              local_hhi->dn, local_hhi->memberof);
+                              hbac_ctx->ldap_basedn, local_hhi->dn,
+                              local_hhi->memberof);
     if (req == NULL) {
         DEBUG(1, ("hbac_get_rules_send failed.\n"));
         goto fail;
diff --git a/src/providers/ipa/ipa_access.h b/src/providers/ipa/ipa_access.h
index bd221c5..e3ba5c3 100644
--- a/src/providers/ipa/ipa_access.h
+++ b/src/providers/ipa/ipa_access.h
@@ -59,6 +59,7 @@ struct hbac_ctx {
     size_t groups_count;
     const char **groups;
     bool offline;
+    char *ldap_basedn;
 };
 
 void ipa_access_handler(struct be_req *be_req);
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index 1fc881f..f6d7a87 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -122,33 +122,6 @@ struct dp_option ipa_def_krb5_opts[] = {
     { "krb5_validate", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }
 };
 
-int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn)
-{
-    const char *s;
-    char *dn;
-    char *p;
-    int l;
-
-    s = domain;
-    dn = talloc_strdup(memctx, "dc=");
-
-    while ((p = strchr(s, '.'))) {
-        l = p - s;
-        dn = talloc_asprintf_append_buffer(dn, "%.*s,dc=", l, s);
-        if (!dn) {
-            return ENOMEM;
-        }
-        s = p + 1;
-    }
-    dn = talloc_strdup_append_buffer(dn, s);
-    if (!dn) {
-        return ENOMEM;
-    }
-
-    *basedn = dn;
-    return EOK;
-}
-
 int ipa_get_options(TALLOC_CTX *memctx,
                     struct confdb_ctx *cdb,
                     const char *conf_path,
@@ -388,11 +361,6 @@ done:
     return ret;
 }
 
-/* the following define is used to keep track of * the options in the krb5
- * module, so that if they change and ipa is not updated correspondingly
- * this will trigger a runtime abort error */
-#define IPA_KRB5_OPTS_TEST 8
-
 int ipa_get_auth_options(struct ipa_options *ipa_opts,
                          struct confdb_ctx *cdb,
                          const char *conf_path,
diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 75be55e..9818906 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -32,10 +32,11 @@ struct ipa_service {
     struct krb5_service *krb5_service;
 };
 
-/* the following define is used to keep track of the options in the ldap
- * module, so that if they change and ipa is not updated correspondingly
+/* the following defines are used to keep track of the options in the ldap and
+ * krb5 modules, so that if they change and ipa is not updated correspondingly
  * this will trigger a runtime abort error */
 #define IPA_OPTS_BASIC_TEST 32
+#define IPA_KRB5_OPTS_TEST 9
 
 enum ipa_basic_opt {
     IPA_DOMAIN = 0,
@@ -59,6 +60,8 @@ struct ipa_options {
     struct krb5_ctx *auth_ctx;
 };
 
+int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn);
+
 /* options parsers */
 int ipa_get_options(TALLOC_CTX *memctx,
                     struct confdb_ctx *cdb,
diff --git a/src/providers/ipa/ipa_utils.c b/src/providers/ipa/ipa_utils.c
new file mode 100644
index 0000000..504a877
--- /dev/null
+++ b/src/providers/ipa/ipa_utils.c
@@ -0,0 +1,57 @@
+/*
+    SSSD
+
+    IPA Provider Utility Functions
+
+    Authors:
+        Simo Sorce <ssorce at redhat.com>, Sumit Bose <sbose at redhat.com>
+
+    Copyright (C) 2009-2010 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 "providers/ipa/ipa_common.h"
+
+int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn)
+{
+    const char *s;
+    char *dn;
+    char *p;
+    int l;
+
+    if (!domain || !basedn) {
+        return EINVAL;
+    }
+
+    s = domain;
+    dn = talloc_strdup(memctx, "dc=");
+
+    while ((p = strchr(s, '.'))) {
+        l = p - s;
+        dn = talloc_asprintf_append_buffer(dn, "%.*s,dc=", l, s);
+        if (!dn) {
+            return ENOMEM;
+        }
+        s = p + 1;
+    }
+    dn = talloc_strdup_append_buffer(dn, s);
+    if (!dn) {
+        return ENOMEM;
+    }
+
+    *basedn = dn;
+    return EOK;
+}
diff --git a/src/tests/ipa_ldap_opt-tests.c b/src/tests/ipa_ldap_opt-tests.c
index 4b4667a..50c129d 100644
--- a/src/tests/ipa_ldap_opt-tests.c
+++ b/src/tests/ipa_ldap_opt-tests.c
@@ -24,17 +24,67 @@
 
 #include <check.h>
 #include <stdlib.h>
+#include <talloc.h>
 
 #include "providers/ipa/ipa_common.h"
 #include "providers/ldap/sdap.h"
+#include "providers/krb5/krb5_common.h"
 #include "tests/common.h"
 
+struct test_domain {
+    const char *domain;
+    const char *basedn;
+};
+
+struct test_domain test_domains[] = {
+    { "abc", "dc=abc"},
+    { "a.b.c", "dc=a,dc=b,dc=c"},
+    { NULL, NULL}
+};
+
+START_TEST(test_domain_to_basedn)
+{
+    int ret;
+    int i;
+    TALLOC_CTX *tmp_ctx;
+    char *basedn;
+
+    tmp_ctx = talloc_new(NULL);
+    fail_unless(tmp_ctx != NULL, "talloc_new failed");
+
+    ret = domain_to_basedn(tmp_ctx, NULL, &basedn);
+    fail_unless(ret == EINVAL,
+                "domain_to_basedn does not fail with EINVAL if domain is NULL");
+
+    ret = domain_to_basedn(tmp_ctx, "abc", NULL);
+    fail_unless(ret == EINVAL,
+                "domain_to_basedn does not fail with EINVAL if basedn is NULL");
+
+    for(i=0; test_domains[i].domain != NULL; i++) {
+        ret = domain_to_basedn(tmp_ctx, test_domains[i].domain, &basedn);
+        fail_unless(ret == EOK, "domain_to_basedn failed");
+        fail_unless(strcmp(basedn, test_domains[i].basedn) == 0,
+                    "domain_to_basedn returned wrong basedn, "
+                    "get [%s], expected [%s]", basedn, test_domains[i].basedn);
+        talloc_free(basedn);
+    }
+
+    talloc_free(tmp_ctx);
+}
+END_TEST
+
 START_TEST(test_check_num_opts)
 {
     fail_if(IPA_OPTS_BASIC_TEST != SDAP_OPTS_BASIC);
 }
 END_TEST
 
+START_TEST(test_check_krb5_num_opts)
+{
+    fail_if(KRB5_OPTS != IPA_KRB5_OPTS_TEST);
+}
+END_TEST
+
 Suite *ipa_ldap_opt_suite (void)
 {
     Suite *s = suite_create ("ipa_ldap_opt");
@@ -42,8 +92,13 @@ Suite *ipa_ldap_opt_suite (void)
     TCase *tc_ipa_ldap_opt = tcase_create ("ipa_ldap_opt");
 
     tcase_add_test (tc_ipa_ldap_opt, test_check_num_opts);
+    tcase_add_test (tc_ipa_ldap_opt, test_check_krb5_num_opts);
     suite_add_tcase (s, tc_ipa_ldap_opt);
 
+    TCase *tc_ipa_utils = tcase_create ("ipa_utils");
+    tcase_add_test (tc_ipa_utils, test_domain_to_basedn);
+    suite_add_tcase (s, tc_ipa_utils);
+
     return s;
 }
 
-- 
1.6.6.1



More information about the sssd-devel mailing list