[Freeipa-devel] [PATCH] 0201 Add support for an external trust to Active Directory domain

Alexander Bokovoy abokovoy at redhat.com
Tue Jun 7 16:00:18 UTC 2016


On Tue, 07 Jun 2016, Martin Babinsky wrote:
>On 06/06/2016 12:33 PM, Alexander Bokovoy wrote:
>>Hi,
>>
>>this patch adds support for external trust to Active Directory.
>>
>>External trust is a trust that can be created between Active Directory
>>domains that are in different forests or between an Active Directory
>>domain. Since FreeIPA does not support non-Kerberos means of
>>communication, external trust to Windows NT 4.0 or earlier domains is
>>not supported.
>>
>>The external trust is not transitive and can be established to any
>>domain in another forest. This means no access beyond the external
>>domain is possible via the trust link.
>>
>>Resolves: https://fedorahosted.org/freeipa/ticket/5743
>>
>>
>>
>Hi Alexander,
>
>I have a few comments:
>
>1.) there are numerous pep8 errors in the code:
>
>"""
>./ipaserver/dcerpc.py:1044:80: E501 line too long (115 > 79 characters)
>./ipaserver/dcerpc.py:1044:106: E251 unexpected spaces around keyword 
>/ parameter equals
>./ipaserver/dcerpc.py:1044:108: E251 unexpected spaces around keyword 
>/ parameter equals
>./ipaserver/dcerpc.py:1107:80: E501 line too long (110 > 79 characters)
>./ipaserver/dcerpc.py:1108:80: E501 line too long (82 > 79 characters)
>./ipaserver/dcerpc.py:1109:80: E501 line too long (114 > 79 characters)
>./ipaserver/dcerpc.py:1111:80: E501 line too long (91 > 79 characters)
>./ipaserver/dcerpc.py:1272:80: E501 line too long (82 > 79 characters)
>./ipaserver/dcerpc.py:1400:80: E501 line too long (143 > 79 characters)
>./ipaserver/dcerpc.py:1401:80: E501 line too long (88 > 79 characters)
>./ipaserver/plugins/trust.py:66:80: E501 line too long (91 > 79 characters)
>./ipaserver/plugins/trust.py:174:22: E203 whitespace before ':'
>./ipaserver/plugins/trust.py:175:80: E501 line too long (106 > 79 
>characters)
>./ipaserver/plugins/trust.py:201:1: E302 expected 2 blank lines, found 1
>./ipaserver/plugins/trust.py:209:80: E501 line too long (91 > 79 characters)
>./ipaserver/plugins/trust.py:690:9: E124 closing bracket does not 
>match visual indentation
>./ipaserver/plugins/trust.py:694:80: E501 line too long (127 > 79 
>characters)
>"""
>
>Please fix them. You can check whether your commit does not break pep8 
>by running `git show HEAD -U0 | pep8 --diff`.
Thanks for the review.

The lines will have to be long. Sorry, we are not in the age of
80-column terminals anymore. Try to look at the ipaserver/dcerpc.py from
a bigger perspective -- there are currently 224 errors reported by pep8
in this file. Fixing some of them makes little sense -- for example,
pylint hints have to be where they are and that makes lines long, error
messages have to be what they are and changing them to be smaller is
counterproductive.

I'm updated the patch to have most of the lines within 90 columns.

