More forest trust related patches

Tue May 5 15:38:22 MDT 2015

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.

>>>  - 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 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.

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.

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.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150505/6e92c555/attachment.pgp>