URL: https://github.com/freeipa/freeipa/pull/6113 Author: abbra Title: #6113: PAC fixes for Windows Server November 2021 security release Action: opened
PR body: """ ### ipa-kdb: issue PAC_REQUESTER_SID only for TGTs MS-KILE 3.3.5.6.4.8 in revision after Windows Server November 2021 security fixes added the following requirement: - PAC_REQUESTER_SID is only added in TGT case (including referrals and tickets to RODCs) ### ipa-kdb: fix requester SID check according to MS-KILE and MS-SFU updates New versions of MS-KILE and MS-SFU after Windows Server November 2021 security updates add PAC_REQUESTER_SID buffer check behavior: - PAC_REQUESTER_SID should only be added for TGT requests - if PAC_REQUESTER_SID is present, KDC must verify that the cname on the ticket resolves to the account with the same SID as the PAC_REQUESTER_SID. If it doesn't KDC must respond with KDC_ERR_TKT_REVOKED Change requester SID check to skip exact check for non-local PAC_REQUESTER_SID but harden to ensure it comes from the trusted domains we know about. If requester SID is the same as in PAC, we already do cname vs PAC SID verification. With these changes FreeIPA works against Windows Server 2019 with November 2021 security fixes in cross-realm S4U2Self operations. Fixes: https://pagure.io/freeipa/issue/9031 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/6113/head:pr6113 git checkout pr6113
From 7f9a4ea2addbc281f32ff5f7589aaa49e8b0598e Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Fri, 26 Nov 2021 17:40:54 +0200 Subject: [PATCH 1/2] ipa-kdb: issue PAC_REQUESTER_SID only for TGTs MS-KILE 3.3.5.6.4.8 in revision after Windows Server November 2021 security fixes added the following requirement: - PAC_REQUESTER_SID is only added in TGT case (including referrals and tickets to RODCs) Fixes: https://pagure.io/freeipa/issue/9031 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_mspac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 6f7d1ac15da..538cfbba958 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -1148,7 +1148,8 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext, #endif #ifdef HAVE_PAC_REQUESTER_SID - { + /* MS-KILE 3.3.5.6.4.8: add PAC_REQUESTER_SID only in TGT case */ + if ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0) { union PAC_INFO pac_requester_sid; /* == Package PAC_REQUESTER_SID == */ memset(&pac_requester_sid, 0, sizeof(pac_requester_sid)); From 014773e062a931b3168c0d23ec6d9cb30e72862f Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Fri, 26 Nov 2021 11:13:51 +0200 Subject: [PATCH 2/2] ipa-kdb: fix requester SID check according to MS-KILE and MS-SFU updates New versions of MS-KILE and MS-SFU after Windows Server November 2021 security updates add PAC_REQUESTER_SID buffer check behavior: - PAC_REQUESTER_SID should only be added for TGT requests - if PAC_REQUESTER_SID is present, KDC must verify that the cname on the ticket resolves to the account with the same SID as the PAC_REQUESTER_SID. If it doesn't KDC must respond with KDC_ERR_TKT_REVOKED Change requester SID check to skip exact check for non-local PAC_REQUESTER_SID but harden to ensure it comes from the trusted domains we know about. If requester SID is the same as in PAC, we already do cname vs PAC SID verification. With these changes FreeIPA works against Windows Server 2019 with November 2021 security fixes in cross-realm S4U2Self operations. Fixes: https://pagure.io/freeipa/issue/9031 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_mspac.c | 47 ++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 538cfbba958..98922ad0082 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -1697,7 +1697,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, "local [%s], PAC [%s]", dom ? dom : "<failed to display>", sid ? sid : "<failed to display>"); - return KRB5KDC_ERR_POLICY; + return KRB5KDC_ERR_TGT_REVOKED; } } @@ -1709,7 +1709,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, kerr = ipadb_get_principal(context, client_princ, flags, &client_actual); if (kerr != 0) { krb5_klog_syslog(LOG_ERR, "PAC issue: ipadb_get_principal failed."); - return KRB5KDC_ERR_POLICY; + return KRB5KDC_ERR_TGT_REVOKED; } ied = (struct ipadb_e_data *)client_actual->e_data; @@ -1743,7 +1743,7 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, "local [%s] vs PAC [%s]", local_sid ? local_sid : "<failed to display>", pac_sid ? pac_sid : "<failed to display>"); - kerr = KRB5KDC_ERR_POLICY; + kerr = KRB5KDC_ERR_TGT_REVOKED; goto done; } @@ -2005,22 +2005,43 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, /* Check that requester SID is the same as in the PAC entry */ if (requester_sid != NULL) { struct dom_sid client_sid; + bool is_from_trusted_domain = false; kerr = ipadb_get_sid_from_pac(tmpctx, info.info, &client_sid); if (kerr) { goto done; } result = dom_sid_check(&client_sid, requester_sid, true); if (!result) { - /* memctx is freed by the caller */ - char *pac_sid = dom_sid_string(tmpctx, &client_sid); - char *req_sid = dom_sid_string(tmpctx, requester_sid); - krb5_klog_syslog(LOG_ERR, "PAC issue: PAC has a SID " - "different from what PAC requester claims. " - "PAC [%s] vs PAC requester [%s]", - pac_sid ? pac_sid : "<failed to display>", - req_sid ? req_sid : "<failed to display>"); - kerr = KRB5KDC_ERR_POLICY; - goto done; + struct ipadb_context *ipactx = ipadb_get_context(context); + if (!ipactx || !ipactx->mspac) { + return KRB5_KDB_DBNOTINITED; + } + /* 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, + requester_sid, false); + if (result) { + is_from_trusted_domain = true; + break; + } + } + } + + if (!is_from_trusted_domain) { + /* memctx is freed by the caller */ + char *pac_sid = dom_sid_string(tmpctx, &client_sid); + char *req_sid = dom_sid_string(tmpctx, requester_sid); + krb5_klog_syslog(LOG_ERR, "PAC issue: PAC has a SID " + "different from what PAC requester claims. " + "PAC [%s] vs PAC requester [%s]", + pac_sid ? pac_sid : "<failed to display>", + req_sid ? req_sid : "<failed to display>"); + kerr = KRB5KDC_ERR_POLICY; + goto done; + } } }
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure