URL: https://github.com/freeipa/freeipa/pull/6081 Author: rcritten Title: #6081: [Backport][ipa-4-9] Harden PAC processing leftovers Action: opened
PR body: """ This PR was opened automatically because PR #6080 was pushed to master and backport to ipa-4-9 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/6081/head:pr6081 git checkout pr6081
From 3a1f90adacddb839a61d8779f01a1d61f68a0958 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> 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 <aboko...@redhat.com> --- 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 0e0ee36166e..6f272f9fec4 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); From 5080437af824f1c00acbec847f799030ed725481 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> 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 <aboko...@redhat.com> --- 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 6f272f9fec4..6f7d1ac15da 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 : "<failed to display>", - sid ? sid : "<failed to display>"); - 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 : "<failed to display>", + sid ? sid : "<failed to display>"); + 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;
_______________________________________________ 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
