From 4a2d3451f2241fdcc8f5216a2a5d3cc881c41d88 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 13 Jun 2022 12:45:54 +0200 Subject: [PATCH] Resolves: rhbz#2073095 - Harden kerberos ticket validation (additional patch) Resolves: rhbz#2061795 - Unable to lookup AD user if the AD group contains '@' symbol (additional patch) --- ...c-relax-default-for-pac_check-option.patch | 50 +++++++ ...ly-check-sub-domains-for-regex-match.patch | 131 ++++++++++++++++++ sssd.spec | 9 +- 3 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 0001-pac-relax-default-for-pac_check-option.patch create mode 100644 0002-names-only-check-sub-domains-for-regex-match.patch diff --git a/0001-pac-relax-default-for-pac_check-option.patch b/0001-pac-relax-default-for-pac_check-option.patch new file mode 100644 index 0000000..aeccd2f --- /dev/null +++ b/0001-pac-relax-default-for-pac_check-option.patch @@ -0,0 +1,50 @@ +From 26d8601e9b4e35ff89ca9fa72b9db05199096b56 Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Wed, 8 Jun 2022 10:11:15 +0200 +Subject: [PATCH] pac: relax default for pac_check option +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +PAC might not be always present, especially in IPA environments. So the +default of pac_check should not contain 'pac_present'. + +Resolves: https://github.com/SSSD/sssd/issues/5868 + +Reviewed-by: Iker Pedrosa +Reviewed-by: Pavel Březina +(cherry picked from commit 55e93cf1cf4d61c6de7975cbdc97a723545586c0) +--- + src/confdb/confdb.h | 2 +- + src/man/sssd.conf.5.xml | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h +index d9fe571de..83f6be7f9 100644 +--- a/src/confdb/confdb.h ++++ b/src/confdb/confdb.h +@@ -181,7 +181,7 @@ + #define CONFDB_PAC_LIFETIME "pac_lifetime" + #define CONFDB_PAC_CHECK "pac_check" + #define CONFDB_PAC_CHECK_DEFAULT "no_check" +-#define CONFDB_PAC_CHECK_IPA_AD_DEFAULT "pac_present, check_upn, check_upn_dns_info_ex" ++#define CONFDB_PAC_CHECK_IPA_AD_DEFAULT "check_upn, check_upn_dns_info_ex" + + /* InfoPipe */ + #define CONFDB_IFP_CONF_ENTRY "config/ifp" +diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml +index 705447427..e921ba575 100644 +--- a/src/man/sssd.conf.5.xml ++++ b/src/man/sssd.conf.5.xml +@@ -2298,7 +2298,7 @@ pam_gssapi_indicators_map = sudo:pkinit, sudo-i:pkinit + + + Default: no_check (AD and IPA provider +- 'pac_present, check_upn, check_upn_dns_info_ex') ++ 'check_upn, check_upn_dns_info_ex') + + + +-- +2.35.3 + diff --git a/0002-names-only-check-sub-domains-for-regex-match.patch b/0002-names-only-check-sub-domains-for-regex-match.patch new file mode 100644 index 0000000..7747794 --- /dev/null +++ b/0002-names-only-check-sub-domains-for-regex-match.patch @@ -0,0 +1,131 @@ +From 536dc9e4f72503942e659ca0dbd022d3dfac148f Mon Sep 17 00:00:00 2001 +From: Sumit Bose +Date: Thu, 2 Jun 2022 17:02:31 +0200 +Subject: [PATCH] names: only check sub-domains for regex match +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +It is allowed to have different regular-expression to split the input +name for different domains. After the regex is evaluated and a domain +name was found in the input it has to be check if the domain name +corresponds to the domain the regex is coming from. + +E.g. with the implicit files provider enabled the file provider might +use a simple default regex while and additional IPA or AD provider will +have a more complex one which e.g. properly handles @-characters in +names. When evaluation in input the simple regex will come first and +will split the name but will miss part of the user name part if the name +contains an @-character. Currently SSSD check if the found domain name +matches any of the know domains or sub-domains which is wrong because +the regex was coming from the files provider and hence it should only +handle its own objects. + +With this patch not all domains are checked but only the current one and +its sub-domains, if any. This behavior is also mentioned in a comment +already in the code. As a result in the above example the check with +the results form the simple regex with fail and then the more complex +regex of the other domain will be used which can split the name +properly. + +Resolves: https://github.com/SSSD/sssd/issues/6055 + +Reviewed-by: Alexey Tikhonov +Reviewed-by: Tomáš Halman +(cherry picked from commit 9656516b9af2b3ea4627eab42f11c7667564020f) +--- + src/tests/cmocka/test_fqnames.c | 50 +++++++++++++++++++++++++++++++++ + src/util/usertools.c | 2 +- + 2 files changed, 51 insertions(+), 1 deletion(-) + +diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c +index 406ef55a9..5de4faf9a 100644 +--- a/src/tests/cmocka/test_fqnames.c ++++ b/src/tests/cmocka/test_fqnames.c +@@ -318,6 +318,41 @@ static int parse_name_test_setup(void **state) + return 0; + } + ++static int parse_name_test_two_names_ctx_setup(void **state) ++{ ++ struct parse_name_test_ctx *test_ctx; ++ struct sss_names_ctx *nctx1 = NULL; ++ struct sss_names_ctx *nctx2 = NULL; ++ struct sss_domain_info *dom; ++ int ret; ++ ++ assert_true(leak_check_setup()); ++ ++ test_ctx = talloc_zero(global_talloc_context, struct parse_name_test_ctx); ++ assert_non_null(test_ctx); ++ ++ ret = sss_names_init_from_args(test_ctx, SSS_DEFAULT_RE, ++ "%1$s@%2$s", &nctx1); ++ assert_int_equal(ret, EOK); ++ ++ ret = sss_names_init_from_args(test_ctx, SSS_IPA_AD_DEFAULT_RE, ++ "%1$s@%2$s", &nctx2); ++ assert_int_equal(ret, EOK); ++ ++ test_ctx->dom = create_test_domain(test_ctx, DOMNAME, FLATNAME, ++ NULL, nctx1); ++ assert_non_null(test_ctx->dom); ++ ++ dom = create_test_domain(test_ctx, DOMNAME2, FLATNAME2, ++ NULL, nctx2); ++ assert_non_null(dom); ++ DLIST_ADD_END(test_ctx->dom, dom, struct sss_domain_info *); ++ ++ check_leaks_push(test_ctx); ++ *state = test_ctx; ++ return 0; ++} ++ + static int parse_name_test_teardown(void **state) + { + struct parse_name_test_ctx *test_ctx = talloc_get_type(*state, +@@ -448,6 +483,18 @@ void test_init_nouser(void **state) + assert_int_not_equal(ret, EOK); + } + ++void test_different_regexps(void **state) ++{ ++ struct parse_name_test_ctx *test_ctx = talloc_get_type(*state, ++ struct parse_name_test_ctx); ++ parse_name_check(test_ctx, NAME"@"DOMNAME, NULL, EOK, NAME, DOMNAME); ++ parse_name_check(test_ctx, NAME"@"DOMNAME2, NULL, EOK, NAME, DOMNAME2); ++ parse_name_check(test_ctx, NAME"@WITH_AT@"DOMNAME2, NULL, EOK, NAME"@WITH_AT", DOMNAME2); ++ parse_name_check(test_ctx, FLATNAME"\\"NAME, NULL, EOK, FLATNAME"\\"NAME, NULL); ++ parse_name_check(test_ctx, FLATNAME2"\\"NAME, NULL, EOK, NAME, DOMNAME2); ++ parse_name_check(test_ctx, FLATNAME2"\\"NAME"@WITH_AT", NULL, EOK, NAME"@WITH_AT", DOMNAME2); ++} ++ + void sss_parse_name_fail(void **state) + { + struct parse_name_test_ctx *test_ctx = talloc_get_type(*state, +@@ -502,6 +549,9 @@ int main(int argc, const char *argv[]) + cmocka_unit_test_setup_teardown(sss_parse_name_fail, + parse_name_test_setup, + parse_name_test_teardown), ++ cmocka_unit_test_setup_teardown(test_different_regexps, ++ parse_name_test_two_names_ctx_setup, ++ parse_name_test_teardown), + }; + + /* Set debug level to invalid value so we can decide if -d 0 was used. */ +diff --git a/src/util/usertools.c b/src/util/usertools.c +index 511fb2d5d..91df7129e 100644 +--- a/src/util/usertools.c ++++ b/src/util/usertools.c +@@ -321,7 +321,7 @@ static struct sss_domain_info * match_any_domain_or_subdomain_name( + return dom; + } + +- return find_domain_by_name(dom, dmatch, true); ++ return find_domain_by_name_ex(dom, dmatch, true, SSS_GND_SUBDOMAINS); + } + + int sss_parse_name_for_domains(TALLOC_CTX *memctx, +-- +2.35.3 + diff --git a/sssd.spec b/sssd.spec index 9a890d2..4ec2bae 100644 --- a/sssd.spec +++ b/sssd.spec @@ -27,14 +27,15 @@ Name: sssd Version: 2.7.1 -Release: 1%{?dist} +Release: 2%{?dist} Summary: System Security Services Daemon License: GPLv3+ URL: https://github.com/SSSD/sssd/ Source0: https://github.com/SSSD/sssd/releases/download/%{version}/sssd-%{version}.tar.gz ### Patches ### -#Patch0001: +Patch0001: 0001-pac-relax-default-for-pac_check-option.patch +Patch0002: 0002-names-only-check-sub-domains-for-regex-match.patch ### Dependencies ### @@ -1059,6 +1060,10 @@ fi %systemd_postun_with_restart sssd.service %changelog +* Mon Jun 13 2022 Alexey Tikhonov - 2.7.1-2 +- Resolves: rhbz#2073095 - Harden kerberos ticket validation (additional patch) +- Resolves: rhbz#2061795 - Unable to lookup AD user if the AD group contains '@' symbol (additional patch) + * Sat Jun 4 2022 Alexey Tikhonov - 2.7.1-1 - Resolves: rhbz#2069376 - Rebase SSSD for RHEL 9.1 - Resolves: rhbz#1893192 - sdap_nested_group_deref_direct_process() triggers internal watchdog for large data sets