Cross realm S4U2Self patches rebased on import-lorikeet-heimdal branch

[Andrew Bartlett]

On Fri, 2018-09-28 at 21:30 +0530, Isaac Boukris via samba-technical
wrote:
> On Tue, Sep 25, 2018 at 2:21 PM Andrew Bartlett <abartlet at samba.org> wrote:
> > 
> > On Mon, 2018-09-24 at 13:43 +0530, Isaac Boukris wrote:
> > > > Note, with new heimdal I somehow get the transitive-check errors
> > > > which I previously only had with transitive trust (with a child
> > > > domain involved).
> > > > 
> > > > See this intringin error below:
> > > > Kerberos: TGS-REQ DC7$@SAMBA2008R2.EXAMPLE.COM from
> > > > ipv4:127.0.0.27:16308 for HOST/dc7.samba2008r2.example.com at SAMBA200
> > > > 8R2.EXAMPLE.COM [canonicalize, renewable, forwardable]
> > > > Kerberos: s4u2self DC7$@SAMBA2008R2.EXAMPLE.COM impersonating Admin
> > > > istrator at ADDOM.SAMBA.EXAMPLE.COM to service HOST/dc7.samba2008r2.ex
> > > > ample.com at SAMBA2008R2.EXAMPLE.COM
> > > > Kerberos: cross-realm SAMBA2008R2.EXAMPLE.COM ->
> > > > SAMBA2008R2.EXAMPLE.COM via [ADDOM.SAMBA.EXAMPLE.COM]
> > > > Kerberos: cross-realm SAMBA2008R2.EXAMPLE.COM ->
> > > > SAMBA2008R2.EXAMPLE.COM: no transit allowed through realm
> > > > ADDOM.SAMBA.EXAMPLE.COM from SAMBA2008R2.EXAMPLE.COM to
> > > > SAMBA2008R2.EXAMPLE.COM
> > > > 
> > > 
> > > I think I figured this error, see attached possible patch which I
> > > want
> > > to submit heimdal upstream (and to replace with it, the
> > > transitive-trust poc commits).
> 
> Actually that patch is wrong. I mean I still think the comment is
> correct, that if client and server realm are the same then even if the
> tgt realm is different it doesn't necessarily mean it is a transit
> realm. However it still may be one (as it will be in case of s4u2self
> with transitive trust).
> I'll leave that alone for now, since as I understood, in samba we
> don't really care about transit check, and may eventually disable it
> altogether.
> 
> > > I had hoped it would solve other transit errors I've seen before my
> > > changes, but alas it hadn't (these seem related to netbios and lower
> > > realm).
> > 
> > OK.
> > 
> > > > Pipeline still running, but I guess there would be some failures:
> > > > https://gitlab.com/samba-team/devel/samba/pipelines/30858709
> > > 
> > > The pipeline failed many krb5 torture tests, I'm looking around to
> > > see
> > > if I can figure out something, and mainly if my changes have
> > > introduced new errors.
> > > 
> > > I think one significant change in cross realm client code between the
> > > two version, is the order of capath vs referral in
> > > _krb5_get_cred_kdc_any() which has changed (likely to break some
> > > torture expectations).
> 
> If I revert that change for testing (see attached), it gets rid of all
> the transit errors I've seen before my changes.
> I think this error (KRB5KDC_ERR_PATH_NOT_ACCEPTED) comes from
> get_cred_kdc_referral() when the server name is short and canonicalize
> flag is off, and therefore was not seen by the caller when we used to
> fallback to capath.
> 
> [old] $ cat selftest_2018-09-23_10\:50.log |grep "KDC Policy rejects
> transited path" |wc -l
> 296
> [new] $ cat selftest_2018-09-28_15\:06.log |grep "KDC Policy rejects
> transited path" |wc -l
> 0
> 
> [old] $ grep "failure:" selftest_2018-09-23_10\:50.log  |wc -l
> 1756
> [new]$ grep "failure:" selftest_2018-09-28_15\:06.log  |wc -l
> 1700
> 
> Assuming we are ok with that change of order of methods (capath vs
> referrals), I'll try to confirm and update the expectation in the
> torture test.

That is the right approach.

> Otherwise, we might want to introduce new flags to better control what
> method to chose, and use those in samba.

If reordering the tests is a nightmare, then do this.  Or we can carry
your patch.  It seems unlikely we will ever get to a pure upstream
Heimdal anyway (I'm having difficultly getting even simple patches
upstream). 

> > Any help you can provide to make these tests pass again would be most
> > appreciated.  It is a big task, but we are close to having it pass
> > 'make test', which is actually pretty amazing given the amount of
> > change.
> 
> It is really amazing, I can tell :)

Thanks!

Andrew Bartlett

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