>
>2.)
>
>I have difficulty understanding the following code:
>
>"""
>+        self.__allow_behavior = 0
>
>         domain_validator = DomainValidator(api)
>         self.configured = domain_validator.is_configured()
>
>         if self.configured:
>             self.local_flatname = domain_validator.flatname
>             self.local_dn = domain_validator.dn
>             self.__populate_local_domain()
>
>+    def allow_behavior(self, behavior_set):
>+        if type(behavior_set) is int:
>+           behavior_set = [behavior_set]
>+        if TRUST_JOIN_EXTERNAL in behavior_set:
>+           self.__allow_behavior |= TRUST_JOIN_EXTERNAL
>+
>"""
>I assume this is made like this to accommodate setting some other 
>behavior flags which can pop up in the future (otherwise a simple 
>Boolean to indicate external trust should be enough), int hat case I 
>would propose to rewrite it in this form and document it:
>
>
>"""
>def allow_behavior(self, *flags):
>    """
>    Set the expected trust join behavior
>    :param flags: trust flags to set
>    """
>    for flag in flags:
>        self.__allow_behavior |= flag
>"""
>
>Also, I am not a big fan of doubly underscored methods/variables since 
>they do not 'hide' anything (Python's name mangling can be bypassed 
>with ease anyway). If you want to indicate that the 'allow_behavior' 
>attribute belongs to the internal implementation of the class prefix 
>it with single underscore.
You cannot have a property and a method named the same in the same
class. I changed the code to follow your other suggestion. I kept the
__allow_behavior as it is, though, because there is no difference
between __ and _ semantically to a person who would be reading the code
in question but I keep __ memorized. ;)

>
>3.)
>
>"""
>         if self.remote_domain.info['dns_domain'] != 
>self.remote_domain.info['dns_forest']:
>-            raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'], 
>domain=self.remote_domain.info['dns_domain'])
>+            if not (self.__allow_behavior & TRUST_JOIN_EXTERNAL):
>+                raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'], 
>domain=self.remote_domain.info['dns_domain'])
>
>         if not self.remote_domain.read_only:
>             trustdom_pass = samba.generate_random_password(128, 128)
>             self.get_realmdomains()
>-            self.remote_domain.establish_trust(self.local_domain, 
>trustdom_pass, trust_type)
>-            self.local_domain.establish_trust(self.remote_domain, 
>trustdom_pass, trust_type)
>+            self.remote_domain.establish_trust(self.local_domain, 
>trustdom_pass, trust_type, bool(self.__allow_behavior & 
>TRUST_JOIN_EXTERNAL))
>+            self.local_domain.establish_trust(self.remote_domain, 
>trustdom_pass, trust_type, bool(self.__allow_behavior & 
>TRUST_JOIN_EXTERNAL))
>             # if trust is inbound, we don't need to verify it because 
>AD DC will respond
>             # with WERR_NO_SUCH_DOMAIN -- in only does verification 
>for outbound trusts.
>             ...
>
>"""
>
>Please use a separate variable to store the result of bitwise AND you 
>pass to the methods, e.g.:
>
>"""
>join_as_external = bool(self.__allow_behavior & TRUST_JOIN_EXTERNAL)
>if self.remote_domain.info['dns_domain'] != 
>self.remote_domain.info['dns_forest']:
>    ...
>
>"""
>
>There is enough copy-pasta we have to deal with when fixing bugs, 
>better not add any more of that.
Thanks, changed this part.

>4.)
>
>In trust-find post-callback you can use 
>`attrs.single_value.get('ipanttrustattributes', 0)` to get trust 
>attributes from the LDAP entry (they haven't been converted to dicts 
>yet). It makes code bit more readable. The same can be said for 
>trust-show post-callback.
>
>"""
>   def post_callback(self, ldap, entries, truncated, *args, **options):
>        if options.get('pkey_only', False):
>             return truncated
>
>        for attrs in entries:
>             # Translate ipanttrusttype to trusttype if --raw not used
>             trust_type = attrs.get('ipanttrusttype', [None])[0]
>+            attributes = attrs.get('ipanttrustattributes', [0])[0]
>             if not options.get('raw', False) and trust_type is not None:
>-                attrs['trusttype'] = 
>trust_type_string(attrs['ipanttrusttype'][0])
>+                attrs['trusttype'] = [trust_type_string(trust_type, 
>attributes)]
>                 del attrs['ipanttrusttype']
>+                if attributes:
>+                    del attrs['ipanttrustattributes']
>"""
Updated patch is attached.

