v1-0002-dblink-Add-SCRAM-pass-through-authentication.patch

From 51fb5cfb2bc07c70d558b16ded33c43ebecb6575 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Mon, 20 Jan 2025 15:34:10 -0300
Subject: [PATCH v1 2/2] dblink: Add SCRAM pass-through authentication

This enables SCRAM authentication for dblink_fdw when connecting to a
foreign server using the same approach as it was implemented for
postgres_fdw on 761c79508e.
---
 contrib/dblink/Makefile            |   1 +
 contrib/dblink/dblink.c            | 111 ++++++++++++++++++++++-
 contrib/dblink/meson.build         |   5 ++
 contrib/dblink/t/001_auth_scram.pl | 137 +++++++++++++++++++++++++++++
 doc/src/sgml/dblink.sgml           |  81 ++++++++++++++++-
 5 files changed, 330 insertions(+), 5 deletions(-)
 create mode 100644 contrib/dblink/t/001_auth_scram.pl

diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile
index d4c7ed625a..fde0b49ddb 100644
--- a/contrib/dblink/Makefile
+++ b/contrib/dblink/Makefile
@@ -13,6 +13,7 @@ PGFILEDESC = "dblink - connect to other PostgreSQL databases"
 
 REGRESS = dblink
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 4af8cbd1d0..68d9dbcc6f 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -43,6 +43,8 @@
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
+#include "commands/defrem.h"
+#include "common/base64.h"
 #include "executor/spi.h"
 #include "foreign/foreign.h"
 #include "funcapi.h"
@@ -127,6 +129,16 @@ static bool is_valid_dblink_option(const PQconninfoOption *options,
 static int	applyRemoteGucs(PGconn *conn);
 static void restoreLocalGucs(int nestlevel);
 
+static bool
+UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user);
+
+static void
+appendSCRAMKeysInfo(StringInfo buf);
+
+static bool
+is_valid_dblink_fdw_option(const PQconninfoOption *options, const char *option,
+						   Oid context);
+
 static PGconn *
 connect_pg_server(char *conname_or_str, remoteConn *rconn);
 
