From 0e618c36ed74c240f7acd071ccb7bfd405b2d827 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Tue, 22 Nov 2022 14:43:21 +0100
Subject: [PATCH 19/19] pac: relax default check
To avoid issues with the UPN check during PAC validation when
'ldap_user_principal' is set to a not existing attribute to skip reading
user principals a new 'pac_check' option, 'check_upn_allow_missing' is
added to the default options. With this option only a log message is
shown but the check will not fail.
Resolves: https://github.com/SSSD/sssd/issues/6451
(cherry picked from commit 51b11db8b99a77ba5ccf6f850c2e81b5a6ee9f79)
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
---
src/confdb/confdb.h | 2 +-
src/man/sssd.conf.5.xml | 30 +++++++++++++++++++++++++++++-
src/providers/ad/ad_pac_common.c | 24 ++++++++++++++++++++----
src/util/pac_utils.c | 10 ++++++++++
src/util/util.h | 2 ++
5 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 83f6be7f9..5fda67585 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 "check_upn, check_upn_dns_info_ex"
+#define CONFDB_PAC_CHECK_IPA_AD_DEFAULT "check_upn, check_upn_allow_missing, 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 7a9920815..d9f4a7481 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2275,6 +2275,34 @@ pam_gssapi_indicators_map = sudo:pkinit, sudo-i:pkinit
consistent.</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>check_upn_allow_missing</term>
+ <listitem>
+ <para>This option should be used together
+ with 'check_upn' and handles the case where
+ a UPN is set on the server-side but is not
+ read by SSSD. The typical example is a
+ FreeIPA domain where 'ldap_user_principal'
+ is set to a not existing attribute name.
+ This was typically done to work-around
+ issues in the handling of enterprise
+ principals. But this is fixed since quite
+ some time and FreeIPA can handle enterprise
+ principals just fine and there is no need
+ anymore to set 'ldap_user_principal'.</para>
+ <para>Currently this option is set by
+ default to avoid regressions in such
+ environments. A log message will be added
+ to the system log and SSSD's debug log in
+ case a UPN is found in the PAC but not in
+ SSSD's cache. To avoid this log message it
+ would be best to evaluate if the
+ 'ldap_user_principal' option can be removed.
+ If this is not possible, removing
+ 'check_upn' will skip the test and avoid the
+ log message.</para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term>upn_dns_info_present</term>
<listitem>
@@ -2305,7 +2333,7 @@ pam_gssapi_indicators_map = sudo:pkinit, sudo-i:pkinit
</para>
<para>
Default: no_check (AD and IPA provider
- 'check_upn, check_upn_dns_info_ex')
+ 'check_upn, check_upn_allow_missing, check_upn_dns_info_ex')
</para>
</listitem>
</varlistentry>
diff --git a/src/providers/ad/ad_pac_common.c b/src/providers/ad/ad_pac_common.c
index 79f79b7a7..fcb54cd2c 100644
--- a/src/providers/ad/ad_pac_common.c
+++ b/src/providers/ad/ad_pac_common.c
@@ -215,10 +215,26 @@ errno_t check_upn_and_sid_from_user_and_pac(struct ldb_message *msg,
DEBUG(SSSDBG_MINOR_FAILURE, "User object does not have a UPN but PAC "
"says otherwise, maybe ldap_user_principal option is set.\n");
if (pac_check_opts & CHECK_PAC_CHECK_UPN) {
- DEBUG(SSSDBG_CRIT_FAILURE,
- "UPN is missing but PAC UPN check required, "
- "PAC validation failed.\n");
- return ERR_CHECK_PAC_FAILED;
+ if (pac_check_opts & CHECK_PAC_CHECK_UPN_ALLOW_MISSING) {
+ DEBUG(SSSDBG_IMPORTANT_INFO,
+ "UPN is missing but PAC UPN check required, "
+ "PAC validation failed. However, "
+ "'check_upn_allow_missing' is set and the error is "
+ "ignored. To make this message go away please check "
+ "why the UPN is not read from the server. In FreeIPA "
+ "environments 'ldap_user_principal' is most probably "
+ "set to a non-existing attribute name to avoid "
+ "issues with enterprise principals. This is not "
+ "needed anymore with recent versions of FreeIPA.\n");
+ sss_log(SSS_LOG_CRIT, "PAC validation issue, please check "
+ "sssd_pac.log for details");
+ return EOK;
+ } else {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "UPN is missing but PAC UPN check required, "
+ "PAC validation failed.\n");
+ return ERR_CHECK_PAC_FAILED;
+ }
}
}
diff --git a/src/util/pac_utils.c b/src/util/pac_utils.c
index c53b0c082..4499d8dfd 100644
--- a/src/util/pac_utils.c
+++ b/src/util/pac_utils.c
@@ -64,6 +64,8 @@ static errno_t check_check_pac_opt(const char *inp, uint32_t *check_pac_flags)
flags |= CHECK_PAC_CHECK_UPN_DNS_INFO_EX;
flags |= CHECK_PAC_UPN_DNS_INFO_PRESENT;
flags |= CHECK_PAC_CHECK_UPN;
+ } else if (strcasecmp(list[c], CHECK_PAC_CHECK_UPN_ALLOW_MISSING_STR) == 0) {
+ flags |= CHECK_PAC_CHECK_UPN_ALLOW_MISSING;
} else {
DEBUG(SSSDBG_OP_FAILURE, "Unknown value [%s] for pac_check.\n",
list[c]);
@@ -72,6 +74,14 @@ static errno_t check_check_pac_opt(const char *inp, uint32_t *check_pac_flags)
}
}
+ if ((flags & CHECK_PAC_CHECK_UPN_ALLOW_MISSING)
+ && !(flags & CHECK_PAC_CHECK_UPN)) {
+ DEBUG(SSSDBG_CONF_SETTINGS,
+ "pac_check option '%s' is set but '%s' is not set, this means "
+ "the UPN is not checked.\n",
+ CHECK_PAC_CHECK_UPN_ALLOW_MISSING_STR, CHECK_PAC_CHECK_UPN_STR);
+ }
+
ret = EOK;
done:
diff --git a/src/util/util.h b/src/util/util.h
index 6d9111874..4b2651c2c 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -818,6 +818,8 @@ uint64_t get_spend_time_us(uint64_t st);
#define CHECK_PAC_CHECK_UPN_DNS_INFO_EX (1 << 3)
#define CHECK_PAC_UPN_DNS_INFO_EX_PRESENT_STR "upn_dns_info_ex_present"
#define CHECK_PAC_UPN_DNS_INFO_EX_PRESENT (1 << 4)
+#define CHECK_PAC_CHECK_UPN_ALLOW_MISSING_STR "check_upn_allow_missing"
+#define CHECK_PAC_CHECK_UPN_ALLOW_MISSING (1 << 5)
errno_t get_pac_check_config(struct confdb_ctx *cdb, uint32_t *pac_check_opts);
#endif /* __SSSD_UTIL_H__ */
--
2.37.3