Re: [HACKERS] [PATCH v5] GSSAPI encryption support
On Fri, Feb 26, 2016 at 5:02 AM, Robbie Harwood wrote: > Michael Paquier writes: >> We need to be careful here, backward-compatibility is critical for >> both the client and the server, or to put it in other words, an >> uptables client should still be able to connect a patched server, and >> vice-versa. This is an area where it is important to not break any >> third-part tool, either using libpq or reimplementing the frontend >> protocol. > > Which is why in my introduction to the v4 patch I explicitly mentioned > that this was missing. I wanted to talk about how this should be > implemented, since I feel that I received no feedback on that design > From last time. It's not hard to port that design over, if desired. I gave my opinion about the parametrization here a couple of months back, and thought that it was rather a neat design. I still think so: http://www.postgresql.org/message-id/cab7npqrgq60pwlb-cllrvmoqwpabnu-961w+xgpvg62rmsk...@mail.gmail.com In short: 1) Introduction of new pg_hba parameter require_encrypt, defaulting to off, enforcing the clients to have encryption. This way an administrator of a new server can prevent connections of old clients if he/she wants to have all the connections excrypted. 2) Client-side parameter, that you named previously gss_encrypt, to let a client decide if he wishes to do encryption or not. >> So I finally began to dive into your new patch... And after testing >> this is breaking the authentication protocol for GSS. I have been able >> to connect to a server once, but at the second attempt and after >> connection is failing with the following error: >> psql: expected authentication request from server, but received ioltas > > Interesting. I will see if I can find anything. The capture posted > earlier (thanks David!) suggests that the client doesn't expect the > encrypted data. > > I suspect what has happened is a race between the client buffering data > From the socket and the client processing the completion of GSSAPI > authentication. This kind of issue is why my v1 handled GSSAPI at the > protocol level, not at the transport level. I think we end up with > encrypted data in the buffer that's supposed to be decrypted, and since > the GSSAPI blob starts with \x00, it doesn't know what to do. > > I'll cut a v6 with most of the changes we've talked about here. It > should address this issue, but I suspect that no one will be happy about > how, since the client essentially needs to "un-read" some data. Let's be sure that we come out with something rock-solid here, the code paths taken for authentication do not require an authenticated user, so any bugs introduced could have dangerous consequences for the backend. > As a side note, this would also explain why I can't reproduce the issue, > since I'm running in very low-latency environments (three VMs on my > laptop). I'm doing my tests in single VM, with both krb5kdc and Postgres running together. >> Also, something that is missing is the parametrization that has been >> discussed the last time this patch was on the mailing list. Where is >> the capacity to control if a client is connecting to a server that is >> performing encryption and downgrade to non-ecrypted connection should >> the server not be able to support it? Enforcing a client to require >> encryption support using pg_hba.conf was as well a good thing. Some >> more infrastructure is needed here, I thought that there was an >> agreement previously regarding that. > > This does not match my impression of the discussion, but I would be > happy to be wrong about that since it is less work for me. OK, good to know. I had the opposite impression actually :) See above. >> Also, and to make the review a bit easier, it would be good to split >> the patch into smaller pieces (thanks for waiting on my input here, >> this became quite clear after looking at this code). Basically, >> pg_GSS_error is moved to a new file, bringing with it pg_GSS_recvauth >> because the error routine is being used by the new ones you are >> introducing: be_gssapi_write, etc. The split that this patch is doing >> is a bit confusing, all the GSS-related stuff is put within one single >> file: >> - read/write routines >> - authentication routine >> - constants >> - routines for error handling >> Mixing read/write routines with the authentication routine looks >> wrong, because the read-write routines just need to create a >> dependency with for example be-secure.c on the backend. In short, >> where before authentication and secure read/writes after >> authentication get linked to each other, and create a dependency that >> did not exist before. >> >> For the sake of clarity I would suggest the following split: >> - be-gss-common.c, where all the constants and the error handling >> routine are located. >> - Let authrecv in auth.c >> - Move only the read/write routines to the new file be-[secure-]gssapi.c >> Splitting the patches into one that moves around the routines
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
Michael Paquier writes: > On Tue, Feb 16, 2016 at 2:45 AM, Robbie Harwood wrote: >> David Steele writes: >>> 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: >>> >>> 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. >>> >>> 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. >> >> What I can't tell from looking at your methodology is whether both the >> client and server were running my patches or no. There's no fallback >> here (I'd like to talk about how that should work, with example from >> v1-v3, if people have ideas). This means that both the client and the >> server need to be running my patches for the moment. Is this your >> setup? > > We need to be careful here, backward-compatibility is critical for > both the client and the server, or to put it in other words, an > uptables client should still be able to connect a patched server, and > vice-versa. This is an area where it is important to not break any > third-part tool, either using libpq or reimplementing the frontend > protocol. Which is why in my introduction to the v4 patch I explicitly mentioned that this was missing. I wanted to talk about how this should be implemented, since I feel that I received no feedback on that design From last time. It's not hard to port that design over, if desired. > So I finally began to dive into your new patch... And after testing > this is breaking the authentication protocol for GSS. I have been able > to connect to a server once, but at the second attempt and after > connection is failing with the following error: > psql: expected authentication request from server, but received ioltas Interesting. I will see if I can find anything. The capture posted earlier (thanks David!) suggests that the client doesn't expect the encrypted data. I suspect what has happened is a race between the client buffering data From the socket and the client processing the completion of GSSAPI authentication. This kind of issue is why my v1 handled GSSAPI at the protocol level, not at the transport level. I think we end up with encrypted data in the buffer that's supposed to be decrypted, and since the GSSAPI blob starts with \x00, it doesn't know what to do. I'll cut a v6 with most of the changes we've talked about here. It should address this issue, but I suspect that no one will be happy about how, since the client essentially needs to "un-read" some data. As a side note, this would also explain why I can't reproduce the issue, since I'm running in very low-latency environments (three VMs on my laptop). > Also, something that is missing is the parametrization that has been > discussed the last time this patch was on the mailing list. Where is > the capacity to control if a client is connecting to a server that is > performing encryption and downgrade to non-ecrypted connection should > the server not be able to support it? Enforcing a client to require > encryption support using pg_hba.conf was as well a good thing. Some > more infrastructure is needed here, I thought that there was an > agreement previously regarding that. This does not match my impression of the discussion, but I would be happy to be wrong about that since it is less work for me. > Also, and to make the review a bit easier, it would be good to split > the patch into smaller pieces (thanks for waiting on my input here, > this became quite clear after looking at this code). Basically, > pg_GSS_error is moved to a new file, bringing with it pg_GSS_recvauth > because the error routine is being used by the new ones you are > introducing: be_gssapi_write, etc. The split that this patch is doing > is a bit confusing, all the GSS-related stuff is put within one single > file: > - read/write routines > - authentication routine > - constants > - routines for error handling > Mixing read/write routines with the authentication routine looks > wrong, because the read-write routines just need to create a > dependency with for example be-secure.c on the backend. In short, > where before authentication and secure read/writes after > authentication
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
On 2/25/16 2:08 AM, Michael Paquier wrote: > On Wed, Feb 24, 2016 at 7:12 PM, Robbie Harwood wrote: >> >> Not that I can immediately see. As long as the client and server are >> both patched, everything should work. My process is the same as with >> previous versions of this patchset [0], and though I'm using FreeIPA >> there is no reason it shouldn't work with any other KDC (MIT, for >> instance[1]) provided the IPA calls are converted. > > I used a custom krb5kdc set up manually, and all my connection > attempts are working on HEAD, not with your patch (both client and > server patched). I've got the same setup with the same results. >> I am curious, though - I haven't changed any of the authentication code >> in v4/v5 from what's in ~master, so how often can you log in using >> GSSAPI using master? > > My guess is that there is something not been correctly cleaned up when > closing the connection. The first attempt worked for me, not after. I was able to get in again after a number of failed attempts, though the number varied. -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
On Wed, Feb 24, 2016 at 7:12 PM, Robbie Harwood wrote: > David Steele writes: > >> On 2/15/16 12:45 PM, Robbie Harwood wrote: >>> David Steele writes: >>> 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. >>> >>> It applied to head of master (57c932475504d63d8f8a68fc6925d7decabc378a) >>> for me (`patch -p1 < v4-GSSAPI-encryption-support.patch`). I rebased it >>> anyway and cut a v5 anyway, just to be sure. It's attached, and >>> available on github as well: >>> https://github.com/frozencemetery/postgres/commit/dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 >> >> It could have been my mistake. I'll give it another try when you have a >> new patch. > > Please do let me know how v5 goes. If you run into trouble, in addition > to the logs you helpfully provided before, I'd like a traffic dump (pcap > preferable; I need tcp/udp port 88 for Kerberos and tcp port 5432 or > whatever you're running postgres on) if possible. Thanks! > 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: >>> >>> What I can't tell from looking at your methodology is whether both the >>> client and server were running my patches or no. There's no fallback >>> here (I'd like to talk about how that should work, with example from >>> v1-v3, if people have ideas). This means that both the client and the >>> server need to be running my patches for the moment. Is this your >>> setup? >> >> I was testing on a system with no version of PostgreSQL installed. I >> applied your patch to master and then ran both server and client from >> that patched version. Is there something I'm missing? > > Not that I can immediately see. As long as the client and server are > both patched, everything should work. My process is the same as with > previous versions of this patchset [0], and though I'm using FreeIPA > there is no reason it shouldn't work with any other KDC (MIT, for > instance[1]) provided the IPA calls are converted. I used a custom krb5kdc set up manually, and all my connection attempts are working on HEAD, not with your patch (both client and server patched). > I am curious, though - I haven't changed any of the authentication code > in v4/v5 from what's in ~master, so how often can you log in using > GSSAPI using master? My guess is that there is something not been correctly cleaned up when closing the connection. The first attempt worked for me, not after. -- 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 v5] GSSAPI encryption support
On Tue, Feb 16, 2016 at 2:45 AM, Robbie Harwood wrote: > David Steele writes: >> 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. > > It applied to head of master (57c932475504d63d8f8a68fc6925d7decabc378a) > for me (`patch -p1 < v4-GSSAPI-encryption-support.patch`). I rebased it > anyway and cut a v5 anyway, just to be sure. It's attached, and > available on github as well: > https://github.com/frozencemetery/postgres/commit/dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 v5 is applying fine for me. There were two diff hunks in routine.sgml but nothing to worry about. >> 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. > > What I can't tell from looking at your methodology is whether both the > client and server were running my patches or no. There's no fallback > here (I'd like to talk about how that should work, with example from > v1-v3, if people have ideas). This means that both the client and the > server need to be running my patches for the moment. Is this your > setup? We need to be careful here, backward-compatibility is critical for both the client and the server, or to put it in other words, an uptables client should still be able to connect a patched server, and vice-versa. This is an area where it is important to not break any third-part tool, either using libpq or reimplementing the frontend protocol. So I finally began to dive into your new patch... And after testing this is breaking the authentication protocol for GSS. I have been able to connect to a server once, but at the second attempt and after connection is failing with the following error: psql: expected authentication request from server, but received ioltas Also, something that is missing is the parametrization that has been discussed the last time this patch was on the mailing list. Where is the capacity to control if a client is connecting to a server that is performing encryption and downgrade to non-ecrypted connection should the server not be able to support it? Enforcing a client to require encryption support using pg_hba.conf was as well a good thing. Some more infrastructure is needed here, I thought that there was an agreement previously regarding that. Also, and to make the review a bit easier, it would be good to split the patch into smaller pieces (thanks for waiting on my input here, this became quite clear after looking at this code). Basically, pg_GSS_error is moved to a new file, bringing with it pg_GSS_recvauth because the error routine is being used by the new ones you are introducing: be_gssapi_write, etc. The split that this patch is doing is a bit confusing, all the GSS-related stuff is put within one single file: - read/write routines - authentication routine - constants - routines for error handling Mixing read/write routines with the authentication routine looks wrong, because the read-write routines just need to create a dependency with for example be-secure.c on the backend. In short, where before authentication and secure read/writes after authentication get linked to each other, and create a dependency that did not exist before. For the sake of clarity I would suggest the following split: - be-gss-common.c, where all the constants and the error handling routine are located. - Let authrecv in auth.c - Move only the
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
David Steele writes: > On 2/15/16 12:45 PM, Robbie Harwood wrote: >> David Steele writes: >> >>> 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. >> >> It applied to head of master (57c932475504d63d8f8a68fc6925d7decabc378a) >> for me (`patch -p1 < v4-GSSAPI-encryption-support.patch`). I rebased it >> anyway and cut a v5 anyway, just to be sure. It's attached, and >> available on github as well: >> https://github.com/frozencemetery/postgres/commit/dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 > > It could have been my mistake. I'll give it another try when you have a > new patch. Please do let me know how v5 goes. If you run into trouble, in addition to the logs you helpfully provided before, I'd like a traffic dump (pcap preferable; I need tcp/udp port 88 for Kerberos and tcp port 5432 or whatever you're running postgres on) if possible. Thanks! >>> 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: >> >> What I can't tell from looking at your methodology is whether both the >> client and server were running my patches or no. There's no fallback >> here (I'd like to talk about how that should work, with example from >> v1-v3, if people have ideas). This means that both the client and the >> server need to be running my patches for the moment. Is this your >> setup? > > I was testing on a system with no version of PostgreSQL installed. I > applied your patch to master and then ran both server and client from > that patched version. Is there something I'm missing? Not that I can immediately see. As long as the client and server are both patched, everything should work. My process is the same as with previous versions of this patchset [0], and though I'm using FreeIPA there is no reason it shouldn't work with any other KDC (MIT, for instance[1]) provided the IPA calls are converted. I am curious, though - I haven't changed any of the authentication code in v4/v5 from what's in ~master, so how often can you log in using GSSAPI using master? [0]: https://mivehind.net/2015/06/11/kerberized-postgresql/ [1]: http://web.mit.edu/kerberos/krb5-devel/doc/admin/install_kdc.html signature.asc Description: PGP signature
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
On 2/15/16 12:45 PM, Robbie Harwood wrote: > David Steele writes: > >> 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. > > It applied to head of master (57c932475504d63d8f8a68fc6925d7decabc378a) > for me (`patch -p1 < v4-GSSAPI-encryption-support.patch`). I rebased it > anyway and cut a v5 anyway, just to be sure. It's attached, and > available on github as well: > https://github.com/frozencemetery/postgres/commit/dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 It could have been my mistake. I'll give it another try when you have a new patch. >> 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: > > What I can't tell from looking at your methodology is whether both the > client and server were running my patches or no. There's no fallback > here (I'd like to talk about how that should work, with example from > v1-v3, if people have ideas). This means that both the client and the > server need to be running my patches for the moment. Is this your > setup? I was testing on a system with no version of PostgreSQL installed. I applied your patch to master and then ran both server and client from that patched version. Is there something I'm missing? -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] [PATCH v5] GSSAPI encryption support
David Steele writes: > 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. It applied to head of master (57c932475504d63d8f8a68fc6925d7decabc378a) for me (`patch -p1 < v4-GSSAPI-encryption-support.patch`). I rebased it anyway and cut a v5 anyway, just to be sure. It's attached, and available on github as well: https://github.com/frozencemetery/postgres/commit/dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 > 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. What I can't tell from looking at your methodology is whether both the client and server were running my patches or no. There's no fallback here (I'd like to talk about how that should work, with example from v1-v3, if people have ideas). This means that both the client and the server need to be running my patches for the moment. Is this your setup? Thanks for taking it for a spin! --Robbie From dc10e3519f0f6c67f79abd157dc8ff1a1c293f53 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 | 583 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| 474 + src/interfaces/libpq/fe-secure.c| 16 +- src/interfaces/libpq/libpq-int.h| 16 +- 18 files changed, 1191 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 b3f3abe..a5bd629 100755 --- a/configure +++ b/configure @@ -713,6 +713,7 @@ with_systemd with_selinux with_openssl krb_srvtab +with_gssapi with_python with_perl with_tcl @@ -5491,6 +5492,7 @@ $as_echo "$with_gssapi" >&6; } + # # Kerberos configuration parameters # diff --git a/configure.in b/configure.in index 0bd90d7..4fd8f05 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
