From 32b7c1d012a0904420edc61bf94be66253a6c84a Mon Sep 17 00:00:00 2001

From: Alexander Bokovoy <abokovoy@redhat.com>

Date: Thu, 28 May 2015 08:33:51 +0000

Subject: [PATCH] ipa-kdb: filter out group membership from MS-PAC for exact

 SID matches too

When incoming SID blacklist contains exact SIDs of users and groups,

attempt to filter them out as well, according to [MS-PAC] 4.1.1.2.

Note that we treat user's SID and primary group RID filtering as violation

of the KDC policy because the resulting MS-PAC will have no user SID or

primary group and thus will be invalid.

For group RIDs we filter them out. According to [MS-KILE] 3.3.5.6.3.1

it is OK to have empty group RIDs array as GroupCount SHOULD be

equal to Groups.MembershipCount returned by SamrGetGroupsForUser

[MS-SAMR] 3.1.5.9.1, not MUST, thus it may be empty.

Part of fix for https://bugzilla.redhat.com/show_bug.cgi?id=1222475

Reviewed-By: Tomas Babej <tbabej@redhat.com>

---

 daemons/ipa-kdb/ipa_kdb_mspac.c | 100 +++++++++++++++++++++++++++++++++++++++-

 1 file changed, 99 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c

index 74ee2f3fd4b81bd3433c9ff9c77f7434b72e7f4d..3e6024f55071e95c6d40869e31d879baf627a3b9 100644

--- a/daemons/ipa-kdb/ipa_kdb_mspac.c

+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c

@@ -1320,6 +1320,22 @@ static void filter_logon_info_log_message(struct dom_sid *sid)

     }

 }

+static void filter_logon_info_log_message_rid(struct dom_sid *sid, uint32_t rid)

+{

+    char *domstr = NULL;

+

+    domstr = dom_sid_string(NULL, sid);

+    if (domstr) {

+        krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID [%s-%d] is not allowed "

+                                  "from a trusted source and will be excluded.", domstr, rid);

+        talloc_free(domstr);

+    } else {

+        krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID is not allowed "

+                                  "from a trusted source and will be excluded."

+                                  "Unable to allocate memory to display SID.");

+    }

+}

+

 static krb5_error_code filter_logon_info(krb5_context context,

                                          TALLOC_CTX *memctx,

                                          krb5_data realm,

@@ -1331,9 +1347,21 @@ static krb5_error_code filter_logon_info(krb5_context context,

      * attempt at getting us to sign fake credentials with the help of a

      * compromised trusted realm */

+    /* NOTE: there are two outcomes from filtering:

+     * REJECT TICKET -- ticket is rejected if domain SID of

+     *                  the principal with MS-PAC is filtered out or

+     *                  its primary group RID is filtered out

+     *

+     * REMOVE SID    -- SIDs are removed from the list of SIDs associated

+     *                  with the principal if they are filtered out

+     *                  This applies also to secondary RIDs of the principal

+     *                  if domain_sid-<secondary RID> is filtered out

+     */

+

     struct ipadb_context *ipactx;

     struct ipadb_adtrusts *domain;

-    int i, j, k, count;

+    int i, j, k, l, count;

+    uint32_t rid;

     bool result;

     char *domstr = NULL;

@@ -1380,6 +1408,76 @@ static krb5_error_code filter_logon_info(krb5_context context,

         }

     }

+    /* Check if this user's SIDs membership is filtered too */

+    for(k = 0; k < domain->len_sid_blacklist_incoming; k++) {

+        /* Short-circuit if there are no RIDs. This may happen if we filtered everything already.

+         * In normal situation there would be at least primary gid as RID in the RIDs array

+         * but if we filtered out the primary RID, this MS-PAC is invalid */

+        count = info->info->info3.base.groups.count;

+        result = dom_sid_is_prefix(info->info->info3.base.domain_sid,

+                                   &domain->sid_blacklist_incoming[k]);

+        if (result) {

+            i = 0;

+            j = 0;

+            if (domain->sid_blacklist_incoming[k].num_auths - info->info->info3.base.domain_sid->num_auths != 1) {

+                krb5_klog_syslog(LOG_ERR, "Incoming SID blacklist element matching domain [%s with SID %s] "

+                                          "has more than one RID component. Invalid check skipped.",

+                                 domain->domain_name, domain->domain_sid);

+                break;

+            }

+            rid = domain->sid_blacklist_incoming[k].sub_auths[domain->sid_blacklist_incoming[k].num_auths - 1];

+            if (rid == info->info->info3.base.rid) {

+                filter_logon_info_log_message_rid(info->info->info3.base.domain_sid, rid);

+                /* Actual user's SID is filtered out */

+                return KRB5KDC_ERR_POLICY;

+            }

+            if (rid == info->info->info3.base.primary_gid) {

+                /* User's primary group SID is filtered out */

+                return KRB5KDC_ERR_POLICY;

+            }

+            if (count == 0) {

+                /* Having checked actual user's SID and primary group SID, and having no other RIDs,

+                 * skip checks below and continue to next blacklist element */

+                continue;

+            }

+

+            do {

+                if (rid == info->info->info3.base.groups.rids[i].rid) {

+                    filter_logon_info_log_message_rid(info->info->info3.base.domain_sid, rid);

+                    /* If this is just a non-primary RID, we simply remove it from the array of RIDs */

+                    l = count - i - j - 1;

+                    if (l != 0) {

+                         memmove(info->info->info3.base.groups.rids+i,

+                                 info->info->info3.base.groups.rids+i+1,

+                                 sizeof(struct samr_RidWithAttribute)*l);

+                    }

+                    j++;

+                } else {

+                    i++;

+                }

+            } while ((i + j) < count);

+

+            if (j != 0) {

+                count = count-j;

+                if (count == 0) {

+                    /* All RIDs were filtered out. Unusual but MS-KILE 3.3.5.6.3.1 says SHOULD, not MUST for GroupCount */

+                    info->info->info3.base.groups.count = 0;

+                    talloc_free(info->info->info3.base.groups.rids);

+                    info->info->info3.base.groups.rids = NULL;

+                } else {

+                    info->info->info3.base.groups.rids = talloc_realloc(memctx,

+                                                                        info->info->info3.base.groups.rids,

+                                                                        struct samr_RidWithAttribute, count);

+                    if (!info->info->info3.base.groups.rids) {

+                        info->info->info3.base.groups.count = 0;

+                        return ENOMEM;

+                    }

+                    info->info->info3.base.groups.count = count;

+                }

+            }

+        }

+    }

+

     /* According to MS-KILE 25.0, info->info->info3.sids may be non zero, so check

      * should include different possibilities into account

      * */

-- 

2.1.0