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

Sumit Bose sbose at redhat.com
Thu Mar 25 15:57:28 UTC 2010


On Tue, Mar 16, 2010 at 11:55:23AM +0100, Sumit Bose wrote:
> 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

Hi,

this is a new version of the patch where
- the fixes to the option counter and the test are removed, they are
  addressed by 'Regression test against RHBZ #576856' and 'Add
  krb5_kpasswd to IPA provider'
- the change to code and the test are split into two patches

bye,
Sumit
-------------- next part --------------
>From f1d3b45cb82f8a80a5a8b22e644afcbdb8141097 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 25 Mar 2010 16:21:12 +0100
Subject: [PATCH 1/2] Fix LDAP search paths for IPA HBAC

- use domain_to_basedn() to construct LDAP search paths for IPA HBAC
- move domain_to_basedn() to a separate file to simplify the build of
  a 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 |   27 -------------------
 src/providers/ipa/ipa_common.h |    4 ++-
 src/providers/ipa/ipa_utils.c  |   57 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 84 insertions(+), 43 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 2ad63d0..bed0666 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -123,33 +123,6 @@ struct dp_option ipa_def_krb5_opts[] = {
     { "krb5_kpasswd", DP_OPT_STRING, NULL_STRING, NULL_STRING }
 };
 
-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,
diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 54da6c8..7762818 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -32,7 +32,7 @@ struct ipa_service {
     struct krb5_service *krb5_service;
 };
 
-/* the following define is used to keep track of the options in the ldap
+/* the following defines are used to keep track of the options in the ldap
  * module, so that if they change and ipa is not updated correspondingly
  * this will trigger a runtime abort error */
 #define IPA_OPTS_BASIC_TEST 32
@@ -64,6 +64,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;
+}
-- 
1.6.6.1

-------------- next part --------------
>From 80389f60ae529b1c3c19892943096be9801f29cf Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 25 Mar 2010 16:24:25 +0100
Subject: [PATCH 2/2] Add a test for domain_to_basedn()

---
 src/tests/ipa_ldap_opt-tests.c |   47 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/src/tests/ipa_ldap_opt-tests.c b/src/tests/ipa_ldap_opt-tests.c
index e9a9ea3..f0c0d40 100644
--- a/src/tests/ipa_ldap_opt-tests.c
+++ b/src/tests/ipa_ldap_opt-tests.c
@@ -24,12 +24,55 @@
 
 #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);
@@ -46,6 +89,10 @@ Suite *ipa_ldap_opt_suite (void)
     tcase_add_test (tc_ipa_ldap_opt, test_check_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