URL: https://github.com/SSSD/sssd/pull/226 Author: mzidek-rh Title: #226: Config check regex Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/226/head:pr226 git checkout pr226
From 2250c6b3b433fa0c907874bec1935006258daf44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 4 Apr 2017 18:01:02 +0200 Subject: [PATCH 1/5] VALIDATORS: Add subdomain section Add separate rule for subdomain sections. Resolves: https://pagure.io/SSSD/sssd/issue/3356 --- src/config/cfg_rules.ini | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 70be88364..4cc0534b8 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -11,7 +11,8 @@ section = ifp section = secrets section = kcm section_re = ^secrets/users/[0-9]\+$ -section_re = ^domain/.*$ +section_re = ^domain/[^/\@]\+$ +section_re = ^domain/[^/\@]\+/[^/\@]\+$ section_re = ^application/.*$ [rule/allowed_sssd_options] @@ -699,3 +700,17 @@ validator = ini_allowed_options section_re = ^application/.*$ option = inherit_from + +[rule/allowed_subdomain_options] +validator = ini_allowed_options +section_re = ^domain/[^/\@]\+/[^/\@]\+$ + +option = ldap_search_base +option = ldap_user_search_base +option = ldap_group_search_base +option = ldap_netgroup_search_base +option = ldap_service_search_base +option = ad_server +option = ad_backup_server +option = ad_site +option = use_fully_qualified_names From d0a78d2b4fbe90a1064760ba84bafde847cf44e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 4 Apr 2017 19:07:12 +0200 Subject: [PATCH 2/5] VALIDATORS: Remove application section domain Application domains can use the same options as normal domains section with one more additional option. We could either duplicate all options from the domain section also in the application domain section + add the one additional aoption or add this one option to the domain section even though it is not meant to be used there to avoid duplication of all domain options in the rule for application section. It would be could to enhance the validators in libini to allow something like 'include' section in order to avoid this issue in the future. Resolves: https://pagure.io/SSSD/sssd/issue/3356 --- src/config/cfg_rules.ini | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 4cc0534b8..6e9bf940b 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -695,10 +695,7 @@ option = ldap_user_uid_number option = ldap_user_uuid option = ldap_use_tokengroups -[rule/allowed_application_options] -validator = ini_allowed_options -section_re = ^application/.*$ - +# For application domains option = inherit_from [rule/allowed_subdomain_options] From 386336f670c5d3ca0ad5acaf4a9966dcc6ada06a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 4 Apr 2017 20:06:40 +0200 Subject: [PATCH 3/5] VALIDATORS: Escape spcial regex chars The rule allowed_domain_options did not work because of bad regex. Resolves: https://pagure.io/SSSD/sssd/issue/3356 --- src/config/cfg_rules.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 6e9bf940b..10a8473b3 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -295,7 +295,7 @@ option = responder_idle_timeout [rule/allowed_domain_options] validator = ini_allowed_options -section_re = ^(domain|application)/.*$ +section_re = ^\(domain\|application\)/.*$ option = debug option = debug_level From 40cec35ccaf51e22545c62c471635d93c84949df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Fri, 26 May 2017 19:58:48 +0200 Subject: [PATCH 4/5] TESTS: Add unit tests for cfg validation Add infrastructure for unit tests for validators. --- Makefile.am | 15 +++ src/tests/cmocka/config_check.c | 255 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) create mode 100644 src/tests/cmocka/config_check.c diff --git a/Makefile.am b/Makefile.am index 18cafbc42..6512b8b49 100644 --- a/Makefile.am +++ b/Makefile.am @@ -252,6 +252,7 @@ if HAVE_CMOCKA dp_opt_tests \ responder-get-domains-tests \ sbus-internal-tests \ + config_check_tests \ sss_sifp-tests \ test_search_bases \ test_ldap_auth \ @@ -2431,6 +2432,20 @@ sbus_internal_tests_LDADD = \ libsss_debug.la \ libsss_test_common.la +config_check_tests_SOURCES = \ + src/tests/cmocka/config_check.c \ + $(NULL) +config_check_tests_CFLAGS = \ + $(AM_CFLAGS) \ + $(NULL) +config_check_tests_LDADD = \ + $(CMOCKA_LIBS) \ + $(SSSD_LIBS) \ + libsss_util.la \ + libsss_debug.la \ + libsss_test_common.la \ + $(NULL) + test_find_uid_SOURCES = \ src/tests/cmocka/test_find_uid.c \ src/util/find_uid.c \ diff --git a/src/tests/cmocka/config_check.c b/src/tests/cmocka/config_check.c new file mode 100644 index 000000000..e5e0821e7 --- /dev/null +++ b/src/tests/cmocka/config_check.c @@ -0,0 +1,255 @@ +/* + Authors: + Michal Zidek <mzi...@redhat.com> + + Copyright (C) 2014 Red Hat + + Config file validators test + + 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 <popt.h> +#include <ini_configobj.h> + +#include "util/util.h" +#include "util/sss_ini.h" +#include "tests/cmocka/common_mock.h" + +#ifdef HAVE_LIBINI_CONFIG_V1_3 + +#define RULES_PATH ABS_SRC_DIR"/src/config/cfg_rules.ini" + +struct sss_ini_initdata { + char **error_list; + struct ref_array *ra_success_list; + struct ref_array *ra_error_list; + struct ini_cfgobj *sssd_config; + struct value_obj *obj; + const struct stat *cstat; + struct ini_cfgfile *file; +}; + +void config_check_test_common(const char *cfg_string, + size_t num_errors_expected, + const char **errors_expected) +{ + struct sss_ini_initdata *init_data; + size_t num_errors; + char **strs; + int ret; + TALLOC_CTX *tmp_ctx; + + tmp_ctx = talloc_new(NULL); + assert_non_null(tmp_ctx); + + init_data = sss_ini_initdata_init(tmp_ctx); + + ret = ini_config_file_from_mem(discard_const(cfg_string), + strlen(cfg_string), + &init_data->file); + assert_int_equal(ret, EOK); + + ret = ini_config_create(&(init_data->sssd_config)); + assert_int_equal(ret, EOK); + + ret = ini_config_parse(init_data->file, + INI_STOP_ON_ANY, + INI_MV1S_OVERWRITE, + INI_PARSE_NOWRAP, + init_data->sssd_config); + assert_int_equal(ret, EOK); + + ret = sss_ini_call_validators_strs(tmp_ctx, init_data, + RULES_PATH, + &strs, &num_errors); + assert_int_equal(ret, EOK); + + /* Output from validators */ + for (int i = 0; i < num_errors; i++) { + /* Keep this printf loop for faster debugging */ + printf("%s\n", strs[i]); + } + + for (int i = 0; i < num_errors && i <= num_errors_expected; i++) { + assert_string_equal(strs[i], errors_expected[i]); + } + + /* Check if the number of errors is the same */ + assert_int_equal(num_errors_expected, num_errors); +} + +void config_check_test_bad_section_name(void **state) +{ + char cfg_str[] = "[sssssssssssssd]"; + const char *expected_errors[] = { + "[rule/allowed_sections]: Section [sssssssssssssd] is not allowed. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_sssd_option_name(void **state) +{ + char cfg_str[] = "[sssd]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_sssd_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'sssd'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_pam_option_name(void **state) +{ + char cfg_str[] = "[pam]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_pam_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'pam'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_nss_option_name(void **state) +{ + char cfg_str[] = "[nss]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_nss_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'nss'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_pac_option_name(void **state) +{ + char cfg_str[] = "[pac]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_pac_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'pac'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_ifp_option_name(void **state) +{ + char cfg_str[] = "[ifp]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_ifp_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'ifp'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_domain_option_name(void **state) +{ + char cfg_str[] = "[domain/A.test\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_subdomain_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'domain/A.test'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_appdomain_option_name(void **state) +{ + char cfg_str[] = "[application/myapp\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_subdomain_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'application/myapp'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_bad_subdom_option_name(void **state) +{ + char cfg_str[] = "[domain/A.test/B.A.test]\n" + "debug_leTYPOvel = 10\n"; + const char *expected_errors[] = { + "[rule/allowed_sssd_options]: Attribute 'debug_leTYPOvel' is not allowed in section 'domain/A.test/B.A.test'. Check for typos.", + }; + + config_check_test_common(cfg_str, 1, expected_errors); +} + +void config_check_test_good_sections(void **state) +{ + char cfg_str[] = "[sssd]\n" + "[pam]\n" + "[nss]\n" + "[domain/testdom.test]\n" + "[domain/testdom.test/testsubdom.testdom.test]\n" + "[application/myapp]\n" + "[secrets]\n" + "[ifp]\n" + "[pac]\n"; + const char *expected_errors[] = {NULL}; + + config_check_test_common(cfg_str, 0, expected_errors); +} + +int main(int argc, const char *argv[]) +{ + poptContext pc; + int opt; + struct poptOption long_options[] = { + POPT_AUTOHELP + SSSD_DEBUG_OPTS + POPT_TABLEEND + }; + + const struct CMUnitTest tests[] = { + cmocka_unit_test(config_check_test_bad_section_name), + cmocka_unit_test(config_check_test_bad_sssd_option_name), + cmocka_unit_test(config_check_test_bad_pam_option_name), + cmocka_unit_test(config_check_test_bad_nss_option_name), + cmocka_unit_test(config_check_test_bad_pac_option_name), + cmocka_unit_test(config_check_test_bad_ifp_option_name), + cmocka_unit_test(config_check_test_good_sections), + }; + + /* 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(); + return cmocka_run_group_tests(tests, NULL, NULL); +} + +#else /* !HAVE_LIBINI_CONFIG_V1_3 */ + +int main(int argc, const char *argv[]) +{ + fprintf(stderr, "%s requires newer version of libini\n", argv[0]); + return 0; +} + +#endif /* HAVE_LIBINI_CONFIG_V1_3 */ From a0389b2e6f5367fdd99598fe06aed59a1bddfd8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 30 May 2017 12:05:39 +0200 Subject: [PATCH 5/5] MAN: Fix typo in trusted domain section --- src/man/sssd.conf.5.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index c71287029..0f7822b29 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -2912,7 +2912,7 @@ ldap_user_extra_attrs = phone:telephoneNumber <para> Some options used in the domain section can also be used in the trusted domain section, that is, in a section called - <quote>[domain/<replaceable>DOMAIN_NAME</replaceable>]/<replaceable>TRUSTED_DOMAIN_NAME</replaceable>]</quote>. + <quote>[domain/<replaceable>DOMAIN_NAME</replaceable>/<replaceable>TRUSTED_DOMAIN_NAME</replaceable>]</quote>. Currently supported options in the trusted domain section are: </para> <para>ldap_search_base,</para>
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
