Re: [HACKERS] [PATCH v4] GSSAPI encryption support

[David Steele]

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 

Re: [HACKERS] [PATCH v4] GSSAPI encryption support

[Robbie Harwood]

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

[Michael Paquier]

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

[Michael Paquier]

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

[Robert Haas]

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

[Robbie Harwood]

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
+