Current semantics for channel-bindings in GSSAPI

[Simo Sorce]

On Thu, 2020-03-19 at 11:05 +0100, Isaac Boukris wrote:
> Hi all,
> 
> On Tue, Mar 10, 2020 at 5:25 PM Stefan Metzmacher <metze at samba.org> wrote:
> > Am 10.03.20 um 17:18 schrieb Isaac Boukris:
> > > On Tue, Mar 10, 2020 at 4:54 PM Stefan Metzmacher <metze at samba.org> wrote:
> > > > Am 10.03.20 um 16:34 schrieb Isaac Boukris:
> > > > > On Tue, Mar 10, 2020 at 4:23 PM Stefan Metzmacher <metze at samba.org> wrote:
> > > > > > > As discussed last week, we want the following changes.
> > > > > > > 
> > > > > > > - MIT should match Heimdal behavior and only error if client bindings
> > > > > > > are not all zeros.
> > > > > > > - Both Heimdal/MIT should return channel-bound flag if the bindings did match.
> > > > > > > - Both Heimdal/MIT should take advantage of KERB_AP_OPTIONS_CBT if
> > > > > > > present if authenticator, in which case if the server passed bindings
> > > > > > > they must match.
> > > > > > > - Both Heimdal/MIT should provide a conf option to asset the client
> > > > > > > system supports channel-bindings, causing KERB_AP_OPTIONS_CBT to be
> > > > > > > sent in any ap-req.
> > > > > > > 
> > > > > > > I submitted wip PR #1047 upstream MIT based on the above.
> > > > > > > 
> > > > > > > @metze, would that satisfy samba's requirements?
> > > > > > 
> > > > > > I looked briefly and the core changes look good,
> > > > > > but (as always :-) I think krb5.conf option alone are unflexible
> > > > > > and I'd really like to get rid of autogenerated krb5.conf files and
> > > > > > global exporting "KRB5_CONFIG". So APIs to turn this on from the
> > > > > > application would be great.
> > > > > 
> > > > > Ok, so we'd need a new cred-option to override it by the application.
> > > > 
> > > > If we can agree on a way to implement that:-)
> > > > 
> > > > Using gss_set_cred_option() would be the simplest solution,
> > > > but it got rejected for GSS_KRB5_CRED_NO_TRANSIT_CHECK_X.
> > > > Passing cred_store to gss_acquire_cred_from() would also work
> > > > and I'm not sure if/how gss_create_sec_context() +
> > > > gss_set_sec_context_option() would work.
> > > > 
> > > > gss_set_sec_context_option() would be the most flexible way
> > > > and may be useful for more things I plan to implement.
> 
> Looking more at PR 685 code, I can't help but thinking:
> If we are ok with gss_create_sec_context(), maybe it would be better
> to bring that forward instead of GSS_C_CHANNEL_BOUND_FLAG return flag.

I strongly believe we should proceed with the plan we drafted as it
minimized the need to change applications.
I think adding gss_create_sec_contetx() is a good thing in general, but
it will not be backwards compatible, so its use will be limited to
ifdef blocks for a long time and that is ugly and we shouldn't require
every application to do that right away.

> I think we should be able to accomplish the above requirements with
> gss_create_sec_context(), the initiator could create it with
> 'client_aware_cb' flag (overriding global), and the acceptor could set
> 'channel_bindings_enforcement_level' to require level=2.
> I think that would help making eventual changes to current
> applications code, simpler.
> 
> btw, do we really need gss_create_sec_context() +
> gss_set_sec_context_option(), can't we use just
> gss_create_sec_context(flags) ?

Sometimes options will be more than flags. Not sure we want to repeat
the problem of having a limited set of flags that will be used up
pretty quickly and then we are stuck with some stuff set via flags and
som via options.

> > > Honestly I'd say we can start with the krb5.conf option, I think it
> > > has value anyway as it allows to protect applications system-wide
> > > without the need to update them.
> > 
> > Yes, a global option should be there!
> > 
> > > Then eventually, use cred/context
> > > options to override it, as we decide.
> > > 
> > > btw, as mentioned "off-list" Windows seem to skip channel-bindings
> > > check if the client omits the checksum altogether, even in level=2. I
> > > think it is a bug, and we shouldn't return channel-bound flag it that
> > > case.
> > 
> > I think your code is fine as it only sets cb_match = true if we did the
> > memcmp and the 16 bytes match exactly.
> 
> Yeah, I've also reached out to MS and according to them this will be
> fixed in future Windows versions as well.
> 

-- 
Simo Sorce
RHEL Crypto Team
Red Hat, Inc