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