Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * Stephen Frost (sfr...@snowman.net) wrote: > * Stephen Frost (sfr...@snowman.net) wrote: > > * David Christensen (da...@pgguru.net) wrote: > > > Ok, based on the interdiff there, I'm happy with that last change. > > > Marking > > > as Ready For Committer. > > > > Great, thanks! > > > > I'm going to go through it again myself but I feel reasonably good about > > it and if nothing else pops and there aren't objections, I'll push it > > before feature freeze. > > Alright, I've cleaned up a few of the error messages to consistently use > GSSAPI (instead of a mix of GSSAPI and gssapi) and run the changes through > pgindent. Updated patch attached. cfbot looks happy. Feeling like > this is looking just about ready to go. And pushed, thanks again! Time to watch the buildfarm.. :) Stephen signature.asc Description: PGP signature
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * Stephen Frost (sfr...@snowman.net) wrote: > * David Christensen (da...@pgguru.net) wrote: > > Ok, based on the interdiff there, I'm happy with that last change. Marking > > as Ready For Committer. > > Great, thanks! > > I'm going to go through it again myself but I feel reasonably good about > it and if nothing else pops and there aren't objections, I'll push it > before feature freeze. Alright, I've cleaned up a few of the error messages to consistently use GSSAPI (instead of a mix of GSSAPI and gssapi) and run the changes through pgindent. Updated patch attached. cfbot looks happy. Feeling like this is looking just about ready to go. Thanks! Stephen From c79373e14fed0e7b019415bce98e2c72a9ada24c Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Mon, 28 Feb 2022 20:17:55 -0500 Subject: [PATCH] Add support for Kerberos credential delegation Support GSSAPI/Kerberos credentials being delegated to the server by a client. With this, a user authenticating to PostgreSQL using Kerberos (GSSAPI) credentials can choose to delegate their credentials to the PostgreSQL server (which can choose to accept them, or not), allowing the server to then use those delegated credentials to connect to another service, such as with postgres_fdw or dblink or theoretically any other service which is able to be authenticated using Kerberos. Both postgres_fdw and dblink are changed to allow non-superuser password-less connections but only when GSSAPI credentials have been delegated to the server by the client and GSSAPI is used to authenticate to the remote system. Authors: Stephen Frost, Peifeng Qiu Reviewed-By: David Christensen Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com --- contrib/dblink/dblink.c | 127 --- contrib/dblink/expected/dblink.out| 4 +- contrib/postgres_fdw/connection.c | 72 +++- .../postgres_fdw/expected/postgres_fdw.out| 19 +- contrib/postgres_fdw/option.c | 6 + contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +- doc/src/sgml/config.sgml | 17 + doc/src/sgml/dblink.sgml | 5 +- doc/src/sgml/libpq.sgml | 41 +++ doc/src/sgml/monitoring.sgml | 9 + doc/src/sgml/postgres-fdw.sgml| 7 +- src/backend/catalog/system_views.sql | 3 +- src/backend/foreign/foreign.c | 1 + src/backend/libpq/auth.c | 13 +- src/backend/libpq/be-gssapi-common.c | 53 +++ src/backend/libpq/be-secure-gssapi.c | 26 +- src/backend/utils/activity/backend_status.c | 1 + src/backend/utils/adt/pgstatfuncs.c | 21 +- src/backend/utils/init/postinit.c | 8 +- src/backend/utils/misc/guc_tables.c | 10 + src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/catalog/pg_proc.dat | 6 +- src/include/libpq/auth.h | 1 + src/include/libpq/be-gssapi-common.h | 3 + src/include/libpq/libpq-be.h | 2 + src/include/utils/backend_status.h| 1 + src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-auth.c| 15 +- src/interfaces/libpq/fe-connect.c | 17 + src/interfaces/libpq/fe-secure-gssapi.c | 23 +- src/interfaces/libpq/libpq-fe.h | 1 + src/interfaces/libpq/libpq-int.h | 2 + src/test/kerberos/Makefile| 3 + src/test/kerberos/t/001_auth.pl | 331 -- src/test/perl/PostgreSQL/Test/Utils.pm| 27 ++ src/test/regress/expected/rules.out | 11 +- 36 files changed, 755 insertions(+), 136 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 78a8bcee6e..55f75eff36 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -48,6 +48,7 @@ #include "funcapi.h" #include "lib/stringinfo.h" #include "libpq-fe.h" +#include "libpq/libpq-be.h" #include "libpq/libpq-be-fe-helpers.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -111,7 +112,8 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode); static char *generate_relation_name(Relation rel); static void dblink_connstr_check(const char *connstr); -static void dblink_security_check(PGconn *conn, remoteConn *rconn); +static bool dblink_connstr_has_pw(const char *connstr); +static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr); static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res, bool fail, const char *fmt,...) pg_attribute_printf(5, 6); static char *get_connect_string(const char *servername); @@ -213,7 +215,7 @@ dblink_get_conn(char
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * David Christensen (da...@pgguru.net) wrote: > Ok, based on the interdiff there, I'm happy with that last change. Marking > as Ready For Committer. Great, thanks! I'm going to go through it again myself but I feel reasonably good about it and if nothing else pops and there aren't objections, I'll push it before feature freeze. Thanks! Stephen signature.asc Description: PGP signature
Re: Kerberos delegation support in libpq and postgres_fdw
Ok, based on the interdiff there, I'm happy with that last change. Marking as Ready For Committer. Best, David
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* David Christensen (da...@pgguru.net) wrote:
> Reviewed v8; largely looking good, though I notice this hunk, which may
> arguably be a bug fix, but doesn't appear to be relevant to this specific
> patch, so could probably be debated independently (and if a bug, should
> probably be backpatched):
>
> diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
> index 4229d2048c..11d41979c6 100644
> --- a/contrib/postgres_fdw/option.c
> +++ b/contrib/postgres_fdw/option.c
> @@ -288,6 +288,9 @@ InitPgFdwOptions(void)
> {"sslcert", UserMappingRelationId, true},
> {"sslkey", UserMappingRelationId, true},
>
> + /* gssencmode is also libpq option, same to above. */
> + {"gssencmode", UserMappingRelationId, true},
> +
> {NULL, InvalidOid, false}
> };
Hmm, yeah, hard to say if that makes sense at a user-mapping level or
not. Agreed that we could have an independent discussion regarding
that and if it should be back-patched, so removed it from this patch.
> That said, should "gssdeleg" be exposed as a user mapping? (This shows up
> in postgresql_fdw; not sure if there are other places that would be
> relevant, like in dblink somewhere as well, just a thought.)
Ah, yeah, that certainly makes sense to have as optional for a user
mapping. dblink doesn't have the distinction between server-level
options and user mapping options (as it doesn't have user mappings at
all really) so it doesn't have something similar.
Updated patch attached.
Thanks!
Stephen
From 87642bc75e7d4f3d986d4f100e6ee00711155bc7 Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Mon, 28 Feb 2022 20:17:55 -0500
Subject: [PATCH] Add support for Kerberos credential delegation
Support GSSAPI/Kerberos credentials being delegated to the server by a
client. With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.
Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
delegated to the server by the client and GSSAPI is used to
authenticate to the remote system.
Authors: Stephen Frost, Peifeng Qiu
Reviewed-By: David Christensen
Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com
---
contrib/dblink/dblink.c | 127 ---
contrib/dblink/expected/dblink.out| 4 +-
contrib/postgres_fdw/connection.c | 72 +++-
.../postgres_fdw/expected/postgres_fdw.out| 19 +-
contrib/postgres_fdw/option.c | 6 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +-
doc/src/sgml/config.sgml | 17 +
doc/src/sgml/dblink.sgml | 5 +-
doc/src/sgml/libpq.sgml | 41 +++
doc/src/sgml/monitoring.sgml | 9 +
doc/src/sgml/postgres-fdw.sgml| 7 +-
src/backend/catalog/system_views.sql | 3 +-
src/backend/foreign/foreign.c | 1 +
src/backend/libpq/auth.c | 13 +-
src/backend/libpq/be-gssapi-common.c | 51 +++
src/backend/libpq/be-secure-gssapi.c | 26 +-
src/backend/utils/activity/backend_status.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 20 +-
src/backend/utils/init/postinit.c | 8 +-
src/backend/utils/misc/guc_tables.c | 10 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/catalog/pg_proc.dat | 6 +-
src/include/libpq/auth.h | 1 +
src/include/libpq/be-gssapi-common.h | 3 +
src/include/libpq/libpq-be.h | 2 +
src/include/utils/backend_status.h| 1 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-auth.c| 15 +-
src/interfaces/libpq/fe-connect.c | 17 +
src/interfaces/libpq/fe-secure-gssapi.c | 23 +-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 2 +
src/test/kerberos/Makefile| 3 +
src/test/kerberos/t/001_auth.pl | 331 --
src/test/perl/PostgreSQL/Test/Utils.pm| 27 ++
src/test/regress/expected/rules.out | 11 +-
36 files changed, 752 insertions(+), 136 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e..533eb90e62 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -48,6 +48,7 @@
#include "funcapi.h"
#include "lib/stringinfo.h"
#include "libpq-fe.h"
+#include "libpq/libpq-be.h"
#include
Re: Kerberos delegation support in libpq and postgres_fdw
Reviewed v8; largely looking good, though I notice this hunk, which may
arguably be a bug fix, but doesn't appear to be relevant to this specific
patch, so could probably be debated independently (and if a bug, should
probably be backpatched):
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 4229d2048c..11d41979c6 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -288,6 +288,9 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
+
{NULL, InvalidOid, false}
};
That said, should "gssdeleg" be exposed as a user mapping? (This shows up
in postgresql_fdw; not sure if there are other places that would be
relevant, like in dblink somewhere as well, just a thought.)
Best,
David
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * David Christensen (da...@pgguru.net) wrote: > On Wed, Apr 5, 2023 at 3:30 PM Stephen Frost wrote: > > Per GSS docs, seems like we should be comparing to GSS_C_NO_CREDENTIAL > > and validating that the gflags has the `deleg_flag` bit set before > > considering whether there are valid credentials; in practice this might be > > the same effect (haven't looked at what that symbol actually resolves to, > > but NULL would be sensible). > > > > GSS_C_NO_CREDENTIAL is indeed NULL, but updated to that anyway to be a > > bit cleaner and also added an explicit check that GSS_C_DELEG_FLAG was > > set in gflags. > > + proxy = NULL; > [...] > + if (proxy != GSS_C_NO_CREDENTIAL && gflags & GSS_C_DELEG_FLAG) > > We should probably also initialize "proxy" to GSS_C_NO_CREDENTIAL as well, > yes? Sure, done, and updated for both auth.c and be-secure-gssapi.c > > > + /* > > > + * Set KRB5CCNAME for this backend, so that later calls to > > gss_acquire_cred > > > + * will find the proxied credentials we stored. > > > + */ > > > > > > So I'm not seeing this in other use in the code; I assume this is just > > used by the krb5 libs? > > > > Not sure I'm following. gss_acquire_cred() is called in > > src/interfaces/libpq/fe-gssapi-common.c. > > I just meant the KRB5CCNAME envvar itself; looks like my assumption was > right. Ah, yes, that's correct. > So on a re-read of the v7 patch, there seems to be a bit of inconsistent > usage between delegation and proxying; i.e., the field itself is called > gss_proxy in the gssstatus struct, authentication messages, etc, but the > setting and docs refer to GSS delegation. Are there subtle distinctions > between these? It seems like this patch is using them interchangeably, so > it might be good to settle on one terminology here unless there are already > well-defined categories for where to use one and where to use the other. That's a fair point and so I've updated the patch to consistently use 'delegated credentials' and similar to match the Kerberos documentation. In Kerberos there is *also* the concept of proxied credentials which are very very similar to delegated credentials (they're actually "constrainted delegations") but they're not exactly the same and that isn't what we're doing with this particular patch (though I hope that once we get support for unconstrained delegation, which is what this patch is doing, we can then go add support for constrainted delegations). Updated patch attached. Thanks! Stephen From 0be69ca2720b2091d2ae79b7f5c91fa30cc27d13 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Mon, 28 Feb 2022 20:17:55 -0500 Subject: [PATCH] Add support for Kerberos credential delegation Support GSSAPI/Kerberos credentials being delegated to the server by a client. With this, a user authenticating to PostgreSQL using Kerberos (GSSAPI) credentials can choose to delegate their credentials to the PostgreSQL server (which can choose to accept them, or not), allowing the server to then use those delegated credentials to connect to another service, such as with postgres_fdw or dblink or theoretically any other service which is able to be authenticated using Kerberos. Both postgres_fdw and dblink are changed to allow non-superuser password-less connections but only when GSSAPI credentials have been delegated to the server by the client and GSSAPI is used to authenticate to the remote system. Authors: Stephen Frost, Peifeng Qiu Reviewed-By: David Christensen Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com --- contrib/dblink/dblink.c | 127 --- contrib/dblink/expected/dblink.out| 4 +- contrib/postgres_fdw/connection.c | 72 +++- .../postgres_fdw/expected/postgres_fdw.out| 19 +- contrib/postgres_fdw/option.c | 3 + contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +- doc/src/sgml/config.sgml | 17 + doc/src/sgml/dblink.sgml | 5 +- doc/src/sgml/libpq.sgml | 41 +++ doc/src/sgml/monitoring.sgml | 9 + doc/src/sgml/postgres-fdw.sgml| 7 +- src/backend/catalog/system_views.sql | 3 +- src/backend/foreign/foreign.c | 1 + src/backend/libpq/auth.c | 13 +- src/backend/libpq/be-gssapi-common.c | 51 +++ src/backend/libpq/be-secure-gssapi.c | 26 +- src/backend/utils/activity/backend_status.c | 1 + src/backend/utils/adt/pgstatfuncs.c | 20 +- src/backend/utils/init/postinit.c | 8 +- src/backend/utils/misc/guc_tables.c | 10 + src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/catalog/pg_proc.dat | 6 +- src/include/libpq/auth.h | 1 + src/include/libpq/be-gssapi-common.h | 3 + src/include/libpq/libpq-be.h
Re: Kerberos delegation support in libpq and postgres_fdw
On Wed, Apr 5, 2023 at 3:30 PM Stephen Frost wrote:
> Greetings,
>
> * David Christensen (david...@pgguru.net) wrote:
> > Did a code review pass here; here is some feedback.
>
> Thanks!
>
> > + /* If password was used to connect, make sure it was one provided
> */
> > + if (PQconnectionUsedPassword(conn) &&
> dblink_connstr_has_pw(connstr))
> > + return;
> >
> > Do we need to consider whether these passwords are the same? Is there
> a different vector where a different password could be acquired from a
> different source (PGPASSWORD, say) while both of these criteria are true?
> Seems like it probably doesn't matter that much considering we only checked
> Password alone in previous version of this code.
>
> Note that this patch isn't really changing how these checks are being
> done but more moving them around and allowing a GSSAPI-based approach
> with credential delegation to also be allowed.
>
> That said, as noted in the comments above dblink_connstr_check():
>
> * For non-superusers, insist that the connstr specify a password, except
> * if GSSAPI credentials have been proxied (and we check that they are used
> * for the connection in dblink_security_check later). This prevents a
> * password or GSSAPI credentials from being picked up from .pgpass, a
> * service file, the environment, etc. We don't want the postgres user's
> * passwords or Kerberos credentials to be accessible to non-superusers.
>
> The point of these checks is, indeed, to ensure that environmental
> values such as a .pgpass or variable don't end up getting picked up and
> used (or, if they do, we realize it post-connection and then throw away
> the connection).
>
> libpq does explicitly prefer to use the password passed in as part of
> the connection string and won't attempt to look up passwords in a
> .pgpass file or similar if a password has been included in the
> connection string.
>
The case I think I was thinking of was (manufactured) when we connected to
a backend with one password but the dblink or postgresql_fdw includes an
explicit password to a different server. But now I'm thinking that this
PQconnectionUsedPassword() is checking the outgoing connection for dblink
itself, not the connection of the backend that connected to the main
server, so I think this objection is moot, like you say.
> Looks like the pg_gssinfo struct hides the `proxy_creds` def behind:
> >
> > #if defined(ENABLE_GSS) | defined(ENABLE_SSPI)
> > typedef struct
> > {
> > gss_buffer_desc outbuf; /* GSSAPI output token
> buffer */
> > #ifdef ENABLE_GSS
> > ...
> > bool proxy_creds;/* GSSAPI Delegated/proxy
> credentials */
> > #endif
> > } pg_gssinfo;
> > #endif
>
> ... right, proxy_creds only exists (today anyway) if ENABLE_GSS is set.
>
> > Which means that the later check in `be_gssapi_get_proxy()` we have:
[analysis snipped]
Fairly confident the analysis here is wrong, further, the cfbot seems to
> agree that there isn't a compile failure here:
>
> https://cirrus-ci.com/task/6589717672624128
>
> [20:19:15.985] gss: NO
>
> (we always build with SSPI on Windows, per
> src/include/port/win32_port.h).
>
Cool; since we have coverage for that case seems like my concern was
unwarranted.
[snip]
> >
> >
> > Only superusers may connect to foreign servers without password
> > -authentication, so always specify the password
> option
> > -for user mappings belonging to non-superusers.
> > +authentication or using gssapi proxied credentials, so specify the
> > +password option for user mappings belonging to
> > +non-superusers who are not able to proxy GSSAPI credentials.
> >
> >
> >
> > s/gssapi/GSSAPI/; this is kind of confusing, as this makes it sound like
> only superuser may use GSSAPI proxied credentials, which I disbelieve to be
> true. Additionally, it sounds like you're wanting to explicitly maintain a
> denylist for users to not be allowed proxying; is that correct?
>
> Updated to GSSAPI and reworded in the updated patch (attached).
> Certainly open to suggestions on how to improve the documentation here.
> There is no 'denylist' for users when it comes to GSSAPI proxied
> credentials. If there's a use-case for that then it could be added in
> the future.
>
Okay, I think your revisions here seem more clear, thanks.
>
> > ---
> >
> > libpq/auth.c:
> >
> > if (proxy != NULL)
> > {
> > pg_store_proxy_credential(proxy);
> > port->gss->proxy_creds = true;
> > }
> >
> > Per GSS docs, seems like we should be comparing to GSS_C_NO_CREDENTIAL
> and validating that the gflags has the `deleg_flag` bit set before
> considering whether there are valid credentials; in practice this might be
> the same effect (haven't looked at what that symbol actually resolves to,
> but NULL would be sensible).
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* David Christensen (david...@pgguru.net) wrote:
> Did a code review pass here; here is some feedback.
Thanks!
> + /* If password was used to connect, make sure it was one provided */
> + if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr))
> + return;
>
> Do we need to consider whether these passwords are the same? Is there a
> different vector where a different password could be acquired from a
> different source (PGPASSWORD, say) while both of these criteria are true?
> Seems like it probably doesn't matter that much considering we only checked
> Password alone in previous version of this code.
Note that this patch isn't really changing how these checks are being
done but more moving them around and allowing a GSSAPI-based approach
with credential delegation to also be allowed.
That said, as noted in the comments above dblink_connstr_check():
* For non-superusers, insist that the connstr specify a password, except
* if GSSAPI credentials have been proxied (and we check that they are used
* for the connection in dblink_security_check later). This prevents a
* password or GSSAPI credentials from being picked up from .pgpass, a
* service file, the environment, etc. We don't want the postgres user's
* passwords or Kerberos credentials to be accessible to non-superusers.
The point of these checks is, indeed, to ensure that environmental
values such as a .pgpass or variable don't end up getting picked up and
used (or, if they do, we realize it post-connection and then throw away
the connection).
libpq does explicitly prefer to use the password passed in as part of
the connection string and won't attempt to look up passwords in a
.pgpass file or similar if a password has been included in the
connection string.
> Looks like the pg_gssinfo struct hides the `proxy_creds` def behind:
>
> #if defined(ENABLE_GSS) | defined(ENABLE_SSPI)
> typedef struct
> {
> gss_buffer_desc outbuf; /* GSSAPI output token buffer */
> #ifdef ENABLE_GSS
> ...
> bool proxy_creds;/* GSSAPI Delegated/proxy credentials */
> #endif
> } pg_gssinfo;
> #endif
... right, proxy_creds only exists (today anyway) if ENABLE_GSS is set.
> Which means that the later check in `be_gssapi_get_proxy()` we have:
>
> /*
> * Return if GSSAPI delegated/proxy credentials were included on this
> * connection.
> */
> bool
> be_gssapi_get_proxy(Port *port)
> {
> if (!port || !port->gss)
> return NULL;
>
> return port->gss->proxy_creds;
> }
but we don't build be-secure-gssapi.c, where this function is added,
unless --with-gssapi is included, from src/backend/libpq/Makefile:
ifeq ($(with_gssapi),yes)
OBJS += be-gssapi-common.o be-secure-gssapi.o
endif
Further, src/include/libpq/libpq-be.h has a matching #ifdef ENABLE_GSS
for the function declarations:
#ifdef ENABLE_GSS
/*
* Return information about the GSSAPI authenticated connection
*/
extern bool be_gssapi_get_auth(Port *port);
extern bool be_gssapi_get_enc(Port *port);
extern const char *be_gssapi_get_princ(Port *port);
extern bool be_gssapi_get_proxy(Port *port);
> So in theory it'd be possible to have SSPI enabled but GSS disabled and we'd
> fail to compile in that case. (It may be that this routine is never
> *actually* called in that case, just noting compile-time considerations.)
> I'm not seeing guards in the actual PQ* routines, but don't think I've done
> an exhaustive search.
Fairly confident the analysis here is wrong, further, the cfbot seems to
agree that there isn't a compile failure here:
https://cirrus-ci.com/task/6589717672624128
[20:19:15.985] gss: NO
(we always build with SSPI on Windows, per
src/include/port/win32_port.h).
> gss_accept_deleg
>
>
> +
> +Forward (delegate) GSS credentials to the server. The default is
> +disable which means credentials will not be
> forwarded
> +to the server. Set this to enable to have
> +credentials forwarded when possible.
>
> When is this not possible? Server policy? External factors?
The Kerberos credentials have to be forwardable for them to be allowed
to be forwarded and the server has to be configured to accept them.
>
>
> Only superusers may connect to foreign servers without password
> -authentication, so always specify the password option
> -for user mappings belonging to non-superusers.
> +authentication or using gssapi proxied credentials, so specify the
> +password option for user mappings belonging to
> +non-superusers who are not able to proxy GSSAPI credentials.
>
>
>
> s/gssapi/GSSAPI/; this is kind of confusing, as this makes it sound like only
> superuser may use GSSAPI proxied credentials, which I disbelieve to be true.
> Additionally, it sounds like you're wanting to explicitly maintain a
Re: Kerberos delegation support in libpq and postgres_fdw
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Did a code review pass here; here is some feedback.
+ /* If password was used to connect, make sure it was one provided */
+ if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr))
+ return;
Do we need to consider whether these passwords are the same? Is there a
different vector where a different password could be acquired from a different
source (PGPASSWORD, say) while both of these criteria are true? Seems like it
probably doesn't matter that much considering we only checked Password alone in
previous version of this code.
---
Looks like the pg_gssinfo struct hides the `proxy_creds` def behind:
#if defined(ENABLE_GSS) | defined(ENABLE_SSPI)
typedef struct
{
gss_buffer_desc outbuf; /* GSSAPI output token buffer */
#ifdef ENABLE_GSS
...
boolproxy_creds;/* GSSAPI Delegated/proxy credentials */
#endif
} pg_gssinfo;
#endif
Which means that the later check in `be_gssapi_get_proxy()` we have:
/*
* Return if GSSAPI delegated/proxy credentials were included on this
* connection.
*/
bool
be_gssapi_get_proxy(Port *port)
{
if (!port || !port->gss)
return NULL;
return port->gss->proxy_creds;
}
So in theory it'd be possible to have SSPI enabled but GSS disabled and we'd
fail to compile in that case. (It may be that this routine is never *actually*
called in that case, just noting compile-time considerations.) I'm not seeing
guards in the actual PQ* routines, but don't think I've done an exhaustive
search.
---
gss_accept_deleg
+
+Forward (delegate) GSS credentials to the server. The default is
+disable which means credentials will not be
forwarded
+to the server. Set this to enable to have
+credentials forwarded when possible.
When is this not possible? Server policy? External factors?
---
Only superusers may connect to foreign servers without password
-authentication, so always specify the password option
-for user mappings belonging to non-superusers.
+authentication or using gssapi proxied credentials, so specify the
+password option for user mappings belonging to
+non-superusers who are not able to proxy GSSAPI credentials.
s/gssapi/GSSAPI/; this is kind of confusing, as this makes it sound like only
superuser may use GSSAPI proxied credentials, which I disbelieve to be true.
Additionally, it sounds like you're wanting to explicitly maintain a denylist
for users to not be allowed proxying; is that correct?
---
libpq/auth.c:
if (proxy != NULL)
{
pg_store_proxy_credential(proxy);
port->gss->proxy_creds = true;
}
Per GSS docs, seems like we should be comparing to GSS_C_NO_CREDENTIAL and
validating that the gflags has the `deleg_flag` bit set before considering
whether there are valid credentials; in practice this might be the same effect
(haven't looked at what that symbol actually resolves to, but NULL would be
sensible).
Are there other cases we might need to consider here, like valid credentials,
but they are expired? (GSS_S_CREDENTIALS_EXPIRED)
---
+ /*
+* Set KRB5CCNAME for this backend, so that later calls to
gss_acquire_cred
+* will find the proxied credentials we stored.
+*/
So I'm not seeing this in other use in the code; I assume this is just used by
the krb5 libs?
Similar q's for the other places the pg_gss_accept_deleg are used.
---
+int
+PQconnectionUsedGSSAPI(const PGconn *conn)
+{
+ if (!conn)
+ return false;
+ if (conn->gssapi_used)
+ return true;
+ else
+ return false;
+}
Micro-gripe: this routine seems like could be simpler, though the compiler
probably has the same thing to say for either, so maybe code clarity is better
as written:
int
PQconnectionUsedGSSAPI(const PGconn *conn)
{
return conn && conn->gssapi_used;
}
---
Anything required for adding meson support? I notice src/test/kerberos has
Makefile updated, but no meson.build files are changed.
---
Two tests in src/test/kerberos/t/001_auth.pl at :535 and :545 have the same
test description:
+ 'succeeds with GSS-encrypted access required and hostgssenc hba and
credentials not forwarded',
Since the first test has only `gssencmode` defined (so implicit `gssdeleg`
value) and the second has `gssdeleg=disable` I'd suggest that the test on :545
should have its description updated to add the word "explicitly":
'succeeds with GSS-encrypted access required and hostgssenc hba and
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Stephen Frost (sfr...@snowman.net) wrote:
> * Greg Stark (st...@mit.edu) wrote:
> > The CFBot says there's a function be_gssapi_get_proxy() which is
> > undefined. Presumably this is a missing #ifdef or a definition that
> > should be outside an #ifdef.
>
> Yup, just a couple of missing #ifdef's.
>
> Updated and rebased patch attached.
... and a few more. Apparently hacking on a plane without enough sleep
leads to changing ... and unchanging configure flags before testing.
Thanks,
Stephen
From 9808fd4eb4920e40468709af7325a27b18066cec Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Mon, 28 Feb 2022 20:17:55 -0500
Subject: [PATCH] Add support for Kerberos credential delegation
Support GSSAPI/Kerberos credentials being delegated to the server by a
client. With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.
Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
proxied to the server by the client and GSSAPI is used to authenticate
to the remote system.
Authors: Stephen Frost, Peifeng Qiu
Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com
---
contrib/dblink/dblink.c | 127 ---
contrib/dblink/expected/dblink.out| 4 +-
contrib/postgres_fdw/connection.c | 72 +++-
.../postgres_fdw/expected/postgres_fdw.out| 19 +-
contrib/postgres_fdw/option.c | 3 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +-
doc/src/sgml/config.sgml | 17 +
doc/src/sgml/dblink.sgml | 5 +-
doc/src/sgml/libpq.sgml | 41 +++
doc/src/sgml/monitoring.sgml | 9 +
doc/src/sgml/postgres-fdw.sgml| 5 +-
src/backend/catalog/system_views.sql | 3 +-
src/backend/foreign/foreign.c | 1 +
src/backend/libpq/auth.c | 13 +-
src/backend/libpq/be-gssapi-common.c | 51 +++
src/backend/libpq/be-secure-gssapi.c | 26 +-
src/backend/utils/activity/backend_status.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 20 +-
src/backend/utils/init/postinit.c | 8 +-
src/backend/utils/misc/guc_tables.c | 10 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/catalog/pg_proc.dat | 6 +-
src/include/libpq/auth.h | 1 +
src/include/libpq/be-gssapi-common.h | 3 +
src/include/libpq/libpq-be.h | 2 +
src/include/utils/backend_status.h| 1 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-auth.c| 15 +-
src/interfaces/libpq/fe-connect.c | 17 +
src/interfaces/libpq/fe-secure-gssapi.c | 23 +-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 2 +
src/test/kerberos/Makefile| 3 +
src/test/kerberos/t/001_auth.pl | 335 --
src/test/perl/PostgreSQL/Test/Utils.pm| 27 ++
src/test/regress/expected/rules.out | 11 +-
36 files changed, 752 insertions(+), 135 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e..d4dd338055 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -48,6 +48,7 @@
#include "funcapi.h"
#include "lib/stringinfo.h"
#include "libpq-fe.h"
+#include "libpq/libpq-be.h"
#include "libpq/libpq-be-fe-helpers.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -111,7 +112,8 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
static char *generate_relation_name(Relation rel);
static void dblink_connstr_check(const char *connstr);
-static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+static bool dblink_connstr_has_pw(const char *connstr);
+static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
static char *get_connect_string(const char *servername);
@@ -213,7 +215,7 @@ dblink_get_conn(char *conname_or_str,
errmsg("could not establish connection"),
errdetail_internal("%s", msg)));
}
- dblink_security_check(conn, rconn);
+ dblink_security_check(conn, rconn, connstr);
if
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Greg Stark (st...@mit.edu) wrote:
> The CFBot says there's a function be_gssapi_get_proxy() which is
> undefined. Presumably this is a missing #ifdef or a definition that
> should be outside an #ifdef.
Yup, just a couple of missing #ifdef's.
Updated and rebased patch attached.
Thanks!
Stephen
From 450a8749d04af54e8214a2ab357fbec7849a485b Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Mon, 28 Feb 2022 20:17:55 -0500
Subject: [PATCH] Add support for Kerberos credential delegation
Support GSSAPI/Kerberos credentials being delegated to the server by a
client. With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.
Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
proxied to the server by the client and GSSAPI is used to authenticate
to the remote system.
Authors: Stephen Frost, Peifeng Qiu
Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com
---
contrib/dblink/dblink.c | 120 ---
contrib/dblink/expected/dblink.out| 4 +-
contrib/postgres_fdw/connection.c | 70 +++-
.../postgres_fdw/expected/postgres_fdw.out| 19 +-
contrib/postgres_fdw/option.c | 3 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +-
doc/src/sgml/config.sgml | 17 +
doc/src/sgml/dblink.sgml | 5 +-
doc/src/sgml/libpq.sgml | 41 +++
doc/src/sgml/monitoring.sgml | 9 +
doc/src/sgml/postgres-fdw.sgml| 5 +-
src/backend/catalog/system_views.sql | 3 +-
src/backend/foreign/foreign.c | 1 +
src/backend/libpq/auth.c | 13 +-
src/backend/libpq/be-gssapi-common.c | 51 +++
src/backend/libpq/be-secure-gssapi.c | 26 +-
src/backend/utils/activity/backend_status.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 20 +-
src/backend/utils/init/postinit.c | 8 +-
src/backend/utils/misc/guc_tables.c | 10 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/catalog/pg_proc.dat | 6 +-
src/include/libpq/auth.h | 1 +
src/include/libpq/be-gssapi-common.h | 3 +
src/include/libpq/libpq-be.h | 2 +
src/include/utils/backend_status.h| 1 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-auth.c| 15 +-
src/interfaces/libpq/fe-connect.c | 17 +
src/interfaces/libpq/fe-secure-gssapi.c | 23 +-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 2 +
src/test/kerberos/Makefile| 3 +
src/test/kerberos/t/001_auth.pl | 335 --
src/test/perl/PostgreSQL/Test/Utils.pm| 27 ++
src/test/regress/expected/rules.out | 11 +-
36 files changed, 743 insertions(+), 135 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e..292377566b 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -48,6 +48,7 @@
#include "funcapi.h"
#include "lib/stringinfo.h"
#include "libpq-fe.h"
+#include "libpq/libpq-be.h"
#include "libpq/libpq-be-fe-helpers.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -111,7 +112,8 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
static char *generate_relation_name(Relation rel);
static void dblink_connstr_check(const char *connstr);
-static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+static bool dblink_connstr_has_pw(const char *connstr);
+static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
static char *get_connect_string(const char *servername);
@@ -213,7 +215,7 @@ dblink_get_conn(char *conname_or_str,
errmsg("could not establish connection"),
errdetail_internal("%s", msg)));
}
- dblink_security_check(conn, rconn);
+ dblink_security_check(conn, rconn, connstr);
if (PQclientEncoding(conn) != GetDatabaseEncoding())
PQsetClientEncoding(conn, GetDatabaseEncodingName());
freeconn = true;
@@ -307,7 +309,7 @@ dblink_connect(PG_FUNCTION_ARGS)
}
/* check password
Re: Kerberos delegation support in libpq and postgres_fdw
The CFBot says there's a function be_gssapi_get_proxy() which is undefined. Presumably this is a missing #ifdef or a definition that should be outside an #ifdef. [14:05:21.532] dblink.c: In function ‘dblink_security_check’: [14:05:21.532] dblink.c:2606:38: error: implicit declaration of function ‘be_gssapi_get_proxy’ [-Werror=implicit-function-declaration] [14:05:21.532] 2606 | if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_proxy(MyProcPort)) [14:05:21.532] | ^~~ [14:05:21.532] cc1: all warnings being treated as errors [13:56:28.789] dblink.c.obj : error LNK2019: unresolved external symbol be_gssapi_get_proxy referenced in function dblink_connstr_check [13:56:29.040] contrib\dblink\dblink.dll : fatal error LNK1120: 1 unresolved externals On Mon, 20 Mar 2023 at 09:30, Stephen Frost wrote: > > Greetings, > > * Stephen Frost (sfr...@snowman.net) wrote: > > * Michael Paquier (mich...@paquier.xyz) wrote: > > > On Mon, Sep 19, 2022 at 02:05:39PM -0700, Jacob Champion wrote: > > > > It's not prevented, because a password is being used. In my tests I'm > > > > connecting as an unprivileged user. > > > > > > > > You're claiming that the middlebox shouldn't be doing this. If this new > > > > default behavior were the historical behavior, then I would have agreed. > > > > But the cat's already out of the bag on that, right? It's safe today. > > > > And if it's not safe today for some other reason, please share why, and > > > > maybe I can work on a patch to try to prevent people from doing it. > > > > > > Please note that this has been marked as returned with feedback in the > > > current CF, as this has remained unanswered for a bit more than three > > > weeks. > > > > There's some ongoing discussion about how to handle outbound connections > > from the server ending up picking up credentials from the server's > > environment (that really shouldn't be allowed unless specifically asked > > for..), that's ultimately an independent change from what this patch is > > doing. > > That got committed, which is great, though it didn't go quite as far as > I had been hoping regarding dealing with outbound connections from the > server- perhaps we should make it clear at least for postgres_fdw that > it might be good for administrators to explicitly say which options are > allowed for a given user-map when it comes to how authentication is > done to the remote server? Seems like mostly a documentation > improvement, I think? Or should we have some special handling around > that option for postgres_fdw/dblink? > > > Here's an updated version which does address Robert's concerns around > > having this disabled by default and having options on both the server > > and client side saying if it is to be enabled or not. Also added to > > pg_stat_gssapi a field that indicates if credentials were proxied or not > > and made some other improvements and added additional regression tests > > to test out various combinations. > > I've done some self-review and also reworked how the security checks are > done to be sure that we're not ending up pulling credentials from the > environment (with added regression tests to check for it too). If > there's remaining concerns around that, please let me know. Of course, > other review would be great also. Presently though: > > - Rebased up to today > - Requires explicitly being enabled on client and server > - Authentication to a remote server via dblink or postgres_fdw with > GSSAPI requires that credentials were proxied by the client to the > server, except if the superuser set 'password_required' to false on > the postgres_fdw (which has lots of caveats around it in the > documentation because it's inherently un-safe to do). > - Includes updated documentation > - Quite a few additional regression tests to check for unrelated > credentials coming from the environment in either cases where > credentials have been proxied and in cases where they haven't. > - Only changes to existing regression tests for dblink/postgres_fdw are > in the error message wording updates. > > Thanks! > > Stephen -- greg
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * Stephen Frost (sfr...@snowman.net) wrote: > * Michael Paquier (mich...@paquier.xyz) wrote: > > On Mon, Sep 19, 2022 at 02:05:39PM -0700, Jacob Champion wrote: > > > It's not prevented, because a password is being used. In my tests I'm > > > connecting as an unprivileged user. > > > > > > You're claiming that the middlebox shouldn't be doing this. If this new > > > default behavior were the historical behavior, then I would have agreed. > > > But the cat's already out of the bag on that, right? It's safe today. > > > And if it's not safe today for some other reason, please share why, and > > > maybe I can work on a patch to try to prevent people from doing it. > > > > Please note that this has been marked as returned with feedback in the > > current CF, as this has remained unanswered for a bit more than three > > weeks. > > There's some ongoing discussion about how to handle outbound connections > from the server ending up picking up credentials from the server's > environment (that really shouldn't be allowed unless specifically asked > for..), that's ultimately an independent change from what this patch is > doing. That got committed, which is great, though it didn't go quite as far as I had been hoping regarding dealing with outbound connections from the server- perhaps we should make it clear at least for postgres_fdw that it might be good for administrators to explicitly say which options are allowed for a given user-map when it comes to how authentication is done to the remote server? Seems like mostly a documentation improvement, I think? Or should we have some special handling around that option for postgres_fdw/dblink? > Here's an updated version which does address Robert's concerns around > having this disabled by default and having options on both the server > and client side saying if it is to be enabled or not. Also added to > pg_stat_gssapi a field that indicates if credentials were proxied or not > and made some other improvements and added additional regression tests > to test out various combinations. I've done some self-review and also reworked how the security checks are done to be sure that we're not ending up pulling credentials from the environment (with added regression tests to check for it too). If there's remaining concerns around that, please let me know. Of course, other review would be great also. Presently though: - Rebased up to today - Requires explicitly being enabled on client and server - Authentication to a remote server via dblink or postgres_fdw with GSSAPI requires that credentials were proxied by the client to the server, except if the superuser set 'password_required' to false on the postgres_fdw (which has lots of caveats around it in the documentation because it's inherently un-safe to do). - Includes updated documentation - Quite a few additional regression tests to check for unrelated credentials coming from the environment in either cases where credentials have been proxied and in cases where they haven't. - Only changes to existing regression tests for dblink/postgres_fdw are in the error message wording updates. Thanks! Stephen From 03a799699b42d3f9d9ed017bd448f606b7a2761d Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Mon, 28 Feb 2022 20:17:55 -0500 Subject: [PATCH] Add support for Kerberos credential delegation Support GSSAPI/Kerberos credentials being delegated to the server by a client. With this, a user authenticating to PostgreSQL using Kerberos (GSSAPI) credentials can choose to delegate their credentials to the PostgreSQL server (which can choose to accept them, or not), allowing the server to then use those delegated credentials to connect to another service, such as with postgres_fdw or dblink or theoretically any other service which is able to be authenticated using Kerberos. Both postgres_fdw and dblink are changed to allow non-superuser password-less connections but only when GSSAPI credentials have been proxied to the server by the client and GSSAPI is used to authenticate to the remote system. Authors: Stephen Frost, Peifeng Qiu Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com --- contrib/dblink/dblink.c | 118 +++--- contrib/dblink/expected/dblink.out| 4 +- contrib/postgres_fdw/connection.c | 68 +++- .../postgres_fdw/expected/postgres_fdw.out| 19 +- contrib/postgres_fdw/option.c | 3 + contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +- doc/src/sgml/config.sgml | 17 + doc/src/sgml/dblink.sgml | 5 +- doc/src/sgml/libpq.sgml | 41 +++ doc/src/sgml/monitoring.sgml | 9 + doc/src/sgml/postgres-fdw.sgml| 5 +- src/backend/catalog/system_views.sql | 3 +- src/backend/foreign/foreign.c | 1 + src/backend/libpq/auth.c
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Michael Paquier (mich...@paquier.xyz) wrote:
> On Mon, Sep 19, 2022 at 02:05:39PM -0700, Jacob Champion wrote:
> > It's not prevented, because a password is being used. In my tests I'm
> > connecting as an unprivileged user.
> >
> > You're claiming that the middlebox shouldn't be doing this. If this new
> > default behavior were the historical behavior, then I would have agreed.
> > But the cat's already out of the bag on that, right? It's safe today.
> > And if it's not safe today for some other reason, please share why, and
> > maybe I can work on a patch to try to prevent people from doing it.
>
> Please note that this has been marked as returned with feedback in the
> current CF, as this has remained unanswered for a bit more than three
> weeks.
There's some ongoing discussion about how to handle outbound connections
from the server ending up picking up credentials from the server's
environment (that really shouldn't be allowed unless specifically asked
for..), that's ultimately an independent change from what this patch is
doing.
Here's an updated version which does address Robert's concerns around
having this disabled by default and having options on both the server
and client side saying if it is to be enabled or not. Also added to
pg_stat_gssapi a field that indicates if credentials were proxied or not
and made some other improvements and added additional regression tests
to test out various combinations.
Thanks,
Stephen
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 78a8bcee6e..713ef7c248 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2589,7 +2589,7 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
{
if (!superuser())
{
- if (!PQconnectionUsedPassword(conn))
+ if (!(PQconnectionUsedPassword(conn) || PQconnectionUsedGSSAPI(conn)))
{
libpqsrv_disconnect(conn);
if (rconn)
@@ -2597,8 +2597,8 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
- errmsg("password is required"),
- errdetail("Non-superuser cannot connect if the server does not request a password."),
+ errmsg("password or GSSAPI is required"),
+ errdetail("Non-superuser cannot connect if the server does not request a password or use GSSAPI."),
errhint("Target server's authentication method must be changed.")));
}
}
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d5fc61446a..77656ccf80 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -171,7 +171,8 @@ ALTER SERVER testserver1 OPTIONS (
sslcrl 'value',
--requirepeer 'value',
krbsrvname 'value',
- gsslib 'value'
+ gsslib 'value',
+ gssdeleg 'value'
--replication 'value'
);
-- Error, invalid list syntax
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 984e4d168a..bb6f309907 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -286,6 +286,9 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
+
{NULL, InvalidOid, false}
};
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 1e50be137b..1a43bdf55b 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -185,7 +185,8 @@ ALTER SERVER testserver1 OPTIONS (
sslcrl 'value',
--requirepeer 'value',
krbsrvname 'value',
- gsslib 'value'
+ gsslib 'value',
+ gssdeleg 'value'
--replication 'value'
);
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ecd9aa73ef..a931996968 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1170,6 +1170,23 @@ include_dir 'conf.d'
+
+ gss_accept_deleg (boolean)
+
+ gss_accept_deleg configuration parameter
+
+
+
+
+Sets whether GSSAPI delegation should be accepted from the client.
+The default is off meaning credentials from the client will
+NOT be accepted. Changing this to on will make the server
+accept credentials delegated to it from the client. This parameter can only be
+set in the postgresql.conf file or on the server command line.
+
+
+
+
db_user_namespace (boolean)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0e7ae70c70..9f3e12b2a7 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1879,6 +1879,18 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
+
+ gssdeleg
+
+
+Forward (delegate) GSS credentials to the server. The default is
+disable
Re: Kerberos delegation support in libpq and postgres_fdw
On Mon, Sep 19, 2022 at 02:05:39PM -0700, Jacob Champion wrote: > It's not prevented, because a password is being used. In my tests I'm > connecting as an unprivileged user. > > You're claiming that the middlebox shouldn't be doing this. If this new > default behavior were the historical behavior, then I would have agreed. > But the cat's already out of the bag on that, right? It's safe today. > And if it's not safe today for some other reason, please share why, and > maybe I can work on a patch to try to prevent people from doing it. Please note that this has been marked as returned with feedback in the current CF, as this has remained unanswered for a bit more than three weeks. -- Michael signature.asc Description: PGP signature
Re: Kerberos delegation support in libpq and postgres_fdw
On 9/19/22 10:05, Stephen Frost wrote: > This is coming across as if it's a surprise of some kind when it > certainly isn't.. If the delegated credentials are being used to > authenticate and establish the connection from that backend to another > system then, yes, naturally that means that the keys provided are coming > from the client and the client knows them. I think it may be surprising to end users that credential delegation lets them trivially break transport encryption. Like I said before, it was a surprise to me, because the cryptosystems I'm familiar with prevent that. If it wasn't surprising to you, I could really have used a heads up back when I asked you about it. > The idea of arranging to > have an admin's credentials used to authenticate to another system where > the backend is actually controlled by a non-admin user is, in fact, the > issue in what is being outlined above as that's clearly a situation > where the user's connection is being elevated to an admin level. Yes, controlled elevation is the goal in the scenario I'm describing. > That's > also something that we try to avoid having happen because it's not > really a good idea, which is why we require a password today for the > connection to be established (postgres_fdw/connection.c: > > Non-superuser cannot connect if the server does not request a password. A password is being used in this scenario. The password is the secret being stolen. The rest of your email describes a scenario different from what I'm attacking here. Here's my sample HBA line for the backend again: hostgssenc all all ... password I'm using password authentication with a Kerberos-encrypted channel. It's similar to protecting password authentication with TLS and a client cert: hostssl all all ... password clientcert=verify-* > Consider that, in general, the user could also simply directly connect > to the other system themselves No, because they don't have the password. They don't have USAGE on the foreign table, so they can't see the password in the USER MAPPING. With the new default introduced in this patch, they can now steal the password by delegating their credentials and cracking the transport encryption. This bypasses the protections that are documented for the pg_user_mappings view. > Consider SSH instead of PG. What you're pointing out, accurately, is > that if an admin were to install their keys into a user's .ssh directory > unencrypted and then the user logged into the system, they'd then be > able to SSH to another system with the admin's credentials and then > they'd need the admin's credentials to decrypt the traffic, but that if, > instead, the user brings their own credentials then they could > potentially decrypt the connection between the systems. Is that really > the issue here? No, it's not the issue here. This is more like setting up a restricted shell that provides limited access to a resource on another machine (analogous to the foreign table). The user SSHs into this restricted shell, and then invokes an admin-blessed command whose implementation uses some credentials (which they cannot read, analogous to the USER MAPPING) over an encrypted channel to access the backend resource. In this situation an admin would want to ensure that the encrypted tunnel couldn't be weakened by the client, so that they can't learn how to bypass the blessed command and connect to the backend directly. Unlike SSH, we've never supported credential delegation, and now we're introducing it. So my claim is, it's possible for someone who was previously in a secure situation to be broken by the new default. >> So the client can decrypt backend communications that make use of its >> delegated key material. (This also means that gssencmode is a lot >> weaker than I expected.) > > The backend wouldn't be able to establish the connection in the first > place without those delegated credentials. That's not true in the situation I'm describing; hopefully my comments above help clarify. > The server in the middle should *not* be using its own Kerberos > credentials to establish the connection to the other system- that's > elevating the credentials used for that connection and is something that > should be prevented for non-superusers already (see above). It's not prevented, because a password is being used. In my tests I'm connecting as an unprivileged user. You're claiming that the middlebox shouldn't be doing this. If this new default behavior were the historical behavior, then I would have agreed. But the cat's already out of the bag on that, right? It's safe today. And if it's not safe today for some other reason, please share why, and maybe I can work on a patch to try to prevent people from doing it. Thanks, --Jacob
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings, * Jacob Champion (jchamp...@timescale.com) wrote: > On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion wrote: > > So my question is this: does substituting my credentials for the admin's > > credentials let me weaken or break the transport encryption on the > > backend connection, and grab the password that I'm not supposed to have > > access to as a front-end client? > > With some further research: yes, it does. > > If a DBA is using a GSS encrypted tunnel to communicate to a foreign > server, accepting delegation by default means that clients will be > able to break that backend encryption at will, because the keys in use > will be under their control. This is coming across as if it's a surprise of some kind when it certainly isn't.. If the delegated credentials are being used to authenticate and establish the connection from that backend to another system then, yes, naturally that means that the keys provided are coming from the client and the client knows them. The idea of arranging to have an admin's credentials used to authenticate to another system where the backend is actually controlled by a non-admin user is, in fact, the issue in what is being outlined above as that's clearly a situation where the user's connection is being elevated to an admin level. That's also something that we try to avoid having happen because it's not really a good idea, which is why we require a password today for the connection to be established (postgres_fdw/connection.c: Non-superuser cannot connect if the server does not request a password. ). Consider that, in general, the user could also simply directly connect to the other system themselves instead of having a PG backend make that connection for them- the point in doing it from PG would be to avoid having to pass all the data back through the client's system. Consider SSH instead of PG. What you're pointing out, accurately, is that if an admin were to install their keys into a user's .ssh directory unencrypted and then the user logged into the system, they'd then be able to SSH to another system with the admin's credentials and then they'd need the admin's credentials to decrypt the traffic, but that if, instead, the user brings their own credentials then they could potentially decrypt the connection between the systems. Is that really the issue here? Doesn't seem like that's where the concern should be in this scenario. > > Maybe there's some ephemeral exchange going on that makes it too hard to > > attack in practice, or some other mitigations. > > There is no forward secrecy, ephemeral exchange, etc. to mitigate this [1]: > >The Kerberos protocol in its basic form does not provide perfect >forward secrecy for communications. If traffic has been recorded by >an eavesdropper, then messages encrypted using the KRB_PRIV message, >or messages encrypted using application-specific encryption under >keys exchanged using Kerberos can be decrypted if the user's, >application server's, or KDC's key is subsequently discovered. > > So the client can decrypt backend communications that make use of its > delegated key material. (This also means that gssencmode is a lot > weaker than I expected.) The backend wouldn't be able to establish the connection in the first place without those delegated credentials. > > I'm trying to avoid writing Wireshark dissector > > code, but maybe that'd be useful either way... > > I did end up filling out the existing PGSQL dissector so that it could > decrypt GSSAPI exchanges (with the use of a keytab, that is). If you'd > like to give it a try, the patch, based on Wireshark 3.7.1, is > attached. Note the GPLv2 license. It isn't correct code yet, because I > didn't understand how packet reassembly worked in Wireshark when I > started writing the code, so really large GSSAPI messages that are > split across multiple TCP packets will confuse the dissector. But it's > enough to prove the concept. > > To see this in action, set up an FDW connection that uses gssencmode > (so the server in the middle will need its own Kerberos credentials). The server in the middle should *not* be using its own Kerberos credentials to establish the connection to the other system- that's elevating the credentials used for that connection and is something that should be prevented for non-superusers already (see above). We do allow that when a superuser is involved because they are considered to essentially have OS-level privileges and therefore could see those credentials anyway, but that's not the case for non-superusers. Thanks, Stephen signature.asc Description: PGP signature
Re: Kerberos delegation support in libpq and postgres_fdw
On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion wrote:
> So my question is this: does substituting my credentials for the admin's
> credentials let me weaken or break the transport encryption on the
> backend connection, and grab the password that I'm not supposed to have
> access to as a front-end client?
With some further research: yes, it does.
If a DBA is using a GSS encrypted tunnel to communicate to a foreign
server, accepting delegation by default means that clients will be
able to break that backend encryption at will, because the keys in use
will be under their control.
> Maybe there's some ephemeral exchange going on that makes it too hard to
> attack in practice, or some other mitigations.
There is no forward secrecy, ephemeral exchange, etc. to mitigate this [1]:
The Kerberos protocol in its basic form does not provide perfect
forward secrecy for communications. If traffic has been recorded by
an eavesdropper, then messages encrypted using the KRB_PRIV message,
or messages encrypted using application-specific encryption under
keys exchanged using Kerberos can be decrypted if the user's,
application server's, or KDC's key is subsequently discovered.
So the client can decrypt backend communications that make use of its
delegated key material. (This also means that gssencmode is a lot
weaker than I expected.)
> I'm trying to avoid writing Wireshark dissector
> code, but maybe that'd be useful either way...
I did end up filling out the existing PGSQL dissector so that it could
decrypt GSSAPI exchanges (with the use of a keytab, that is). If you'd
like to give it a try, the patch, based on Wireshark 3.7.1, is
attached. Note the GPLv2 license. It isn't correct code yet, because I
didn't understand how packet reassembly worked in Wireshark when I
started writing the code, so really large GSSAPI messages that are
split across multiple TCP packets will confuse the dissector. But it's
enough to prove the concept.
To see this in action, set up an FDW connection that uses gssencmode
(so the server in the middle will need its own Kerberos credentials).
Capture traffic starting from the kinit through the query on the
foreign table. Export the client's key material into a keytab, and set
up Wireshark to use that keytab for decryption. When credential
forwarding is *not* in use, Wireshark will be able to decrypt the
initial client connection, but it won't be able to see anything inside
the foreign server connection. When credential forwarding is in use,
Wireshark will be able to decrypt both connections.
Thanks,
--Jacob
[1] https://www.rfc-editor.org/rfc/rfc4120
From 0cf3153a6044edae42037e45aee0fc88352f Mon Sep 17 00:00:00 2001
From: Jacob Champion
Date: Wed, 14 Sep 2022 13:08:22 -0700
Subject: [PATCH] pgsql: decrypt GSS-encrypted channels and tokens if possible
License: GPLv2
Add dissection for a conversation that starts with a GSSAPI encryption
request, and pass those packets along to the gssapi dissector. This
allows parts of the conversation to be decrypted if Wireshark has been
set up with a KRB5 keytab.
Additionally, break apart the PGSQL_AUTH_GSSAPI_SSPI_DATA case into
separate GSSAPI and SSPI cases, and when we're using GSSAPI, dissect the
authentication tokens that are passed between the client and server.
This incidentally fixes an off-by-four bug in the
PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE case; the offset was not being
updated correctly before.
The new code copies the dissection strategy of multipart, and adds a new
last_nongss_frame marker similar to the STARTTLS handling code's
last_nontls_frame.
---
epan/dissectors/packet-pgsql.c | 121 +++--
1 file changed, 116 insertions(+), 5 deletions(-)
diff --git a/epan/dissectors/packet-pgsql.c b/epan/dissectors/packet-pgsql.c
index c935eec1f8..7efe1c003f 100644
--- a/epan/dissectors/packet-pgsql.c
+++ b/epan/dissectors/packet-pgsql.c
@@ -14,6 +14,8 @@
#include
+#include "packet-dcerpc.h"
+#include "packet-gssapi.h"
#include "packet-tls-utils.h"
#include "packet-tcp.h"
@@ -22,6 +24,7 @@ void proto_reg_handoff_pgsql(void);
static dissector_handle_t pgsql_handle;
static dissector_handle_t tls_handle;
+static dissector_handle_t gssapi_handle;
static int proto_pgsql = -1;
static int hf_frontend = -1;
@@ -80,10 +83,13 @@ static int hf_constraint_name = -1;
static int hf_file = -1;
static int hf_line = -1;
static int hf_routine = -1;
+static int hf_gssapi_length = -1;
static gint ett_pgsql = -1;
static gint ett_values = -1;
+static expert_field ei_gssapi_decryption_not_possible = EI_INIT;
+
#define PGSQL_PORT 5432
static gboolean pgsql_desegment = TRUE;
static gboolean first_message = TRUE;
@@ -92,11 +98,14 @@ typedef enum {
PGSQL_AUTH_STATE_NONE, /* No authentication seen or used */
PGSQL_AUTH_SASL_REQUESTED, /* Server sends SASL auth request with
supported SASL mechanisms*/
PGSQL_AUTH_SASL_CONTINUE,/* Server and/or
Re: Kerberos delegation support in libpq and postgres_fdw
On 4/8/22 05:21, Stephen Frost wrote: > Added a few more tests and updated the documentation too. Sadly, seems > we've missed the deadline for v15 though for lack of feedback on these. > Would really like to get some other folks commenting as these are new > pg_hba and postgresql.conf options being added. Sorry for the incredibly long delay; I lost track of this thread during the email switch. I'm testing the patch with various corner cases to try to figure out how it behaves, so this isn't a full review, but I wanted to jump through some of the emails I missed and at least give you some responses. As an overall note, I think the patch progression, and adding more explicit control over when credentials may be delegated, is very positive, and +1 for the proposed libpq connection option elsewhere in the thread. On 4/7/22 21:21, Stephen Frost wrote: >> That an admin might have a credential cache that's picked up and used >> for connections from a regular user backend to another system strikes me >> as an altogether concerning idea. Even so, in such a case, the admin >> would have had to set up the user mapping with 'password required = >> false' or it wouldn't have worked for a non-superuser anyway, so I'm not >> sure that I'm too worried about this case. > > To address this, I also added a new GUC which allows an administrator to > control what the credential cache is set to for user-authenticated > backends, with a default of MEMORY:, which should generally be safe and > won't cause a user backend to pick up on a file-based credential cache > which might exist on the server somewhere. This gives the administrator > the option to set it to more-or-less whatever they'd like though, so if > they want to set it to a file-based credential cache, then they can do > so (I did put some caveats about doing that into the documentation as I > don't think it's generally a good idea to do...). I'm not clear on how this handles the collision case. My concern was with a case where you have more than one foreign table/server, and they need to use separate credentials. It's not obvious to me how changing the location of a (single, backend-global) cache mitigates that problem. I'm also missing something about why password_required=false is necessary (as opposed to simply setting a password in the USER MAPPING). My current test case doesn't make use of password_required=false and it appears to work just fine. On 4/6/22 12:27, Stephen Frost wrote: >> Another danger might be disclosure/compromise of middlebox secrets? Is >> it possible for someone who has one half of the credentials to snoop on >> a gssenc connection between the proxy Postgres and the backend >> Postgres? > > A compromised middlebox would, of course, be an issue- for any kind of > delegated credentials (which certainly goes for cleartext passwords > being passed along, and that's currently the only thing we support..). > One nice thing about GSSAPI is that the client and the server validate > each other, so it wouldn't just be 'any' middle-box but would have to be > one that was actually a trusted system in the infrastructure which has > somehow been compromised and was still trusted. I wasn't clear enough, sorry -- I mean that we have to prove that defaulting allow_cred_delegation to true doesn't cause the compromise of existing deployments. As an example, right now I'm trying to characterize behavior with the following pg_hba setup on the foreign server: hostgssenc all all ... password So in other words we're using GSS as transport encryption only, not as an authentication provider. On the middlebox, we create a FOREIGN SERVER/TABLE that points to this, and set up a USER MAPPING (with no USAGE rights) that contains the necessary password. (I'm using a plaintext password to make it more obvious what the danger is, not suggesting that this would be good practice.) As far as I can tell, to make this work today, a server admin has to set up a local credential cache with the keys for some one-off principal. It doesn't have to be an admin principal, because the point is just to provide transport protection for the password, so it's not really particularly scary to make it available to user backends. But this new proposed feature lets the client override that credential cache, substituting their own credentials, for which they have all the Kerberos symmetric key material. So my question is this: does substituting my credentials for the admin's credentials let me weaken or break the transport encryption on the backend connection, and grab the password that I'm not supposed to have access to as a front-end client? I honestly don't know the answer; GSSAPI is a black box that defers to Kerberos and there's a huge number of specs that I've been slowly making my way through. But in my tests, if I turn on credential forwarding, Wireshark is suddenly able to use the *client's* keys to decrypt pieces of the TGS's conversations with the *middlebox*,
Re: Kerberos delegation support in libpq and postgres_fdw
On Fri, Apr 8, 2022 at 11:29 AM Stephen Frost wrote:
> > + allow_cred_delegation
> >
> > First, I again recommend not choosing words at random to abbreviate.
> > "delegate_credentials" would be shorter and clearer. Second, I think
> > we need to decide whether we envision just having one parameter here
> > for every kind of credential delegation that libpq might ever support,
> > or whether this is really something specific to GSS. If the latter,
> > the name should mention GSS.
>
> delegate_credentials seems to imply that the server has some kind of
> control over the act of delegating credentials, which isn't really the
> case. The client has to decide to delegate credentials and it does that
> independent of the server- the server side just gets to either accept
> those delegated credentials, or ignore them.
Oh ... I thought this was a libpq parameter to control the client
behavior. I guess I didn't read it carefully enough.
> Regarding the client side, it is the case that GSSAPIDelegateCredentials
> in ssh defaults to no, so it seems like the next iteration of the patch
> should probably include a libpq option similar to that ssh_config
> option. As I mentioned before, users already can decide if they'd like
> proxyable credentials or not when they kinit, though more generally this
> is set as a environment-wide policy, but we can add an option and
> disable it by default.
+1.
> This isn't actually something we have a choice in, really, it's from the
> Kerberos library. MEMORY is the library's in-memory credential cache.
> Other possible values are FILE:/some/file, DIR:/some/dir, API:, and
> others. Documentaton is available here:
> https://web.mit.edu/kerberos/krb5-1.12/doc/basic/ccache_def.html
Well, I was just going by the fact that this string ("MEMORY:") seems
to be being interpreted in our code, not the library.
> > I wonder whether we really quite this many cases. But if we do they
> > probably need better and more consistent naming.
>
> I wouldn't want to end up with values that could end up conflicting with
> real values that a user might want to specify, so the choice of
> 'environment' and empty-value were specifically chosen to avoid that
> risk. If we're worried that doing so isn't sufficient or is too
> confusing, the better option would likely be to have another GUC that
> controls if we unset, ignore, or set the value to what the other GUC
> says to set it to. I'm fine with that if you agree.
Yeah, I thought of that, and it might be the way to go. I wasn't too
sure we needed the explicit-unset behavior as an option, but I defer
to you on that.
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Robert Haas (robertmh...@gmail.com) wrote:
> On Fri, Apr 8, 2022 at 8:21 AM Stephen Frost wrote:
> > Added an explicit 'environment' option to allow for, basically, existing
> > behavior, where we don't mess with the environment variable at all,
> > though I kept the default as MEMORY since I don't think it's really
> > typical that folks actually want regular user backends to inherit the
> > credential cache of the server.
> >
> > Added a few more tests and updated the documentation too. Sadly, seems
> > we've missed the deadline for v15 though for lack of feedback on these.
> > Would really like to get some other folks commenting as these are new
> > pg_hba and postgresql.conf options being added.
>
> I don't think this patch is quite baked enough to go in even if the
> deadline hadn't formally passed, but I'm happy to offer a few opinions
> ... especially if we can also try to sort out a plan for getting that
> wider-checksums thing you mentioned done for v16.
Sure.
> + /* gssencmode is also libpq option, same to above. */
> + {"gssencmode", UserMappingRelationId, true},
>
> I really hate names like this that are just a bunch of stuff strung
> together with no punctuation and some arbitrary abbreviations thrown
> in for good measure. But since the libpq parameter already exists it's
> hard to argue we should do anything else here.
Well, yeah.
> + allow_cred_delegation
>
> First, I again recommend not choosing words at random to abbreviate.
> "delegate_credentials" would be shorter and clearer. Second, I think
> we need to decide whether we envision just having one parameter here
> for every kind of credential delegation that libpq might ever support,
> or whether this is really something specific to GSS. If the latter,
> the name should mention GSS.
delegate_credentials seems to imply that the server has some kind of
control over the act of delegating credentials, which isn't really the
case. The client has to decide to delegate credentials and it does that
independent of the server- the server side just gets to either accept
those delegated credentials, or ignore them.
In terms of having a prefix, this is certainly something that I'd like
to see SSPI support added for as well (perhaps that can be in v16 too)
and so it's definitely not GSS-exclusive among the authentication
methods that we have today. In that sense, this option falls into the
same category as 'include_realm' and 'krb_realm' in that it applies to
more than one, but not all, of our authentication methods.
> I also suggest that the default value of this option should be false,
> rather than true. I would be unhappy if ssh started defaulting to
> ForwardAgent=yes, because that's less secure and I don't want my
> credentials shared with random servers without me making a choice to
> do that. Similarly here I think we should default to the more secure
> option.
This is a bit backwards from how it works though- this option is about
if the server will accept delegated credentials, not if the client sends
them. If your client was set to ForwardAgent=yes, would you be happy if
the server's default was AllowAgentForwarding=no? (At least on the
system I'm looking at, the current default is AllowAgentForwarding=yes
in sshd_config).
Regarding the client side, it is the case that GSSAPIDelegateCredentials
in ssh defaults to no, so it seems like the next iteration of the patch
should probably include a libpq option similar to that ssh_config
option. As I mentioned before, users already can decide if they'd like
proxyable credentials or not when they kinit, though more generally this
is set as a environment-wide policy, but we can add an option and
disable it by default.
> +
> +
> +Sets the location of the Kerberos credential cache to be used for
> +regular user backends which go through authentication. The default
> is
> +MEMORY:, which is where delegated credentials
> +are stored (and is otherwise empty). Care should be used when
> changing
> +this value- setting it to a file-based credential cache will mean
> that
> +user backends could potentially use any credentials stored to access
> +other systems.
> +If this parameter is set to an empty string, then the variable will
> be
> + explicit un-set and the system-dependent default is used, which may be a
> + file-based credential cache with the same caveats as previously
> + mentioned. If the special value 'environment' is used, then the variable
> + is left untouched and will be whatever was set in the environment at
> + startup time.
>
> "MEMORY:" seems like a pretty weird choice of arbitrary string. Is it
> supposed to look like a Windows drive letter or pseudo-device, or
> what? I'm not sure exactly what's better here, but I just think this
> doesn't look like anything else we've got today. And then we've got a
> second special environment, "environment", which looks
Re: Kerberos delegation support in libpq and postgres_fdw
On Fri, Apr 8, 2022 at 8:21 AM Stephen Frost wrote:
> Added an explicit 'environment' option to allow for, basically, existing
> behavior, where we don't mess with the environment variable at all,
> though I kept the default as MEMORY since I don't think it's really
> typical that folks actually want regular user backends to inherit the
> credential cache of the server.
>
> Added a few more tests and updated the documentation too. Sadly, seems
> we've missed the deadline for v15 though for lack of feedback on these.
> Would really like to get some other folks commenting as these are new
> pg_hba and postgresql.conf options being added.
Hi,
I don't think this patch is quite baked enough to go in even if the
deadline hadn't formally passed, but I'm happy to offer a few opinions
... especially if we can also try to sort out a plan for getting that
wider-checksums thing you mentioned done for v16.
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
I really hate names like this that are just a bunch of stuff strung
together with no punctuation and some arbitrary abbreviations thrown
in for good measure. But since the libpq parameter already exists it's
hard to argue we should do anything else here.
+ allow_cred_delegation
First, I again recommend not choosing words at random to abbreviate.
"delegate_credentials" would be shorter and clearer. Second, I think
we need to decide whether we envision just having one parameter here
for every kind of credential delegation that libpq might ever support,
or whether this is really something specific to GSS. If the latter,
the name should mention GSS.
I also suggest that the default value of this option should be false,
rather than true. I would be unhappy if ssh started defaulting to
ForwardAgent=yes, because that's less secure and I don't want my
credentials shared with random servers without me making a choice to
do that. Similarly here I think we should default to the more secure
option.
+
+
+Sets the location of the Kerberos credential cache to be used for
+regular user backends which go through authentication. The default is
+MEMORY:, which is where delegated credentials
+are stored (and is otherwise empty). Care should be used when changing
+this value- setting it to a file-based credential cache will mean that
+user backends could potentially use any credentials stored to access
+other systems.
+If this parameter is set to an empty string, then the variable will be
+ explicit un-set and the system-dependent default is used, which may be a
+ file-based credential cache with the same caveats as previously
+ mentioned. If the special value 'environment' is used, then the variable
+ is left untouched and will be whatever was set in the environment at
+ startup time.
"MEMORY:" seems like a pretty weird choice of arbitrary string. Is it
supposed to look like a Windows drive letter or pseudo-device, or
what? I'm not sure exactly what's better here, but I just think this
doesn't look like anything else we've got today. And then we've got a
second special environment, "environment", which looks completely
different: now it's lower-case and without the colon. And then empty
string is special too.
I wonder whether we really quite this many cases. But if we do they
probably need better and more consistent naming.
The formatting here also looks weird.
+#ifndef PG_KRB_USER_CCACHE
+#define PG_KRB_USER_CCACHE "MEMORY:"
+#endif
At the risk of stating the obvious, the general idea of a #define is
that you define things in one place and then use the defined symbol
rather than the original value everywhere. This patch takes the
less-useful approach of defining two different symbols for the same
string in different files. This one has this #ifndef/#endif guard here
which I think it probably shouldn't, since the choice of string
probably shouldn't be compile-time configurable, but it also won't
work, because there's no similar guard in the other file.
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Stephen Frost (sfr...@snowman.net) wrote:
> The new krb_user_ccache is a lot closer to 'global', though it's
> specifically for user-authenticated backends (allowing the postmaster
> and other things like replication connections to use whatever the
> credential cache is set to by the administrator on startup), but that
> seems like it makes sense to me- generally you're not going to want
> regular user backends to be accessing the credential cache of the
> 'postgres' unix account on the server.
Added an explicit 'environment' option to allow for, basically, existing
behavior, where we don't mess with the environment variable at all,
though I kept the default as MEMORY since I don't think it's really
typical that folks actually want regular user backends to inherit the
credential cache of the server.
Added a few more tests and updated the documentation too. Sadly, seems
we've missed the deadline for v15 though for lack of feedback on these.
Would really like to get some other folks commenting as these are new
pg_hba and postgresql.conf options being added.
Thanks!
Stephen
From bd248c3fd82d04d3c12bf6c777f861134a45a101 Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Thu, 7 Apr 2022 15:34:39 -0400
Subject: [PATCH] Add support for Kerberos credential delegation
Accept GSSAPI/Kerberos delegated credentials. With this, a user could
authenticate to PostgreSQL using Kerberos credentials, delegate
credentials to the PostgreSQL server, and then the PostgreSQL server
could use those credentials to connect to another service, such as with
postgres_fdw or dblink or theoretically any other authenticated
connection which is able to use delegated credentials.
If an administrator prefers to not allow credentials to be delegated to
the server, they can be disallowed using a new pg_hba option for gss
called 'allow_cred_delegation'.
A new server GUC has also been introduced to allow an administrator to
control what the kerberos credential cache is configured to for user
authenticated backends, krb_user_ccache. This defaults to MEMORY:,
which is where delegated credentials are stored (and is otherwise empty,
avoiding the risk of an administrator's credentials on the server being
mistakenly picked up and used).
Original patch by: Peifeng Qiu, whacked around some by me.
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/co1pr05mb8023cc2cb575e0faad7df4f8a8...@co1pr05mb8023.namprd05.prod.outlook.com
---
contrib/dblink/dblink.c | 6 +-
.../postgres_fdw/expected/postgres_fdw.out| 2 +-
contrib/postgres_fdw/option.c | 3 +
doc/src/sgml/client-auth.sgml | 13 ++
doc/src/sgml/config.sgml | 28
doc/src/sgml/libpq.sgml | 19 +++
src/backend/libpq/auth.c | 27 +++-
src/backend/libpq/be-gssapi-common.c | 51 +++
src/backend/libpq/be-secure-gssapi.c | 19 ++-
src/backend/libpq/hba.c | 19 +++
src/backend/utils/adt/hbafuncs.c | 4 +
src/backend/utils/init/postinit.c | 8 +-
src/backend/utils/misc/guc.c | 15 ++
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/libpq/auth.h | 1 +
src/include/libpq/be-gssapi-common.h | 3 +
src/include/libpq/hba.h | 1 +
src/include/libpq/libpq-be.h | 3 +
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-auth.c| 12 +-
src/interfaces/libpq/fe-connect.c | 12 ++
src/interfaces/libpq/fe-secure-gssapi.c | 3 +-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
src/test/kerberos/Makefile| 3 +
src/test/kerberos/t/001_auth.pl | 128 --
src/test/perl/PostgreSQL/Test/Utils.pm| 27
27 files changed, 391 insertions(+), 20 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index a06d4bd12d..e5b70e084e 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2643,7 +2643,7 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
{
if (!superuser())
{
- if (!PQconnectionUsedPassword(conn))
+ if (!(PQconnectionUsedPassword(conn) || PQconnectionUsedGSSAPI(conn)))
{
PQfinish(conn);
ReleaseExternalFD();
@@ -2652,8 +2652,8 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
- errmsg("password is required"),
- errdetail("Non-superuser cannot connect if the server does not request a password."),
+ errmsg("password or GSSAPI is required"),
+ errdetail("Non-superuser cannot connect if the server does not request a password or use GSSAPI."),
errhint("Target server's authentication method must be
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
* Stephen Frost (sfr...@snowman.net) wrote:
> * Jacob Champion (pchamp...@vmware.com) wrote:
> > On Fri, 2022-03-11 at 19:39 -0500, Stephen Frost wrote:
> > > Even so, I’m not against adding an option… but exactly how would that
> > > option be configured? Server level? On the HBA line? role level..?
> >
> > In the OPTIONS for CREATE SERVER, maybe? At least for the FDW case.
>
> I'm a bit confused on this. The option to allow or not allow delegated
> credentials couldn't be something that's in the CREATE SERVER for FDWs
> as it applies to more than just FDWs but also dblink and anything else
> where we reach out from PG to contact some other system.
Thinking it through further, it seems like the right place to allow an
administrator to control if credentials are allowed to be delegated is
through a pg_hba option. Attached patch adds such an option.
> > > > Also, we're
> > > > globally ignoring whatever ccache was set by an administrator. Can't
> > > > two postgres_fdw connections from the same backend process require
> > > > different settings?
> > >
> > > Settings..? Perhaps, but delegated credentials aren’t really
> > > settings, so not really sure what you’re suggesting here.
> >
> > I mean that one backend server might require delegated credentials, and
> > another might require whatever the admin has already set up in the
> > ccache, and the user might want to use tables from both servers in the
> > same session.
>
> That an admin might have a credential cache that's picked up and used
> for connections from a regular user backend to another system strikes me
> as an altogether concerning idea. Even so, in such a case, the admin
> would have had to set up the user mapping with 'password required =
> false' or it wouldn't have worked for a non-superuser anyway, so I'm not
> sure that I'm too worried about this case.
To address this, I also added a new GUC which allows an administrator to
control what the credential cache is set to for user-authenticated
backends, with a default of MEMORY:, which should generally be safe and
won't cause a user backend to pick up on a file-based credential cache
which might exist on the server somewhere. This gives the administrator
the option to set it to more-or-less whatever they'd like though, so if
they want to set it to a file-based credential cache, then they can do
so (I did put some caveats about doing that into the documentation as I
don't think it's generally a good idea to do...).
> > > > I notice that gss_store_cred_into() has a companion,
> > > > gss_acquire_cred_from(). Is it possible to use that to pull out our
> > > > delegated credential explicitly by name, instead of stomping on the
> > > > global setup?
> > >
> > > Not really sure what is meant here by global setup..? Feeling like
> > > this is a follow on confusion from maybe mixing server vs client
> > > libpq?
> >
> > By my reading, the gss_store_cred_into() call followed by
> > the setenv("KRB5CCNAME", ...) is effectively performing global
> > configuration for the process. Any KRB5CCNAME already set up by the
> > server admin is going to be ignored from that point onward. Is that
> > accurate?
>
> The process, yes, but I guess I disagree on that being 'global'- it's
> just for that PG backend process.
The new krb_user_ccache is a lot closer to 'global', though it's
specifically for user-authenticated backends (allowing the postmaster
and other things like replication connections to use whatever the
credential cache is set to by the administrator on startup), but that
seems like it makes sense to me- generally you're not going to want
regular user backends to be accessing the credential cache of the
'postgres' unix account on the server.
> Attached is an updated patch which adds the gss_release_creds call, a
> function in libpq to allow checking if the libpq connection was made
> using GSS, changes to dblink to have it check for password-or-gss when
> connecting to a remote system, and tests for dblink and postgres_fdw to
> make sure that this all works correctly.
I've added a couple more tests to address the new options too, along
with documentation for them. This is starting to feel reasonably decent
to me, at least as a first pass at supporting kerberos credential
delegation, which is definitely a feature I've been hoping we would get
into PG for quite a while. Would certainly appreciate some feedback on
this (from anyone who'd like to comment), though I know we're getting
into the last few hours before feature freeze ends.
Updated patch attached.
Thanks!
Stephen
From 83b2979e252866ce86a9bfb3d0b631db2f5b8404 Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Thu, 7 Apr 2022 15:34:39 -0400
Subject: [PATCH] Add support for Kerberos credential delegation
Accept GSSAPI/Kerberos delegated credentials. With this, a user could
authenticate to PostgreSQL using Kerberos credentials, delegate
credentials to the PostgreSQL server, and then the PostgreSQL server
Re: Kerberos delegation support in libpq and postgres_fdw
Greetinsg, * Jacob Champion (pchamp...@vmware.com) wrote: > On Fri, 2022-03-11 at 19:39 -0500, Stephen Frost wrote: > > On Fri, Mar 11, 2022 at 18:55 Jacob Champion wrote: > > > [5] says we have to free the proxy credential with GSS_Release_cred(); > > > I don't see that happening anywhere, but I may have missed it. > > > > I’m not sure that it’s really necessary or worthwhile to do that at > > process end since … the process is about to end. I suppose we could > > provide a function that a user could call to ask for it to be > > released sooner if we really wanted..? > > Do we have to keep the credential handle around once we've stored it > into the MEMORY: cache, though? Just seems like a leak that someone > will have to plug eventually, even if it doesn't really impact things > now. We don't, so I've fixed that in the attached. Not sure it's that big a deal but I don't think it hurts anything either. > > > It seems like there should be significant security implications to > > > allowing delegation across the board. Especially since one postgres_fdw > > > might delegate to another server, and then another... Should this be > > > opt-in, maybe via a connection parameter? > > > > This is already opt-in- at kinit time a user can decide if they’d > > like a proxy-able ticket or not. I don’t know that we really need to > > have our own option for it … tho I’m not really against adding such > > an option either. > > I don't really have experience with the use case. Is it normal for > kinit users to have to decide once, globally, whether they want > everything they interact with to be able to proxy their credentials? It > just seems like you'd want more fine-grained control over who gets to > masquerade as you. Yes, that's pretty typical for kinit users- they usually go with whatever the org policy is. Now, you're not wrong about wanting more fine-grained control, which is what's known as 'constrained delegation'. That's something that Kerberos in general supports these days though it's more complicated and requires additional code to do. That's something that I think we could certainly add later on. > > > Similarly, it feels a little strange that the server would allow the > > > client to unilaterally force the use of a delegated credential. I think > > > that should be opt-in on the server side too, unless there's some > > > context I'm missing around why that's safe. > > > > Perhaps you could explain what isn’t safe about accepting a delegated > > credential from a client..? I am not away of a risk to accepting > > such a delegated credential. > > My initial impression is that this is effectively modifying the USER > MAPPING that the admin has set up. I'd be worried about an open > credential proxy being used to bypass firewall or HBA restrictions, for > instance -- you might not be able to connect as an admin from your > machine, but you might be able to connect by bouncing through a proxy. > (What damage you can do is going to be limited by what the server > extensions can do, of course.) I'm not sure that I really see the concern here. Also, in order for this to work, the user mapping would have to be created with "password required = false". Maybe that's something we revisit later, but it seems like a good way to allow an admin to have control over this. > Another danger might be disclosure/compromise of middlebox secrets? Is > it possible for someone who has one half of the credentials to snoop on > a gssenc connection between the proxy Postgres and the backend > Postgres? A compromised middlebox would, of course, be an issue- for any kind of delegated credentials (which certainly goes for cleartext passwords being passed along, and that's currently the only thing we support..). One nice thing about GSSAPI is that the client and the server validate each other, so it wouldn't just be 'any' middle-box but would have to be one that was actually a trusted system in the infrastructure which has somehow been compromised and was still trusted. > > Even so, I’m not against adding an option… but exactly how would that > > option be configured? Server level? On the HBA line? role level..? > > In the OPTIONS for CREATE SERVER, maybe? At least for the FDW case. I'm a bit confused on this. The option to allow or not allow delegated credentials couldn't be something that's in the CREATE SERVER for FDWs as it applies to more than just FDWs but also dblink and anything else where we reach out from PG to contact some other system. > > > If I'm reading it right, we're resetting the default credential in the > > > MEMORY cache, so if you're a libpq client doing your own GSSAPI work, > > > I'm guessing you might not be happy with this behavior. > > > > This is just done on the server side and not the client side..? > > Yeah, I misread the patch, sorry. No worries. > > > Also, we're > > > globally ignoring whatever ccache was set by an administrator. Can't > > > two postgres_fdw
Re: Kerberos delegation support in libpq and postgres_fdw
On Fri, 2022-03-11 at 19:39 -0500, Stephen Frost wrote:
>
> On Fri, Mar 11, 2022 at 18:55 Jacob Champion wrote:
> >
> > [5] says we have to free the proxy credential with GSS_Release_cred();
> > I don't see that happening anywhere, but I may have missed it.
>
> I’m not sure that it’s really necessary or worthwhile to do that at
> process end since … the process is about to end. I suppose we could
> provide a function that a user could call to ask for it to be
> released sooner if we really wanted..?
Do we have to keep the credential handle around once we've stored it
into the MEMORY: cache, though? Just seems like a leak that someone
will have to plug eventually, even if it doesn't really impact things
now.
> > It seems like there should be significant security implications to
> > allowing delegation across the board. Especially since one postgres_fdw
> > might delegate to another server, and then another... Should this be
> > opt-in, maybe via a connection parameter?
>
> This is already opt-in- at kinit time a user can decide if they’d
> like a proxy-able ticket or not. I don’t know that we really need to
> have our own option for it … tho I’m not really against adding such
> an option either.
I don't really have experience with the use case. Is it normal for
kinit users to have to decide once, globally, whether they want
everything they interact with to be able to proxy their credentials? It
just seems like you'd want more fine-grained control over who gets to
masquerade as you.
> > Similarly, it feels a little strange that the server would allow the
> > client to unilaterally force the use of a delegated credential. I think
> > that should be opt-in on the server side too, unless there's some
> > context I'm missing around why that's safe.
>
> Perhaps you could explain what isn’t safe about accepting a delegated
> credential from a client..? I am not away of a risk to accepting
> such a delegated credential.
My initial impression is that this is effectively modifying the USER
MAPPING that the admin has set up. I'd be worried about an open
credential proxy being used to bypass firewall or HBA restrictions, for
instance -- you might not be able to connect as an admin from your
machine, but you might be able to connect by bouncing through a proxy.
(What damage you can do is going to be limited by what the server
extensions can do, of course.)
Another danger might be disclosure/compromise of middlebox secrets? Is
it possible for someone who has one half of the credentials to snoop on
a gssenc connection between the proxy Postgres and the backend
Postgres?
> Even so, I’m not against adding an option… but exactly how would that
> option be configured? Server level? On the HBA line? role level..?
In the OPTIONS for CREATE SERVER, maybe? At least for the FDW case.
> > If I'm reading it right, we're resetting the default credential in the
> > MEMORY cache, so if you're a libpq client doing your own GSSAPI work,
> > I'm guessing you might not be happy with this behavior.
>
> This is just done on the server side and not the client side..?
Yeah, I misread the patch, sorry.
> > Also, we're
> > globally ignoring whatever ccache was set by an administrator. Can't
> > two postgres_fdw connections from the same backend process require
> > different settings?
>
> Settings..? Perhaps, but delegated credentials aren’t really
> settings, so not really sure what you’re suggesting here.
I mean that one backend server might require delegated credentials, and
another might require whatever the admin has already set up in the
ccache, and the user might want to use tables from both servers in the
same session.
> > I notice that gss_store_cred_into() has a companion,
> > gss_acquire_cred_from(). Is it possible to use that to pull out our
> > delegated credential explicitly by name, instead of stomping on the
> > global setup?
>
> Not really sure what is meant here by global setup..? Feeling like
> this is a follow on confusion from maybe mixing server vs client
> libpq?
By my reading, the gss_store_cred_into() call followed by
the setenv("KRB5CCNAME", ...) is effectively performing global
configuration for the process. Any KRB5CCNAME already set up by the
server admin is going to be ignored from that point onward. Is that
accurate?
Thanks,
--Jacob
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
On Fri, Mar 11, 2022 at 18:55 Jacob Champion wrote:
> On Mon, 2022-02-28 at 20:28 -0500, Stephen Frost wrote:
> > Will add to the CF for consideration.
>
> GSSAPI newbie here, so caveat lector.
No worries, thanks for your interest!
> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
> > index efc53f3135..6f820a34f1 100644
> > --- a/src/backend/libpq/auth.c
> > +++ b/src/backend/libpq/auth.c
> > @@ -920,6 +920,7 @@ pg_GSS_recvauth(Port *port)
> > int mtype;
> > StringInfoData buf;
> > gss_buffer_desc gbuf;
> > + gss_cred_id_t proxy;
> >
> > /*
> >* Use the configured keytab, if there is one. Unfortunately,
> Heimdal
> > @@ -949,6 +950,9 @@ pg_GSS_recvauth(Port *port)
> >*/
> > port->gss->ctx = GSS_C_NO_CONTEXT;
> >
> > + proxy = NULL;
> > + port->gss->proxy_creds = false;
> > +
> > /*
> >* Loop through GSSAPI message exchange. This exchange can consist
> of
> >* multiple messages sent in both directions. First message is
> always from
> > @@ -999,7 +1003,7 @@ pg_GSS_recvauth(Port *port)
> >
>>gss->outbuf,
> >
>,
> >
>NULL,
> > -
>NULL);
> > +
>);
> >
> > /* gbuf no longer used */
> > pfree(buf.data);
> > @@ -1011,6 +1015,12 @@ pg_GSS_recvauth(Port *port)
> >
> > CHECK_FOR_INTERRUPTS();
> >
> > + if (proxy != NULL)
> > + {
> > + pg_store_proxy_credential(proxy);
> > + port->gss->proxy_creds = true;
> > + }
> > +
>
> Some implementation docs [1] imply that a delegated_cred_handle is only
> valid if the ret_flags include GSS_C_DELEG_FLAG. The C-binding RFC [2],
> though, says that we can rely on it being set to GSS_C_NO_CREDENTIAL if
> no handle was sent...
>
> I don't know if there are any implementation differences here, but in
> any case I think it'd be more clear to use the GSS_C_NO_CREDENTIAL
> spelling (instead of NULL) here, if we do decide not to check
> ret_flags.
Hmmm, yeah, that seems like it might be better and is something I’ll take a
look at.
[5] says we have to free the proxy credential with GSS_Release_cred();
> I don't see that happening anywhere, but I may have missed it.
I’m not sure that it’s really necessary or worthwhile to do that at process
end since … the process is about to end. I suppose we could provide a
function that a user could call to ask for it to be released sooner if we
really wanted..?
> maj_stat = gss_init_sec_context(_stat,
> > -
> GSS_C_NO_CREDENTIAL,
> > +
> proxy,
> >
> >gctx,
> >
> conn->gtarg_nam,
> >
> GSS_C_NO_OID,
> > -
> GSS_C_MUTUAL_FLAG,
> > +
> GSS_C_MUTUAL_FLAG | GSS_C_DELEG_FLAG,
> > 0,
> >
> GSS_C_NO_CHANNEL_BINDINGS,
> >
> (ginbuf.value == NULL) ? GSS_C_NO_BUFFER : ,
> > diff --git a/src/interfaces/libpq/fe-secure-gssapi.c
> b/src/interfaces/libpq/fe-secure-gssapi.c
> > index 6ea52ed866..566c89f52f 100644
> > --- a/src/interfaces/libpq/fe-secure-gssapi.c
> > +++ b/src/interfaces/libpq/fe-secure-gssapi.c
> > @@ -631,7 +631,7 @@ pqsecure_open_gss(PGconn *conn)
> >*/
> > major = gss_init_sec_context(, conn->gcred, >gctx,
> >
> conn->gtarg_nam, GSS_C_NO_OID,
> > -
> GSS_REQUIRED_FLAGS, 0, 0, , NULL,
> > +
> GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, , NULL,
>
> It seems like there should be significant security implications to
> allowing delegation across the board. Especially since one postgres_fdw
> might delegate to another server, and then another... Should this be
> opt-in, maybe via a connection parameter?
This is already opt-in- at kinit time a user can decide if they’d like a
proxy-able ticket or not. I don’t know that we really need to have our own
option for it … tho I’m not really against adding such an option either.
(It also looks like there are some mechanisms for further constraining
> delegation scope, either by administrator policy or otherwise [3, 4].
> Might be a good thing for a v2 of this feature to have.)
Yes, constrained delegation is a pretty neat extension to Kerberos and one
I’d like to look at later as a future enhancement but I don’t think it
needs to be in the initial version.
Similarly, it feels a little strange that the server would allow the
> client to unilaterally force the use of a delegated credential. I think
> that should be opt-in on the server side too, unless there's some
> context I'm missing around why that's safe.
Perhaps you could explain what isn’t safe about accepting a delegated
credential from a client..? I am not away of a risk to accepting such a
delegated credential. Even so, I’m not against adding an option… but
exactly how would that option be configured? Server level? On the HBA
line? role level..?
> + /* Make the proxy credential only available to current process */
> > + major =
Re: Kerberos delegation support in libpq and postgres_fdw
On Mon, 2022-02-28 at 20:28 -0500, Stephen Frost wrote:
> Will add to the CF for consideration.
GSSAPI newbie here, so caveat lector.
> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
> index efc53f3135..6f820a34f1 100644
> --- a/src/backend/libpq/auth.c
> +++ b/src/backend/libpq/auth.c
> @@ -920,6 +920,7 @@ pg_GSS_recvauth(Port *port)
> int mtype;
> StringInfoData buf;
> gss_buffer_desc gbuf;
> + gss_cred_id_t proxy;
>
> /*
>* Use the configured keytab, if there is one. Unfortunately, Heimdal
> @@ -949,6 +950,9 @@ pg_GSS_recvauth(Port *port)
>*/
> port->gss->ctx = GSS_C_NO_CONTEXT;
>
> + proxy = NULL;
> + port->gss->proxy_creds = false;
> +
> /*
>* Loop through GSSAPI message exchange. This exchange can consist of
>* multiple messages sent in both directions. First message is always
> from
> @@ -999,7 +1003,7 @@ pg_GSS_recvauth(Port *port)
>
> >gss->outbuf,
>
> ,
>
> NULL,
> -
> NULL);
> +
> );
>
> /* gbuf no longer used */
> pfree(buf.data);
> @@ -1011,6 +1015,12 @@ pg_GSS_recvauth(Port *port)
>
> CHECK_FOR_INTERRUPTS();
>
> + if (proxy != NULL)
> + {
> + pg_store_proxy_credential(proxy);
> + port->gss->proxy_creds = true;
> + }
> +
Some implementation docs [1] imply that a delegated_cred_handle is only
valid if the ret_flags include GSS_C_DELEG_FLAG. The C-binding RFC [2],
though, says that we can rely on it being set to GSS_C_NO_CREDENTIAL if
no handle was sent...
I don't know if there are any implementation differences here, but in
any case I think it'd be more clear to use the GSS_C_NO_CREDENTIAL
spelling (instead of NULL) here, if we do decide not to check
ret_flags.
[5] says we have to free the proxy credential with GSS_Release_cred();
I don't see that happening anywhere, but I may have missed it.
> maj_stat = gss_init_sec_context(_stat,
> -
> GSS_C_NO_CREDENTIAL,
> + proxy,
>
> >gctx,
>
> conn->gtarg_nam,
>
> GSS_C_NO_OID,
> -
> GSS_C_MUTUAL_FLAG,
> +
> GSS_C_MUTUAL_FLAG | GSS_C_DELEG_FLAG,
> 0,
>
> GSS_C_NO_CHANNEL_BINDINGS,
>
> (ginbuf.value == NULL) ? GSS_C_NO_BUFFER : ,
> diff --git a/src/interfaces/libpq/fe-secure-gssapi.c
> b/src/interfaces/libpq/fe-secure-gssapi.c
> index 6ea52ed866..566c89f52f 100644
> --- a/src/interfaces/libpq/fe-secure-gssapi.c
> +++ b/src/interfaces/libpq/fe-secure-gssapi.c
> @@ -631,7 +631,7 @@ pqsecure_open_gss(PGconn *conn)
>*/
> major = gss_init_sec_context(, conn->gcred, >gctx,
>
> conn->gtarg_nam, GSS_C_NO_OID,
> -
> GSS_REQUIRED_FLAGS, 0, 0, , NULL,
> +
> GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, , NULL,
It seems like there should be significant security implications to
allowing delegation across the board. Especially since one postgres_fdw
might delegate to another server, and then another... Should this be
opt-in, maybe via a connection parameter?
(It also looks like there are some mechanisms for further constraining
delegation scope, either by administrator policy or otherwise [3, 4].
Might be a good thing for a v2 of this feature to have.)
Similarly, it feels a little strange that the server would allow the
client to unilaterally force the use of a delegated credential. I think
that should be opt-in on the server side too, unless there's some
context I'm missing around why that's safe.
> + /* Make the proxy credential only available to current process */
> + major = gss_store_cred_into(,
> + cred,
> + GSS_C_INITIATE, /* credential only used for starting libpq
> connection */
> +
Re: Kerberos delegation support in libpq and postgres_fdw
Greetings,
(Dropping the original poster as their email address apparently no
longer works)
* Peter Eisentraut (peter.eisentr...@enterprisedb.com) wrote:
> On 22.07.21 10:39, Peifeng Qiu wrote:
> >I've slightly modified the patch to support "gssencmode" and added TAP
> >tests.
>
> For the TAP tests, please put then under src/test/kerberos/, instead of
> copying the whole infrastructure to contrib/postgres_fdw/. Just make a new
> file, for example t/002_postgres_fdw_proxy.pl, and put your tests there.
I've incorporated the tests into the existing kerberos/001_auth.pl as
there didn't seem any need to create another file.
> Also, you can put code and tests in one patch, no need to separate.
Done. Also rebased and updated for the changes in the TAP testing
infrastructure and other changes. Also added code to track if
credentials were forwarded or not and to log that information.
> I wonder if this feature would also work in dblink. Since there is no
> substantial code changes in postgres_fdw itself as part of this patch, I
> would suspect yes. Can you check?
Yup, this should work fine. I didn't include any explicit testing of
postgres_fdw or dblink in this, yet. Instead, for the moment at least,
I've added to the connection log message an indiciation of if
credentials were passed along with the connection along with tests of
both the negative case and the positive case. Not sure if that's useful
information to have in pg_stat_gssapi, but if so, then we could add it
there pretty easily.
I'm happy to try and get testing with postgres_fdw and dblink working
soon though, assuming there aren't any particular objections to moving
this forward.
Will add to the CF for consideration.
Thanks,
Stephen
From c8aff4ae7595647fb5c82ea2f726c2d5b866765c Mon Sep 17 00:00:00 2001
From: Stephen Frost
Date: Mon, 28 Feb 2022 20:17:55 -0500
Subject: [PATCH] Add support for Kerberos credential delegation
Accept GSSAPI/Kerberos delegated credentials. With this, a user could
authenticate to PostgreSQL using Kerberos credentials, delegate
credentials to the PostgreSQL server, and then the PostgreSQL server
could use those credentials to connect to another service, such as with
postgres_fdw or dblink or theoretically any other authenticated
connection which is able to use delegated credentials.
Original patch by: Peifeng Qiu, whacked around some by me.
---
.../postgres_fdw/expected/postgres_fdw.out| 2 +-
contrib/postgres_fdw/option.c | 3 ++
src/backend/libpq/auth.c | 12 -
src/backend/libpq/be-gssapi-common.c | 44 +++
src/backend/libpq/be-secure-gssapi.c | 26 ++-
src/backend/utils/init/postinit.c | 8 ++--
src/include/libpq/be-gssapi-common.h | 3 ++
src/include/libpq/libpq-be.h | 2 +
src/interfaces/libpq/fe-auth.c| 9 +++-
src/interfaces/libpq/fe-secure-gssapi.c | 2 +-
src/test/kerberos/t/001_auth.pl | 25 +++
src/test/perl/PostgreSQL/Test/Utils.pm| 27
12 files changed, 146 insertions(+), 17 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index f210f91188..a73a468d89 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -188,7 +188,7 @@ ALTER USER MAPPING FOR public SERVER testserver1
ALTER USER MAPPING FOR public SERVER testserver1
OPTIONS (ADD sslmode 'require');
ERROR: invalid option "sslmode"
-HINT: Valid options in this context are: user, password, sslpassword, password_required, sslcert, sslkey
+HINT: Valid options in this context are: user, password, sslpassword, password_required, sslcert, sslkey, gssencmode
-- But we can add valid ones fine
ALTER USER MAPPING FOR public SERVER testserver1
OPTIONS (ADD sslpassword 'dummy');
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 572591a558..05922cfe6d 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -262,6 +262,9 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
+
{NULL, InvalidOid, false}
};
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index efc53f3135..6f820a34f1 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -920,6 +920,7 @@ pg_GSS_recvauth(Port *port)
int mtype;
StringInfoData buf;
gss_buffer_desc gbuf;
+ gss_cred_id_t proxy;
/*
* Use the configured keytab, if there is one. Unfortunately, Heimdal
@@ -949,6 +950,9 @@ pg_GSS_recvauth(Port *port)
*/
port->gss->ctx = GSS_C_NO_CONTEXT;
+ proxy = NULL;
+ port->gss->proxy_creds = false;
+
/*
* Loop through GSSAPI message exchange. This exchange can consist
Re: Kerberos delegation support in libpq and postgres_fdw
> On 3 Nov 2021, at 13:41, Daniel Gustafsson wrote: > > This patch no longer applies following the Perl namespace changes, can you > please submit a rebased version? Marking the patch "Waiting on Author". As the thread has stalled, and the OP email bounces, I'm marking this patch Returned with Feedback. Please feel free to resubmit a new entry in case anyone picks this up. -- Daniel Gustafsson https://vmware.com/
Re: Kerberos delegation support in libpq and postgres_fdw
This patch no longer applies following the Perl namespace changes, can you please submit a rebased version? Marking the patch "Waiting on Author". -- Daniel Gustafsson https://vmware.com/
Re: Kerberos delegation support in libpq and postgres_fdw
On 22.07.21 10:39, Peifeng Qiu wrote: I've slightly modified the patch to support "gssencmode" and added TAP tests. For the TAP tests, please put then under src/test/kerberos/, instead of copying the whole infrastructure to contrib/postgres_fdw/. Just make a new file, for example t/002_postgres_fdw_proxy.pl, and put your tests there. Also, you can put code and tests in one patch, no need to separate. I wonder if this feature would also work in dblink. Since there is no substantial code changes in postgres_fdw itself as part of this patch, I would suspect yes. Can you check?
Re: Kerberos delegation support in libpq and postgres_fdw
Hi all.
I've slightly modified the patch to support "gssencmode" and added TAP tests.
Best regards,
Peifeng Qiu
From: Peifeng Qiu
Sent: Tuesday, July 20, 2021 11:05 AM
To: pgsql-hackers@lists.postgresql.org ;
Magnus Hagander ; Stephen Frost ; Tom
Lane
Subject: Kerberos delegation support in libpq and postgres_fdw
Hi hackers.
This is the patch to add kerberos delegation support in libpq, which
enables postgres_fdw to connect to another server and authenticate
as the same user to the current login user. This will obsolete my
previous patch which requires keytab file to be present on the fdw
server host.
After the backend accepts the gssapi context, it may also get a
proxy credential if permitted by policy. I previously made a hack
to pass the pointer of proxy credential directly into libpq. It turns
out that the correct way to do this is store/acquire using credential
cache within local process memory to prevent leak.
Because no password is needed when querying foreign table via
kerberos delegation, the "password_required" option in user
mapping must be set to false by a superuser. Other than this, it
should work with normal user.
I only tested it manually in a very simple configuration currently.
I will go on to work with TAP tests for this.
How do you feel about this patch? Any feature/security concerns
about this?
Best regards,
Peifeng Qiu
commit a413b020afb663b5f89722b480c1b453e4304a36
Author: Peifeng Qiu
Date: Thu Jul 22 17:27:21 2021 +0900
kerberos delegation
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 4593cbc540..855f9af09c 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -247,6 +247,9 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /* gssencmode is also libpq option, same to above. */
+ {"gssencmode", UserMappingRelationId, true},
+
{NULL, InvalidOid, false}
};
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..235c9b32f5 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -918,6 +918,7 @@ pg_GSS_recvauth(Port *port)
int mtype;
StringInfoData buf;
gss_buffer_desc gbuf;
+ gss_cred_id_t proxy;
/*
* Use the configured keytab, if there is one. Unfortunately, Heimdal
@@ -947,6 +948,8 @@ pg_GSS_recvauth(Port *port)
*/
port->gss->ctx = GSS_C_NO_CONTEXT;
+ proxy = NULL;
+
/*
* Loop through GSSAPI message exchange. This exchange can consist of
* multiple messages sent in both directions. First message is always from
@@ -997,7 +1000,7 @@ pg_GSS_recvauth(Port *port)
>gss->outbuf,
,
NULL,
- NULL);
+ );
/* gbuf no longer used */
pfree(buf.data);
@@ -1009,6 +1012,9 @@ pg_GSS_recvauth(Port *port)
CHECK_FOR_INTERRUPTS();
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
if (port->gss->outbuf.length != 0)
{
/*
diff --git a/src/backend/libpq/be-gssapi-common.c b/src/backend/libpq/be-gssapi-common.c
index 38f58def25..cd243994c8 100644
--- a/src/backend/libpq/be-gssapi-common.c
+++ b/src/backend/libpq/be-gssapi-common.c
@@ -92,3 +92,40 @@ pg_GSS_error(const char *errmsg,
(errmsg_internal("%s", errmsg),
errdetail_internal("%s: %s", msg_major, msg_minor)));
}
+
+void
+pg_store_proxy_credential(gss_cred_id_t cred)
+{
+ OM_uint32 major, minor;
+ gss_OID_set mech;
+ gss_cred_usage_t usage;
+ gss_key_value_element_desc cc;
+ gss_key_value_set_desc ccset;
+
+ cc.key = "ccache";
+ cc.value = "MEMORY:";
+ ccset.count = 1;
+ ccset.elements =
+
+ /* Make the proxy credential only available to current process */
+ major = gss_store_cred_into(,
+ cred,
+ GSS_C_INITIATE, /* credential only used for starting libpq connection */
+ GSS_C_NULL_OID, /* store all */
+ true, /* overwrite */
+ true, /* make default */
+ ,
+ ,
+ );
+
+
+ if (major != GSS_S_COMPLETE)
+ {
+ pg_GSS_error("gss_store_cred", major, minor);
+ }
+
+ /* quite strange that gss_store_cred doesn't work with "KRB5CCNAME=MEMORY:",
+ * we have to use gss_store_cred_into instead and set the env for later
+ * gss_acquire_cred calls. */
+ putenv("KRB5CCNAME=MEMORY:");
+}
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 316ca65db5..e27d517dea 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -497,6 +497,7 @@ secure_open_gssapi(Port *port)
bool complete_next = false;
OM_uint32 major,
minor;
+ gss_cred_id_t proxy;
/*
* Allocate subsidiary Port data for GSSAPI operations.
@@ -588,7 +589,8 @@ secure_open_gssapi(Port *port)
GSS_C_NO_CREDENTIAL, ,
GSS_C_NO_CHANNEL_BINDINGS,
>gss->name, NU
Kerberos delegation support in libpq and postgres_fdw
Hi hackers.
This is the patch to add kerberos delegation support in libpq, which
enables postgres_fdw to connect to another server and authenticate
as the same user to the current login user. This will obsolete my
previous patch which requires keytab file to be present on the fdw
server host.
After the backend accepts the gssapi context, it may also get a
proxy credential if permitted by policy. I previously made a hack
to pass the pointer of proxy credential directly into libpq. It turns
out that the correct way to do this is store/acquire using credential
cache within local process memory to prevent leak.
Because no password is needed when querying foreign table via
kerberos delegation, the "password_required" option in user
mapping must be set to false by a superuser. Other than this, it
should work with normal user.
I only tested it manually in a very simple configuration currently.
I will go on to work with TAP tests for this.
How do you feel about this patch? Any feature/security concerns
about this?
Best regards,
Peifeng Qiu
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 8cc23ef7fb..235c9b32f5 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -918,6 +918,7 @@ pg_GSS_recvauth(Port *port)
int mtype;
StringInfoData buf;
gss_buffer_desc gbuf;
+ gss_cred_id_t proxy;
/*
* Use the configured keytab, if there is one. Unfortunately, Heimdal
@@ -947,6 +948,8 @@ pg_GSS_recvauth(Port *port)
*/
port->gss->ctx = GSS_C_NO_CONTEXT;
+ proxy = NULL;
+
/*
* Loop through GSSAPI message exchange. This exchange can consist of
* multiple messages sent in both directions. First message is always from
@@ -997,7 +1000,7 @@ pg_GSS_recvauth(Port *port)
>gss->outbuf,
,
NULL,
- NULL);
+ );
/* gbuf no longer used */
pfree(buf.data);
@@ -1009,6 +1012,9 @@ pg_GSS_recvauth(Port *port)
CHECK_FOR_INTERRUPTS();
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
if (port->gss->outbuf.length != 0)
{
/*
diff --git a/src/backend/libpq/be-gssapi-common.c b/src/backend/libpq/be-gssapi-common.c
index 38f58def25..cd243994c8 100644
--- a/src/backend/libpq/be-gssapi-common.c
+++ b/src/backend/libpq/be-gssapi-common.c
@@ -92,3 +92,40 @@ pg_GSS_error(const char *errmsg,
(errmsg_internal("%s", errmsg),
errdetail_internal("%s: %s", msg_major, msg_minor)));
}
+
+void
+pg_store_proxy_credential(gss_cred_id_t cred)
+{
+ OM_uint32 major, minor;
+ gss_OID_set mech;
+ gss_cred_usage_t usage;
+ gss_key_value_element_desc cc;
+ gss_key_value_set_desc ccset;
+
+ cc.key = "ccache";
+ cc.value = "MEMORY:";
+ ccset.count = 1;
+ ccset.elements =
+
+ /* Make the proxy credential only available to current process */
+ major = gss_store_cred_into(,
+ cred,
+ GSS_C_INITIATE, /* credential only used for starting libpq connection */
+ GSS_C_NULL_OID, /* store all */
+ true, /* overwrite */
+ true, /* make default */
+ ,
+ ,
+ );
+
+
+ if (major != GSS_S_COMPLETE)
+ {
+ pg_GSS_error("gss_store_cred", major, minor);
+ }
+
+ /* quite strange that gss_store_cred doesn't work with "KRB5CCNAME=MEMORY:",
+ * we have to use gss_store_cred_into instead and set the env for later
+ * gss_acquire_cred calls. */
+ putenv("KRB5CCNAME=MEMORY:");
+}
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 316ca65db5..e27d517dea 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -497,6 +497,7 @@ secure_open_gssapi(Port *port)
bool complete_next = false;
OM_uint32 major,
minor;
+ gss_cred_id_t proxy;
/*
* Allocate subsidiary Port data for GSSAPI operations.
@@ -588,7 +589,8 @@ secure_open_gssapi(Port *port)
GSS_C_NO_CREDENTIAL, ,
GSS_C_NO_CHANNEL_BINDINGS,
>gss->name, NULL, , NULL,
- NULL, NULL);
+ NULL, );
+
if (GSS_ERROR(major))
{
pg_GSS_error(_("could not accept GSSAPI security context"),
@@ -605,6 +607,9 @@ secure_open_gssapi(Port *port)
complete_next = true;
}
+ if (proxy != NULL)
+ pg_store_proxy_credential(proxy);
+
/* Done handling the incoming packet, reset our buffer */
PqGSSRecvLength = 0;
diff --git a/src/include/libpq/be-gssapi-common.h b/src/include/libpq/be-gssapi-common.h
index c07d7e7c5a..62d60ffbd8 100644
--- a/src/include/libpq/be-gssapi-common.h
+++ b/src/include/libpq/be-gssapi-common.h
@@ -18,9 +18,11 @@
#include
#else
#include
+#include
#endif
extern void pg_GSS_error(const char *errmsg,
OM_uint32 maj_stat, OM_uint32 min_stat);
+extern void pg_store_proxy_credential(gss_cred_id_t cred);
#endif /* BE_GSSAPI_COMMON_H */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 3421ed4685..e0d342124e 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -62,6 +62,7 @@ pg_GSS_continue(PGconn *conn,