@@ -1907,7 +1919,7 @@ dblink_fdw_validator(PG_FUNCTION_ARGS)
 	{
 		DefElem    *def = (DefElem *) lfirst(cell);
 
-		if (!is_valid_dblink_option(options, def->defname, context))
+		if (!is_valid_dblink_fdw_option(options, def->defname, context))
 		{
 			/*
 			 * Unknown option, or invalid option for the context specified, so
@@ -2765,6 +2777,9 @@ get_connect_string(ForeignServer *foreign_server, UserMapping *user_mapping)
 		if (aclresult != ACLCHECK_OK)
 			aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, foreign_server->servername);
 
+		if (MyProcPort->has_scram_keys && UseScramPassthrough(foreign_server, user_mapping))
+				appendSCRAMKeysInfo(&buf);
+
 		foreach(cell, fdw->options)
 		{
 			DefElem    *def = lfirst(cell);
@@ -2946,6 +2961,21 @@ is_valid_dblink_option(const PQconninfoOption *options, const char *option,
 	return true;
 }
 
+
+/*
+ * Same as is_valid_dblink_option but also check for only dblink_fdw specific
+ * options.
+ */
+static bool
+is_valid_dblink_fdw_option(const PQconninfoOption *options, const char *option,
+					   Oid context)
+{
+	if (strcmp(option, "use_scram_passthrough") == 0)
+		return true;
+
+	return is_valid_dblink_option(options, option, context);
+}
+
 /*
  * Copy the remote session's values of GUCs that affect datatype I/O
  * and apply them locally in a new GUC nesting level.  Returns the new
@@ -3016,6 +3046,69 @@ restoreLocalGucs(int nestlevel)
 		AtEOXact_GUC(true, nestlevel);
 }
 
+/*
+ * Append SCRAM client key and server key information from the global
+ * MyProcPort into the given StringInfo buffer.
+ */
+static void
+appendSCRAMKeysInfo(StringInfo buf)
+{
+	int			len;
+	int			encoded_len;
+	char        *client_key;
+	char        *server_key;
+
+
+	len = pg_b64_enc_len(sizeof(MyProcPort->scram_ClientKey));
+	/* don't forget the zero-terminator */
+	client_key = palloc0(len + 1);
+	encoded_len = pg_b64_encode((const char *) MyProcPort->scram_ClientKey,
+								sizeof(MyProcPort->scram_ClientKey),
+								client_key, len);
+	if (encoded_len < 0)
+		elog(ERROR, "could not encode SCRAM client key");
+
+	len = pg_b64_enc_len(sizeof(MyProcPort->scram_ServerKey));
+	/* don't forget the zero-terminator */
+	server_key = palloc0(len + 1);
+	encoded_len = pg_b64_encode((const char *) MyProcPort->scram_ServerKey,
+								sizeof(MyProcPort->scram_ServerKey),
+								server_key, len);
+	if (encoded_len < 0)
+		elog(ERROR, "could not encode SCRAM server key");
+
+	appendStringInfo(buf, "scram_client_key='%s'", client_key);
+	appendStringInfo(buf, "scram_server_key='%s'", server_key);
+
+	pfree(client_key);
+	pfree(server_key);
+}
+
+
+static bool
+UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user)
+{
+	ListCell      *cell;
+
+	foreach(cell, foreign_server->options)
+	{
+		DefElem    *def = lfirst(cell);
+
+		if (strcmp(def->defname, "use_scram_passthrough") == 0)
+			return defGetBoolean(def);
+	}
+
+	foreach(cell, user->options)
+	{
+		DefElem    *def = (DefElem *) lfirst(cell);
+
+		if (strcmp(def->defname, "use_scram_passthrough") == 0)
+			return defGetBoolean(def);
+	}
+
+	return false;
+}
+
 /*
  * Connect to remote server. If connstr_or_srvname maps to a foreign server,
  * the associated properties and user mapping properties is also used to open
@@ -3031,6 +3124,7 @@ connect_pg_server(char *connstr_or_srvname, remoteConn *rconn)
 	char	      *srvname;
 	Oid			   serverid;
 	UserMapping   *user_mapping;
+	bool           useScramPassthrough = false;
 	Oid			   userid = GetUserId();
 
 	static const PQconninfoOption *options = NULL;
@@ -3061,13 +3155,19 @@ connect_pg_server(char *connstr_or_srvname, remoteConn *rconn)
 	{
 		serverid            = foreign_server->serverid;
 		user_mapping        = GetUserMapping(userid, serverid);
+		useScramPassthrough = MyProcPort->has_scram_keys && UseScramPassthrough(foreign_server, user_mapping);
 
 		connstr = get_connect_string(foreign_server, user_mapping);
 	}
 	else
 		connstr = connstr_or_srvname;
 
-	dblink_connstr_check(connstr);
+	/*
+	 * Verify the set of connection parameters only if scram pass-through
+	 * is not being used because the password is not necessary.
+	 */
+	if (!useScramPassthrough)
+		dblink_connstr_check(connstr);
 
 	/* first time, allocate or get the custom wait event */
 	if (dblink_we_connect == 0)
@@ -3087,7 +3187,12 @@ connect_pg_server(char *connstr_or_srvname, remoteConn *rconn)
 				 errdetail_internal("%s", msg)));
 	}
 
-	dblink_security_check(conn, rconn, connstr);
+	/*
+	 * Perform post-connection security checks only if scram pass-through
+	 * is not being used because the password is not necessary.
+	 */
+	if (!useScramPassthrough)
+		dblink_security_check(conn, rconn, connstr);
 
 	/* attempt to set client encoding to match server encoding, if needed */
 	if (PQclientEncoding(conn) != GetDatabaseEncoding())
diff --git a/contrib/dblink/meson.build b/contrib/dblink/meson.build
index 3ab7866828..dfd8eb6877 100644
--- a/contrib/dblink/meson.build
+++ b/contrib/dblink/meson.build
@@ -36,4 +36,9 @@ tests += {
     ],
     'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
   },
+  'tap': {
+    'tests': [
+      't/001_auth_scram.pl',
+    ],
+  },
 }
