>From daf86e381d190e70cbd945d46bbdd723dd677d9b Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 8 Oct 2013 20:59:22 +0200 Subject: [PATCH 6/6] AD: Add extended access filter https://fedorahosted.org/sssd/ticket/2082 Adds a new option that allows the admin to specify a LDAP access filter that can be applied globally, per-domain or per-forest. --- Makefile.am | 28 ++- src/man/sssd-ad.5.xml | 41 +++- src/providers/ad/ad_access.c | 213 ++++++++++++++++++- src/providers/ad/ad_init.c | 5 +- src/tests/cmocka/test_ad_access_filter.c | 341 +++++++++++++++++++++++++++++++ 5 files changed, 622 insertions(+), 6 deletions(-) create mode 100644 src/tests/cmocka/test_ad_access_filter.c diff --git a/Makefile.am b/Makefile.am index ff1e71e72d90a6eff658a40e2ca24c1929b31aa5..140b0e804df4971d3d1877d5bd473eb9f36c8fd1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -149,7 +149,8 @@ if HAVE_CMOCKA sss_nss_idmap-tests \ dyndns-tests \ fqnames-tests \ - test_sss_idmap + test_sss_idmap \ + ad_access_filter_tests endif check_PROGRAMS = \ @@ -1371,6 +1372,31 @@ test_sss_idmap_LDADD = \ $(SSSD_INTERNAL_LTLIBS) \ libsss_test_common.la +ad_access_filter_tests_SOURCES = \ + $(TEST_MOCK_OBJ) \ + $(sssd_be_SOURCES) \ + src/util/sss_ldap.c \ + src/util/sss_krb5.c \ + src/util/find_uid.c \ + src/util/user_info_msg.c \ + src/tests/cmocka/test_ad_access_filter.c +ad_access_filter_tests_CFLAGS = \ + $(AM_CFLAGS) \ + $(SYSTEMD_LOGIN_CFLAGS) \ + -DUNIT_TESTING +ad_access_filter_tests_LDADD = \ + $(PAM_LIBS) \ + $(CMOCKA_LIBS) \ + $(SSSD_LIBS) \ + $(CARES_LIBS) \ + $(KRB5_LIBS) \ + $(SSSD_INTERNAL_LTLIBS) \ + $(SYSTEMD_LOGIN_LIBS) \ + libsss_idmap.la \ + libsss_ldap_common.la \ + libsss_krb5_common.la \ + libsss_test_common.la + endif noinst_PROGRAMS = pam_test_client diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml index 41bd3ba0536bcc3038eea6befecf84f882aa5d30..64231cc450ab8b0fb81decfcf27d50ad5e6b9155 100644 --- a/src/man/sssd-ad.5.xml +++ b/src/man/sssd-ad.5.xml @@ -180,11 +180,46 @@ ldap_id_mapping = False for this option to have an effect. - Example: + The option also supports specifying different + filters per domain or forest. This + extended filter would consist of: + KEYWORD:NAME:FILTER. + The keyword can be either DOM, + FOREST or missing. + + + If the keyword equals to DOM + or is missing, then NAME specifies + the domain or subdomain the filter applies to. + If the keyword equals to FOREST, + then the filter equals to all domains from the + forest specified by NAME. + + + Multiple filters can be separated with the + ? character, similarly to how + search bases work. + + + The most specific match is always used. For + example, if the option specified filter + for a domain the user is a member of and a + global filter, the per-domain filter would + be applied. If there are more matches with + the same specification, the first one is used. + + + Examples: -access_provider = ad -ad_access_filter = memberOf=cn=allowedusers,ou=Groups,dc=example,dc=com +# apply filter on domain called dom1 only: +dom1:(memberOf=cn=admins,ou=groups,dc=dom1,dc=com) + +# apply filter on domain called dom2 only: +DOM:dom2:(memberOf=cn=admins,ou=groups,dc=dom2,dc=com) + +# apply filter on forest called EXAMPLE.COM only: +FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com) Default: Not set diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c index 59e2f2b81a3f9d1850d673551be079f42a6a2ac5..68d0200f769297084dfae11bc838716333acdf07 100644 --- a/src/providers/ad/ad_access.c +++ b/src/providers/ad/ad_access.c @@ -33,6 +33,196 @@ ad_access_done(struct tevent_req *req); static errno_t ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn); +/* + * More advanced format can be used to restrict the filter to a specific + * domain or a specific forest. This format is KEYWORD:NAME:FILTER + * + * KEYWORD can be one of DOM or FOREST + * KEYWORD can be missing + * NAME is a label. + * - if KEYWORD equals DOM or missing completely, the filter is applied + * for users from domain named NAME only + * - if KEYWORD equals FOREST, the filter is applied on users from + * forest named NAME only + * examples of valid filters are: + * apply filter on domain called dom1 only: + * dom1:(memberOf=cn=admins,ou=groups,dc=dom1,dc=com) + * apply filter on domain called dom2 only: + * DOM:dom2:(memberOf=cn=admins,ou=groups,dc=dom2,dc=com) + * apply filter on forest called EXAMPLE.COM only: + * FOREST:EXAMPLE.COM:(memberOf=cn=admins,ou=groups,dc=example,dc=com) + * + * If any of the extended formats are used, the filter MUST be enclosed + * already. + */ + +/* From least specific */ +#define AD_FILTER_GENERIC 0x01 +#define AD_FILTER_FOREST 0x02 +#define AD_FILTER_DOMAIN 0x04 + +#define KW_FOREST "FOREST" +#define KW_DOMAIN "DOM" + +/* parse filter in the format domain_name:filter */ +static errno_t +parse_sub_filter(TALLOC_CTX *mem_ctx, const char *full_filter, + char **filter, char **sub_name, int *flags, + const int flagconst) +{ + char *specdelim; + + specdelim = strchr(full_filter, ':'); + if (specdelim == NULL) return EINVAL; + + /* Make sure the filter is already enclosed in brackets */ + if (*(specdelim+1) != '(') return EINVAL; + + *sub_name = talloc_strndup(mem_ctx, full_filter, specdelim - full_filter); + *filter = talloc_strdup(mem_ctx, specdelim+1); + if (*sub_name == NULL || *filter == NULL) return ENOMEM; + + *flags = flagconst; + return EOK; +} + +static inline errno_t +parse_dom_filter(TALLOC_CTX *mem_ctx, const char *dom_filter, + char **filter, char **domname, int *flags) +{ + return parse_sub_filter(mem_ctx, dom_filter, filter, domname, + flags, AD_FILTER_DOMAIN); +} + +static inline errno_t +parse_forest_filter(TALLOC_CTX *mem_ctx, const char *forest_filter, + char **filter, char **forest_name, int *flags) +{ + return parse_sub_filter(mem_ctx, forest_filter, filter, forest_name, + flags, AD_FILTER_FOREST); +} + + +static errno_t +parse_filter(TALLOC_CTX *mem_ctx, const char *full_filter, + char **filter, char **spec, int *flags) +{ + char *kwdelim, *specdelim; + + if (filter == NULL || spec == NULL || flags == NULL) return EINVAL; + + kwdelim = strchr(full_filter, ':'); + if (kwdelim != NULL) { + specdelim = strchr(kwdelim+1, ':'); + + if (specdelim == NULL) { + /* There is a single keyword. Treat it as a domain name */ + return parse_dom_filter(mem_ctx, full_filter, filter, spec, flags); + } else if (strncmp(full_filter, "DOM", kwdelim-full_filter) == 0) { + /* The format must be DOM:domain_name:filter */ + if (specdelim && specdelim-kwdelim <= 1) { + /* Check if there is some domain_name */ + return EINVAL; + } + + return parse_dom_filter(mem_ctx, kwdelim + 1, filter, spec, flags); + } else if (strncmp(full_filter, "FOREST", kwdelim-full_filter) == 0) { + /* The format must be FOREST:forest_name:filter */ + if (specdelim && specdelim-kwdelim <= 1) { + /* Check if there is some domain_name */ + return EINVAL; + } + + return parse_forest_filter(mem_ctx, kwdelim + 1, + filter, spec, flags); + } + + /* Malformed option */ + DEBUG(SSSDBG_CRIT_FAILURE, + ("Keyword in filter [%s] did not match expected format\n", + full_filter)); + return EINVAL; + } + + /* No keyword. Easy. */ + *filter = talloc_strdup(mem_ctx, full_filter); + if (*filter == NULL) return ENOMEM; + + *spec = NULL; + *flags = AD_FILTER_GENERIC; + return EOK; +} + +static errno_t +ad_parse_access_filter(TALLOC_CTX *mem_ctx, + struct sss_domain_info *dom, + const char *filter_list, + char **_filter) +{ + char **filters; + int nfilters; + errno_t ret; + char *best_match; + int best_flags; + char *filter; + char *spec; + int flags; + TALLOC_CTX *tmp_ctx; + + tmp_ctx = talloc_new(mem_ctx); + if (tmp_ctx == NULL) { + ret = ENOMEM; + goto done; + } + + ret = split_on_separator(tmp_ctx, filter_list, '?', true, true, + &filters, &nfilters); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + ("Cannot parse the list of ad_access_filters\n")); + goto done; + } + + best_match = NULL; + best_flags = 0; + for (int i=0; i < nfilters; i++) { + ret = parse_filter(tmp_ctx, filters[i], &filter, &spec, &flags); + if (ret != EOK) { + /* Skip the faulty filter. At worst, the user won't be + * allowed access */ + DEBUG(SSSDBG_MINOR_FAILURE, ("Access filter [%s] could not be " + "parsed, skipping\n", filters[i])); + continue; + } + + if (flags & AD_FILTER_DOMAIN && strcasecmp(spec, dom->name) != 0) { + /* If the filter specifies a domain, it must match the + * domain the user comes from + */ + continue; + } + + if (flags & AD_FILTER_FOREST && strcasecmp(spec, dom->forest) != 0) { + /* If the filter specifies a forest, it must match the + * forest the user comes from + */ + continue; + } + + if (flags > best_flags) { + best_flags = flags; + best_match = filter; + } + } + + ret = EOK; + /* Make sure the result is enclosed in brackets */ + *_filter = sdap_get_access_filter(mem_ctx, best_match); +done: + talloc_free(tmp_ctx); + return ret; +} + struct ad_access_state { struct tevent_context *ev; struct ad_access_ctx *ctx; @@ -40,6 +230,7 @@ struct ad_access_state { struct be_ctx *be_ctx; struct sss_domain_info *domain; + char *filter; struct sdap_id_conn_ctx **clist; int cindex; }; @@ -67,6 +258,14 @@ ad_access_send(TALLOC_CTX *mem_ctx, state->be_ctx = be_ctx; state->domain = domain; + ret = ad_parse_access_filter(state, domain, ctx->sdap_access_ctx->filter, + &state->filter); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, ("Could not determine the best filter\n")); + ret = ERR_ACCESS_DENIED; + goto done; + } + state->clist = talloc_zero_array(state, struct sdap_id_conn_ctx *, 3); if (state->clist == NULL) { ret = ENOMEM; @@ -102,13 +301,25 @@ ad_access_step(struct tevent_req *req, struct sdap_id_conn_ctx *conn) { struct tevent_req *subreq; struct ad_access_state *state; + struct sdap_access_ctx *req_ctx; state = tevent_req_data(req, struct ad_access_state); + req_ctx = talloc(state, struct sdap_access_ctx); + if (req_ctx == NULL) { + return ENOMEM; + } + req_ctx->id_ctx = state->ctx->sdap_access_ctx->id_ctx; + req_ctx->filter = state->filter; + memcpy(&req_ctx->access_rule, + state->ctx->sdap_access_ctx->access_rule, + sizeof(int) * LDAP_ACCESS_LAST); + subreq = sdap_access_send(req, state->ev, state->be_ctx, - state->domain, state->ctx->sdap_access_ctx, + state->domain, req_ctx, conn, state->pd); if (req == NULL) { + talloc_free(req_ctx); return ENOMEM; } tevent_req_set_callback(subreq, ad_access_done, req); diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c index d7f41a563442332743b21f1e1cebdd0ee4abf5eb..d06efbd082bd6bba74fb6616c7dd722c99244988 100644 --- a/src/providers/ad/ad_init.c +++ b/src/providers/ad/ad_init.c @@ -402,7 +402,10 @@ sssm_ad_access_init(struct be_ctx *bectx, access_ctx->sdap_access_ctx->access_rule[0] = LDAP_ACCESS_EXPIRE; filter = dp_opt_get_cstring(access_ctx->ad_options, AD_ACCESS_FILTER); if (filter != NULL) { - access_ctx->sdap_access_ctx->filter = sdap_get_access_filter( + /* The processing of the extended filter is performed during the access + * check itself + */ + access_ctx->sdap_access_ctx->filter = talloc_strdup( access_ctx->sdap_access_ctx, filter); if (access_ctx->sdap_access_ctx->filter == NULL) { diff --git a/src/tests/cmocka/test_ad_access_filter.c b/src/tests/cmocka/test_ad_access_filter.c new file mode 100644 index 0000000000000000000000000000000000000000..e31f2dc37d7f78b0fa871839a57bd62a09c53944 --- /dev/null +++ b/src/tests/cmocka/test_ad_access_filter.c @@ -0,0 +1,341 @@ +/* + Authors: + Jakub Hrozek + + Copyright (C) 2013 Red Hat + + SSSD tests: AD access control filter tests + + 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 +#include +#include + +/* In order to access opaque types */ +#include "providers/ad/ad_access.c" + +#include "tests/cmocka/common_mock.h" + +#define DOM_NAME "parent_dom" + +struct ad_access_test_ctx { + struct sss_domain_info *dom; +}; + +static struct ad_access_test_ctx *test_ctx; + +void ad_access_filter_test_setup(void **state) +{ + assert_true(leak_check_setup()); + test_ctx = talloc_zero(global_talloc_context, + struct ad_access_test_ctx); + assert_non_null(test_ctx); + + test_ctx->dom = talloc_zero(test_ctx, struct sss_domain_info); + assert_non_null(test_ctx->dom); + + test_ctx->dom->name = talloc_strdup(test_ctx->dom, DOM_NAME); + assert_non_null(test_ctx->dom->name); +} + +void ad_access_filter_test_teardown(void **state) +{ + talloc_free(test_ctx); + assert_true(leak_check_teardown()); +} + +struct filter_parse_result { + const int result; + const char *best_match; +}; + +static void test_parse_filter_generic(const char *filter_in, + struct filter_parse_result *expected) +{ + errno_t ret; + TALLOC_CTX *tmp_ctx; + char *best_match; + + assert_non_null(expected); + + tmp_ctx = talloc_new(global_talloc_context); + assert_non_null(tmp_ctx); + check_leaks_push(tmp_ctx); + + ret = ad_parse_access_filter(tmp_ctx, test_ctx->dom, filter_in, + &best_match); + assert_int_equal(ret, expected->result); + if (expected->result != EOK) { + goto done; + } + + if (expected->best_match != NULL) { + assert_string_equal(best_match, expected->best_match); + } else { + assert_true(best_match == NULL); + } + talloc_free(best_match); + +done: + assert_true(check_leaks_pop(tmp_ctx) == true); + talloc_free(tmp_ctx); +} + +/* Test that if one filter is provided, it is returned as-is + */ +void test_single_filter(void **state) +{ + struct filter_parse_result expected = { + .result = EOK, + .best_match = "(name=foo)" + }; + + test_parse_filter_generic("name=foo", &expected); + test_parse_filter_generic("(name=foo)", &expected); + test_parse_filter_generic(DOM_NAME":(name=foo)", &expected); + test_parse_filter_generic("DOM:"DOM_NAME":(name=foo)", &expected); +} + +/* Test that if more filters are provided, the best match is returned */ +void test_filter_order(void **state) +{ + struct filter_parse_result expected = { + .result = EOK, + .best_match = "(name=foo)" + }; + + test_parse_filter_generic("name=foo?name=bar", &expected); + test_parse_filter_generic(DOM_NAME":(name=foo)?name=bar", &expected); + test_parse_filter_generic("name=bla?"DOM_NAME":(name=foo)?name=bar", &expected); + /* Test that another foreign domain wouldn't match */ + test_parse_filter_generic("anotherdom:(name=bla)?"DOM_NAME":(name=foo)", &expected); + test_parse_filter_generic("anotherdom:(name=bla)?(name=foo)", &expected); +} + +void test_filter_no_match(void **state) +{ + struct filter_parse_result expected = { + .result = EOK, + .best_match = NULL + }; + + test_parse_filter_generic("anotherdom:(name=bla)?yetanother:(name=foo)", &expected); +} + + +void parse_test_setup(void **state) +{ + assert_true(leak_check_setup()); +} + +void parse_test_teardown(void **state) +{ + assert_true(leak_check_teardown()); +} + +struct parse_result { + const int result; + const char *filter; + const char *spec; + const int flags; +}; + +static void test_parse_generic(const char *filter_in, struct parse_result *expected) +{ + errno_t ret; + TALLOC_CTX *tmp_ctx; + char *filter; + char *spec; + int flags; + + assert_non_null(expected); + + tmp_ctx = talloc_new(global_talloc_context); + assert_non_null(tmp_ctx); + check_leaks_push(tmp_ctx); + + ret = parse_filter(tmp_ctx, filter_in, &filter, &spec, &flags); + + assert_int_equal(ret, expected->result); + if (expected->result != EOK) { + goto done; + } + + if (expected->filter != NULL) { + assert_string_equal(filter, expected->filter); + } else { + assert_true(filter == NULL); + } + talloc_free(filter); + + if (expected->spec != NULL) { + assert_string_equal(spec, expected->spec); + } else { + assert_true(spec == NULL); + } + talloc_free(spec); + + assert_int_equal(flags, expected->flags); + +done: + assert_true(check_leaks_pop(tmp_ctx) == true); + talloc_free(tmp_ctx); +} + +void test_parse_plain(void **state) +{ + struct parse_result expected = { + .result = EOK, + .filter = "name=foo", + .spec = NULL, + .flags = AD_FILTER_GENERIC + }; + + test_parse_generic("name=foo", &expected); +} + +void test_parse_dom_without_kw(void **state) +{ + struct parse_result expected = { + .result = EOK, + .filter = "(name=foo)", + .spec = "mydom", + .flags = AD_FILTER_DOMAIN + }; + + test_parse_generic("mydom:(name=foo)", &expected); + + /* Check we can handle domain called DOM */ + struct parse_result expected2 = { + .result = EOK, + .filter = "(name=foo)", + .spec = "DOM", + .flags = AD_FILTER_DOMAIN + }; + + test_parse_generic("DOM:(name=foo)", &expected2); +} + +void test_parse_dom_kw(void **state) +{ + struct parse_result expected = { + .result = EOK, + .filter = "(name=foo)", + .spec = "mydom", + .flags = AD_FILTER_DOMAIN + }; + + test_parse_generic("DOM:mydom:(name=foo)", &expected); +} + +void test_parse_forest_kw(void **state) +{ + struct parse_result expected = { + .result = EOK, + .filter = "(name=foo)", + .spec = "myforest", + .flags = AD_FILTER_FOREST + }; + + test_parse_generic("FOREST:myforest:(name=foo)", &expected); +} + + +void test_parse_malformed(void **state) +{ + struct parse_result expected = { + .result = EINVAL, + }; + + test_parse_generic("DOM:", &expected); + test_parse_generic("DOM::", &expected); + test_parse_generic("DOM:mydom:", &expected); + test_parse_generic("DOM:mydom:name=foo", &expected); + test_parse_generic("DOM::(name=foo)", &expected); + test_parse_generic("BLABLABLA:mydom:name=foo", &expected); +} + +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_plain, + parse_test_setup, + parse_test_teardown), + + unit_test_setup_teardown(test_parse_dom_without_kw, + parse_test_setup, + parse_test_teardown), + + unit_test_setup_teardown(test_parse_dom_kw, + parse_test_setup, + parse_test_teardown), + + unit_test_setup_teardown(test_parse_forest_kw, + parse_test_setup, + parse_test_teardown), + + unit_test_setup_teardown(test_parse_malformed, + parse_test_setup, + parse_test_teardown), + + unit_test_setup_teardown(test_single_filter, + ad_access_filter_test_setup, + ad_access_filter_test_teardown), + + unit_test_setup_teardown(test_filter_order, + ad_access_filter_test_setup, + ad_access_filter_test_teardown), + + unit_test_setup_teardown(test_filter_no_match, + ad_access_filter_test_setup, + ad_access_filter_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); + + tests_set_cwd(); + + return run_tests(tests); +} -- 1.8.3.1