-- 
/ Alexander Bokovoy
-------------- next part --------------
From e9e8b4af5bf834aaceb9a7835733bd3641d5f93c Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Mon, 6 Jun 2016 08:55:44 +0300
Subject: [PATCH 1/5] trusts: Add support for an external trust to Active 
 Directory domain

External trust is a trust that can be created between Active Directory
domains that are in different forests or between an Active Directory
domain. Since FreeIPA does not support non-Kerberos means of
communication, external trust to Windows NT 4.0 or earlier domains is
not supported.

The external trust is not transitive and can be established to any
domain in another forest. This means no access beyond the external
domain is possible via the trust link.

Resolves: https://fedorahosted.org/freeipa/ticket/5743
---
 API.txt                    |  3 ++-
 ipaserver/dcerpc.py        | 50 +++++++++++++++++++++++++++----------
 ipaserver/plugins/trust.py | 61 +++++++++++++++++++++++++++++++++++-----------
 3 files changed, 86 insertions(+), 28 deletions(-)

diff --git a/API.txt b/API.txt
index f170930..d5fbc27 100644
--- a/API.txt
+++ b/API.txt
@@ -5208,12 +5208,13 @@ arg: Str('cn', cli_name='name')
 option: Str('version?')
 output: Output('result')
 command: trust_add
-args: 1,14,3
+args: 1,15,3
 arg: Str('cn', cli_name='realm')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Int('base_id?', cli_name='base_id')
 option: Bool('bidirectional?', cli_name='two_way', default=False)
+option: Bool('external?', cli_name='external', default=False)
 option: Int('range_size?', cli_name='range_size')
 option: StrEnum('range_type?', cli_name='range_type', values=[u'ipa-ad-trust-posix', u'ipa-ad-trust'])
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 25ffbfa..5f56643 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -77,6 +77,11 @@ and Samba4 python bindings.
 TRUST_ONEWAY        = 1
 TRUST_BIDIRECTIONAL = 3
 
+# Trust join behavior
+# External trust -- allow creating trust to a non-root domain in the forest
+TRUST_JOIN_EXTERNAL = 1
+
+
 def is_sid_valid(sid):
     try:
         security.dom_sid(sid)
@@ -1037,7 +1042,7 @@ class TrustDomainInstance(object):
             # We can ignore the error here -- setting up name suffix routes may fail
             pass
 
