Subject: Kerberos delegation support in libpq and postgres_fdw

From: Peifeng Qiu <[email-masked]>
Date: Tue, 20 Jul 2021 03:05:48 +0000

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)
 										  &port->gss->outbuf,
 										  &gflags,
 										  NULL,
-										  NULL);
+										  &proxy);
 
 		/* 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 = &cc;
+
+	/* 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. */
+	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, &input,
 									   GSS_C_NO_CHANNEL_BINDINGS,
 									   &port->gss->name, NULL, &output, NULL,
-									   NULL, NULL);
+									   NULL, &proxy);
+
 		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 <gssapi.h>
 #else
 #include <gssapi/gssapi.h>
+#include <gssapi/gssapi_ext.h>
 #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, int payloadlen)
 				lmin_s;
 	gss_buffer_desc ginbuf;
 	gss_buffer_desc goutbuf;
+	gss_cred_id_t proxy;
 
 	/*
 	 * On first call, there's no input token. On subsequent calls, read the
@@ -94,12 +95,16 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
 		ginbuf.value = NULL;
 	}
 
+	/* Check if we can aquire a proxy credential. */
+	if (!pg_GSS_have_cred_cache(&proxy))
+		proxy = GSS_C_NO_CREDENTIAL;
+
 	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 c783a53734..781af4227c 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,
 								 &output, NULL, NULL);
 
 	/* GSS Init Sec Context uses the whole packet, so clear it */

---

From: Peifeng Qiu <[email-masked]>
Date: Thu, 22 Jul 2021 08:39:53 +0000

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: [email-masked] <[email-masked]>; Magnus Hagander <[email-masked]>; Stephen Frost <[email-masked]>; Tom Lane <[email-masked]>
Subject: Kerberos delegation support in libpq and postgres_fdw

Hi hackers.

Best regards,
Peifeng Qiu

commit a413b020afb663b5f89722b480c1b453e4304a36
Author: Peifeng Qiu <[email-masked]>
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}
 	};

 	/*
 
+	proxy = NULL;
+
 	/*

 		CHECK_FOR_INTERRUPTS();
 
+		if (proxy != NULL)
 
 	/*
 
+		if (proxy != NULL)

 
 
 	/*

 	/* GSS Init Sec Context uses the whole packet, so clear it */
commit a24714cc5507ce6027eef6f923b8d24e7e1ce208
Author: Peifeng Qiu <[email-masked]>
Date:   Thu Jul 22 17:27:36 2021 +0900

    TAP test

