Re: [PATCH v20] GSSAPI encryption support

Robbie Harwood <rharwood@redhat.com>

From: Robbie Harwood <rharwood@redhat.com>
To: Stephen Frost <sfrost@snowman.net>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org, Nico Williams <nico@cryptonector.com>, Michael Paquier <michael@paquier.xyz>, Alvaro Herrera <alvherre@2ndquadrant.com>, David Steele <david@pgmasters.net>
Date: 2019-03-06T03:55:23Z
Lists: pgsql-hackers

Attachments

Stephen Frost <sfrost@snowman.net> writes:

> * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
>
>> Or maybe we avoid that, and you rename be-secure-gssapi.c to just
>> be-gssapi.c and also combine that with the contents of
>> be-gssapi-common.c.
>
> I don't know why we would need to, or want to, combine
> be-secure-gssapi.c and be-gssapi-common.c, they do have different
> roles in that be-gssapi-common.c is used even if you aren't doing
> encryption, while bs-secure-gssapi.c is specifically for handling the
> encryption side of things.
>
> Then again, be-gssapi-common.c does currently only have the one
> function in it, and it's not like there's an option to compile for
> *just* GSS authentication (and not encryption), or at least, I don't
> think that would be a useful option to have..  Robbie?

Yeah, I think I'm opposed to making that an option.

Worth pointing out here: up until v6, I had this structured differently,
with all the GSSAPI code in fe-gssapi.c and be-gssapi.c.  The current
separation was suggested by Michael Paquier for ease of reading and to
keep the code churn down.

>> (And similarly in libpq.)
>
> I do agree that we should try to keep the frontend and backend code
> structures similar in this area, that seems to make sense.

Well, I don't know if it's an argument in either direction, but: on the
frontend we have about twice as much shared code in fe-gssapi-common.c
(pg_GSS_have_ccache() and pg_GSS_load_servicename()).

>> I don't see any tests in the patch.  We have a Kerberos test suite at
>> src/test/kerberos/ and an SSL test suite at src/test/ssl/.  You can
>> get some ideas there.
>
> Yeah, I was going to comment on that as well.  We definitely should
> implement tests around this.

Attached.  Please note that I don't really speak perl.  There's a pile
of duplicated code in 002_enc.pl that probably should be shared between
the two.  (It would also I think be possible for 001_auth.pl to set up
the KDC and for 002_enc.pl to then use it.)

Thanks,
--Robbie

Commits

  1. GSSAPI encryption support

  2. Fix typo