[Freeipa-devel] [PATCH] 0118 add support for subdomains

[Sumit Bose]

On Wed, Oct 02, 2013 at 10:31:21PM +0200, Tomas Babej wrote:
> On 10/01/2013 05:15 PM, Alexander Bokovoy wrote:
> >On Mon, 30 Sep 2013, Alexander Bokovoy wrote:
> >>On Mon, 30 Sep 2013, Tomas Babej wrote:
> >>>On 09/28/2013 10:01 PM, Alexander Bokovoy wrote:
> >>>>On Fri, 27 Sep 2013, Sumit Bose wrote:
> >>>>>On Fri, Sep 27, 2013 at 03:53:08PM +0300, Alexander Bokovoy wrote:
> >>>>>>On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
> >>>>>>>On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
> >>>>>>>>On Mon, 23 Sep 2013, Alexander Bokovoy wrote:
> >>>>>>>>>On Mon, 23 Sep 2013, Martin Kosek wrote:
> >>>>>>>>>>>>However, we don't have trust type available so it needs
> >>>>>>to discovered
> >>>>>>>>>>>>every time. This doesn't play well with the framework, it
> >>>>>>is simply not
> >>>>>>>>>>>>expecting dynamic containers.
> >>>>>>>>>>>
> >>>>>>>>>>>This doesn't sound like a big obstacle to me. Right now
> >>>>>>the trust_type lookup
> >>>>>>>>>>>is done in trust_show.execute() for some reason, which is
> >>>>>>not the best place to
> >>>>>>>>>>>do it IMHO. Doing it in trust.get_dn() instead should
> >>>>>>simplify things enough to
> >>>>>>>>>>>make parent_object work.
> >>>>>>>>>>
> >>>>>>>>>>Yup, get_dn() is the method where object DN lookup should
> >>>>>>be done. See for
> >>>>>>>>>>example host.py plugin get_dn method, we also do a dynamic
> >>>>>>lookup for correct
> >>>>>>>>>>host name.
> >>>>>>>>>I'll see if that would work.
> >>>>>>>>>
> >>>>>>>>>>the best way to implement dynamic DN gathering is the
> >>>>>>get_dn() method. That
> >>>>>>>>>>way, it could be implemented in one place and all commands
> >>>>>>could take advantage
> >>>>>>>>>>of it instead of re-implementing it several times in
> >>>>>>pre_callback - this is
> >>>>>>>>>>just hackish.
> >>>>>>>>>I'd suggest you look into the code. The commands use
> >>>>>>pre_callback for a
> >>>>>>>>>different purpose than implementing dynamic DN gathering.
> >>>>>>>>>
> >>>>>>>>>>I think it would have been very useful to have a design
> >>>>>>page before sending a
> >>>>>>>>>>patch. It is then easier to make design decisions without
> >>>>>>having to dig into
> >>>>>>>>>>the patch.
> >>>>>>>>>The design page is there for long time:
> >>>>>>>>>http://www.freeipa.org/page/V3/Transitive_Trusts
> >>>>>>>>Ok, here is new version of the patch and updated
> >>>>>>>>version of my 0117
> >>>>>>>>patch as Sumit noticed I've sent wrong version.
> >>>>>>>Ok, here is updated 0118 which fixes API.txt change for
> >>>>>>trustdomain_add
> >>>>>>>-- I renamed trustdomain_create to trustdomain_add but
> >>>>>>>forgot to rerun
> >>>>>>>makeapi.
> >>>>>>New edition attached for all subdomain-related patches:
> >>>>>
> >>>>>I did some tests and all is working as expected.
> >>>>>
> >>>>>>
> >>>>>>freeipa-abbra-0117-ipaserver-dcerpc.py-populate-forest-trust-informatio-3.patch
> >>>>>>
> >>>>>>
> >>>>>>Use realmdomains to report name suffix routes at the
> >>>>>>time we establish trust
> >>>>>>
> >>>>>>freeipa-abbra-0118-trusts-support-subdomains-in-a-forest-3.patch
> >>>>>>Introduce trustdomain-* commands to fetch list of domains associated
> >>>>>>with a forest trust and allow filtering them off
> >>>>>
> >>>>>We talked on irc that ipaNTSupportedEncryptionTypes in the filter
> >>>>>for the trusted domains should be replace by a different attribute.
> >>>>>Because of an error in ipasam the
> >>>>>ipaNTSupportedEncryptionTypes is only
> >>>>>set in recent versions and might not be present in the
> >>>>>directory trees of
> >>>>>older versions.
> >>>>Fixed in the attached patch 0118 version 4.
> >>>>
> >>>>Also attached first attempt to implement transiting through trusted
> >>>>domains, as patch 0123. In this patch we grant transition only if all
> >>>>three realms (client, transited realm, and server realm) match any of
> >>>>our trusted domains and our domain. This is probably a bit
> >>>>wider but it
> >>>>worked for me bidirectionally, from a child domain to a
> >>>>service in IPA,
> >>>>and from IPA realm to a service in a child domain of a forest trust.
> >>>>
> >>>>
> >>>>
> >>>>_______________________________________________
> >>>>Freeipa-devel mailing list
> >>>>Freeipa-devel at redhat.com
> >>>>https://www.redhat.com/mailman/listinfo/freeipa-devel
> >>>
> >>>Hi,
> >>>
> >>>here are my comments:
> >>>
> >>>*PATCH 117*
> >>>
> >>>+    def get_realmdomains(self):
> >>>+        """
> >>>+        Generate list of records for forest trust information about
> >>>+        our realm domains. Note that the list generated currently
> >>>+        includes only top level domains, no exclusion
> >>>domains, and no TDO objects
> >>>+        as we handle the latter in a separte way
> >>>+        """
> >>>
> >>>A nitpick typo: separte -> separate.
> >>Fixed.
> >>
> >>>
> >>>Also, there's trailing whitespace in the patch:
> >>>
> >>>Applying: ipaserver/dcerpc.py: populate forest trust
> >>>information using realmdomains
> >>>/home/tbabej/dev/freeipa/.git/rebase-apply/patch:62: trailing
> >>>whitespace.
> >>>      Only top level name and top level name exclusions are
> >>>handled here.
> >>>/home/tbabej/dev/freeipa/.git/rebase-apply/patch:174: trailing
> >>>whitespace.
> >>>
> >>>warning: 2 lines add whitespace errors.
> >>Fixed.
> >>
> >>>
> >>>
> >>>*PATCH 119*
> >>>
> >>>We also need to change the frontend tests that cover this
> >>>functionality:
> >>>
> >>>======================================================================
> >>>FAIL: Test the ``ipalib.frontend.Command.args`` instance attribute.
> >>>----------------------------------------------------------------------
> >>>Traceback (most recent call last):
> >>>File "/usr/lib/python2.7/site-packages/nose/case.py", line
> >>>197, in runTest
> >>>  self.test(*self.arg)
> >>>File
> >>>"/home/tbabej/dev/freeipa/ipatests/test_ipalib/test_frontend.py",
> >>>line 283, in test_args
> >>>  assert str(e) == 'arg2: required argument after optional'
> >>>AssertionError
> >>>
> >>>See ipatests/test_ipalib/test_frontend.py, line 281:
> >>>
> >>>      # Test ValueError, required after optional:
> >>>      e = raises(ValueError, self.get_instance, args=('arg1?', 'arg2'))
> >>>      assert str(e) == 'arg2: required argument after optional'
> >>Ok, will fix. This patch is not essential, of course, so we can decide
> >>what to do with it later.
> >>
> >>>
> >>>
> >>>*PATCH 120*
> >>>
> >>>When I try to add a trust, I get internal error:
> >>>
> >>>echo $AD_PASSWORD | ipa trust-add --type=ad $AD_DOMAIN --admin
> >>>Administrator --password
> >>>
> >>>[Wed Sep 25 10:28:53.978664 2013] [:error] [pid 7905] ipa:
> >>>ERROR: non-public: IndexError: tuple index out of range
> >>>[Wed Sep 25 10:28:53.978702 2013] [:error] [pid 7905]
> >>>Traceback (most recent call last):
> >>>[Wed Sep 25 10:28:53.978708 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
> >>>line 333, in wsgi_execute
> >>>[Wed Sep 25 10:28:53.978713 2013] [:error] [pid 7905] result =
> >>>self.Command[name](*args, **options)
> >>>[Wed Sep 25 10:28:53.978720 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
> >>>436, in __call__
> >>>[Wed Sep 25 10:28:53.978725 2013] [:error] [pid 7905]     ret
> >>>= self.run(*args, **options)
> >>>[Wed Sep 25 10:28:53.978730 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
> >>>755, in run
> >>>[Wed Sep 25 10:28:53.978734 2013] [:error] [pid 7905] result =
> >>>self.execute(*args, **options)
> >>>[Wed Sep 25 10:28:53.978739 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipalib/plugins/trust.py",
> >>>line 338, in execute
> >>>[Wed Sep 25 10:28:53.978744 2013] [:error] [pid 7905]
> >>>self.add_range(range_name, dom_sid, *keys, **options)
> >>>[Wed Sep 25 10:28:53.978748 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipalib/plugins/trust.py",
> >>>line 549, in add_range
> >>>[Wed Sep 25 10:28:53.978755 2013] [:error] [pid 7905] quiet=True)
> >>>[Wed Sep 25 10:28:53.978759 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipaserver/dcerpc.py", line
> >>>507, in search_in_dc
> >>>[Wed Sep 25 10:28:53.978764 2013] [:error] [pid 7905]     info
> >>>= self.__retrieve_trusted_domain_gc_list(domain)
> >>>[Wed Sep 25 10:28:53.978769 2013] [:error] [pid 7905]   File
> >>>"/usr/lib/python2.7/site-packages/ipaserver/dcerpc.py", line
> >>>595, in __retrieve_trusted_domain_gc_list
> >>>[Wed Sep 25 10:28:53.978774 2013] [:error] [pid 7905]
> >>>info['auth'] = self._domains[domain][2]
> >>>[Wed Sep 25 10:28:53.978778 2013] [:error] [pid 7905]
> >>>IndexError: tuple index out of range
> >>>[Wed Sep 25 10:28:53.979248 2013] [:error] [pid 7905] ipa:
> >>>INFO: admin at DOM006.TBAD.IPA.COM: trust_add(u'tbad.ipa.com',
> >>>trust_type=u'ad', realm_admin=u'Administrator',
> >>>realm_passwd=u'********', all=False, raw=False,
> >>>version=u'2.65'): IndexError
> >>>
> >>>I think we need to do the following changes here:
> >>>
> >>>diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
> >>>index fa5c449..4ac0a5f 100644
> >>>--- a/ipaserver/dcerpc.py
> >>>+++ b/ipaserver/dcerpc.py
> >>>@@ -565,7 +565,6 @@ class DomainValidator(object):
> >>>       Returns dictionary with following keys
> >>>            name       -- NetBIOS name of the trusted domain
> >>>            dns_domain -- DNS name of the trusted domain
> >>>-             auth       -- encrypted credentials for trusted
> >>>domain account
> >>>            gc         -- array of tuples (server, port) for
> >>>Global Catalog
> >>>       """
> >>>       if domain in self._info:
> >>>@@ -592,7 +591,6 @@ class DomainValidator(object):
> >>>           self._domains = self.get_trusted_domains()
> >>>
> >>>       info = dict()
> >>>-        info['auth'] = self._domains[domain][2]
> >>>       servers = []
> >>>
> >>>       if result:
> >>>
> >>>After applying this fix, I get:
> >>>
> >>>tbabej at vm-006 freeipa]$ echo $AD_PASSWORD | ipa trust-add
> >>>--type=ad $AD_DOMAIN --admin Administrator --password
> >>>ipa: ERROR: CIFS server communication error: code "-1073741811",
> >>>                message "Unexpected information received"
> >>>(both may be "None")
> >>>
> >>>I was unable to track this one down in a reasonable timeframe,
> >>>I suggest we continue on IRC.
> >>I've fixed this. At the time we establish trust, there could be a race
> >>condition when cross-realm TGT is not yet ready so we cannot rely on it
> >>when fetching domains. As we have administrator's credentials here, I've
> >>added use of them in addition to Kerberos.
> >>
> >>
> >>I'll send new patchset shortly.
> >New patchset is attached.
> >
> >1. Added test update for ipalib/frontend.py changes
> >2. Used LDAPQuery as base for trustdomain_enable|disable commands as
> >  suggested by Honza.
> >3. Fixed issues with removal of trust account password authentication
> >4. Added support to use AD administrator credentials when fetching
> >  subdomains information when we establish trust as Kerberos will not
> >  be available for cross-realm operations yet.
> >5. Patch 0123 is not part of the patchset and should not be committed,
> >  we will discuss exact semantics of transition checks with MIT
> >  Kerberos upstream first.
> >6. Fixed few error paths and dead-end cases like attempt to disable root
> >  domain of the trust (renders trust dead) or enabling it (it is always
> >  enabled).
> >7. Made clear that deleting root domain of the trust is not possible,
> >  use trust-del instead.
> >8. Removed whitespaces where saw.
> >
> >
> >
> 
> Thanks!
> 
> This fixes most of the issues I had.
> 
> To summarize, two issues from the today's functional testing we
> already discussed on IRC:
> 
> 1.) The blacklisting for the child domain does not work (it works
> fine for the root domain).
> Thus, ipa trustdomain-disable for the child domain does not reject
> access to the IPA's resources:
> 
> [tbabej at vm-147 labtool]$ ipa trustdomain-disable
> tbad.idm.lab.eng.brq.redhat.com
> child.tbad.idm.lab.eng.brq.redhat.com
> ------------------------------------------------------------------------------------------------------------------------------------
> Domain child.tbad.idm.lab.eng.brq.redhat.com of trust
> tbad.idm.lab.eng.brq.redhat.com is already not allowed to access IPA
> resources
> ------------------------------------------------------------------------------------------------------------------------------------
> [tbabej at vm-147 labtool]$ kdestroy
> [tbabej at vm-147 labtool]$ kvno -S ldap `hostname`
> kvno: Credentials cache file '/run/user/536/krb5cc/tkt1sLaOS' not
> found while getting client principal name
> [tbabej at vm-147 labtool]$ kinit
> Administrator at CHILD.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
> Password for Administrator at CHILD.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM:
> [tbabej at vm-147 labtool]$ klist
> Ticket cache: DIR::/run/user/536/krb5cc/tktS7Bkhj
> Default principal: Administrator at CHILD.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
> 
> Valid starting       Expires              Service principal
> 10/02/2013 21:28:52  10/03/2013 07:28:52 krbtgt/CHILD.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM at CHILD.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>         renew until 10/03/2013 21:28:46
> [tbabej at vm-147 labtool]$ kvno -S ldap `hostname`
> ldap/vm-147.dom147.tbad.idm.lab.eng.brq.redhat.com at DOM147.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM:
> kvno = 2
> 
> We should have been denied access here.
> 
> 2.) The trust-fetch-domains has somewhat confusing options:
> 
> [tbabej at vm-147 labtool]$ ipa trust-fetch-domains
> tbad.idm.lab.eng.brq.redhat.com --help
> Usage: ipa [global-options] trust-fetch-domains REALM [options]
> 
> Refresh list of the domains associated with the trust
> Options:
>   -h, --help  show this help message and exit
>   --rights    Display the access rights of this entry (requires --all). See
>               ipa man page for details.
>   --all       Retrieve and print all attributes from the server. Affects
>               command output.
>   --raw       Print entries as stored on the server. Only affects output
>               format.
> 
> 
> Please note that I did not test with more than 1 subdomain, since I
> do not have more ADs available.
> 

I have done some testing as well and the patches are working as expected
except the trustdomain-disable issue Tomas mentioned. But I think it
would be sufficient to add a comment to the release notes and fix this
with the next release to not delay this release anymore.

The patches are also working for trusts which were added with older
releases. So ACK from my side for the functional part.

bye,
Sumit
> -- 
> Tomas Babej
> Associate Software Engeneer | Red Hat | Identity Management
> RHCE | Brno Site | IRC: tbabej | freeipa.org
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel