Re: [HACKERS] [PATCH v4] GSSAPI encryption support
Hi Robbie, On 2/10/16 4:06 PM, Robbie Harwood wrote: > Hello friends, > > For your consideration, here is a new version of GSSAPI encryption > support. For those who prefer, it's also available on my github: > https://github.com/frozencemetery/postgres/commit/c92275b6605d7929cda5551de47a4c60aab7179e It tried out this patch and ran into a few problems: 1) It didn't apply cleanly to HEAD. It did apply cleanly on a455878 which I figured was recent enough for testing. I didn't bisect to find the exact commit that broke it. 2) While I was able to apply the patch and get it compiled it seemed pretty flaky - I was only able to logon about 1 in 10 times on average. Here was my testing methodology: a) Build Postgres from a455878 (without your patch), install/configure Kerberos and get everything working. I was able the set the auth method to gss in pg_hba.conf and logon successfully every time. b) On the same system rebuild Postgres from a455878 including your patch and attempt authentication. The problems arose after step 2b. Sometimes I would try to logon twenty times without success and sometimes it only take five or six attempts. I was never able to logon successfully twice in a row. When not successful the client always output this incomplete message (without terminating LF): psql: expected authentication request from server, but received From the logs I can see the server is reporting EOF from the client, though the client does not core dump and prints the above message before exiting. I have attached files that contain server logs at DEBUG5 and tcpdump output for both the success and failure cases. Please let me know if there's any more information you would like me to provide. -- -David da...@pgmasters.net Postgres Log: DEBUG: forked new backend, pid=16531 socket=9 DEBUG: postgres child[16531]: starting with ( DEBUG: postgres DEBUG: ) DEBUG: InitPostgres DEBUG: my backend ID is 2 DEBUG: StartTransaction DEBUG: name: unnamed; blockState: DEFAULT; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: DEBUG: Processing received GSS token of length 667 DEBUG: gss_accept_sec_context major: 0, minor: 0, outlen: 161, outflags: 1b2 DEBUG: sending GSS response token of length 161 DEBUG: sending GSS token of length 161 DEBUG: CommitTransaction DEBUG: name: unnamed; blockState: STARTED; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: DEBUG: unexpected EOF on client connection DEBUG: shmem_exit(0): 1 before_shmem_exit callbacks to make DEBUG: shmem_exit(0): 6 on_shmem_exit callbacks to make DEBUG: proc_exit(0): 3 callbacks to make DEBUG: exit(0) DEBUG: shmem_exit(-1): 0 before_shmem_exit callbacks to make DEBUG: shmem_exit(-1): 0 on_shmem_exit callbacks to make DEBUG: proc_exit(-1): 0 callbacks to make DEBUG: reaping dead processes DEBUG: server process (PID 16531) exited with exit code 0 TCP Dump: 16:48:15.541558 IP6 (hlim 64, next-header TCP (6) payload length: 40) ::1.51100 > ::1.5432: Flags [S], cksum 0x0030 (incorrect -> 0xdcd6), seq 1835476714, win 43690, options [mss 65476,sackOK,TS val 231098240 ecr 0,nop,wscale 7], length 0 0x: 6000 0028 0640 `(.@ 0x0010: 0001 0x0020: 0001 c79c 1538 6d67 26ea ...8mg&. 0x0030: a002 0030 0204 ffc4 .0.. 0x0040: 0402 080a 0dc6 4780 0103 0307 ..G. 16:48:15.541566 IP6 (hlim 64, next-header TCP (6) payload length: 40) ::1.5432 > ::1.51100: Flags [S.], cksum 0x0030 (incorrect -> 0x5a2a), seq 3525598000, ack 1835476715, win 43690, options [mss 65476,sackOK,TS val 231098240 ecr 231098240,nop,wscale 7], length 0 0x: 6000 0028 0640 `(.@ 0x0010: 0001 0x0020: 0001 1538 c79c d224 5b30 .8...$[0 0x0030: 6d67 26eb a012 0030 0204 ffc4 mg&..0.. 0x0040: 0402 080a 0dc6 4780 0dc6 4780 0103 0307 ..G...G. 16:48:15.541573 IP6 (hlim 64, next-header TCP (6) payload length: 32) ::1.51100 > ::1.5432: Flags [.], cksum 0x0028 (incorrect -> 0x2c5c), seq 1835476715, ack 3525598001, win 342, options [nop,nop,TS val 231098240 ecr 231098240], length 0 0x: 6000 0020 0640 `..@ 0x0010: 0001 0x0020: 0001 c79c 1538 6d67 26eb ...8mg&. 0x0030: d224 5b31 8010 0156 0028 0101 080a .$[1...V.(.. 0x0040: 0dc6 4780 0dc6 4780 ..G...G. 16:48:15.541806 IP6 (hlim 64, next-header TCP (6) payload length: 129) ::1.51100 > ::1.5432: Flags [P.], cksum 0x0089 (incorrect -> 0x553d), seq 1835476715:1835476812, ack 3525598001, win 342, options [nop
Re: [HACKERS] [PATCH v4] GSSAPI encryption support
On Fri, Feb 12, 2016 at 3:56 AM, Robbie Harwood wrote: > Michael Paquier writes: >> On Thu, Feb 11, 2016 at 6:06 AM, Robbie Harwood wrote: >>> - The GSSAPI authentication code has been moved without modification. >>> In doing so, the temptation to modify it (flags, error checking, that >>> big comment at the top about things from Athena, etc.) is very large. >>> I do not know whether these changes are best suited to another patch >>> in this series or should be reviewed separately. I am also hesitant >>> to add things beyond the core before I am told this is the right >>> approach. >> >> I would recommend a different patch if code needs to be moved around. >> The move may make sense taken as an independent piece of the >> integration. > > Sorry, are you suggesting separate patch for moving the GSS auth code, > or separate patch for changes to said code? I am happy to move it if > so, just want to be sure. This is based on my first impressions on the patch. Let's discuss more those points once I got a more in-depth look at the patch with what it actually does. In short, there is no need to put more efforts in the coding now :) Sorry to confuse you. >> + * Portions Copyright (c) 2015-2016, Red Hat, Inc. >> + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group >> I think that this part may be a problem... Not sure the feeling of the >> others regarding additional copyright notices. > > Good catch. That's an accident (force of habit). Since I'm pretty sure > this version won't be merged anyway, I'll drop it from the next one. > >> It would be good to add that to the next CF, I will be happy to get a >> look at it. > > Sounds good. Thanks for looking at it! Okay, let's do this. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH v4] GSSAPI encryption support
Michael Paquier writes: > On Thu, Feb 11, 2016 at 6:06 AM, Robbie Harwood wrote: >> >> - The GSSAPI authentication code has been moved without modification. >> In doing so, the temptation to modify it (flags, error checking, that >> big comment at the top about things from Athena, etc.) is very large. >> I do not know whether these changes are best suited to another patch >> in this series or should be reviewed separately. I am also hesitant >> to add things beyond the core before I am told this is the right >> approach. > > I would recommend a different patch if code needs to be moved around. > The move may make sense taken as an independent piece of the > integration. Sorry, are you suggesting separate patch for moving the GSS auth code, or separate patch for changes to said code? I am happy to move it if so, just want to be sure. > + * Portions Copyright (c) 2015-2016, Red Hat, Inc. > + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group > I think that this part may be a problem... Not sure the feeling of the > others regarding additional copyright notices. Good catch. That's an accident (force of habit). Since I'm pretty sure this version won't be merged anyway, I'll drop it from the next one. > It would be good to add that to the next CF, I will be happy to get a > look at it. Sounds good. Thanks for looking at it! signature.asc Description: PGP signature
Re: [HACKERS] [PATCH v4] GSSAPI encryption support
On Thu, Feb 11, 2016 at 8:42 AM, Michael Paquier wrote: > + * Portions Copyright (c) 2015-2016, Red Hat, Inc. > + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group > I think that this part may be a problem... Not sure the feeling of the > others regarding additional copyright notices. Yep. "PostgreSQL Global Development Group" means "whoever it was that contributed", not a specific legal organization. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH v4] GSSAPI encryption support
On Thu, Feb 11, 2016 at 6:06 AM, Robbie Harwood wrote: > For your consideration, here is a new version of GSSAPI encryption > support. For those who prefer, it's also available on my github: > https://github.com/frozencemetery/postgres/commit/c92275b6605d7929cda5551de47a4c60aab7179e Yeah! Glad to see you back. > Some thoughts: > > - The overall design is different this time - GSS encryption sits in > parallel construction to SSL encryption rather than at the protocol > level - so a strict diff probably isn't useful. > > - The GSSAPI authentication code has been moved without modification. > In doing so, the temptation to modify it (flags, error checking, that > big comment at the top about things from Athena, etc.) is very large. > I do not know whether these changes are best suited to another patch > in this series or should be reviewed separately. I am also hesitant > to add things beyond the core before I am told this is the right > approach. I would recommend a different patch if code needs to be moved around. The move may make sense taken as an independent piece of the integration. > - There's no fallback here. I wrote fallback support for versions 1-3, > and the same design could apply here without too much trouble, but I > am hesitant to port it over before the encryption design is approved. > I strongly suspect you will not want to merge this without fallback > support, and that makes sense to me. > > - The client and server code look a lot like each other. This > resemblance is not exact, and my understanding is that server and > client need to compile independently, so I do not know of a way to > rectify this. Suggestions are welcome. At quick glance, I like the direction this is taking. You moved all the communication protocol at a lower level where SSL and secure reads are located, so this results in a neat integration. + * Portions Copyright (c) 2015-2016, Red Hat, Inc. + * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group I think that this part may be a problem... Not sure the feeling of the others regarding additional copyright notices. It would be good to add that to the next CF, I will be happy to get a look at it. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH v4] GSSAPI encryption support
Hello friends, For your consideration, here is a new version of GSSAPI encryption support. For those who prefer, it's also available on my github: https://github.com/frozencemetery/postgres/commit/c92275b6605d7929cda5551de47a4c60aab7179e Some thoughts: - The overall design is different this time - GSS encryption sits in parallel construction to SSL encryption rather than at the protocol level - so a strict diff probably isn't useful. - The GSSAPI authentication code has been moved without modification. In doing so, the temptation to modify it (flags, error checking, that big comment at the top about things from Athena, etc.) is very large. I do not know whether these changes are best suited to another patch in this series or should be reviewed separately. I am also hesitant to add things beyond the core before I am told this is the right approach. - There's no fallback here. I wrote fallback support for versions 1-3, and the same design could apply here without too much trouble, but I am hesitant to port it over before the encryption design is approved. I strongly suspect you will not want to merge this without fallback support, and that makes sense to me. - The client and server code look a lot like each other. This resemblance is not exact, and my understanding is that server and client need to compile independently, so I do not know of a way to rectify this. Suggestions are welcome. Thanks! From c92275b6605d7929cda5551de47a4c60aab7179e Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Tue, 17 Nov 2015 18:34:14 -0500 Subject: [PATCH] Connect encryption support for GSSAPI Existing GSSAPI authentication code is extended to support connection encryption. Connection begins as soon as possible - that is, immediately after the client and server complete authentication. --- configure | 2 + configure.in| 1 + doc/src/sgml/client-auth.sgml | 2 +- doc/src/sgml/runtime.sgml | 20 +- src/Makefile.global.in | 1 + src/backend/libpq/Makefile | 4 + src/backend/libpq/auth.c| 330 +--- src/backend/libpq/be-gssapi.c | 584 src/backend/libpq/be-secure.c | 16 + src/backend/postmaster/postmaster.c | 12 + src/include/libpq/libpq-be.h| 31 ++ src/interfaces/libpq/Makefile | 4 + src/interfaces/libpq/fe-auth.c | 182 --- src/interfaces/libpq/fe-auth.h | 5 + src/interfaces/libpq/fe-connect.c | 10 + src/interfaces/libpq/fe-gssapi.c| 475 + src/interfaces/libpq/fe-secure.c| 16 +- src/interfaces/libpq/libpq-int.h| 16 +- 18 files changed, 1193 insertions(+), 518 deletions(-) create mode 100644 src/backend/libpq/be-gssapi.c create mode 100644 src/interfaces/libpq/fe-gssapi.c diff --git a/configure b/configure index 3dd1b15..7fd7610 100755 --- a/configure +++ b/configure @@ -712,6 +712,7 @@ with_uuid with_selinux with_openssl krb_srvtab +with_gssapi with_python with_perl with_tcl @@ -5488,6 +5489,7 @@ $as_echo "$with_gssapi" >&6; } + # # Kerberos configuration parameters # diff --git a/configure.in b/configure.in index 9398482..b19932e 100644 --- a/configure.in +++ b/configure.in @@ -636,6 +636,7 @@ PGAC_ARG_BOOL(with, gssapi, no, [build with GSSAPI support], krb_srvtab="FILE:\$(sysconfdir)/krb5.keytab" ]) AC_MSG_RESULT([$with_gssapi]) +AC_SUBST(with_gssapi) AC_SUBST(krb_srvtab) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 3b2935c..7d37223 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -915,7 +915,7 @@ omicron bryanh guest1 provides automatic authentication (single sign-on) for systems that support it. The authentication itself is secure, but the data sent over the database connection will be sent unencrypted unless -SSL is used. +SSL or GSSAPI are used. diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index cda05f5..bd8156f 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1880,12 +1880,13 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433 - To prevent spoofing on TCP connections, the best solution is to use - SSL certificates and make sure that clients check the server's certificate. - To do that, the server - must be configured to accept only hostssl connections () and have SSL key and certificate files - (). The TCP client must connect using + To prevent spoofing on TCP connections, the best solutions are either to + use GSSAPI for authentication and encryption or to use SSL certificates and + make sure that clients check the server's certificate. To secure using + SSL, the server must be configured to accept only hostssl + connections () and have SSL key and + certificate files (). The TCP