From fe59e6a0b06926a3d71c6b6f361714d1422d5b0f Mon Sep 17 00:00:00 2001

From: Alexander Bokovoy <abokovoy@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 <abokovoy@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 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 <abokovoy@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 <abokovoy@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 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 : "<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;

-- 

2.33.1