-    def establish_trust(self, another_domain, trustdom_secret, trust_type='bidirectional'):
+    def establish_trust(self, another_domain, trustdom_secret, trust_type='bidirectional', trust_external=False):
         """
         Establishes trust between our and another domain
         Input: another_domain -- instance of TrustDomainInstance, initialized with #retrieve call
@@ -1060,6 +1065,8 @@ class TrustDomainInstance(object):
             info.trust_direction |= lsa.LSA_TRUST_DIRECTION_OUTBOUND
         info.trust_type = lsa.LSA_TRUST_TYPE_UPLEVEL
         info.trust_attributes = 0
+        if trust_external:
+            info.trust_attributes |= lsa.LSA_TRUST_ATTRIBUTE_NON_TRANSITIVE
 
         try:
             dname = lsa.String()
@@ -1096,14 +1103,17 @@ class TrustDomainInstance(object):
             # server as that one doesn't support AES encryption types
             pass
 
-        try:
-            info = self._pipe.QueryTrustedDomainInfo(trustdom_handle, lsa.LSA_TRUSTED_DOMAIN_INFO_INFO_EX)
-            info.trust_attributes |= lsa.LSA_TRUST_ATTRIBUTE_FOREST_TRANSITIVE
-            self._pipe.SetInformationTrustedDomain(trustdom_handle, lsa.LSA_TRUSTED_DOMAIN_INFO_INFO_EX, info)
-        except RuntimeError as e:
-            root_logger.error('unable to set trust to transitive: %s' % (str(e)))
+        if not trust_external:
+            try:
+                info = self._pipe.QueryTrustedDomainInfo(trustdom_handle,
+                                                         lsa.LSA_TRUSTED_DOMAIN_INFO_INFO_EX)
+                info.trust_attributes |= lsa.LSA_TRUST_ATTRIBUTE_FOREST_TRANSITIVE
+                self._pipe.SetInformationTrustedDomain(trustdom_handle,
+                                                       lsa.LSA_TRUSTED_DOMAIN_INFO_INFO_EX, info)
+            except RuntimeError as e:
+                root_logger.error('unable to set trust transitivity status: %s' % (str(e)))
 
-        if self.info['is_pdc']:
+        if self.info['is_pdc'] or trust_external:
             self.update_ftinfo(another_domain)
 
     def verify_trust(self, another_domain):
@@ -1262,6 +1272,7 @@ class TrustDomainJoins(object):
         self.api = api
         self.local_domain = None
         self.remote_domain = None
+        self.__allow_behavior = 0
 
         domain_validator = DomainValidator(api)
         self.configured = domain_validator.is_configured()
@@ -1271,6 +1282,10 @@ class TrustDomainJoins(object):
             self.local_dn = domain_validator.dn
             self.__populate_local_domain()
 
+    def allow_behavior(self, *flags):
+        for f in flags:
+           self.__allow_behavior |= int(f)
+
     def __populate_local_domain(self):
         # Initialize local domain info using kerberos only
         ld = TrustDomainInstance(self.local_flatname)
@@ -1358,14 +1373,19 @@ class TrustDomainJoins(object):
                 realm_passwd
             )
 
+        trust_external = bool(self.__allow_behavior & TRUST_JOIN_EXTERNAL)
         if self.remote_domain.info['dns_domain'] != self.remote_domain.info['dns_forest']:
-            raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'], domain=self.remote_domain.info['dns_domain'])
+            if not trust_external:
+                raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'],
+                                                 domain=self.remote_domain.info['dns_domain'])
 
         if not self.remote_domain.read_only:
             trustdom_pass = samba.generate_random_password(128, 128)
             self.get_realmdomains()
-            self.remote_domain.establish_trust(self.local_domain, trustdom_pass, trust_type)
-            self.local_domain.establish_trust(self.remote_domain, trustdom_pass, trust_type)
+            self.remote_domain.establish_trust(self.local_domain,
+                                               trustdom_pass, trust_type, trust_external)
+            self.local_domain.establish_trust(self.remote_domain,
+                                              trustdom_pass, trust_type, trust_external)
             # if trust is inbound, we don't need to verify it because AD DC will respond
             # with WERR_NO_SUCH_DOMAIN -- in only does verification for outbound trusts.
             result = True
@@ -1381,8 +1401,12 @@ class TrustDomainJoins(object):
         if not(isinstance(self.remote_domain, TrustDomainInstance)):
             self.populate_remote_domain(realm, realm_server, realm_passwd=None)
 
+        trust_external = bool(self.__allow_behavior & TRUST_JOIN_EXTERNAL)
         if self.remote_domain.info['dns_domain'] != self.remote_domain.info['dns_forest']:
-            raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'], domain=self.remote_domain.info['dns_domain'])
+            if not trust_external:
+                raise errors.NotAForestRootError(forest=self.remote_domain.info['dns_forest'],
+                                                 domain=self.remote_domain.info['dns_domain'])
 
-        self.local_domain.establish_trust(self.remote_domain, trustdom_passwd, trust_type)
+        self.local_domain.establish_trust(self.remote_domain,
+                                          trustdom_passwd, trust_type, trust_external)
         return dict(local=self.local_domain, remote=self.remote_domain, verified=False)
diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py
index ee0ab5d..744be93 100644
--- a/ipaserver/plugins/trust.py
+++ b/ipaserver/plugins/trust.py
@@ -62,8 +62,10 @@ except Exception as e:
 
 if api.env.in_server and api.env.context in ['lite', 'server']:
     try:
-        import ipaserver.dcerpc #pylint: disable=F0401
-        from ipaserver.dcerpc import TRUST_ONEWAY, TRUST_BIDIRECTIONAL
+        import ipaserver.dcerpc  # pylint: disable=F0401
+        from ipaserver.dcerpc import (TRUST_ONEWAY,
+                                      TRUST_BIDIRECTIONAL,
+                                      TRUST_JOIN_EXTERNAL)
         import dbus
         import dbus.mainloop.glib
         _bindings_installed = True
@@ -162,11 +164,18 @@ trust_output_params = (
         label=_('Trust type')),
     Str('truststatus',
         label=_('Trust status')),
+    Str('ipantadditionalsuffixes*',
+        label=_('UPN suffixes')),
 )
 
+# Trust type is a combination of ipanttrusttype and ipanttrustattributes
+# We shift trust attributes by 3 bits to left so bit 0 becomes bit 3 and
+# 2+(1 << 3) becomes 10.
 _trust_type_dict = {1 : _('Non-Active Directory domain'),
                     2 : _('Active Directory domain'),
-                    3 : _('RFC4120-compliant Kerberos realm')}
+                    3 : _('RFC4120-compliant Kerberos realm'),
+                    10: _('Non-transitive external trust to a domain in another Active Directory forest')}
+
 _trust_direction_dict = {1 : _('Trusting forest'),
                          2 : _('Trusted forest'),
                          3 : _('Two-way trust')}
@@ -189,14 +198,17 @@ DBUS_IFACE_TRUST = 'com.redhat.idm.trust'
 CRED_STYLE_SAMBA = 1
 CRED_STYLE_KERBEROS = 2
 
-def trust_type_string(level):
+LSA_TRUST_ATTRIBUTE_NON_TRANSITIVE = 0x00000001
+
+def trust_type_string(level, attrs):
     """
     Returns a string representing a type of the trust. The original field is an enum:
       LSA_TRUST_TYPE_DOWNLEVEL  = 0x00000001,
       LSA_TRUST_TYPE_UPLEVEL    = 0x00000002,
       LSA_TRUST_TYPE_MIT        = 0x00000003
     """
-    string = _trust_type_dict.get(int(level), _trust_type_dict_unknown)
+    transitive = int(attrs) & LSA_TRUST_ATTRIBUTE_NON_TRANSITIVE
+    string = _trust_type_dict.get(int(level) | (transitive << 3), _trust_type_dict_unknown)
     return unicode(string)
 
 def trust_direction_string(level):
@@ -677,6 +689,12 @@ sides.
              doc=(_('Establish bi-directional trust. By default trust is inbound one-way only.')),
              default=False,
         ),
+        Bool('external?',
+             label=_('External trust'),
+             cli_name='external',
+             doc=(_('Establish external trust to a domain in another forest. The trust is not transitive beyond the domain.')),
+             default=False,
+        ),
     )
 
     msg_summary = _('Added Active Directory trust for realm "%(value)s"')
@@ -735,12 +753,15 @@ sides.
                 fetch_trusted_domains_over_dbus(self.api, self.log, result['value'])
 
         # Format the output into human-readable values
+        attributes = int(result['result'].get('ipanttrustattributes', [0])[0])
         result['result']['trusttype'] = [trust_type_string(
-            result['result']['ipanttrusttype'][0])]
+            result['result']['ipanttrusttype'][0], attributes)]
         result['result']['trustdirection'] = [trust_direction_string(
             result['result']['ipanttrustdirection'][0])]
         result['result']['truststatus'] = [trust_status_string(
             result['verified'])]
+        if attributes:
+            result['result'].pop('ipanttrustattributes', None)
 
         del result['verified']
         result['result'].pop('ipanttrustauthoutgoing', None)
@@ -929,6 +950,11 @@ sides.
         trust_type = TRUST_ONEWAY
         if options.get('bidirectional', False):
             trust_type = TRUST_BIDIRECTIONAL
+
+        # If we are forced to establish external trust, allow it
+        if options.get('external', False):
+            self.trustinstance.allow_behavior(TRUST_JOIN_EXTERNAL)
+
         # 1. Full access to the remote domain. Use admin credentials and
         # generate random trustdom password to do work on both sides
         if full_join:
@@ -1033,7 +1059,7 @@ class trust_mod(LDAPUpdate):
 class trust_find(LDAPSearch):
     __doc__ = _('Search for trusts.')
     has_output_params = LDAPSearch.has_output_params + trust_output_params +\
-                        (Str('ipanttrusttype'),)
+                        (Str('ipanttrusttype'), Str('ipanttrustattributes'))
 
     msg_summary = ngettext(
         '%(count)d trust matched', '%(count)d trusts matched', 0
@@ -1043,7 +1069,7 @@ class trust_find(LDAPSearch):
     # search needs to be done on a sub-tree scope
     def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, **options):
         # list only trust, not trust domains
-        trust_filter = '(ipaNTTrustPartner=*)'
+        trust_filter = '(&(ipaNTTrustPartner=*)(&(objectclass=ipaIDObject)(objectclass=ipaNTTrustedDomain)))'
         filter = ldap.combine_filters((filters, trust_filter), rules=ldap.MATCH_ALL)
         return (filter, base_dn, ldap.SCOPE_SUBTREE)
 
@@ -1060,10 +1086,13 @@ class trust_find(LDAPSearch):
 
         for attrs in entries:
             # Translate ipanttrusttype to trusttype if --raw not used
-            trust_type = attrs.get('ipanttrusttype', [None])[0]
+            trust_type = attrs.single_value.get('ipanttrusttype', None)
+            attributes = attrs.single_value.get('ipanttrustattributes', 0)
             if not options.get('raw', False) and trust_type is not None:
-                attrs['trusttype'] = trust_type_string(attrs['ipanttrusttype'][0])
+                attrs['trusttype'] = [trust_type_string(trust_type, attributes)]
                 del attrs['ipanttrusttype']
+                if attributes:
+                    del attrs['ipanttrustattributes']
 
         return truncated
 
@@ -1071,7 +1100,7 @@ class trust_find(LDAPSearch):
 class trust_show(LDAPRetrieve):
     __doc__ = _('Display information about a trust.')
     has_output_params = LDAPRetrieve.has_output_params + trust_output_params +\
-                        (Str('ipanttrusttype'), Str('ipanttrustdirection'))
+                        (Str('ipanttrusttype'), Str('ipanttrustdirection'), Str('ipanttrustattributes'))
 
     def execute(self, *keys, **options):
         result = super(trust_show, self).execute(*keys, **options)
@@ -1088,16 +1117,20 @@ class trust_show(LDAPRetrieve):
         # if --raw not used
 
         if not options.get('raw', False):
-            trust_type = entry_attrs.get('ipanttrusttype', [None])[0]
+            trust_type = entry_attrs.single_value.get('ipanttrusttype', None)
+            attributes = entry_attrs.single_value.get('ipanttrustattributes', 0)
             if trust_type is not None:
-                entry_attrs['trusttype'] = trust_type_string(trust_type)
+                entry_attrs['trusttype'] = [trust_type_string(trust_type, attributes)]
                 del entry_attrs['ipanttrusttype']
 
-            dir_str = entry_attrs.get('ipanttrustdirection', [None])[0]
+            dir_str = entry_attrs.single_value.get('ipanttrustdirection', None)
             if dir_str is not None:
                 entry_attrs['trustdirection'] = [trust_direction_string(dir_str)]
                 del entry_attrs['ipanttrustdirection']
 
+            if attributes:
+                del entry_attrs['ipanttrustattributes']
+
         return dn
 
 
-- 
2.7.4



More information about the Freeipa-devel mailing list