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 03cad20d7423d57e92c2957a5c853406cb45ba34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
Date: Tue, 4 Apr 2017 18:01:02 +0200
Subject: [PATCH 1/4] 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 60c84810ed0813a57c84147d6d1f0deb229b1547 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
Date: Tue, 4 Apr 2017 19:07:12 +0200
Subject: [PATCH 2/4] 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 575052da459a8a74c4cd2cd02806a99cd3fe13b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
Date: Tue, 4 Apr 2017 20:06:40 +0200
Subject: [PATCH 3/4] 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 147e4c8767c5be733ba43d41f7e720b14ec80fb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
Date: Fri, 26 May 2017 19:58:48 +0200
Subject: [PATCH 4/4] TESTS: Add unit tests for cfg validation

Add infrastructure for unit tests for validators.
---
 Makefile.am                     |  15 +++
 src/tests/cmocka/config_check.c | 233 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 248 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..cd04f03be
--- /dev/null
+++ b/src/tests/cmocka/config_check.c
@@ -0,0 +1,233 @@
+/*
+    Authors:
+        Michal Zidek <[email protected]>
+
+    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"
+
+#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;
+};
+
+char cfg_domain_sec_typo[] = "[domain/testdom.test]\n";
+
+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_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"
+                     "[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);
+}
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to