De-Revert "Add support for Kerberos credential delegation"

This reverts commit 3d03b24 (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

Author: sfrost

contrib/dblink/dblink.c

@@ -48,6 +48,7 @@
 #include "funcapi.h"
 #include "lib/stringinfo.h"
 #include "libpq-fe.h"
+#include "libpq/libpq-be.h"
 #include "libpq/libpq-be-fe-helpers.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -111,7 +112,8 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat
 static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode);
 static char *generate_relation_name(Relation rel);
 static void dblink_connstr_check(const char *connstr);
-static void dblink_security_check(PGconn *conn, remoteConn *rconn);
+static bool dblink_connstr_has_pw(const char *connstr);
+static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
 static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
 							 bool fail, const char *fmt,...) pg_attribute_printf(5, 6);
 static char *get_connect_string(const char *servername);
@@ -213,7 +215,7 @@ dblink_get_conn(char *conname_or_str,
 					 errmsg("could not establish connection"),
 					 errdetail_internal("%s", msg)));
 		}
-		dblink_security_check(conn, rconn);
+		dblink_security_check(conn, rconn, connstr);
 		if (PQclientEncoding(conn) != GetDatabaseEncoding())
 			PQsetClientEncoding(conn, GetDatabaseEncodingName());
 		freeconn = true;
@@ -307,7 +309,7 @@ dblink_connect(PG_FUNCTION_ARGS)
 	}
 
 	/* check password actually used if not superuser */
-	dblink_security_check(conn, rconn);
+	dblink_security_check(conn, rconn, connstr);
 
 	/* attempt to set client encoding to match server encoding, if needed */
 	if (PQclientEncoding(conn) != GetDatabaseEncoding())
@@ -2584,64 +2586,99 @@ deleteConnection(const char *name)
 				 errmsg("undefined connection name")));
 }
 
+/*
+ * We need to make sure that the connection made used credentials
+ * which were provided by the user, so check what credentials were
+ * used to connect and then make sure that they came from the user.
+ */
 static void
-dblink_security_check(PGconn *conn, remoteConn *rconn)
+dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
 {
-	if (!superuser())
-	{
-		if (!PQconnectionUsedPassword(conn))
-		{
-			libpqsrv_disconnect(conn);
-			if (rconn)
-				pfree(rconn);
+	/* Superuser bypasses security check */
+	if (superuser())
+		return;
 
-			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.")));
-		}
-	}
+	/* If password was used to connect, make sure it was one provided */
+	if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr))
+		return;
+
+#ifdef ENABLE_GSS
+	/* If GSSAPI creds used to connect, make sure it was one delegated */
+	if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_deleg(MyProcPort))
+		return;
+#endif
+
+	/* Otherwise, fail out */
+	libpqsrv_disconnect(conn);
+	if (rconn)
+		pfree(rconn);
+
+	ereport(ERROR,
+			(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+			 errmsg("password or GSSAPI delegated credentials required"),
+			 errdetail("Non-superusers may only connect using credentials they provide, eg: password in connection string or delegated GSSAPI credentials"),
+			 errhint("Ensure provided credentials match target server's authentication method.")));
 }
 
 /*
- * For non-superusers, insist that the connstr specify a password.  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
- * to be accessible to non-superusers.
+ * Function to check if the connection string includes an explicit
+ * password, needed to ensure that non-superuser password-based auth
+ * is using a provided password and not one picked up from the
+ * environment.
  */
-static void
-dblink_connstr_check(const char *connstr)
+static bool
+dblink_connstr_has_pw(const char *connstr)
 {
-	if (!superuser())
-	{
-		PQconninfoOption *options;
-		PQconninfoOption *option;
-		bool		connstr_gives_password = false;
+	PQconninfoOption *options;
+	PQconninfoOption *option;
+	bool		connstr_gives_password = false;
 
-		options = PQconninfoParse(connstr, NULL);
-		if (options)
+	options = PQconninfoParse(connstr, NULL);
+	if (options)
+	{
+		for (option = options; option->keyword != NULL; option++)
 		{
-			for (option = options; option->keyword != NULL; option++)
+			if (strcmp(option->keyword, "password") == 0)
 			{
-				if (strcmp(option->keyword, "password") == 0)
+				if (option->val != NULL && option->val[0] != '\0')
 				{
-					if (option->val != NULL && option->val[0] != '\0')
-					{
-						connstr_gives_password = true;
-						break;
-					}
+					connstr_gives_password = true;
+					break;
 				}
 			}
-			PQconninfoFree(options);
 		}
-
-		if (!connstr_gives_password)
-			ereport(ERROR,
-					(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
-					 errmsg("password is required"),
-					 errdetail("Non-superusers must provide a password in the connection string.")));
+		PQconninfoFree(options);
 	}
+
+	return connstr_gives_password;
+}
+
+/*
+ * For non-superusers, insist that the connstr specify a password, except
+ * if GSSAPI credentials have been delegated (and we check that they are used
+ * for the connection in dblink_security_check later).  This prevents a
+ * password or GSSAPI credentials from being picked up from .pgpass, a
+ * service file, the environment, etc.  We don't want the postgres user's
+ * passwords or Kerberos credentials to be accessible to non-superusers.
+ */
+static void
+dblink_connstr_check(const char *connstr)
+{
+	if (superuser())
+		return;
+
+	if (dblink_connstr_has_pw(connstr))
+		return;
+
+#ifdef ENABLE_GSS
+	if (be_gssapi_get_deleg(MyProcPort))
+		return;
+#endif
+
+	ereport(ERROR,
+			(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
+			 errmsg("password or GSSAPI delegated credentials required"),
+			 errdetail("Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.")));
 }
 
 /*

contrib/dblink/expected/dblink.out

@@ -903,8 +903,8 @@ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO regress_dblink_user;
 SET SESSION AUTHORIZATION regress_dblink_user;
 -- should fail
 SELECT dblink_connect('myconn', 'fdtest');
-ERROR:  password is required
-DETAIL:  Non-superusers must provide a password in the connection string.
+ERROR:  password or GSSAPI delegated credentials required
+DETAIL:  Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials.
 -- should succeed
 SELECT dblink_connect_u('myconn', 'fdtest');
  dblink_connect_u 

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);
 
@@ -384,6 +387,47 @@ make_new_connection(ConnCacheEntry *entry, UserMapping *user)
 		 entry->conn, server->servername, user->umid, user->userid);
 }
 
+/*
+ * 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.
  */
@@ -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.")));
 }
 
 /*

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.

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}
 	};
 

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'
 );