Re: [PATCH v20] GSSAPI encryption support

Greetings Robbie,

* 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).
> Other changes since v19 from Stephen's review:
>
> - About 100 lines of new comments

Thanks for that.

> - pgindent run over code (only the stuff I'm changing; it reports other
> problems on master, but if I understand correctly, that's not on me to
> fix here)

That's correct.

> Thanks for the feedback! And speaking of feedback, this patch is in the
> "Needs Review" state so if people have additional feedback, that would
> be appreciated. I've CC'd people who I can remember reviewing before;
> they should of course feel no obligation to review again if time
> commitments/interests do not allow.

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:

---
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. Also, what
information do you think would be particularly useful on the server
side? I was thinking that the princ used to authenticate, the
encryption type/cipher, and bits used, at least, would be meaningful.
I'm also guessing that we may need to add something to make these
functions not fail on older systems which don't provide the SASL SSF
OID..? I haven't looked into that yet but we should.

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.

I've attached my WIP patch for adding the bits, patched over-top of your
v20 (would be neat if there was a way to tell cfbot to ignore it...).

Thoughts?

Thanks!

Stephen