Re: [PATCH v20] GSSAPI encryption support

From: Robbie Harwood <rharwood(at)redhat(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org, Nico Williams <nico(at)cryptonector(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>
Subject: Re: [PATCH v20] GSSAPI encryption support
Date: 2019-02-12 19:06:44
Message-ID: jlgwom4kerf.fsf@redhat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

>> 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.

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.

>>> ---
>>> psql (12devel)
>>> GSS Encrypted connection (bits: 256)
>>> Type "help" for help.
>>> ---
>>>
>>> I was curious though- is there a good way to get at the encryption
>>> type used..? I haven't had much luck in reading the RFPs and poking
>>> around, but perhaps I'm just not looking in the right place.
>>
>> 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.

>> For Kerberos-specific introspection, MIT krb5 and Heimdal both
>> support an interface called lucid contexts that allows this kind of
>> introspection (and some other gross layering violations) for use with
>> the kernel. Look at gssapi_krb5.h (it's called that in both). I
>> don't think it's worth supporting rfc1964 at this point (it's
>> 1DES-only).
>
> I agree that there's no sense in supporting 1DES-only.
>
> I also, frankly, don't agree with the characterization that wanting to
> know what the agreed-upon encryption algorithm is constitutes a gross
> layering violation, but that's really an independent discussion that we
> can have in a pub somewhere. ;)

Yeah I won't defend it very hard :)

> 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.

> 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.

>> (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.

Thanks,
--Robbie

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-02-12 19:16:33 Re: monitoring CREATE INDEX [CONCURRENTLY]
Previous Message Andrey Lepikhov 2019-02-12 17:47:32 Re: [PATCH] xlogreader: do not read a file block twice