From a60b978d360b28589ea78eb144d864acf952ae39 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Thu, 11 Nov 2021 20:32:05 +0200 Subject: [PATCH] Harden PAC processing -- trusted domains Handle SIDs of the trusted domains during S4U extensions Signed-off-by: Alexander Bokovoy --- freeipa-harden-pac-2.patch | 222 +++++++++++++++++++++++++++++++++++++ freeipa.spec | 7 +- 2 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 freeipa-harden-pac-2.patch diff --git a/freeipa-harden-pac-2.patch b/freeipa-harden-pac-2.patch new file mode 100644 index 0000000..abfc547 --- /dev/null +++ b/freeipa-harden-pac-2.patch @@ -0,0 +1,222 @@ +From fe59e6a0b06926a3d71c6b6f361714d1422d5b0f Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +Date: Thu, 11 Nov 2021 09:58:09 +0200 +Subject: [PATCH 1/2] ipa-kdb: honor SID from the host or service entry + +If the SID was explicitly set for the host or service entry, honor it +when issuing PAC. For normal services and hosts we don't allocate +individual SIDs but for cifs/... principals on domain members we do as +they need to login to Samba domain controller. + +Related: https://pagure.io/freeipa/issue/9031 + +Signed-off-by: Alexander Bokovoy +--- + daemons/ipa-kdb/ipa_kdb_mspac.c | 46 ++++++++++++++++++++------------- + 1 file changed, 28 insertions(+), 18 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c +index 0e0ee3616..6f272f9fe 100644 +--- a/daemons/ipa-kdb/ipa_kdb_mspac.c ++++ b/daemons/ipa-kdb/ipa_kdb_mspac.c +@@ -653,6 +653,28 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, + * clear it after detecting the changes */ + info3->base.acct_flags = ACB_USE_AES_KEYS; + ++ ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, ++ "ipaNTSecurityIdentifier", &strres); ++ if (ret) { ++ /* SID is mandatory for all but host/services */ ++ if (!(is_host || is_service)) { ++ return ret; ++ } ++ info3->base.rid = 0; ++ } else { ++ ret = ipadb_string_to_sid(strres, &sid); ++ free(strres); ++ if (ret) { ++ return ret; ++ } ++ ret = sid_split_rid(&sid, &info3->base.rid); ++ if (ret) { ++ return ret; ++ } ++ } ++ ++ /* If SID was present prefer using it even for hosts and services ++ * but we still need to set the account flags correctly */ + if ((is_host || is_service)) { + /* it is either host or service, so get the hostname first */ + char *sep = strchr(info3->base.account_name.string, '/'); +@@ -661,29 +683,17 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, + sep ? sep + 1 : info3->base.account_name.string); + if (is_master) { + /* Well known RID of domain controllers group */ +- info3->base.rid = 516; ++ if (info3->base.rid == 0) { ++ info3->base.rid = 516; ++ } + info3->base.acct_flags |= ACB_SVRTRUST; + } else { + /* Well known RID of domain computers group */ +- info3->base.rid = 515; ++ if (info3->base.rid == 0) { ++ info3->base.rid = 515; ++ } + info3->base.acct_flags |= ACB_WSTRUST; + } +- } else { +- ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, +- "ipaNTSecurityIdentifier", &strres); +- if (ret) { +- /* SID is mandatory */ +- return ret; +- } +- ret = ipadb_string_to_sid(strres, &sid); +- free(strres); +- if (ret) { +- return ret; +- } +- ret = sid_split_rid(&sid, &info3->base.rid); +- if (ret) { +- return ret; +- } + } + + ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, &deref_results); +-- +2.33.1 + + +From 21af43550aa0a31e1ec5240578bd64fcbdd4ee24 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +Date: Thu, 11 Nov 2021 10:16:47 +0200 +Subject: [PATCH 2/2] ipa-kdb: validate domain SID in incoming PAC for trusted + domains for S4U + +Previously, ipadb_check_logon_info() was called only for cross-realm +case. Now we call it for both in-realm and cross-realm cases. In case of +the S4U2Proxy, we would be passed a PAC of the original caller which +might be a principal from the trusted realm. We cannot validate that PAC +against our local client DB entry because this is the proxy entry which +is guaranteed to have different SID. + +In such case, validate the SID of the domain in PAC against our realm +and any trusted doman but skip an additional check of the DB entry in +the S4U2Proxy case. + +Related: https://pagure.io/freeipa/issue/9031 + +Signed-off-by: Alexander Bokovoy +--- + daemons/ipa-kdb/ipa_kdb_mspac.c | 54 ++++++++++++++++++++++++++------- + 1 file changed, 43 insertions(+), 11 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c +index 6f272f9fe..6f7d1ac15 100644 +--- a/daemons/ipa-kdb/ipa_kdb_mspac.c ++++ b/daemons/ipa-kdb/ipa_kdb_mspac.c +@@ -1637,11 +1637,13 @@ static void filter_logon_info_log_message_rid(struct dom_sid *sid, uint32_t rid) + static krb5_error_code check_logon_info_consistent(krb5_context context, + TALLOC_CTX *memctx, + krb5_const_principal client_princ, ++ krb5_boolean is_s4u, + struct PAC_LOGON_INFO_CTR *info) + { + krb5_error_code kerr = 0; + struct ipadb_context *ipactx; + bool result; ++ bool is_from_trusted_domain = false; + krb5_db_entry *client_actual = NULL; + struct ipadb_e_data *ied = NULL; + int flags = 0; +@@ -1671,14 +1673,36 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + result = dom_sid_check(&ipactx->mspac->domsid, + info->info->info3.base.domain_sid, true); + if (!result) { +- /* memctx is freed by the caller */ +- char *sid = dom_sid_string(memctx, info->info->info3.base.domain_sid); +- char *dom = dom_sid_string(memctx, &ipactx->mspac->domsid); +- krb5_klog_syslog(LOG_ERR, "PAC issue: PAC record claims domain SID different " +- "to local domain SID: local [%s], PAC [%s]", +- dom ? dom : "", +- sid ? sid : ""); +- return KRB5KDC_ERR_POLICY; ++ /* In S4U case we might be dealing with the PAC issued by the trusted domain */ ++ if (is_s4u && (ipactx->mspac->trusts != NULL)) { ++ /* Iterate through list of trusts and check if this SID belongs to ++ * one of the domains we trust */ ++ for(int i = 0 ; i < ipactx->mspac->num_trusts ; i++) { ++ result = dom_sid_check(&ipactx->mspac->trusts[i].domsid, ++ info->info->info3.base.domain_sid, true); ++ if (result) { ++ is_from_trusted_domain = true; ++ break; ++ } ++ } ++ } ++ ++ if (!result) { ++ /* memctx is freed by the caller */ ++ char *sid = dom_sid_string(memctx, info->info->info3.base.domain_sid); ++ char *dom = dom_sid_string(memctx, &ipactx->mspac->domsid); ++ krb5_klog_syslog(LOG_ERR, "PAC issue: PAC record claims domain SID different " ++ "to local domain SID or any trusted domain SID: " ++ "local [%s], PAC [%s]", ++ dom ? dom : "", ++ sid ? sid : ""); ++ return KRB5KDC_ERR_POLICY; ++ } ++ } ++ ++ if (is_s4u && is_from_trusted_domain) { ++ /* If the PAC belongs to a user from the trusted domain, we cannot compare SIDs */ ++ return 0; + } + + kerr = ipadb_get_principal(context, client_princ, flags, &client_actual); +@@ -1703,6 +1727,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, + goto done; + } + ++ + kerr = ipadb_get_sid_from_pac(memctx, info->info, &client_sid); + if (kerr) { + goto done; +@@ -1956,6 +1981,7 @@ krb5_error_code filter_logon_info(krb5_context context, + static krb5_error_code ipadb_check_logon_info(krb5_context context, + krb5_const_principal client_princ, + krb5_boolean is_cross_realm, ++ krb5_boolean is_s4u, + krb5_data *pac_blob, + struct dom_sid *requester_sid) + { +@@ -1999,8 +2025,11 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, + + if (!is_cross_realm) { + /* For local realm case we need to check whether the PAC is for our user +- * but we don't need to process further */ +- kerr = check_logon_info_consistent(context, tmpctx, client_princ, &info); ++ * but we don't need to process further. In S4U2Proxy case when the client ++ * is ours but operates on behalf of the cross-realm principal, we will ++ * search through the trusted domains but otherwise skip the exact SID check ++ * as we are not responsible for the principal from the trusted domain */ ++ kerr = check_logon_info_consistent(context, tmpctx, client_princ, is_s4u, &info); + goto done; + } + +@@ -2251,7 +2280,10 @@ static krb5_error_code ipadb_verify_pac(krb5_context context, + #endif + + kerr = ipadb_check_logon_info(context, +- client_princ, is_cross_realm, &pac_blob, ++ client_princ, ++ is_cross_realm, ++ (flags & KRB5_KDB_FLAGS_S4U), ++ &pac_blob, + requester_sid); + if (kerr != 0) { + goto done; +-- +2.33.1 + diff --git a/freeipa.spec b/freeipa.spec index bdc7368..aea453b 100644 --- a/freeipa.spec +++ b/freeipa.spec @@ -196,7 +196,7 @@ Name: %{package_name} Version: %{IPA_VERSION} -Release: 3%{?rc_version:.%rc_version}%{?dist} +Release: 4%{?rc_version:.%rc_version}%{?dist} Summary: The Identity, Policy and Audit system License: GPLv3+ @@ -222,6 +222,7 @@ Patch1001: 1001-Change-branding-to-IPA-and-Identity-Management.patch # RHEL spec file only: END Patch0001: 0001-Make-Dogtag-return-XML-for-ipa-cert-find.patch Patch0002: freeipa-harden-pac.patch +Patch0003: freeipa-harden-pac-2.patch # For the timestamp trick in patch application BuildRequires: diffstat @@ -1705,6 +1706,10 @@ fi %endif %changelog +* Thu Nov 11 2021 Alexander Bokovoy - 4.9.7-4 +- Hardening for CVE-2020-25717 part 2 +- Handle S4U for users from trusted domains + * Wed Nov 10 2021 Alexander Bokovoy - 4.9.7-3 - Hardening for CVE-2020-25717 - Generate SIDs for IPA users and groups by default