Heimdal upgrade, really happening this time

[Andrew Bartlett]

On Mon, 2021-08-09 at 10:53 +0200, Stefan Metzmacher wrote:
> Am 09.08.21 um 03:37 schrieb Andrew Bartlett:
> > On Mon, 2021-08-09 at 11:12 +1200, Andrew Bartlett via samba-
> > technical
> > wrote:
> > > On Fri, 2021-07-09 at 22:29 +1200, Andrew Bartlett via samba-
> > > technical
> > > wrote:
> > > > We now have a mostly-working branch of current Heimdal on
> > > > current
> > > > Samba, compiling on all our supported system, which is pretty
> > > > impressive.
> > > 
> > > I just wanted to wrap back to the list with an update.  Thanks to
> > > some
> > > great work with Luke Howard recently, host of our pull requests
> > > with
> > > Heimdal have either been merged or will be shortly (as in, I made
> > > the
> > > requested changes and expect them to be accepted).
> > > 
> > > This means that we are actually fairly close to upstream Heimdal,
> > > closer than we ever have been I dare to suggest.
> > > 
> > > The remaining changes outstanding are:
> > > ...
> > 
> > There are also, which were on the Samba side, the attached.
> > 
> > I'm not really sure about them - I think
> > 
> > source4/heimdal/lib/krb5/init_creds_pw.c
> > KRB5_NT_ENTERPRISE_PRINCIPAL
> > ctx->flags.canonicalize = 1
> > 
> > is trying to do the same as the Samba-side commit:
> > 
> > testprogs/blackbox/ --enterprise --canonicalize
> > 
> > Is that the case, and so could we drop the Heimdal side now?
> 
> I don't think enterprise principals will work without canonicalize
> and we have also non-blackbox cases we need to handle.
> 
> Just try and check if all our tests still work.
> It seems our C code uses krb5_get_init_creds_opt_set_canonicalize(),
> so we may not need that patch.

OK, I will do.  

That horrible krb5.kdc.canon C based tests show what happens if you
have enterprise without canonicalise, I wish the KDCs never accepted
it...

> > I also don't know what 
> > source4/heimdal/lib/krb5/mcache.c anonymous resolving
> > is for or fixes.  Can you shed some light on this?
> 
> This needed in order to have memory credential caches, which are not
> part of the global credential cache collection, but are still
> available
> to be opened by explicit name, which is the exact usage we need for
> any
> in memory caches.

Yeah, that's exactly how we use them.

> The whole global credential cache collection magic seems to be very
> dangerous
> for application like samba, which need to use kerberos on behalf of
> different unrelated identities.

Indeed.

> We already had very strange things happen with MIT, which where very
> hard to debug,
> setenv("KRB5CCNAME", "MEMORY:libads", 1) completely ignores the
> ':libads' part!
> It always iterates over *all* caches with a "MEMORY:" prefix and use
> the first cache
> that has a tgt in the used realm, which may means we authenticate as
> a completely
> unexpected user (maybe administrator), while we want to do an LDAP
> operation
> on behalf of the local machine account.

Yikes!  I'm still lost about what the patch is, but now I know this
much I'll be sure not to drop it! 

If you could tidy up and submit upstream that would be awesome.  I
submitted a lot of your work up already, but I don't think I can
explain the code enough for this one, it still confuses me.

Thanks so much!

Andrew Bartlett

-- 
Andrew Bartlett (he/him)       https://samba.org/~abartlet/
Samba Team Member (since 2001) https://samba.org
Samba Team Lead, Catalyst IT   https://catalyst.net.nz/services/samba

Samba Development and Support, Catalyst IT - Expert Open Source
Solutions