De-Revert "Add support for Kerberos credential delegation"

This reverts commit 3d03b24c3 (Revert Add support for Kerberos
credential delegation) which was committed on the grounds of concern
about portability, but on further review and discussion, it's clear that
we are better off explicitly requiring MIT Kerberos as that appears to
be the only GSSAPI library currently that's under proper maintenance
and ongoing development.  The API used for storing credentials was added
to MIT Kerberos over a decade ago while for the other libraries which
appear to be mainly based on Heimdal, which exists explicitly to be a
re-implementation of MIT Kerberos, the API never made it to a released
version (even though it was added to the Heimdal git repo over 5 years
ago..).

This post-feature-freeze change was approved by the RMT.

Discussion: https://postgr.es/m/ZDDO6jaESKaBgej0%40tamriel.snowman.net

4 files changed, 74 insertions, 26 deletions

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 2969351e9a9..75d93d6eadf 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -17,6 +17,7 @@
 #include "catalog/pg_user_mapping.h"
 #include "commands/defrem.h"
 #include "funcapi.h"
+#include "libpq/libpq-be.h"
 #include "libpq/libpq-be-fe-helpers.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -149,6 +150,8 @@ static void pgfdw_finish_pre_subcommit_cleanup(List *pending_entries,
 static void pgfdw_finish_abort_cleanup(List *pending_entries,
 									   List *cancel_requested,
 									   bool toplevel);
+static void pgfdw_security_check(const char **keywords, const char **values,
+								 UserMapping *user, PGconn *conn);
 static bool UserMappingPasswordRequired(UserMapping *user);
 static bool disconnect_cached_connections(Oid serverid);
 
@@ -385,6 +388,47 @@ make_new_connection(ConnCacheEntry *entry, UserMapping *user)
 }
 
 /*
+ * Check that non-superuser has used password or delegated credentials
+ * to establish connection; otherwise, he's piggybacking on the
+ * postgres server's user identity. See also dblink_security_check()
+ * in contrib/dblink and check_conn_params.
+ */
+static void
+pgfdw_security_check(const char **keywords, const char **values, UserMapping *user, PGconn *conn)
+{
+	/* Superusers bypass the check */
+	if (superuser_arg(user->userid))
+		return;
+
+#ifdef ENABLE_GSS
+	/* Connected via GSSAPI with delegated credentials- all good. */
+	if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_deleg(MyProcPort))
+		return;
+#endif
+
+	/* Ok if superuser set PW required false. */
+	if (!UserMappingPasswordRequired(user))
+		return;
+
+	/* Connected via PW, with PW required true, and provided non-empty PW. */
+	if (PQconnectionUsedPassword(conn))
+	{
+		/* ok if params contain a non-empty password */
+		for (int i = 0; keywords[i] != NULL; i++)
+		{
+			if (strcmp(keywords[i], "password") == 0 && values[i][0] != '\0')
+				return;
+		}
+	}
+
+	ereport(ERROR,
+			(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+			 errmsg("password or GSSAPI delegated credentials required"),
+			 errdetail("Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials."),
+			 errhint("Target server's authentication method must be changed or password_required=false set in the user mapping attributes.")));
+}
+
+/*
  * Connect to remote server using specified server and user mapping properties.
  */
 static PGconn *
@@ -495,19 +539,8 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
 							server->servername),
 					 errdetail_internal("%s", pchomp(PQerrorMessage(conn)))));
 
-		/*
-		 * Check that non-superuser has used password to establish connection;
-		 * otherwise, he's piggybacking on the postgres server's user
-		 * identity. See also dblink_security_check() in contrib/dblink and
-		 * check_conn_params.
-		 */
-		if (!superuser_arg(user->userid) && UserMappingPasswordRequired(user) &&
-			!PQconnectionUsedPassword(conn))
-			ereport(ERROR,
-					(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-					 errmsg("password is required"),
-					 errdetail("Non-superuser cannot connect if the server does not request a password."),
-					 errhint("Target server's authentication method must be changed or password_required=false set in the user mapping attributes.")));
+		/* Perform post-connection security checks */
+		pgfdw_security_check(keywords, values, user, conn);
 
 		/* Prepare new session for use */
 		configure_remote_session(conn);
