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 <pgsql-hackers@lists.postgresql.org>; Magnus Hagander <magnus@hagander.net>; Stephen Frost <sfrost@snowman.net>; Tom Lane <tgl@sss.pgh.pa.us>
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

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?

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/

On 3 Nov 2021, at 13:41, Daniel Gustafsson <daniel@yesql.se> 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/

Greetings,

(Dropping the original poster as their email address apparently no
longer works)

* Peter Eisentraut (peter.eisentraut@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

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)
&port->gss->outbuf,
&gflags,
NULL,
-										  NULL);
+										  &proxy);

/* 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]https://docs.oracle.com/cd/E36784_01/html/E36875/gss-accept-sec-context-3gss.html imply that a delegated_cred_handle is only
valid if the ret_flags include GSS_C_DELEG_FLAG. The C-binding RFC [2]https://datatracker.ietf.org/doc/html/rfc2744#section-5.1,
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]: https://datatracker.ietf.org/doc/html/rfc2743#page-50
I don't see that happening anywhere, but I may have missed it.

maj_stat = gss_init_sec_context(&min_stat,
-									GSS_C_NO_CREDENTIAL,
+									proxy,
&conn->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 : &ginbuf,
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(&minor, conn->gcred, &conn->gctx,
conn->gtarg_nam, GSS_C_NO_OID,
-								 GSS_REQUIRED_FLAGS, 0, 0, &input, NULL,
+								 GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, &input, 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(&minor,
+		cred,
+		GSS_C_INITIATE, /* credential only used for starting libpq connection */
+		GSS_C_NULL_OID, /* store all */
+		true, /* overwrite */
+		true, /* make default */
+		&ccset,
+		&mech,
+		&usage);
+
+
+	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. */
+	setenv("KRB5CCNAME", GSS_MEMORY_CACHE, 1);

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. 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?

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?

Thanks,
--Jacob

[1]: https://docs.oracle.com/cd/E36784_01/html/E36875/gss-accept-sec-context-3gss.html
[2]: https://datatracker.ietf.org/doc/html/rfc2744#section-5.1
[3]: https://datatracker.ietf.org/doc/html/rfc5896
[4]: https://web.mit.edu/kerberos/krb5-latest/doc/appdev/gssapi.html#constrained-delegation-s4u
[5]: https://datatracker.ietf.org/doc/html/rfc2743#page-50

Greetings,

On Fri, Mar 11, 2022 at 18:55 Jacob Champion <pchampion@vmware.com> 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)

&port->gss->outbuf,

&gflags,

NULL,

-

NULL);

+

&proxy);

/* 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(&min_stat,

-

GSS_C_NO_CREDENTIAL,

+

proxy,

&conn->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 : &ginbuf,

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(&minor, conn->gcred, &conn->gctx,

conn->gtarg_nam, GSS_C_NO_OID,

-

GSS_REQUIRED_FLAGS, 0, 0, &input, NULL,

+

GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, &input, 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 = gss_store_cred_into(&minor,
+             cred,
+             GSS_C_INITIATE, /* credential only used for starting libpq

connection */

+             GSS_C_NULL_OID, /* store all */
+             true, /* overwrite */
+             true, /* make default */
+             &ccset,
+             &mech,
+             &usage);
+
+
+     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. */
+     setenv("KRB5CCNAME", GSS_MEMORY_CACHE, 1);

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..?

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 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?

Thanks,

Stephen

On Fri, 2022-03-11 at 19:39 -0500, Stephen Frost wrote:

On Fri, Mar 11, 2022 at 18:55 Jacob Champion <pchampion@vmware.com> 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

Greetinsg,

* Jacob Champion (pchampion@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 <pchampion@vmware.com> 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 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.

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.

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.

Thoughts?

Thanks!

Stephen

Greetings,

* Stephen Frost (sfrost@snowman.net) wrote:

* Jacob Champion (pchampion@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

Greetings,

* Stephen Frost (sfrost@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

On Fri, Apr 8, 2022 at 8:21 AM Stephen Frost <sfrost@snowman.net> 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.

+ <term><literal>allow_cred_delegation</literal></term>

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.

+      <listitem>
+       <para>
+        Sets the location of the Kerberos credential cache to be used for
+        regular user backends which go through authentication.  The default is
+        <filename>MEMORY:</filename>, 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

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Fri, Apr 8, 2022 at 8:21 AM Stephen Frost <sfrost@snowman.net> 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.

+ <term><literal>allow_cred_delegation</literal></term>

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.

+      <listitem>
+       <para>
+        Sets the location of the Kerberos credential cache to be used for
+        regular user backends which go through authentication.  The default is
+        <filename>MEMORY:</filename>, 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.

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

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.

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.

Yeah, the other #define should have gone away and been changed to use
the above. That should be easy enough to fix.

Thanks!

Stephen

On Fri, Apr 8, 2022 at 11:29 AM Stephen Frost <sfrost@snowman.net> wrote:

+ <term><literal>allow_cred_delegation</literal></term>

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

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*, including session keys,
and that doesn't make me feel very good about the strength of the crypto
when the middlebox starts talking to the backend foreign server.

Maybe there's some ephemeral exchange going on that makes it too hard to
attack in practice, or some other mitigations. But Wireshark doesn't
understand how to dissect the libpq gssenc exchange, and I don't know
the specs well enough yet, so I can't really prove it either way. Do you
know more about the underlying GSS exchange, or else which specs cover
the low-level details? I'm trying to avoid writing Wireshark dissector
code, but maybe that'd be useful either way...

--Jacob

On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion <jchampion@timescale.com> 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]https://www.rfc-editor.org/rfc/rfc4120:

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

Greetings,

* Jacob Champion (jchampion@timescale.com) wrote:

On Thu, Jul 7, 2022 at 4:24 PM Jacob Champion <jchampion@timescale.com> 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

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

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