[Freeipa-devel] [PATCH] 0080 rewrite SID comparison to take into account different SID forms

[Alexander Bokovoy]

Hi,

Domain validator code in ipaserver/dcerpc.py verifies that a SID belongs
to one of our trusted domains. This verification was expecting that SID
is for some resource within trusted domain and ignored the case when it
is the SID of the trusted domain, i.e. when sid has form like
S-1-5-21-16904141-148189700-2149043814 rather than
S-1-5-21-16904141-148189700-2149043814-512 (Domain Admins).

The latter is what idrange-add command uses. 

So comparing SID with SID was done by stripping last component (RID).
In case of idrange-add stripping last RID was making a SID that could
never compare to a trusted domain SID.

Somehow the code worked for me in Fedora and started failing on RHEL6.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 6efa779009b86bca31e95ef3640b179576d21eff Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Tue, 25 Sep 2012 17:25:42 +0300
Subject: [PATCH 2/2] Change the way SID comparison is done for belonging to
 trusted domain

Fixes trust use on RHEL 6.
---
 ipaserver/dcerpc.py | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index ae0738db0de08a6fbc01808a5bd702418fe17778..7108bc8235e00b975ec152d1fbf2519f4679b5c6 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -132,7 +132,8 @@ class DomainValidator(object):
             (entries, truncated) = self.ldap.find_entries(filter=filter, base_dn=cn_trust,
                                                           attrs_list=[self.ATTR_TRUSTED_SID, 'dn'])
 
-            return entries
+            result = map (lambda entry: security.dom_sid(entry[1][self.ATTR_TRUSTED_SID][0]), entries)
+            return result
         except errors.NotFound, e:
             return []
 
@@ -144,17 +145,7 @@ class DomainValidator(object):
         # Parse sid string to see if it is really in a SID format
         try:
             test_sid = security.dom_sid(sid)
-        except TypeError:
-            return False
-        (dom, sid_rid) = test_sid.split()
-        sid_dom = str(dom)
-        # Now we have domain prefix of the sid as sid_dom string and can
-        # analyze it against known prefixes
-        if sid_dom.find(security.SID_NT_AUTHORITY) != 0:
-            # Ignore any potential SIDs that are not S-1-5-*
-            return False
-        if sid_dom.find(self.sid) == 0:
-            # A SID from our own domain cannot be treated as trusted domain's SID
+        except TypeError, e:
             return False
         # At this point we have SID_NT_AUTHORITY family SID and really need to
         # check it against prefixes of domain SIDs we trust to
@@ -166,8 +157,11 @@ class DomainValidator(object):
             return False
         # We have non-zero list of trusted domains and have to go through them
         # one by one and check their sids as prefixes
-        for (dn, domaininfo) in self._domains:
-            if sid_dom.find(domaininfo[self.ATTR_TRUSTED_SID][0]) == 0:
+        test_sid_subauths = test_sid.sub_auths
+        for domsid in self._domains:
+            sub_auths = domsid.sub_auths
+            num_auths = min(test_sid.num_auths, domsid.num_auths)
+            if test_sid_subauths[:num_auths] == sub_auths[:num_auths]:
                 return True
         return False
 
@@ -401,8 +395,10 @@ class TrustDomainInstance(object):
         result = retrieve_netlogon_info_2(self,
                                           netlogon.NETLOGON_CONTROL_TC_VERIFY,
                                           another_domain.info['dns_domain'])
-        if (unicode(result.trusted_dc_name)[2:] == another_domain.info['dc'] and
-            result.tc_connection_status == (0, 'WERR_OK')):
+        if (result and (result.flags and netlogon.NETLOGON_VERIFY_STATUS_RETURNED)):
+            # netr_LogonControl2Ex() returns non-None result only if overall call
+            # result was WERR_OK which means verification was correct.
+            # We only check that it was indeed status for verification process
             return True
         return False
 
-- 
1.7.12