Re: [PATCH v19] GSSAPI encryption support
Stephen Frost writes: > Greetings Robbie, > > * Dmitry Dolgov (9erthali...@gmail.com) wrote: >> > On Tue, Oct 2, 2018 at 11:12 PM Robbie Harwood wrote: >> > >> > Michael Paquier writes: >> > >> > > On Mon, Aug 06, 2018 at 05:23:28PM -0400, Robbie Harwood wrote: >> > >> If you're in a position where you're using Kerberos (or most other >> > >> things from the GSSAPI) for authentication, the encryption comes at >> > >> little to no additional setup cost. And then you get all the security >> > >> benefits outlined in the docs changes. >> > > >> > > Please note that the last patch set does not apply anymore, so I have >> > > moved it to CF 2018-11 and marked it as waiting on author. >> > >> > Indeed. Please find v19 attached. It's just a rebase; no architecture >> > changes. >> >> Unfortunately, patch needs another rebase, could you do this? In the meantime >> I'll move it to the next CF. > > This patch needs a few minor changes to get it back to working, but I'm > happy to say that with those changes, it seems to be working rather well > for me. Thanks Stephen! Dmitry, I will make an update (and address Stephen's feedback) hopefully soon. Thanks, --Robbie signature.asc Description: PGP signature
Re: [PATCH v19] GSSAPI encryption support
On Tue, Dec 4, 2018 at 10:20 AM Stephen Frost wrote: > (and I have to wonder- if we want nearly all callers of > WaitLatch/WaitLatchOrSocket to use WL_EXIT_ON_PM_DEATH, maybe we should > make that the default and allow it to be overridden..? ... That is what I proposed. It was Heikki who talked me into the opt-in solution, but using an assertion to make sure you handle it one way or the other: https://www.postgresql.org/message-id/6417314e-93d5-ed2d-9012-8d6e9ed21778%40iki.fi Perhaps I should have sought more opinions. Please feel free to start a new thread on that if you don't like the way it was done. -- Thomas Munro http://www.enterprisedb.com
Re: [PATCH v19] GSSAPI encryption support
Greetings Robbie, * Dmitry Dolgov (9erthali...@gmail.com) wrote: > > On Tue, Oct 2, 2018 at 11:12 PM Robbie Harwood wrote: > > > > Michael Paquier writes: > > > > > On Mon, Aug 06, 2018 at 05:23:28PM -0400, Robbie Harwood wrote: > > >> If you're in a position where you're using Kerberos (or most other > > >> things from the GSSAPI) for authentication, the encryption comes at > > >> little to no additional setup cost. And then you get all the security > > >> benefits outlined in the docs changes. > > > > > > Please note that the last patch set does not apply anymore, so I have > > > moved it to CF 2018-11 and marked it as waiting on author. > > > > Indeed. Please find v19 attached. It's just a rebase; no architecture > > changes. > > Unfortunately, patch needs another rebase, could you do this? In the meantime > I'll move it to the next CF. This patch needs a few minor changes to get it back to working, but I'm happy to say that with those changes, it seems to be working rather well for me. I'm working on a more comprehensive review but I wanted to go ahead and get these first minor items out of the way: Needs the same treatment done in ab69ea9, for all the WaitLatchOrSocket calls in be-secure-gssapi.c: - WaitLatchOrSocket(MyLatch, WL_SOCKET_WRITEABLE, port->sock, 0, + WaitLatchOrSocket(MyLatch, WL_SOCKET_WRITEABLE | WL_EXIT_ON_PM_DEATH, port->sock, 0, WAIT_EVENT_GSS_OPEN_SERVER); (and I have to wonder- if we want nearly all callers of WaitLatch/WaitLatchOrSocket to use WL_EXIT_ON_PM_DEATH, maybe we should make that the default and allow it to be overridden..? Also, does ModifyWaitEvent() need to also do some checking here..? Should be_tls_read()'s waitfor settings also include WL_EXIT_ON_PM_DEATH?) Needed a minor adjustment in src/interfaces/libpq/fe-connect.c due to conflict with 6e5f8d4. Otherwise, it's building and running with all flavors of client and server-side GSS-related options (require/disable/prefer and host/hostgss/hostnogss). Not surprisingly, trying to connect from a newer psql w/ PGGSSMODE=require (explicitly requesting encryption from the server) with an older server blows up, but there's not much we can do for that. Using prefer works, with an extra roundtrip to discover the server doesn't understand GSSAPI encryption and then falling back. Also, when connecting from an older psql to a newer server, if pg_hba.conf has 'host' or 'hostnogss', everything works great (though without GSSAPI encryption, of course), while an entry of 'hostgss' returns the typical 'no pg_hba.conf entry found', as you'd expect. Just in general, it seems like the code could use a lot more comments. :) Also, it needs some pgindent run over it. That's about all I have time to cover for today, but maybe you could try to spruce up the comments (I'm at least a fan of function-level comments, in particular, explaining how they're to be used, et al..), see if you can get pgindent run over the changes and make the above-mentioned fixes and then perhaps we can get cfbot to do its thing, ask the other folks on the thread who were having issues before to retry with this patch, if possible, and I'll start doing a more thorough code review later this week. Thanks! Stephen signature.asc Description: PGP signature
Re: [PATCH v19] GSSAPI encryption support
> On Tue, Oct 2, 2018 at 11:12 PM Robbie Harwood wrote: > > Michael Paquier writes: > > > On Mon, Aug 06, 2018 at 05:23:28PM -0400, Robbie Harwood wrote: > >> If you're in a position where you're using Kerberos (or most other > >> things from the GSSAPI) for authentication, the encryption comes at > >> little to no additional setup cost. And then you get all the security > >> benefits outlined in the docs changes. > > > > Please note that the last patch set does not apply anymore, so I have > > moved it to CF 2018-11 and marked it as waiting on author. > > Indeed. Please find v19 attached. It's just a rebase; no architecture > changes. Unfortunately, patch needs another rebase, could you do this? In the meantime I'll move it to the next CF.
Re: [PATCH v19] GSSAPI encryption support
Michael Paquier writes: > On Mon, Aug 06, 2018 at 05:23:28PM -0400, Robbie Harwood wrote: >> If you're in a position where you're using Kerberos (or most other >> things from the GSSAPI) for authentication, the encryption comes at >> little to no additional setup cost. And then you get all the security >> benefits outlined in the docs changes. > > Please note that the last patch set does not apply anymore, so I have > moved it to CF 2018-11 and marked it as waiting on author. Indeed. Please find v19 attached. It's just a rebase; no architecture changes. I'll set commitfest status back to "Needs Review" once I've sent this mail. Thanks, --Robbie signature.asc Description: PGP signature >From 4a91571db6873da46becf2f7ad0cd9055df2 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Thu, 10 May 2018 16:12:03 -0400 Subject: [PATCH] libpq GSSAPI encryption support On both the frontend and backend, prepare for GSSAPI encryption support by moving common code for error handling into a separate file. Fix a TODO for handling multiple status messages in the process. Eliminate the OIDs, which have not been needed for some time. Add frontend and backend encryption support functions. Keep the context initiation for authentication-only separate on both the frontend and backend in order to avoid concerns about changing the requested flags to include encryption support. In postmaster, pull GSSAPI authorization checking into a shared function. Also share the initiator name between the encryption and non-encryption codepaths. Modify pqsecure_write() to take a non-const pointer. The pointer will not be modified, but this change (or a const-discarding cast, or a malloc()+memcpy()) is necessary for GSSAPI due to const/struct interactions in C. For HBA, add "hostgss" and "hostnogss" entries that behave similarly to their SSL counterparts. "hostgss" requires either "gss", "trust", or "reject" for its authentication. Simiarly, add a "gssmode" parameter to libpq. Supported values are "disable", "require", and "prefer". Notably, negotiation will only be attempted if credentials can be acquired. Move credential acquisition into its own function to support this behavior. Finally, add documentation for everything new, and update existing documentation on connection security. Thanks to Michael Paquier for the Windows fixes. --- doc/src/sgml/client-auth.sgml | 75 -- doc/src/sgml/libpq.sgml | 57 +++- doc/src/sgml/runtime.sgml | 77 +- src/backend/libpq/Makefile | 4 + src/backend/libpq/auth.c| 107 +++- src/backend/libpq/be-gssapi-common.c| 64 + src/backend/libpq/be-gssapi-common.h| 26 ++ src/backend/libpq/be-secure-gssapi.c| 321 ++ src/backend/libpq/be-secure.c | 16 ++ src/backend/libpq/hba.c | 51 +++- src/backend/postmaster/pgstat.c | 3 + src/backend/postmaster/postmaster.c | 64 - src/include/libpq/hba.h | 4 +- src/include/libpq/libpq-be.h| 11 +- src/include/libpq/libpq.h | 3 + src/include/libpq/pqcomm.h | 5 +- src/include/pgstat.h| 3 +- src/interfaces/libpq/Makefile | 4 + src/interfaces/libpq/fe-auth.c | 82 +- src/interfaces/libpq/fe-connect.c | 232 +++- src/interfaces/libpq/fe-gssapi-common.c | 128 + src/interfaces/libpq/fe-gssapi-common.h | 23 ++ src/interfaces/libpq/fe-secure-gssapi.c | 345 src/interfaces/libpq/fe-secure.c| 16 +- src/interfaces/libpq/libpq-fe.h | 3 +- src/interfaces/libpq/libpq-int.h| 30 ++- src/tools/msvc/Mkvcbuild.pm | 10 + 27 files changed, 1571 insertions(+), 193 deletions(-) create mode 100644 src/backend/libpq/be-gssapi-common.c create mode 100644 src/backend/libpq/be-gssapi-common.h create mode 100644 src/backend/libpq/be-secure-gssapi.c create mode 100644 src/interfaces/libpq/fe-gssapi-common.c create mode 100644 src/interfaces/libpq/fe-gssapi-common.h create mode 100644 src/interfaces/libpq/fe-secure-gssapi.c diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index c2114021c3..6f9f2b7560 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -108,6 +108,8 @@ hostnossl database user host database user IP-address IP-mask auth-method auth-options hostssldatabase user IP-address IP-mask auth-method auth-options hostnossl database user IP-address IP-mask auth-method auth-options +hostgssdatabase user IP-address IP-mask auth-method auth-options +hostnogss database user IP-address IP-mask auth-method auth-options The meaning of the fields is as follows: @@ -128,9 +130,10 @@ hostnossl database user This record matches connection attempts made using TCP/IP. -