@@ -561,7 +594,8 @@ UserMappingPasswordRequired(UserMapping *user)
 }
 
 /*
- * For non-superusers, insist that the connstr specify a password.  This
+ * For non-superusers, insist that the connstr specify a password or that the
+ * user provided their own GSSAPI delegated credentials.  This
  * prevents a password from being picked up from .pgpass, a service file, the
  * environment, etc.  We don't want the postgres user's passwords,
  * certificates, etc to be accessible to non-superusers.  (See also
@@ -576,6 +610,12 @@ check_conn_params(const char **keywords, const char **values, UserMapping *user)
 	if (superuser_arg(user->userid))
 		return;
 
+#ifdef ENABLE_GSS
+	/* ok if the user provided their own delegated credentials */
+	if (be_gssapi_get_deleg(MyProcPort))
+		return;
+#endif
+
 	/* ok if params contain a non-empty password */
 	for (i = 0; keywords[i] != NULL; i++)
 	{
@@ -589,8 +629,8 @@ check_conn_params(const char **keywords, const char **values, UserMapping *user)
 
 	ereport(ERROR,
 			(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-			 errmsg("password is required"),
-			 errdetail("Non-superusers must provide a password in the user mapping.")));
+			 errmsg("password or GSSAPI delegated credentials required"),
+			 errdetail("Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping.")));
 }
 
 /*
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 8f6a04f71b6..fd5752bd5bf 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -171,7 +171,8 @@ ALTER SERVER testserver1 OPTIONS (
 	sslcrl 'value',
 	--requirepeer 'value',
 	krbsrvname 'value',
-	gsslib 'value'
+	gsslib 'value',
+	gssdeleg 'value'
 	--replication 'value'
 );
 -- Error, invalid list syntax
@@ -9840,8 +9841,8 @@ CREATE FOREIGN TABLE pg_temp.ft1_nopw (
 	c8 user_enum
 ) SERVER loopback_nopw OPTIONS (schema_name 'public', table_name 'ft1');
 SELECT 1 FROM ft1_nopw LIMIT 1;
-ERROR:  password is required
-DETAIL:  Non-superusers must provide a password in the user mapping.
+ERROR:  password or GSSAPI delegated credentials required
+DETAIL:  Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping.
 -- If we add a password to the connstr it'll fail, because we don't allow passwords
 -- in connstrs only in user mappings.
 ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
@@ -9853,16 +9854,16 @@ HINT:  Perhaps you meant the option "passfile".
 -- This won't work with installcheck, but neither will most of the FDW checks.
 ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
 SELECT 1 FROM ft1_nopw LIMIT 1;
-ERROR:  password is required
-DETAIL:  Non-superuser cannot connect if the server does not request a password.
+ERROR:  password or GSSAPI delegated credentials required
+DETAIL:  Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials.
 HINT:  Target server's authentication method must be changed or password_required=false set in the user mapping attributes.
 -- Unpriv user cannot make the mapping passwordless
 ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password_required 'false');
 ERROR:  password_required=false is superuser-only
 HINT:  User mappings with the password_required option set to false may only be created or modified by the superuser.
 SELECT 1 FROM ft1_nopw LIMIT 1;
-ERROR:  password is required
-DETAIL:  Non-superuser cannot connect if the server does not request a password.
+ERROR:  password or GSSAPI delegated credentials required
+DETAIL:  Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials.
 HINT:  Target server's authentication method must be changed or password_required=false set in the user mapping attributes.
 RESET ROLE;
 -- But the superuser can
@@ -9890,8 +9891,8 @@ DROP USER MAPPING FOR CURRENT_USER SERVER loopback_nopw;
 -- This will fail again as it'll resolve the user mapping for public, which
 -- lacks password_required=false
 SELECT 1 FROM ft1_nopw LIMIT 1;
-ERROR:  password is required
-DETAIL:  Non-superusers must provide a password in the user mapping.
+ERROR:  password or GSSAPI delegated credentials required
+DETAIL:  Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping.
 RESET ROLE;
 -- The user mapping for public is passwordless and lacks the password_required=false
 -- mapping option, but will work because the current user is a superuser.
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 4229d2048c3..fe40d50c6dd 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -288,6 +288,12 @@ InitPgFdwOptions(void)
 		{"sslcert", UserMappingRelationId, true},
 		{"sslkey", UserMappingRelationId, true},
 
+		/*
+		 * gssdeleg is also a libpq option but should be allowed in a user
+		 * mapping context too
+		 */
+		{"gssdeleg", UserMappingRelationId, true},
+
 		{NULL, InvalidOid, false}
 	};
 
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 5bd69339dfc..c05046f8676 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -185,7 +185,8 @@ ALTER SERVER testserver1 OPTIONS (
 	sslcrl 'value',
 	--requirepeer 'value',
 	krbsrvname 'value',
-	gsslib 'value'
+	gsslib 'value',
+	gssdeleg 'value'
 	--replication 'value'
 );