diff --git a/contrib/postgres_fdw/kerberos/.gitignore b/contrib/postgres_fdw/kerberos/.gitignore
new file mode 100644
index 0000000000..871e943d50
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/.gitignore
@@ -0,0 +1,2 @@
+# Generated by test suite
+/tmp_check/
diff --git a/contrib/postgres_fdw/kerberos/Makefile b/contrib/postgres_fdw/kerberos/Makefile
new file mode 100644
index 0000000000..3efec922f0
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/Makefile
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/kerberos
+#
+# Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/kerberos/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = contrib/postgres_fdw/kerberos
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+export with_gssapi with_krb_srvnam
+
+check:
+	$(prove_check)
+
+installcheck:
+	$(prove_installcheck)
+
+clean distclean maintainer-clean:
+	rm -rf tmp_check
diff --git a/contrib/postgres_fdw/kerberos/README b/contrib/postgres_fdw/kerberos/README
new file mode 100644
index 0000000000..40b8758543
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/README
@@ -0,0 +1,45 @@
+contrib/postgres_fdw/kerberos/README
+
+Tests for Kerberos/GSSAPI functionality
+=======================================
+
+This directory contains a test suite for Kerberos/GSSAPI
+functionality.  This requires a full MIT Kerberos installation,
+including server and client tools, and is therefore kept separate and
+not run by default.
+
+CAUTION: The test server run by this test is configured to listen for TCP
+connections on localhost. Any user on the same host is able to log in to the
+test server while the tests are running. Do not run this suite on a multi-user
+system where you don't trust all local users! Also, this test suite creates a
+KDC server that listens for TCP/IP connections on localhost without any real
+access control.
+
+Running the tests
+=================
+
+NOTE: You must have given the --enable-tap-tests argument to configure.
+
+Run
+    make check
+or
+    make installcheck
+You can use "make installcheck" if you previously did "make install".
+In that case, the code in the installation tree is tested.  With
+"make check", a temporary installation tree is built from the current
+sources and then tested.
+
+Either way, this test initializes, starts, and stops a test Postgres
+cluster, as well as a test KDC server.
+
+Requirements
+============
+
+MIT Kerberos server and client tools are required.  Heimdal is not
+supported.
+
+Debian/Ubuntu packages: krb5-admin-server krb5-kdc krb5-user
+
+RHEL/CentOS/Fedora packages: krb5-server krb5-workstation
+
+FreeBSD port: krb5 (base system has Heimdal)
diff --git a/contrib/postgres_fdw/kerberos/t/001_auth.pl b/contrib/postgres_fdw/kerberos/t/001_auth.pl
new file mode 100644
index 0000000000..79cd0bb006
--- /dev/null
+++ b/contrib/postgres_fdw/kerberos/t/001_auth.pl
@@ -0,0 +1,426 @@
+
+# Copyright (c) 2021, PostgreSQL Global Development Group
+
+# Sets up a KDC and then runs a variety of tests to make sure that the
+# GSSAPI/Kerberos authentication and encryption are working properly,
+# that the options in pg_hba.conf and pg_ident.conf are handled correctly,
+# and that the server-side pg_stat_gssapi view reports what we expect to
+# see for each test.
+#
+# Since this requires setting up a full KDC, it doesn't make much sense
+# to have multiple test scripts (since they'd have to also create their
+# own KDC and that could cause race conditions or other problems)- so
+# just add whatever other tests are needed to here.
+#
+# See the README for additional information.
+
+use strict;
+use warnings;
+use TestLib;
+use PostgresNode;
+use Test::More;
+use Time::HiRes qw(usleep);
+use Cwd 'abs_path';
+
+note "start";
+if ($ENV{with_gssapi} eq 'yes')
+{
+	plan tests => 23;
+}
+else
+{
+	plan skip_all => 'GSSAPI/Kerberos not supported by this build';
+}
+
+my ($krb5_bin_dir, $krb5_sbin_dir);
+
+if ($^O eq 'darwin')
+{
+	$krb5_bin_dir  = '/usr/local/opt/krb5/bin';
+	$krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
+}
+elsif ($^O eq 'freebsd')
+{
+	$krb5_bin_dir  = '/usr/local/bin';
+	$krb5_sbin_dir = '/usr/local/sbin';
+}
+elsif ($^O eq 'linux')
+{
+	$krb5_sbin_dir = '/usr/sbin';
+}
+
+my $krb5_config  = 'krb5-config';
+my $kinit        = 'kinit';
+my $kdb5_util    = 'kdb5_util';
+my $kadmin_local = 'kadmin.local';
+my $krb5kdc      = 'krb5kdc';
+
+if ($krb5_bin_dir && -d $krb5_bin_dir)
+{
+	$krb5_config = $krb5_bin_dir . '/' . $krb5_config;
+	$kinit       = $krb5_bin_dir . '/' . $kinit;
+}
+if ($krb5_sbin_dir && -d $krb5_sbin_dir)
+{
+	$kdb5_util    = $krb5_sbin_dir . '/' . $kdb5_util;
+	$kadmin_local = $krb5_sbin_dir . '/' . $kadmin_local;
+	$krb5kdc      = $krb5_sbin_dir . '/' . $krb5kdc;
+}
+
+my $host     = 'auth-test-localhost.postgresql.example.com';
+my $hostaddr = '127.0.0.1';
+my $realm    = 'EXAMPLE.COM';
+
+
+my $tmp_check = abs_path(${TestLib::tmp_check});
+my $krb5_conf   = "$tmp_check/krb5.conf";
+my $kdc_conf    = "$tmp_check/kdc.conf";
+#my $krb5_cache  = "$tmp_check/krb5cc";
+my $krb5_cache  = "MEMORY:";
+my $krb5_log    = "${TestLib::log_path}/krb5libs.log";
+my $kdc_log     = "${TestLib::log_path}/krb5kdc.log";
+my $kdc_port    = get_free_port();
+my $kdc_datadir = "$tmp_check/krb5kdc";
+my $kdc_pidfile = "$tmp_check/krb5kdc.pid";
+my $keytab_client      = "$tmp_check/client.keytab";
+my $keytab_server      = "$tmp_check/server.keytab";
+
+my $dbname      = 'testdb';
+my $testuser    = 'testuser';
+my $test1   = 'test1';
+my $application = '001_auth.pl';
+
+note "setting up Kerberos";
+
+my ($stdout, $krb5_version);
+run_log [ $krb5_config, '--version' ], '>', \$stdout
+  or BAIL_OUT("could not execute krb5-config");
+BAIL_OUT("Heimdal is not supported") if $stdout =~ m/heimdal/;
+$stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
+  or BAIL_OUT("could not get Kerberos version");
+$krb5_version = $1;
+
+append_to_file(
+	$krb5_conf,
+	qq![logging]
+default = FILE:$krb5_log
+kdc = FILE:$kdc_log
+
+[libdefaults]
+default_realm = $realm
+forwardable = false
+
+[realms]
+$realm = {
+    kdc = $hostaddr:$kdc_port
+}!);
+
+append_to_file(
+	$kdc_conf,
+	qq![kdcdefaults]
+!);
+
+# For new-enough versions of krb5, use the _listen settings rather
+# than the _ports settings so that we can bind to localhost only.
+if ($krb5_version >= 1.15)
+{
+	append_to_file(
+		$kdc_conf,
+		qq!kdc_listen = $hostaddr:$kdc_port
+kdc_tcp_listen = $hostaddr:$kdc_port
+!);
+}
+else
+{
+	append_to_file(
+		$kdc_conf,
+		qq!kdc_ports = $kdc_port
+kdc_tcp_ports = $kdc_port
+!);
+}
+
+append_to_file(
+	$kdc_conf,
+	qq!
+[realms]
+$realm = {
+    database_name = $kdc_datadir/principal
+    admin_keytab = FILE:$kdc_datadir/kadm5.keytab
+    acl_file = $kdc_datadir/kadm5.acl
+    key_stash_file = $kdc_datadir/_k5.$realm
+}!);
+
+mkdir $kdc_datadir or die;
+
+# Ensure that we use test's config and cache files, not global ones.
+$ENV{'KRB5_CONFIG'}      = $krb5_conf;
+$ENV{'KRB5_KDC_PROFILE'} = $kdc_conf;
+$ENV{'KRB5CCNAME'}       = $krb5_cache;
+
+my $service_principal = "$ENV{with_krb_srvnam}/$host";
+
+system_or_bail $kdb5_util, 'create', '-s', '-P', 'secret0';
+
+my $test1_password = 'secret1';
+system_or_bail $kadmin_local, '-q', "addprinc -randkey $testuser";
+system_or_bail $kadmin_local, '-q', "addprinc -randkey $service_principal";
+system_or_bail $kadmin_local, '-q', "ktadd -k $keytab_client $testuser";
+system_or_bail $kadmin_local, '-q', "ktadd -k $keytab_server $service_principal";
+
+system_or_bail $krb5kdc, '-P', $kdc_pidfile;
+
+END
+{
+	kill 'INT', `cat $kdc_pidfile` if -f $kdc_pidfile;
+}
+
+note "setting up PostgreSQL instance";
+
+# server node for fdw to connect
+my $server_node = get_new_node('node');
+$server_node->init;
+$server_node->append_conf(
+	'postgresql.conf', qq{
+listen_addresses = '$hostaddr'
+krb_server_keyfile = '$keytab_server'
+log_connections = on
+lc_messages = 'C'
+});
+$server_node->start;
+
+note "setting up user on server";
+$server_node->safe_psql('postgres', "CREATE USER $testuser;");
+# there's no direct way to get the backend pid from postgres_fdw, create a VIEW at server side to provide this.
+$server_node->safe_psql('postgres', 'CREATE VIEW my_pg_stat_gssapi AS
+    SELECT  S.pid,
+            S.gss_auth AS gss_authenticated,
+            S.gss_princ AS principal,
+            S.gss_enc AS encrypted
+    FROM pg_stat_get_activity(NULL) AS S
+    WHERE S.client_port IS NOT NULL AND S.pid = pg_backend_pid();');
+$server_node->safe_psql('postgres', "GRANT ALL ON my_pg_stat_gssapi TO $testuser;");
+
+my $server_node_port = $server_node->port;
+note "setting up PostgreSQL fdw instance";
+# client node that runs postgres fdw
+my $fdw_node = get_new_node('node_fdw');
+$fdw_node->init;
+$fdw_node->append_conf(
+	'postgresql.conf', qq{
+listen_addresses = '$hostaddr'
+krb_server_keyfile = '$keytab_server'
+log_connections = on
+lc_messages = 'C'
+});
+$fdw_node->start;
+
+$fdw_node->safe_psql('postgres', "CREATE USER $testuser;");
+$fdw_node->safe_psql('postgres', "CREATE DATABASE $dbname;");
+$fdw_node->safe_psql($dbname, "CREATE EXTENSION postgres_fdw");
+$fdw_node->safe_psql($dbname, "GRANT ALL ON FOREIGN DATA WRAPPER postgres_fdw TO $testuser");
+
+note "running tests";
+
+sub set_user_mapping_option
+{
+	my ($option, $value, $action) = @_;
+	if (!defined $action)
+	{
+		$action = "SET";
+	}
+
+	my $option_str = "$option";
+	if (defined $value)
+	{
+		$option_str = "$option '$value'";
+	}
+
+	my $q = "ALTER USER MAPPING for $testuser SERVER postgres_server OPTIONS ($action $option_str);";
+	$fdw_node->safe_psql($dbname, $q);
+}
+
+# Test connection success or failure, and if success, that query returns true.
+sub test_access
+{
+	my ($query, $expected_res, $test_name,
+		$expect_output, @expect_log_msgs)
+	  = @_;
+	test_access_gssmode($query, "prefer", $expected_res, $test_name,
+		$expect_output, @expect_log_msgs)
+}
+
+sub test_access_gssmode_disable
+{
+	my ($query, $expected_res, $test_name,
+		$expect_output, @expect_log_msgs)
+	  = @_;
+	test_access_gssmode($query, "disable", $expected_res, $test_name,
+		$expect_output, @expect_log_msgs)
+}
+
+sub test_access_gssmode
+{
+	my ($query, $mode, $expected_res, $test_name,
+		$expect_output, @expect_log_msgs)
+	  = @_;
+
+	my %params = (sql => $query,);
+
+	# Check log in server node. Obtain log file size before we run any query.
+	# This way we can obtain logs for this psql connection only.
+	my $log_location = -s $server_node->logfile;
+
+	# Run psql in fdw node.
+	my ($ret, $stdout, $stderr) = $fdw_node->psql(
+		$dbname,
+		$query,
+		extra_params => ['-w'],
+		connstr      => "user=$testuser dbname=$dbname host=$host hostaddr=$hostaddr gssencmode=$mode");
+
+
+	is($ret, $expected_res, $test_name);
+	if ($ret != $expected_res)
+	{
+		note $ret;
+		note $stdout;
+		note $stderr;
+	}
+
+	if (defined $expect_output)
+	{
+		if ($expected_res eq 0)
+		{
+			like($stdout, $expect_output, "$test_name: result matches");
+		}
+		else
+		{
+			like($stderr, $expect_output, "$test_name: result matches");
+		}
+	}
+
+	if (@expect_log_msgs)
+	{
+		# Match every message literally.
+		my @regexes = map { qr/\Q$_\E/ } @expect_log_msgs;
+		my $log_contents = TestLib::slurp_file($server_node->logfile, $log_location);
+
+		while (my $regex = shift @regexes)
+		{
+			like($log_contents, $regex, "$test_name: matches");
+		}
+	}
+}
+
+# Setup pg_hba to allow superuser trust login, and testuser gss login.
+# We need to remove the KRB5_CLIENT_KTNAME environemnt first.
+delete $ENV{'KRB5_CLIENT_KTNAME'};
+unlink($fdw_node->data_dir . '/pg_hba.conf');
+$fdw_node->append_conf('pg_hba.conf',
+	qq{local all $ENV{'USER'} trust});
+$fdw_node->append_conf('pg_hba.conf',
+	qq{host all $testuser $hostaddr/32 gss map=mymap});
+$fdw_node->append_conf('pg_ident.conf', qq{mymap  /^(.*)\@$realm\$  \\1});
+$fdw_node->restart;
+
+test_access(
+	'SELECT true FROM pg_stat_gssapi',
+	2,
+	'no access without credential',
+	'/No Kerberos credentials available/'
+);
+
+$ENV{'KRB5_CLIENT_KTNAME'}       = $keytab_client;
+
+test_access(
+	'SELECT true FROM pg_stat_gssapi',
+	0,
+	'success',
+	"/^t\$/"
+);
+
+# create foreign table using unprivileged user
+
+test_access(
+	"CREATE SERVER postgres_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host '$host', hostaddr '$hostaddr', port '$server_node_port', dbname 'postgres')",
+	0,
+	'create server',
+);
+test_access(
+	"CREATE FOREIGN TABLE my_pg_stat_gssapi\(pid int, gss_authenticated boolean, principal text, encrypted boolean\) SERVER postgres_server OPTIONS(table_name 'my_pg_stat_gssapi'); ",
+	0,
+	'create foreign table',
+);
+test_access(
+	"CREATE USER MAPPING for $testuser SERVER postgres_server OPTIONS (user '$testuser');",
+	0,
+	'create user mapping',
+);
+
+test_access(
+	"SELECT gss_authenticated AND encrypted from my_pg_stat_gssapi;",
+	3,
+	'select failed due to password required',
+	"/password is required/"
+);
+
+set_user_mapping_option('password_required', 'false', 'ADD');
+
+test_access(
+	"SELECT gss_authenticated AND encrypted from my_pg_stat_gssapi;",
+	0,
+	'select fdw success, but with trust connection',
+	"/f/"
+);
+
+
+unlink($server_node->data_dir . '/pg_hba.conf');
+$server_node->append_conf('pg_hba.conf',
+	qq{host all all $hostaddr/32 gss map=mymap});
+$server_node->append_conf('pg_ident.conf', qq{mymap  /^(.*)\@$realm\$  \\1});
+$server_node->restart;
+
+test_access(
+	'SELECT gss_authenticated FROM my_pg_stat_gssapi',
+	3,
+	'fails without ticket, due to not forwardable',
+	'/No Kerberos credentials available/',
+);
+
+# Allow ticket forward in krb5.conf
+string_replace_file($krb5_conf, "forwardable = false", "forwardable = true");
+test_access(
+	'SELECT gss_authenticated FROM my_pg_stat_gssapi',
+	0,
+	'success with ticket forward',
+	"/^t\$/"
+);
+
+test_access_gssmode_disable(
+	'SELECT gss_authenticated,encrypted FROM pg_stat_gssapi',
+	0,
+	'login fdw success, encryption disabled',
+	"/^t|f\$/"
+);
+
+test_access_gssmode_disable(
+	'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+	0,
+	'success, encryption enabled for fdw even disabled by login',
+	"/^t|t\$/"
+);
+
+set_user_mapping_option('gssencmode', 'disable', 'ADD');
+
+test_access(
+	'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+	0,
+	'success, encryption disabled for fdw even enabled by login',
+	"/^t|f\$/"
+);
+
+test_access_gssmode_disable(
+	'SELECT gss_authenticated,encrypted FROM my_pg_stat_gssapi',
+	0,
+	'success, encryption disabled both by login and fdw',
+	"/^t|f\$/"
+);
\ No newline at end of file
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 15572abbea..af4245f755 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
   slurp_dir
   slurp_file
   append_to_file
+  string_replace_file
   check_mode_recursive
   chmod_recursive
   check_pg_config
@@ -539,6 +540,32 @@ sub append_to_file
 
 =pod
 
+=item string_replace_file(filename, find, replace)
+
+Find and replace string of a given file.
+
+=cut
+
+sub string_replace_file
+{
+	my ($filename, $find, $replace) = @_;
+	open(my $in, '<', $filename);
+    my $content;
+    while(<$in>)
+    {
+        $_ =~ s/$find/$replace/;
+        $content = $content.$_;
+    }
+	close $in;
+    open(my $out, '>', $filename);
+    print $out $content;
+    close($out);
+
+	return;
+}
+
+=pod
+
 =item check_mode_recursive(dir, expected_dir_mode, expected_file_mode, ignore_list)
 
 Check that all file/dir modes in a directory match the expected values,

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 1 Sep 2021 10:57:20 +0200

> I've slightly modified the patch to support "gssencmode" and added TAP 

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?

---

From: Daniel Gustafsson <[email-masked]>
Date: Wed, 3 Nov 2021 13:41:32 +0100

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/

---

From: Daniel Gustafsson <[email-masked]>
Date: Fri, 26 Nov 2021 14:41:55 +0100

> On 3 Nov 2021, at 13:41, Daniel Gustafsson <[email-masked]> wrote:
> 

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.

---

From: Jacob Champion <[email-masked]>
Date: Thu, 7 Jul 2022 16:24:49 -0700

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

---

From: Jacob Champion <[email-masked]>
Date: Thu, 15 Sep 2022 17:06:05 -0700

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

> 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

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 0cf31522223a6044edae42037e45aee0fc88352f Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email-masked]>
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 <epan/packet.h>
 
