From f2e70ec742cd7aab82b74d7e4b424ba3258da7aa Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Wed, 6 Sep 2017 16:42:20 +0200 Subject: IPA: fix handling of certmap_ctx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch fixes a use-after-free in the AD provider part and initializes the certmap_ctx with data from the cache at startup. Related to https://pagure.io/SSSD/sssd/issue/3508 Reviewed-by: Lukáš Slebodník Reviewed-by: Jakub Hrozek --- src/providers/ipa/ipa_init.c | 7 + src/providers/ipa/ipa_subdomains.c | 53 +------ src/providers/ipa/ipa_subdomains_server.c | 4 +- src/providers/ldap/ldap_common.h | 5 + src/providers/ldap/ldap_id.c | 5 +- src/providers/ldap/sdap.h | 4 +- src/providers/ldap/sdap_certmap.c | 152 +++++++++++++++++++ src/tests/cmocka/test_sdap_certmap.c | 244 ++++++++++++++++++++++++++++++ 8 files changed, 421 insertions(+), 53 deletions(-) create mode 100644 src/providers/ldap/sdap_certmap.c create mode 100644 src/tests/cmocka/test_sdap_certmap.c (limited to 'src') diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c index d291ca654..46ff87f53 100644 --- a/src/providers/ipa/ipa_init.c +++ b/src/providers/ipa/ipa_init.c @@ -680,6 +680,13 @@ static errno_t ipa_init_misc(struct be_ctx *be_ctx, return ENOMEM; } + ret = sdap_init_certmap(sdap_id_ctx, sdap_id_ctx); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to initialized certificate mapping.\n"); + return ret; + } + return EOK; } diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index 8a4657bc0..3d3341a3e 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -311,25 +311,6 @@ struct priv_sss_debug { int level; }; -void ext_debug(void *private, const char *file, long line, const char *function, - const char *format, ...) -{ - va_list ap; - struct priv_sss_debug *data = private; - int level = SSSDBG_OP_FAILURE; - - if (data != NULL) { - level = data->level; - } - - if (DEBUG_IS_SET(level)) { - va_start(ap, format); - sss_vdebug_fn(file, line, function, level, APPEND_LINE_FEED, - format, ap); - va_end(ap); - } -} - static errno_t ipa_certmap_parse_results(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, struct sdap_options *sdap_opts, @@ -344,7 +325,6 @@ static errno_t ipa_certmap_parse_results(TALLOC_CTX *mem_ctx, size_t c; size_t lc = 0; int ret; - struct sss_certmap_ctx *certmap_ctx = NULL; const char **ocs = NULL; bool user_name_hint = false; @@ -444,50 +424,29 @@ static errno_t ipa_certmap_parse_results(TALLOC_CTX *mem_ctx, certmap_list[lc] = NULL; - ret = sss_certmap_init(mem_ctx, ext_debug, NULL, &certmap_ctx); - if (ret != 0) { - DEBUG(SSSDBG_OP_FAILURE, "sss_certmap_init failed.\n"); + ret = sdap_setup_certmap(sdap_opts->sdap_certmap_ctx, certmap_list); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sdap_setup_certmap failed.\n"); goto done; } - for (c = 0; certmap_list[c] != NULL; c++) { - DEBUG(SSSDBG_TRACE_ALL, "Trying to add rule [%s][%d][%s][%s].\n", - certmap_list[c]->name, - certmap_list[c]->priority, - certmap_list[c]->match_rule, - certmap_list[c]->map_rule); - - ret = sss_certmap_add_rule(certmap_ctx, certmap_list[c]->priority, - certmap_list[c]->match_rule, - certmap_list[c]->map_rule, - certmap_list[c]->domains); - if (ret != 0) { - DEBUG(SSSDBG_CRIT_FAILURE, - "sss_certmap_add_rule failed for rule [%s], skipping. " - "Please check for typos and if rule syntax is supported.\n", - certmap_list[c]->name); - goto done; - } - } - ret = sysdb_update_certmap(domain->sysdb, certmap_list, user_name_hint); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_update_certmap failed"); goto done; } - sss_certmap_free_ctx(sdap_opts->certmap_ctx); - sdap_opts->certmap_ctx = talloc_steal(sdap_opts, certmap_ctx); - if (_certmap_list != NULL) { *_certmap_list = certmap_list; + } else { + talloc_free(certmap_list); } + ret = EOK; done: talloc_free(ocs); if (ret != EOK) { - sss_certmap_free_ctx(certmap_ctx); talloc_free(certmap_list); } diff --git a/src/providers/ipa/ipa_subdomains_server.c b/src/providers/ipa/ipa_subdomains_server.c index c908d9cfd..10166d162 100644 --- a/src/providers/ipa/ipa_subdomains_server.c +++ b/src/providers/ipa/ipa_subdomains_server.c @@ -361,8 +361,8 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx, id_ctx->sdap_id_ctx->opts->idmap_ctx; /* Set up the certificate mapping context */ - ad_id_ctx->sdap_id_ctx->opts->certmap_ctx = - id_ctx->sdap_id_ctx->opts->certmap_ctx; + ad_id_ctx->sdap_id_ctx->opts->sdap_certmap_ctx = + id_ctx->sdap_id_ctx->opts->sdap_certmap_ctx; *_ad_id_ctx = ad_id_ctx; return EOK; diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 1acda4147..0510b7d5a 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -362,4 +362,9 @@ sdap_id_ctx_new(TALLOC_CTX *mem_ctx, struct be_ctx *bectx, errno_t sdap_refresh_init(struct be_refresh_ctx *refresh_ctx, struct sdap_id_ctx *id_ctx); +errno_t sdap_init_certmap(TALLOC_CTX *mem_ctx, struct sdap_id_ctx *id_ctx); + +errno_t sdap_setup_certmap(struct sdap_certmap_ctx *sdap_certmap_ctx, + struct certmap_info **certmap_list); +struct sss_certmap_ctx *sdap_get_sss_certmap(struct sdap_certmap_ctx *ctx); #endif /* _LDAP_COMMON_H_ */ diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index 557712e8d..93204d35e 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -252,9 +252,8 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx, } ret = sss_cert_derb64_to_ldap_filter(state, filter_value, attr_name, - ctx->opts->certmap_ctx, - state->domain, - &user_filter); + sdap_get_sss_certmap(ctx->opts->sdap_certmap_ctx), + state->domain, &user_filter); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sss_cert_derb64_to_ldap_filter failed.\n"); diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index c1a156764..a128915eb 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -446,6 +446,8 @@ struct sdap_ext_member_ctx { ext_member_recv_fn_t ext_member_resolve_recv; }; +struct sdap_certmap_ctx; + struct sdap_options { struct dp_option *basic; struct sdap_attr_map *gen_map; @@ -481,7 +483,7 @@ struct sdap_options { enum dc_functional_level dc_functional_level; /* Certificate mapping support */ - struct sss_certmap_ctx *certmap_ctx; + struct sdap_certmap_ctx *sdap_certmap_ctx; }; struct sdap_server_opts { diff --git a/src/providers/ldap/sdap_certmap.c b/src/providers/ldap/sdap_certmap.c new file mode 100644 index 000000000..fcf88a9c6 --- /dev/null +++ b/src/providers/ldap/sdap_certmap.c @@ -0,0 +1,152 @@ + +/* + SSSD + + Authors: + Sumit Bose + + Copyright (C) 2017 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 "util/util.h" +#include "lib/certmap/sss_certmap.h" +#include "providers/ldap/ldap_common.h" + +struct sdap_certmap_ctx { + struct sss_certmap_ctx *certmap_ctx; +}; + +struct priv_sss_debug { + int level; +}; + +static void ext_debug(void *private, const char *file, long line, + const char *function, const char *format, ...) +{ + va_list ap; + struct priv_sss_debug *data = private; + int level = SSSDBG_OP_FAILURE; + + if (data != NULL) { + level = data->level; + } + + if (DEBUG_IS_SET(level)) { + va_start(ap, format); + sss_vdebug_fn(file, line, function, level, APPEND_LINE_FEED, + format, ap); + va_end(ap); + } +} + +struct sss_certmap_ctx *sdap_get_sss_certmap(struct sdap_certmap_ctx *ctx) +{ + return ctx == NULL ? NULL : ctx->certmap_ctx; +} + +errno_t sdap_setup_certmap(struct sdap_certmap_ctx *sdap_certmap_ctx, + struct certmap_info **certmap_list) +{ + int ret; + struct sss_certmap_ctx *sss_certmap_ctx = NULL; + size_t c; + + if (sdap_certmap_ctx == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Missing sdap_certmap_ctx.\n"); + return EINVAL; + } + + if (certmap_list == NULL || *certmap_list == NULL) { + DEBUG(SSSDBG_TRACE_ALL, "No certmap data, nothing to do.\n"); + ret = EOK; + goto done; + } + + ret = sss_certmap_init(sdap_certmap_ctx, ext_debug, NULL, &sss_certmap_ctx); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sss_certmap_init failed.\n"); + goto done; + } + + for (c = 0; certmap_list[c] != NULL; c++) { + DEBUG(SSSDBG_TRACE_ALL, "Trying to add rule [%s][%d][%s][%s].\n", + certmap_list[c]->name, + certmap_list[c]->priority, + certmap_list[c]->match_rule, + certmap_list[c]->map_rule); + + ret = sss_certmap_add_rule(sss_certmap_ctx, certmap_list[c]->priority, + certmap_list[c]->match_rule, + certmap_list[c]->map_rule, + certmap_list[c]->domains); + if (ret != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "sss_certmap_add_rule failed for rule [%s] " + "with error [%d][%s], skipping. " + "Please check for typos and if rule syntax is supported.\n", + certmap_list[c]->name, ret, sss_strerror(ret)); + continue; + } + } + + ret = EOK; + +done: + if (ret == EOK) { + sss_certmap_free_ctx(sdap_certmap_ctx->certmap_ctx); + sdap_certmap_ctx->certmap_ctx = sss_certmap_ctx; + } else { + sss_certmap_free_ctx(sss_certmap_ctx); + } + + return ret; +} + +errno_t sdap_init_certmap(TALLOC_CTX *mem_ctx, struct sdap_id_ctx *id_ctx) +{ + int ret; + bool hint; + struct certmap_info **certmap_list = NULL; + + if (id_ctx->opts->sdap_certmap_ctx == NULL) { + id_ctx->opts->sdap_certmap_ctx = talloc_zero(mem_ctx, + struct sdap_certmap_ctx); + if (id_ctx->opts->sdap_certmap_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n"); + return ENOMEM; + } + } + + ret = sysdb_get_certmap(mem_ctx, id_ctx->be->domain->sysdb, + &certmap_list, &hint); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_get_certmap failed.\n"); + goto done; + } + + ret = sdap_setup_certmap(id_ctx->opts->sdap_certmap_ctx, certmap_list); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sdap_setup_certmap failed.\n"); + goto done; + } + + ret = EOK; + +done: + talloc_free(certmap_list); + + return ret; +} diff --git a/src/tests/cmocka/test_sdap_certmap.c b/src/tests/cmocka/test_sdap_certmap.c new file mode 100644 index 000000000..9df566684 --- /dev/null +++ b/src/tests/cmocka/test_sdap_certmap.c @@ -0,0 +1,244 @@ +/* + Authors: + Sumit Bose + + Copyright (C) 2017 Red Hat + + SSSD tests - sdap certmap + + 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 "providers/ldap/ldap_common.h" +#include "tests/common.h" +#include "db/sysdb.h" + +#define TESTS_PATH "certmap_" BASE_FILE_STEM +#define TEST_CONF_DB "test_sysdb_certmap.ldb" +#define TEST_ID_PROVIDER "ldap" +#define TEST_DOM_NAME "certmap_test" + +struct certmap_info map_a = { discard_const("map_a"), 11, + NULL, discard_const("(abc=def)"), + NULL }; +struct certmap_info map_b = { discard_const("map_b"), UINT_MAX, + NULL, NULL, NULL }; +struct certmap_info *certmap[] = { &map_a, &map_b, NULL }; + +struct certmap_test_ctx { + struct sss_test_ctx *tctx; + struct sdap_id_ctx *id_ctx; +}; + +static int test_sysdb_setup(void **state) +{ + int ret; + struct certmap_test_ctx *test_ctx; + struct sss_test_conf_param params[] = { + { NULL, NULL }, /* Sentinel */ + }; + + assert_true(leak_check_setup()); + + test_ctx = talloc_zero(global_talloc_context, + struct certmap_test_ctx); + assert_non_null(test_ctx); + check_leaks_push(test_ctx); + + test_dom_suite_setup(TESTS_PATH); + + test_ctx->tctx = create_dom_test_ctx(test_ctx, TESTS_PATH, + TEST_CONF_DB, TEST_DOM_NAME, + TEST_ID_PROVIDER, params); + assert_non_null(test_ctx->tctx); + + ret = sysdb_update_certmap(test_ctx->tctx->sysdb, certmap, false); + assert_int_equal(ret, EOK); + + test_ctx->id_ctx = talloc_zero(test_ctx->tctx, struct sdap_id_ctx); + assert_non_null(test_ctx->id_ctx); + + test_ctx->id_ctx->opts = talloc_zero(test_ctx->tctx, struct sdap_options); + assert_non_null(test_ctx->id_ctx->opts); + + test_ctx->id_ctx->be = talloc_zero(test_ctx->tctx, struct be_ctx); + assert_non_null(test_ctx->id_ctx->be); + test_ctx->id_ctx->be->domain = test_ctx->tctx->dom; + + *state = test_ctx; + return 0; +} + +static int test_sysdb_teardown(void **state) +{ + struct certmap_test_ctx *test_ctx = + talloc_get_type(*state, struct certmap_test_ctx); + + test_dom_suite_cleanup(TESTS_PATH, TEST_CONF_DB, TEST_DOM_NAME); + talloc_free(test_ctx->tctx); + assert_true(check_leaks_pop(test_ctx)); + talloc_free(test_ctx); + assert_true(leak_check_teardown()); + return 0; +} + +static void test_sdap_certmap_init(void **state) +{ + int ret; + struct certmap_test_ctx *test_ctx = talloc_get_type(*state, + struct certmap_test_ctx); + + ret = sdap_init_certmap(test_ctx, test_ctx->id_ctx); + assert_int_equal(ret, EOK); + + talloc_free(test_ctx->id_ctx->opts->sdap_certmap_ctx); +} + +static void test_sdap_get_sss_certmap(void **state) +{ + int ret; + struct certmap_test_ctx *test_ctx = talloc_get_type(*state, + struct certmap_test_ctx); + struct sss_certmap_ctx *sss_certmap_ctx; + + sss_certmap_ctx = sdap_get_sss_certmap(NULL); + assert_null(sss_certmap_ctx); + + ret = sdap_init_certmap(test_ctx, test_ctx->id_ctx); + assert_int_equal(ret, EOK); + + sss_certmap_ctx = sdap_get_sss_certmap( + test_ctx->id_ctx->opts->sdap_certmap_ctx); + assert_non_null(sss_certmap_ctx); + + talloc_free(test_ctx->id_ctx->opts->sdap_certmap_ctx); +} + +static void test_sdap_certmap_init_twice(void **state) +{ + int ret; + struct certmap_test_ctx *test_ctx = talloc_get_type(*state, + struct certmap_test_ctx); + struct sdap_certmap_ctx *sdap_certmap_ref; + struct sss_certmap_ctx *sss_certmap_ref; + + ret = sdap_init_certmap(test_ctx, test_ctx->id_ctx); + assert_int_equal(ret, EOK); + + sdap_certmap_ref = test_ctx->id_ctx->opts->sdap_certmap_ctx; + sss_certmap_ref = sdap_get_sss_certmap(sdap_certmap_ref); + + ret = sdap_init_certmap(test_ctx, test_ctx->id_ctx); + assert_int_equal(ret, EOK); + + assert_ptr_equal(sdap_certmap_ref, + test_ctx->id_ctx->opts->sdap_certmap_ctx); + assert_ptr_not_equal(sss_certmap_ref, + sdap_get_sss_certmap(sdap_certmap_ref)); + + talloc_free(test_ctx->id_ctx->opts->sdap_certmap_ctx); +} + + +static void test_sdap_setup_certmap(void **state) +{ + int ret; + struct certmap_test_ctx *test_ctx = talloc_get_type(*state, + struct certmap_test_ctx); + struct sdap_certmap_ctx *sdap_certmap_ref; + struct sss_certmap_ctx *sss_certmap_ref; + + ret = sdap_init_certmap(test_ctx, test_ctx->id_ctx); + assert_int_equal(ret, EOK); + + sdap_certmap_ref = test_ctx->id_ctx->opts->sdap_certmap_ctx; + sss_certmap_ref = sdap_get_sss_certmap(sdap_certmap_ref); + + ret = sdap_setup_certmap(NULL, NULL); + assert_int_equal(ret, EINVAL); + assert_ptr_equal(sdap_certmap_ref, + test_ctx->id_ctx->opts->sdap_certmap_ctx); + assert_ptr_equal(sss_certmap_ref, sdap_get_sss_certmap(sdap_certmap_ref)); + + ret = sdap_setup_certmap(NULL, certmap); + assert_int_equal(ret, EINVAL); + assert_ptr_equal(sdap_certmap_ref, + test_ctx->id_ctx->opts->sdap_certmap_ctx); + assert_ptr_equal(sss_certmap_ref, sdap_get_sss_certmap(sdap_certmap_ref)); + + ret = sdap_setup_certmap(sdap_certmap_ref, certmap); + assert_int_equal(ret, EOK); + assert_ptr_equal(sdap_certmap_ref, + test_ctx->id_ctx->opts->sdap_certmap_ctx); + assert_ptr_not_equal(sss_certmap_ref, + sdap_get_sss_certmap(sdap_certmap_ref)); + + talloc_free(test_ctx->id_ctx->opts->sdap_certmap_ctx); +} + +int main(int argc, const char *argv[]) +{ + int rv; + poptContext pc; + int opt; + struct poptOption long_options[] = { + POPT_AUTOHELP + SSSD_DEBUG_OPTS + POPT_TABLEEND + }; + + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_sdap_certmap_init, + test_sysdb_setup, + test_sysdb_teardown), + cmocka_unit_test_setup_teardown(test_sdap_get_sss_certmap, + test_sysdb_setup, + test_sysdb_teardown), + cmocka_unit_test_setup_teardown(test_sdap_certmap_init_twice, + test_sysdb_setup, + test_sysdb_teardown), + cmocka_unit_test_setup_teardown(test_sdap_setup_certmap, + test_sysdb_setup, + test_sysdb_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_CLI_INIT(debug_level); + + tests_set_cwd(); + rv = cmocka_run_group_tests(tests, NULL, NULL); + + return rv; +} -- cgit