On 03/13/2013 05:23 PM, Martin Kosek wrote:
On 03/13/2013 09:50 AM, Tomas Babej wrote:
On Wed 13 Mar 2013 09:47:09 AM CET, Tomas Babej wrote:
Hi,

SID validation in idrange.py now enforces exact match on SIDs, thus
one can no longer use SID of an object in a trusted domain as a
trusted domain SID.

https://fedorahosted.org/freeipa/ticket/3432

Tomas


_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Just renamed the patch filename to follow the convention.

Tomas

I do not think that the debug message is needed:

+            root_logger.error('No trusted domain with given SID found, '
+                              'listing SIDS for all the trusted domains:')
+            for domain in self._domains:
+                root_logger.error('SID: %s' % self._domains[domain][1])

User will not see it anyway and he can easily get list of SIDs/domains with
"ipa trust-find".

Otherwise the patch looks and works fine. I would just consider renaming the
method from is_trusted_sid_valid_domain to is_trusted_domain_sid_valid. Sounds
better to me, but I have no strong feelings about that.

Martin
Both fixed.

Tomas

>From ee475ccbf0a7849bd8fbd9dba4f79a1b2880f097 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 6 Mar 2013 12:17:28 +0100
Subject: [PATCH] Enforce exact SID match when adding or modifying a ID range

SID validation in idrange.py now enforces exact match on SIDs, thus
one can no longer use SID of an object in a trusted domain as a
trusted domain SID.

https://fedorahosted.org/freeipa/ticket/3432
---
 ipalib/plugins/idrange.py |  2 +-
 ipaserver/dcerpc.py       | 50 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index d8989327a24af2d4aa278a215d934b9ca0fab87b..54f6fbb3e19b9aa01dfde2a8d0c5da4498632386 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -289,7 +289,7 @@ class idrange(LDAPObject):
 
         domain_validator = self.get_domain_validator()
 
-        if not domain_validator.is_trusted_sid_valid(sid):
+        if not domain_validator.is_trusted_domain_sid_valid(sid):
             raise errors.ValidationError(name='domain SID',
                   error=_('SID is not recognized as a valid SID for a '
                           'trusted domain'))
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index b8f83e9a479d2d68cac46000500ddb1051251a22..6253c44fec786572e56883c42b9de7de02b1c352 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -183,37 +183,53 @@ class DomainValidator(object):
         except errors.NotFound, e:
             return []
 
-    def get_domain_by_sid(self, sid):
+    def get_domain_by_sid(self, sid, exact_match=False):
         if not self.domain:
             # our domain is not configured or self.is_configured() never run
             # reject SIDs as we can't check correctness of them
             raise errors.ValidationError(name='sid',
                   error=_('domain is not configured'))
+
         # Parse sid string to see if it is really in a SID format
         try:
             test_sid = security.dom_sid(sid)
-        except TypeError, e:
+        except TypeError:
             raise errors.ValidationError(name='sid',
                   error=_('SID is not valid'))
+
         # At this point we have SID_NT_AUTHORITY family SID and really need to
         # check it against prefixes of domain SIDs we trust to
         if not self._domains:
             self._domains = self.get_trusted_domains()
         if len(self._domains) == 0:
             # Our domain is configured but no trusted domains are configured
-            # This means we can't check the correctness of a trusted domain SIDs
+            # This means we can't check the correctness of a trusted
+            # domain SIDs
             raise errors.ValidationError(name='sid',
                   error=_('no trusted domain is configured'))
-        # We have non-zero list of trusted domains and have to go through them
-        # one by one and check their sids as prefixes
-        test_sid_subauths = test_sid.sub_auths
-        for domain in self._domains:
-            domsid = self._domains[domain][1]
-            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 domain
-        raise errors.NotFound(reason=_('SID does not match any trusted domain'))
+
+        # We have non-zero list of trusted domains and have to go through
+        # them one by one and check their sids as prefixes / exact match
+        # depending on the value of exact_match flag
+        if exact_match:
+            # check exact match of sids
+            for domain in self._domains:
+                if sid == str(self._domains[domain][1]):
+                    return domain
+
+            raise errors.NotFound(reason=_("SID does not match exactly"
+                                           "with any trusted domain's SID"))
+        else:
+            # check as prefixes
+            test_sid_subauths = test_sid.sub_auths
+            for domain in self._domains:
+                domsid = self._domains[domain][1]
+                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 domain
+            raise errors.NotFound(reason=_('SID does not match any '
+                                           'trusted domain'))
 
     def is_trusted_sid_valid(self, sid):
         try:
@@ -223,6 +239,14 @@ class DomainValidator(object):
         else:
             return True
 
+    def is_trusted_domain_sid_valid(self, sid):
+        try:
+            self.get_domain_by_sid(sid, exact_match=True)
+        except (errors.ValidationError, errors.NotFound):
+            return False
+        else:
+            return True
+
     def get_sid_from_domain_name(self, name):
         """Returns binary representation of SID for the trusted domain name
            or None if name is not in the list of trusted domains."""
-- 
1.7.11.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to