+#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 client send further SASL challange-response messages */
-  PGSQL_AUTH_GSSAPI_SSPI_DATA,         /* GSSAPI/SSPI in use */
+  PGSQL_AUTH_GSSAPI,                   /* GSSAPI in use */
+  PGSQL_AUTH_SSPI,                     /* SSPI in use */
 } pgsql_auth_state_t;
 
 typedef struct pgsql_conn_data {
     gboolean    ssl_requested;
+    gboolean    gss_requested;
+    guint32     last_nongss_frame;
     pgsql_auth_state_t auth_state; /* Current authentication state */
 } pgsql_conn_data_t;
 
@@ -189,6 +198,22 @@ static const value_string format_vals[] = {
     { 0, NULL }
 };
 
+static void
+dissect_gssapi_data(tvbuff_t *tvb, gint offset, guint len, packet_info *pinfo,
+                    proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+    tvbuff_t *gssapi_tvb;
+    guint8 *data;
+
+    DISSECTOR_ASSERT(tvb_bytes_exist(tvb, offset, len));
+
+    data = (guint8 *)tvb_memdup(pinfo->pool, tvb, offset, len);
+    gssapi_tvb = tvb_new_child_real_data(tvb, data, len, len);
+
+    add_new_data_source(pinfo, gssapi_tvb, "GSSAPI Data");
+    call_dissector_with_data(gssapi_handle, gssapi_tvb, pinfo, tree, encrypt);
+}
+
 static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
                                  gint n, proto_tree *tree, packet_info *pinfo,
                                  pgsql_conn_data_t *conv_data)