diff --git a/contrib/dblink/t/001_auth_scram.pl b/contrib/dblink/t/001_auth_scram.pl
new file mode 100644
index 0000000000..00fd6d8583
--- /dev/null
+++ b/contrib/dblink/t/001_auth_scram.pl
@@ -0,0 +1,137 @@
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+# Test SCRAM authentication when opening a new connection with a foreign
+# server.
+#
+# The test is executed by testing the SCRAM authentifcation on a looplback
+# connection on the same server and with different servers.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+my $hostaddr = '127.0.0.1';
+my $user = "user01";
+
+my $db0 = "db0";    # For node1
+my $db1 = "db1";    # For node1
+my $db2 = "db2";    # For node2
+my $fdw_server = "db1_fdw";
+my $fdw_server2 = "db2_fdw";
+
+my $node1 = PostgreSQL::Test::Cluster->new('node1');
+my $node2 = PostgreSQL::Test::Cluster->new('node2');
+
+$node1->init;
+$node2->init;
+
+$node1->start;
+$node2->start;
+
+# Test setup
+
+$node1->safe_psql('postgres', qq'CREATE USER $user WITH password \'pass\'');
+$node2->safe_psql('postgres', qq'CREATE USER $user WITH password \'pass\'');
+$ENV{PGPASSWORD} = "pass";
+
+$node1->safe_psql('postgres', qq'CREATE DATABASE $db0');
+$node1->safe_psql('postgres', qq'CREATE DATABASE $db1');
+$node2->safe_psql('postgres', qq'CREATE DATABASE $db2');
+
+setup_table($node1, $db1, "t");
+setup_table($node2, $db2, "t2");
+
+$node1->safe_psql($db0, 'CREATE EXTENSION IF NOT EXISTS dblink');
+setup_fdw_server($node1, $db0, $fdw_server, $node1, $db1);
+setup_fdw_server($node1, $db0, $fdw_server2, $node2, $db2);
+
+setup_user_mapping($node1, $db0, $fdw_server);
+setup_user_mapping($node1, $db0, $fdw_server2);
+
+# Make the user have the same SCRAM key on both servers. Forcing to have the
+# same iteration and salt.
+my $rolpassword = $node1->safe_psql('postgres',
+	qq"SELECT rolpassword FROM pg_authid WHERE rolname = '$user';");
+$node2->safe_psql('postgres', qq"ALTER ROLE $user PASSWORD '$rolpassword'");
+
+setup_pghba($node1);
+setup_pghba($node2);
+
+# End of test setup
+
+test_fdw_auth($node1, $db0, "t", $fdw_server,
+	"SCRAM auth on the same database cluster must succeed");
+
+test_fdw_auth($node1, $db0, "t2", $fdw_server2,
+	"SCRAM auth on a different database cluster must succeed");
+
+# Helper functions
+
+sub test_fdw_auth
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+	my ($node, $db, $tbl, $fdw, $testname) = @_;
+	my $connstr = $node->connstr($db) . qq' user=$user';
+
+	my $ret = $node->safe_psql(
+		$db,
+		qq"SELECT count(1) FROM dblink('$fdw', 'SELECT * FROM $tbl') AS $tbl(a int, b int)",
+		connstr => $connstr);
+
+	is($ret, '10', $testname);
+}
+
+sub setup_pghba
+{
+	my ($node) = @_;
+
+	unlink($node->data_dir . '/pg_hba.conf');
+	$node->append_conf(
+		'pg_hba.conf', qq{
+	local   all             all                                     scram-sha-256
+	host    all             all             $hostaddr/32            scram-sha-256
+	host    all             all             ::1/128                 scram-sha-256
+	});
+
+	$node->restart;
+}
+
+sub setup_user_mapping
+{
+	my ($node, $db, $fdw) = @_;
+
+	$node->safe_psql($db,
+		qq'CREATE USER MAPPING FOR $user SERVER $fdw OPTIONS (user \'$user\');'
+	);
+	$node->safe_psql($db, qq'GRANT USAGE ON FOREIGN SERVER $fdw TO $user;');
+	$node->safe_psql($db, qq'GRANT ALL ON SCHEMA public TO $user');
+}
+
+sub setup_fdw_server
+{
+	my ($node, $db, $fdw, $fdw_node, $dbname) = @_;
+	my $host = $fdw_node->host;
+	my $port = $fdw_node->port;
+
+	$node->safe_psql(
+		$db, qq'CREATE SERVER $fdw FOREIGN DATA WRAPPER dblink_fdw options (
+		host \'$host\', port \'$port\', dbname \'$dbname\', use_scram_passthrough \'true\') '
+	);
+}
+
+sub setup_table
+{
+	my ($node, $db, $tbl) = @_;
+
+	$node->safe_psql($db,
+		qq'CREATE TABLE $tbl AS SELECT g as a, g + 1 as b FROM generate_series(1,10) g(g)'
+	);
+	$node->safe_psql($db, qq'GRANT USAGE ON SCHEMA public TO $user');
+	$node->safe_psql($db, qq'GRANT SELECT ON $tbl TO $user');
+}
+
+done_testing();
+
diff --git a/doc/src/sgml/dblink.sgml b/doc/src/sgml/dblink.sgml
index 81f35986c8..f6e1009c02 100644
--- a/doc/src/sgml/dblink.sgml
+++ b/doc/src/sgml/dblink.sgml
@@ -136,6 +136,82 @@ dblink_connect(text connname, text connstr) returns text
    </para>
   </refsect1>
 
