From c610f72c5b8b28067e24cb72319603e7d906a6d8 Mon Sep 17 00:00:00 2001 From: Sumit Bose 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 , Sumit Bose + + 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 . +*/ + + +#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 #include +#include #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