@@ -220,7 +245,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
                 proto_tree_add_item(tree, hf_sasl_auth_data, tvb, n, length-4, ENC_NA);
                 break;
 
-            case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+            case PGSQL_AUTH_GSSAPI:
+                proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
+                dissect_gssapi_data(tvb, n, length-4, pinfo, tree, NULL);
+                break;
+
+            case PGSQL_AUTH_SSPI:
                 proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-4, ENC_NA);
                 break;
 
@@ -356,6 +386,12 @@ static void dissect_pgsql_fe_msg(guchar type, guint length, tvbuff_t *tvb,
             conv_data->ssl_requested = TRUE;
             break;
 
+        /* GSS request */
+        case 80877104:
+            /* Next reply will be a single byte. */
+            conv_data->gss_requested = TRUE;
+            break;
+
         /* Cancellation request */
         case 80877102:
             proto_tree_add_item(tree, hf_pid, tvb, n,   4, ENC_BIG_ENDIAN);
@@ -430,9 +466,17 @@ static void dissect_pgsql_be_msg(guchar type, guint length, tvbuff_t *tvb,
             siz = (auth_type == PGSQL_AUTH_TYPE_CRYPT ? 2 : 4);
             proto_tree_add_item(tree, hf_salt, tvb, n, siz, ENC_NA);
             break;
+        case PGSQL_AUTH_TYPE_GSSAPI:
+            conv_data->auth_state = PGSQL_AUTH_GSSAPI;
+            break;
+        case PGSQL_AUTH_TYPE_SSPI:
+            conv_data->auth_state = PGSQL_AUTH_SSPI;
+            break;
         case PGSQL_AUTH_TYPE_GSSAPI_SSPI_CONTINUE:
-            conv_data->auth_state = PGSQL_AUTH_GSSAPI_SSPI_DATA;
+            n += 4;
             proto_tree_add_item(tree, hf_gssapi_sspi_data, tvb, n, length-8, ENC_NA);
+            if (conv_data->auth_state == PGSQL_AUTH_GSSAPI)
+                dissect_gssapi_data(tvb, n, length-8, pinfo, tree, NULL);
             break;
         case PGSQL_AUTH_TYPE_SASL:
             conv_data->auth_state = PGSQL_AUTH_SASL_REQUESTED;
@@ -665,6 +709,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
     if (!conn_data) {
         conn_data = wmem_new(wmem_file_scope(), pgsql_conn_data_t);
         conn_data->ssl_requested = FALSE;
+        conn_data->gss_requested = FALSE;
+        conn_data->last_nongss_frame = G_MAXUINT32;
         conn_data->auth_state = PGSQL_AUTH_STATE_NONE;
         conversation_add_proto_data(conversation, proto_pgsql, conn_data);
     }
@@ -703,7 +749,8 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
                 case PGSQL_AUTH_SASL_CONTINUE:
                     typestr = "SASLResponse message";
                     break;
-                case PGSQL_AUTH_GSSAPI_SSPI_DATA:
+                case PGSQL_AUTH_GSSAPI: /* fallthrough */
+                case PGSQL_AUTH_SSPI:
                     typestr = "GSSResponse message";
                     break;
                 default:
@@ -746,6 +793,18 @@ dissect_pgsql_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* dat
     return tvb_captured_length(tvb);
 }
 
+static void
+dissect_gssenc_msg(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, gssapi_encrypt_info_t *encrypt)
+{
+    gint offset = 0, len;
+
+    proto_tree_add_item(tree, hf_gssapi_length, tvb, offset, 4, ENC_BIG_ENDIAN);
+    offset += 4;
+    len = tvb_reported_length_remaining(tvb, offset);
+
+    dissect_gssapi_data(tvb, offset, len, pinfo, tree, encrypt);
+}
+
 /* This function is called once per TCP packet. It sets COL_PROTOCOL and
  * identifies FE/BE messages by adding a ">" or "<" to COL_INFO. Then it
  * arranges for each message to be dissected individually. */
@@ -782,6 +841,43 @@ dissect_pgsql(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data)
         return tvb_captured_length(tvb);
     }
 