+<refsect1>
+   <title>Foreign Data Wrapper</title>
+
+   <para>
+    A Foreign Data Wrapper can be used as a connection name parameter. The foreign
+    server can be created using CREATE SERVER and CREATE USER MAPPING commands. 
+   </para>
+
+   <para>
+    The authentication with the foreign server can be via password on USER
+    MAPPING or using SCRAM pass-through. The
+    <literal>use_scram_passthrough</literal> on CREATE SERVER options controls
+    whether <filename>dblink_fdw</filename> will use the SCRAM pass-through
+    authentication to connect to the foreign server. With SCRAM pass-through
+    authentication, <filename>dblink_fdw</filename> uses SCRAM-hashed secrets
+    instead of plain-text user passwords to connect to the remote server. This
+    avoids storing plain-text user passwords in PostgreSQL system catalogs.
+   </para>
+
+   <para>
+    To use SCRAM pass-through authentication:
+    <itemizedlist>
+     <listitem>
+      <para>
+       The remote server must request SCRAM authentication.  (If desired,
+       enforce this on the client side (FDW side) with the option
+       <literal>require_auth</literal>.)  If another authentication method is
+       requested by the server, then that one will be used normally.
+      </para>
+    </listitem>
+
+    <listitem>
+     <para>
+      The remote server can be of any PostgreSQL version that supports SCRAM.
+      Support for <literal>use_scram_passthrough</literal> is only required on
+      the client side (FDW side).
+     </para>
+    </listitem>
+
+    <listitem>
+     <para>
+      The user mapping password is not used.  (It could be set to support other
+      authentication methods, but that would arguably violate the point of this
+      feature, which is to avoid storing plain-text passwords.)
+     </para>
+    </listitem>
+
+    <listitem>
+     <para>
+     The server running <filename>dblink_fdw</filename> and the remote server
+     must have identical SCRAM secrets (encrypted passwords) for the user being
+     used on <filename>dblink_fdw</filename> to authenticate on the foreign
+     server (same salt and iterations, not merely the same password).
+     </para>
+
+     <para>
+      As a corollary, if FDW connections to multiple hosts are to be made, for
+      example for partitioned foreign tables/sharding, then all hosts must have
+      identical SCRAM secrets for the users involved.
+     </para>
+    </listitem>
+
+    <listitem>
+     <para>
+      The current session on the PostgreSQL instance that makes the outgoing FDW
+      connections also must also use SCRAM authentication for its incoming client
+      connection.  (Hence <quote>pass-through</quote>: SCRAM must be used going in
+      and out.) This is a technical requirement of the SCRAM protocol.
+     </para>
+    </listitem>
+    </itemizedlist>
+   </para>
+
+
+  </refsect1>
+
   <refsect1>
    <title>Notes</title>
 
@@ -181,8 +257,9 @@ SELECT dblink_connect('myconn', 'dbname=postgres options=-csearch_path=');
 (1 row)
 
 -- FOREIGN DATA WRAPPER functionality
--- Note: local connection must require password authentication for this to work properly
---       Otherwise, you will receive the following error from dblink_connect():
+-- Note: local connection that don't use SCRAM pass-through require password 
+--       authentication for this to work properly. Otherwise, you will receive
+--       the following error from dblink_connect():
 --       ERROR:  password is required
 --       DETAIL:  Non-superuser cannot connect if the server does not request a password.
 --       HINT:  Target server's authentication method must be changed.
-- 
2.39.5 (Apple Git-154)