Re: [PATCH v20] GSSAPI encryption support

Greetings,

* Robbie Harwood (rharwood(at)redhat(dot)com) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Robbie Harwood (rharwood(at)redhat(dot)com) wrote:
> >> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> >>> * Robbie Harwood (rharwood(at)redhat(dot)com) wrote:
> >>>
> >>>> Attached please find version 20 of the GSSAPI encryption support.
> >>>> This has been rebased onto master (thanks Stephen for calling out
> >>>> ab69ea9).
> >>>
> >>> I've looked over this again and have been playing with it off-and-on
> >>> for a while now. One thing I was actually looking at implementing
> >>> before we push to commit this was to have similar information to
> >>> what SSL provides on connection, specifically what printSSLInfo()
> >>> prints by way of cipher, bits, etc for the client-side and then
> >>> something like pg_stat_ssl for the server side.
> >>>
> >>> I went ahead and added a printGSSENCInfo(), and then
> >>> PQgetgssSASLSSF(), which calls down into
> >>> gss_inquire_sec_context_by_oid() for GSS_C_SEC_CONTEXT_SASL_SSF to
> >>> get the bits (which also required including gssapi_ext.h), and now
> >>> have psql producing:
> >>
> >> Also, this is an MIT-specific extension: Heimdal doesn't support it.
> >
> > Is that simply because they've not gotten around to it..? From what I
> > saw, this approach is in an accepted RFC, so if they want to implement
> > it, they could do so..?
>
> Yes. Heimdal is less active these days than MIT; they were dormant for
> a while, though are active again. They do have an issue open to track
> this feature: https://github.com/heimdal/heimdal/issues/400

Ok, good, when they add it then hopefully it'll be picked up
more-or-less 'for free'.

> >> While I (a krb5 developer) am fine with a hard MIT dependency, the
> >> project doesn't currently have this. (And if we went that road, I'd
> >> like to use gss_acquire_cred_from() instead of the setenv() in
> >> be-secure-gssapi.c.)
> >
> > We certainly don't want a hard MIT dependency, but if it's following an
> > RFC and we can gracefully fall-back if it isn't available then I think
> > it's acceptable. If there's a better approach which would work with
> > both MIT and Heimdal and ideally is defined through an RFC, that'd be
> > better, but I'm guessing there isn't...
>
> While I think the concept of SASL SSF is standardized, I don't think the
> semantics of this OID are - the OID itself is in the MIT gssapi
> extension space.

We can adjust for that pretty easily, presumably, if another OID ends up
being assigned (though if one already exists, isn't it something that
Heimdal, for example, could potentially decide to just support..?).

> As a historical note, the reason this interface exists in krb5 is of
> course for SASL. In particular, cyrus was reporting treating the SSF of
> all krb5 types as if they were DES. So while technically "assume DES"
> could be a fallback... we probably shouldn't do that.

Agreed, we shouldn't do that.

> >> A call to gss_inquire_context() produces mech type, so you could
> >> print something like "GSS Encrypted connection (Kerberos 256 bits)"
> >> or "GSS encrypted connection (NTLM)". I think this'd be pretty
> >> reasonable.
> >
> > I'm... not impressed with that approach, since "Kerberos" as an
> > encryption algorithm doesn't mean squat- that could be DES or RC4 for
> > all we know.
>
> This is a fair concern.

I will say that I'm alright with listing Kerberos in the message if we
think it's useful- but we should also list the actual encryption
algorithm and bits, if possible (which it sounds like it is, so that's
definitely good).

> > To get where I'd like us to be, however, it sounds like we need to:
> >
> > a) determine if we've got encryption (that's easy, assuming I've done it
> > correctly so far)
>
> Yes.
>
> > b) Figure out if the encryption is being provided by Kerberos (using
> > gss_inquire_context() would do that, right?)
>
> Right - check and log mech_out there, then proceed if it's
> gss_mech_krb5. Probably not worth handling _old and _wrong, especially
> since Heimdal doesn't have them and they shouldn't be in use anyway.
>
> > c) If we're using Kerberos, use the lucid contexts to ask what the
> > actual encryption algorithm used is
>
> Yes. This'll involve some introspection into krb5.h (for enctype
> names), but that's not too hard.

Sounds good.

> > That's a bit obnoxious but it at least seems doable. I don't suppose
> > you know of any example code out there which provides this? Any idea
> > if there's work underway on an RFC to make this, well, suck less?
>
> MIT has some in our test suite (t_enctypes). nfs-utils (GPLv2) also
> uses this interface (NFS is the intended consumer).
>
> There isn't IETF effort in this regard that I'm aware of, but I'll add
> it to the kitten backlog.

Fantastic, thanks.

> >> (With my downstream maintainer hat on, I'd further ask that any such
> >> check not just be a version check in order to allow backporting; in
> >> particular, the el7 krb5-1.15 branch does include support for this OID.)
> >
> > I don't have any issue with that provided we can implement the check in
> > a reasonable way (hint: I'm open to your suggestions on this :).
> >
> >>> I don't think these things are absolutely required to push the patch
> >>> forward, just to be clear, but it's helping my confidence level, at
> >>> least, and would make it closer to on-par with our current SSL
> >>> implementation. They also seem, well, reasonably straight-forward to
> >>> add and quite useful.
> >>
> >> Auditability is definitely reasonable. I think there are a couple
> >> additional wrinkles discussed above, but we can probably get them
> >> sorted.
> >
> > Fantastic. Do you think you'll have time to work through some of the
> > above with me over the next couple of weeks? I was just starting to
> > look into adding things into the beentry to hold information on the
> > server side.
>
> Sure! I'll go ahead and hack up the checks and lucid stuff and get back
> to you.

Great! I'll finish fleshing out the basics of how to have this work
server-side and once the checks and lucid stuff are in on the psql side,
it should be pretty straight-forward to copy that same information into
beentry alongside the SSL info, and expose it through
pg_stat_get_activity() into a pg_stat_gss view.

Thanks!

Stephen