+    if (conn_data && conn_data->gss_requested) {
+        /* Response to GSS request. */
+        switch (tvb_get_guint8(tvb, 0)) {
+        case 'G':   /* Willing to perform GSS encryption */
+            /* Next packet will start using GSS encryption. */
+            conn_data->last_nongss_frame = pinfo->num;
+            break;
+        case 'N':   /* Unwilling to perform GSS encryption */
+        default:    /* ErrorMessage when server does not support SSL. */
+            /* TODO: maybe add expert info here? */
+            break;
+        }
+        conn_data->gss_requested = FALSE;
+        return tvb_captured_length(tvb);
+    }
+
+    if (conn_data && (pinfo->num > conn_data->last_nongss_frame)) {
+        tvbuff_t *subtvb;
+        gssapi_encrypt_info_t  encrypt;
+
+        memset(&encrypt, 0, sizeof(encrypt));
+        encrypt.decrypt_gssapi_tvb = DECRYPT_GSSAPI_NORMAL;
+
+        dissect_gssenc_msg(tvb, pinfo, tree, &encrypt);
+
+        if (encrypt.gssapi_decrypted_tvb){
+                subtvb = encrypt.gssapi_decrypted_tvb;
+                tcp_dissect_pdus(subtvb, pinfo, tree, pgsql_desegment, 5,
+                                 pgsql_length, dissect_pgsql_msg, data);
+        } else if (encrypt.gssapi_encrypted_tvb) {
+                subtvb = encrypt.gssapi_encrypted_tvb;
+                proto_tree_add_expert(tree, pinfo, &ei_gssapi_decryption_not_possible, subtvb, 0, -1);
+        }
+
+        return tvb_captured_length(tvb);
+    }
+
     tcp_dissect_pdus(tvb, pinfo, tree, pgsql_desegment, 5,
                      pgsql_length, dissect_pgsql_msg, data);
     return tvb_captured_length(tvb);
