More forest trust related patches

Sat May 9 01:21:47 MDT 2015

On Tue, 2015-05-05 at 23:38 +0200, Stefan (metze) Metzmacher wrote:
> Am 05.05.2015 um 23:08 schrieb Andrew Bartlett:
> > On Tue, 2015-05-05 at 22:41 +0200, Stefan (metze) Metzmacher wrote:
> >> Hi Andrew,
> >>
> >>>> I moved a lot more stuff to the -ok branch (Note I also changed fixed some
> >>>> of the dsdb_trust_* helper functions compared to the last patchset!)
> >>>>
> >>>> It passed autobuild a few times and it's ready for master from my site.
> >>>>
> >>>> Note that samba-tool domain trust create needs to generate a true
> >>>> utf8 based password if --no-aes-keys is given, this is required
> >>>> because our kerberos client code can't handle random utf16munged passwords
> >>>> for arcfour-hmac-md5 pre-auth yet.
> >>>>
> >>>> However there're a few TODO's in the remaining patches.
> >>>> It's mainly related to bug #11130, where we should allow
> >>>> COMPUTERNAME at REALM and map it to COMPUTERNAME$@REALM.
> >>>> The same applies also for trust accounts (I guess it's just based on the
> >>>> '$').
> >>>> It's allowed as a client and also as a service principal.
> >>>> I added some tests for it and hacked a mostly working (but ugly
> >>>> implementation),
> >>>> Andrew maybe you can work out a better fix :-)
> >>>>
> >>>> Note that winbindd uses MYDOMAIN at OTHERREALM for kinit and generates some
> >>>> warnings
> >>>> without the fix for bug #11130, but it still work fine.
> >>>>
> >>>> Please review and push the -ok patches.
> >>>
> >>> This is really, really good.  The only concern I still have is around
> >>> testing.  We need tests that 
> >>> - walk over all the new samba-tool domain commands.  That is important
> >>> because otherwise we won't even notice if we break them when trying
> >>> python3 upgrades, or other sweeping changes. 
> >>
> >> I guess this is only possible for the non changing commands
> >> for the others we need two domains.
> >>
> >>> - specifically test for the referral shown by behaviour
> >>> HDB_ERR_WRONG_REALM.  This is important because we will soon need to
> >>> update Heimdal, and folks like Debian combine Samba with untested
> >>> upstream versions.
> >>
> >> Ok, I'll see what I can do here.
> > 
> > We also need a test for "heimdal:kdc: generic support for 3part
> > servicePrincipalNames" for similar reasons.
> > 
> > https://git.samba.org/?p=metze/samba/wip.git;a=blobdiff;f=source4/heimdal/kdc/krb5tgs.c;h=ca589e87fa0e5e49c5af4dd9f5e7bafe6f6ddb15;hp=45681775f88f0664174081caf64ac1d735c71df9;hb=3e4deb26d85aabffe389face85adb5d3add4b747;hpb=d32ce4282112888dc2e70f5ae322a2f45c4de652
> 
> actually we could also remove support for 3part principal completely
> as that code path should not be reached.
> The backend returns HDB_ERR_WRONG_REALM as indication for a referral ticket.

OK.  We need to get that Heimdal patch, and the others (removing the
referral handling that didn't make it out of RFC draft) upstream. 

> >>>  - test for (the ban on) changing the trust password over LDAP
> >>
> >> Ok.
> >>
> >>>  - test for listing local groups on the AD DC
> >>
> >> What do you want specifically here?
> >> I think we already test enumerating all groups including validation.
> >>
> >>>  - test different KVNO values on trusts
> >>
> >> What do you mean here exactly?
> >> Changing the password a few times?
> > 
> > Both that, and simply asking for a ticket to an invalid kvno, and
> > checking which password it decrypted.  I'm wanting this code tested in
> > particular:
> > https://git.samba.org/?p=metze/samba/wip.git;a=commit;h=4ca7b60a937242b180bc08f1d7ab8736e8840025
> > 
> > The test probably belongs in the lsa.forest-trust test we discussed a
> > few weeks ago, using the approach of the new krb5.kdc tests.  I realise
> > I'll probably need to help you out with some logistics here.
> 
> How do you want to do that?
> Such a test would typically run against just one KDC.

We can run tests against multiple KDCs if we need to, but I don't think
we need to yet.

> We would need two domains (both with a running kdc)
> and we would need to setup TRUST_AUTH_TYPE_VERSION explicit and
> different on both ends
> in order to test that. But we can't use two environments which already
> trust each other
> and would run other trust tests.

Can't we construct the ticket and request manually if it comes to it?  

In the end, on the wire this is 'just' a special TGS-REQ with a kvno
specified, isn't it?

> I also guess our kvno logic isn't correct generically, we may need to
> return all
> keys (previous and current) and don't provide a kvno for the code path where
> we search for an decryption key. But that problem is also present in the
> non trust case,
> so I'd defer such a change, which might also require core heimdal changes.

Yes, that would. 

> But testing such things is very difficult as it means we might require a
> special
> kdc which would return faked values in a AS-REP and TGS-REP in order to
> verify the behavior
> in the TGS-REQ processing.

Or you use a normal KDC, and then just change the KNVO values :-)

That is the real beauty of the approach I've been using in the krb5.kdc
tests, we decode the ticket, check and if required change values, and
re-encode.  The kvno isn't in the encrypted part, and the KDC protocol
isn't protected by AEAD or similar. 

> The above change simply fixes a problem which didn't happen before
> https://git.samba.org/?p=metze/samba/wip.git;a=object;h=6f8b868a29fe47a3b589616fde97099829933ce0
> 
> >>>  - test the new --local-dc (special_name) handling in Credentials
> >>
> >> Ok.
> >>
> >>> I realise that some of this is tested in integration tests, but I'm
> >>> starting to insist on unit tests (like the great work on the $ removal
> >>> stuff) for KDC changes.  The other issue with the integration tests is
> >>> that a number of tests (validation, namespaces) are being done in the
> >>> environment creation, when these should be done as distinct unit tests. 
> >>
> >> Ok.
> > 
> > Thank you so much for your understanding on this.  
> > 
> >>> I do realise I'm asking for a lot of work, and I'm happy to help on
> >>> this, either between now and SambaXP, or at SambaXP, so we get this done
> >>> right. 
> >>
> >> It would be cool if you could work on a proper fix for bug #11130.
> > 
> > I'll see what I can do.  
> 
> Thanks!
> 
> metze
> 

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba