[RFC] fix bug 12007

[Simo]

On Mon, 2016-07-04 at 21:44 +0300, Uri Simchoni wrote:
> On 07/04/2016 06:50 PM, Simo wrote:
> > 
> > Sorry have been thinking more.
> > 
> <snip>
> > 
> > > 
> > > > 
> > > > > 
> > > > > of Kerberos. The attached fix replaces gss_acquire_cred by
> > > > > gss_krb5_import_cred().
> > This sounds wrong, and we should fix Heimdal if it is trying to re-
> > acquire the same ticket, not try to work around it.
> > 
> > Are you saying the current ccache has a valid TGT but it ignores it
> > and
> > tries to acquire a new one ?
> > 
> > If there are no credentials it is perfectly normal to make an AS
> > request, I do not understand what the problem is here. Do we, by
> > chance
> > copy in some ccache just the LDAP ticket and not the TGT ?
> > 
> The problem I am trying to fix is that winbindd is emitting AS
> requests
> for root at realm, and this happens from within gss_acquire_cred().
> Heimdal
> is trying to acquire the wrong TGT, if no TGT exist in the ccache,
> and
> in a way it has no chance of succeeding whatsoever (it doesn't have
> the
> password of root so what good would a TGT of root do, even if we got
> one?).
> 
> Thinking about it some more, the issue is more fundamental:
> 1. libads expects that the default ccache either has only a TGT of
> the
> "right" user (the one the application has specified for libads to use
> -
> machine account or otherwise), or that its attempt to bind would
> fail.
> Upon failure it would do a kinit and retry.
> 2. Heimdal's gss_acquire_cred() OTOH, when encountering an empty
> ccache,
> tries to get credentials from the default keytab. It is in this code
> path that the AS request for root is made.
> 3. Usually this extra attempt with the keytab would fail, creating
> the
> nuisance with the AS request - maybe we can fix that with Heimdal for
> the case where no keytab exists.
> 4. But what if a keytab does exist, and contains valid password for
> the
> current user? In that case, gss_acquire_cred() might obtain the
> credentials of the current user, whereas libads expected to be of its
> user - could be a username specified via the -U parameter, or the
> machine account.
> 
> Up until the April security release, gensec_gse was used only for SMB
> connections, and there we don't seem to reuse a TGT between
> connections
> - we always do a kinit first (in cli_session_setup_spnego_send()).
> Libads does something new in that respect.
> 
> The fix tries to probe the ccache more gently than
> gss_acquire_cred(). I
> don't know why Heimdal is also trying the keytab, but they would
> probably insist on doing it. Assuming that's true, the fix is to
> always
> do a kinit before the bind attempt (or to improve libads's
> integration
> with gensec and cli_credentials, because those track the content of
> the
> ccache without a need to probe it).
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > I'd like some feedback from those familiar with this code -
> > > > > 1. It could be that the right fix is in Heimdal
> > Definitely if it really is a bug in finding a valid credential in
> > the
> > cache.
> > 
> > > 
> > > > 
> > > > > 
> > > > > 2. The reason for acquiring the credentials (in client
> > > > > context!)
> > > > > seems
> > > > > to be to be able to set GSS_KRB5_CRED_NO_CI_FLAGS_X option on
> > > > > the
> > > > > credentials - not sure what scenario this fixes and how to
> > > > > test
> > > > > there's
> > > > > no degradation there.
> > This is fundamental, otherwise LDAP binds will fail because AD uses
> > the
> > CI flags instead of the SASL negotiation, so we need to be able to
> > control exactly what flags we send (normally both Heimdal and MIT
> > Kerberos send both Confidentiality and Integrity flags
> > unconditionally,
> > because that is mandated by the Spec, but Microsoft interpreted it
> > differently in some protocols).
> > 
> I can test with Windows then. Only tested with Samba AD so far.
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > 3. Perhaps someone can easily determine the MIT behavior - if
> > > > > MIT
> > > > > is not
> > > > > sending this request then maybe the patch should be #ifdef'd
> > > > > on
> > > > > Kerberos
> > > > > type - use the more portable gss_acquire_cred() with MIT
> > > > > Kerberos.
> > We must use gss_acquire_cred() sorry.
> > 
> MIT seems to have a gss_krb5_import_cred() - can't we use that and
> fall
> back to gss_acquire_cred() only for older versions?
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > Thanks,
> > > > > Uri.
> > > > > 
> > > > One more things - the gensec_gse code also makes an extra TGS
> > > > handshake,
> > > > requesting a TGT, because it requires delegation for the
> > > > security
> > > > context. Do we need delegation for LDAP sasl binding/wrapping?
> > > 
> > > Generally no, we don't, it should be somehow selectable, base on
> > > the
> > > target machine's "allowed to delegate to" (IIRC) flag.
> > Brain fart here. We *definitely* almost never want to delegate a
> > TGT,
> > but we definitely want to request a forwardable ticket, so that the
> > server can perform constrained delegation if it is authorized and
> > so
> > configured.
> > 
> > What flags are passed in ? Can you point me at the code path that
> > generates this ?
> > 
> It's in the default gse context flags.
> In  gse_context_init(), we have:
> 
>         gse_ctx->gss_want_flags = GSS_C_MUTUAL_FLAG |
>                                 *GSS_C_DELEG_FLAG* |

This *must* definitely be made conditional IMHO, it is not ok to just
send your TGT by default to third parties, it means
malicious/compromised 3rd parties can simply grab it and then fully
impersonate you to other services.

Simo.

>                                 GSS_C_DELEG_POLICY_FLAG |
>                                 GSS_C_REPLAY_FLAG |
>                                 GSS_C_SEQUENCE_FLAG;
> 
> 
> > 
> > Simo.
> > 
>