@@ -1021,7 +1117,12 @@ proto_register_pgsql(void)
         { &hf_routine,
           { "Routine", "pgsql.routine", FT_STRINGZ, BASE_NONE, NULL, 0,
             "The routine that reported an error.", HFILL }
-        }
+        },
+        { &hf_gssapi_length,
+          { "Length of GSSAPI encrypted data", "pgsql.gss.length", FT_UINT32, BASE_DEC, NULL, 0,
+            "The length of the GSSAPI encrypted blob.",
+            HFILL }
+        },
     };
 
     static gint *ett[] = {
@@ -1029,9 +1130,18 @@ proto_register_pgsql(void)
         &ett_values
     };
 
+    expert_module_t* expert_pgsql;
+
+    static ei_register_info ei[] = {
+        { &ei_gssapi_decryption_not_possible, { "pgsql.gss.decryption_not_possible", PI_UNDECODED, PI_WARN, "The PGSQL dissector could not decrypt the message.", EXPFILL }},
+    };
+
     proto_pgsql = proto_register_protocol("PostgreSQL", "PGSQL", "pgsql");
     proto_register_field_array(proto_pgsql, hf, array_length(hf));
     proto_register_subtree_array(ett, array_length(ett));
+
+    expert_pgsql = expert_register_protocol(proto_pgsql);
+    expert_register_field_array(expert_pgsql, ei, array_length(ei));
 }
 
 void
@@ -1042,6 +1152,7 @@ proto_reg_handoff_pgsql(void)
     dissector_add_uint_with_preference("tcp.port", PGSQL_PORT, pgsql_handle);
 
     tls_handle = find_dissector_add_dependency("tls", proto_pgsql);
+    gssapi_handle = find_dissector_add_dependency("gssapi", proto_pgsql);
 }
 
 /*
-- 
2.25.1

---

From: Stephen Frost <[email-masked]>
Date: Mon, 19 Sep 2022 13:05:17 -0400

Greetings,

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

> delegated key material. (This also means that gssencmode is a lot

The backend wouldn't be able to establish the connection in the first
place without those delegated credentials.

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

---

From: Jacob Champion <[email-masked]>
Date: Mon, 19 Sep 2022 14:05:39 -0700

On 9/19/22 10:05, Stephen Frost wrote:
> 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
> 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

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:

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

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

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

> place without those delegated credentials.

That's not true in the situation I'm describing; hopefully my comments
above help clarify.

> 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

---

From: Michael Paquier <[email-masked]>
Date: Wed, 12 Oct 2022 14:32:31 +0900

> 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