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