Re: [PATCH v20] GSSAPI encryption support

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

When the mech is gss_mech_krb5 under MIT krb5:

psql (12devel)
GSSAPI encrypted connection (krb5 using aes256-cts-hmac-sha384-192)
Type "help" for help.

And the same under Heimdal:

psql (12devel)
GSSAPI encrypted connection (krb5 using aes256-cts-hmac-sha1-96)
Type "help" for help

If the mech weren't gss_krb5, or Lucid introspection failed, but we're a
SASL-aware mech (and using MIT):

psql (12devel)
GSSAPI encrypted connection (~256 bits)
Type "help" for help.

The third case (no lucid, no SASL SSF):

psql (12devel)
GSSAPI encrypted connection (unknown mechanism)
Type "help" for help.

Feel free to tweak these strings as needed.

I've also adjusted the layering somewhat and moved the actual printf()
call down into fe-secure-gssapi.c I don't know whether this model makes
more sense in the long run, but for PoC code it was marginally easier to
reason about.

Another thing I've been thinking about here is whether the SASL SSF
logic is useful. It'll only get invoked when the mechanism isn't
gss_mech_krb5, and only under MIT. SPNEGO (krb5), NTLM (gss-ntlmssp),
IAKERB (krb5), and EAP (moonshot) all don't support
GSS_C_SEC_CONTEXT_SASL_SSF; I actually couldn't come up with another
mechanism that does. I defer to you on whether to remove that, though.

I did some testing with Heimdal since we're using some extensions here,
and found out that the current build machinery doesn't support Heimdal.
(The configure.in detection logic only seems to work for MIT and
Solaris.) However, it's possible to explicitly pass in CFLAGS/LDFLAGS
and it worked prior to my changes, so I've preserved that behavior.

Finally, as this patch touches configure.in, configure needs to be
regenerated; I'm not sure what project policy is on when that happens
(and it produces rather a lot of churn on my machine).

Patch attached after the break; apply on top of -20.

Thanks,
--Robbie