From 9596f9a90f557a6e5b75d10d295e1c2f0f234dec Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Thu, 20 Feb 2025 15:12:48 -0300 Subject: [PATCH v8 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 | 191 +++++++++++++++++++++- contrib/dblink/meson.build | 5 + contrib/dblink/t/001_auth_scram.pl | 249 +++++++++++++++++++++++++++++ doc/src/sgml/dblink.sgml | 81 +++++++++- 5 files changed, 518 insertions(+), 9 deletions(-) create mode 100644 contrib/dblink/t/001_auth_scram.pl diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile index d4c7ed625ab..fde0b49ddbb 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 4497b7a4999..50959c4f5bb 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,11 @@ static bool is_valid_dblink_option(const PQconninfoOption *options, static int applyRemoteGucs(PGconn *conn); static void restoreLocalGucs(int nestlevel); static PGconn *connect_pg_server(char *connstr_or_srvname, remoteConn *rconn, uint32 wait_event_info); +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 bool dblink_connstr_has_required_scram_options(const char *connstr); /* Global */ static remoteConn *pconn = NULL; @@ -1927,7 +1934,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 @@ -2559,6 +2566,68 @@ deleteConnection(const char *name) errmsg("undefined connection name"))); } + /* + * Ensure that require_auth and scram keys are correctly set on connstr. + * SCRAM keys used to pass-through is coming from the initial connection from + * the client with the server. + * + * All required scram options is set by ourself, so we just need to ensure + * that these options are not overwritten by the user. + * + * See appendSCRAMKeysInfo and it's usage for more. + */ +bool +dblink_connstr_has_required_scram_options(const char *connstr) +{ + PQconninfoOption *options; + PQconninfoOption *option; + bool has_scram_server_key = false; + bool has_scram_client_key = false; + bool has_require_auth = false; + bool has_scram_keys = false; + + options = PQconninfoParse(connstr, NULL); + if (options) + { + /* + * We continue iterating even if we found the keys that we need to + * validate to make sure that there is no other declaration of these + * keys that can overwrite the first. + */ + for (option = options; option->keyword != NULL; option++) + { + if (strcmp(option->keyword, "require_auth") == 0) + { + if (option->val != NULL && strcmp(option->val, "scram-sha-256") == 0) + has_require_auth = true; + else + has_require_auth = false; + } + + if (strcmp(option->keyword, "scram_client_key") == 0) + { + if (option->val != NULL && option->val[0] != '\0') + has_scram_client_key = true; + else + has_scram_client_key = false; + } + + if (strcmp(option->keyword, "scram_server_key") == 0) + { + if (option->val != NULL && option->val[0] != '\0') + has_scram_server_key = true; + else + has_scram_server_key = false; + } + } + PQconninfoFree(options); + } + + has_scram_keys = has_scram_client_key && has_scram_server_key && MyProcPort->has_scram_keys; + + return (has_scram_keys && has_require_auth); +} + /* * We need to make sure that the connection made used credentials * which were provided by the user, so check what credentials were @@ -2575,6 +2644,18 @@ dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr) if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr)) return; + /* + * Password was not used to connect, check if scram pass-through is in + * use. + * + * If dblink_connstr_has_required_scram_options is true we assume that + * UseScramPassthrough is also true because the required scram keys is + * only added if UseScramPassthrough is set, and the user is not allowed + * to add the scram keys on fdw and user mapping options. + */ + if (MyProcPort->has_scram_keys && dblink_connstr_has_required_scram_options(connstr)) + return; + #ifdef ENABLE_GSS /* If GSSAPI creds used to connect, make sure it was one delegated */ if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_delegation(MyProcPort)) @@ -2627,12 +2708,14 @@ dblink_connstr_has_pw(const char *connstr) } /* - * 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. + * 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) or if scram pass-through is + * being used. 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. In case of scram pass-through insist that the connstr + * has the required scram pass-through options. */ static void dblink_connstr_check(const char *connstr) @@ -2643,6 +2726,10 @@ dblink_connstr_check(const char *connstr) if (dblink_connstr_has_pw(connstr)) return; + if (MyProcPort->has_scram_keys && dblink_connstr_has_required_scram_options(connstr)) + return; + + #ifdef ENABLE_GSS if (be_gssapi_get_delegation(MyProcPort)) return; @@ -2787,6 +2874,14 @@ get_connect_string(ForeignServer *foreign_server) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, foreign_server->servername); + /* + * First append hardcoded options needed for SCRAM pass-through, so if the + * user overwrite these options we can ereport on dblink_connstr_check and + * dblink_security_check. + */ + if (MyProcPort->has_scram_keys && UseScramPassthrough(foreign_server, user_mapping)) + appendSCRAMKeysInfo(&buf); + foreach(cell, fdw->options) { DefElem *def = lfirst(cell); @@ -2968,6 +3063,22 @@ 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 @@ -3038,6 +3149,70 @@ 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); + appendStringInfo(buf, "require_auth='scram-sha-256' "); + + 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 @@ -3062,6 +3237,7 @@ connect_pg_server(char *connstr_or_srvname, remoteConn *rconn, uint32 wait_event else connstr = connstr_or_srvname; + /* Verify the set of connection parameters. */ dblink_connstr_check(connstr); /* OK to make connection */ @@ -3079,6 +3255,7 @@ connect_pg_server(char *connstr_or_srvname, remoteConn *rconn, uint32 wait_event errdetail_internal("%s", msg))); } + /* Perform post-connection security checks. */ dblink_security_check(conn, rconn, connstr); /* attempt to set client encoding to match server encoding, if needed */ diff --git a/contrib/dblink/meson.build b/contrib/dblink/meson.build index 3ab78668288..dfd8eb6877e 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 00000000000..3a94a27cb0c --- /dev/null +++ b/contrib/dblink/t/001_auth_scram.pl @@ -0,0 +1,249 @@ +# 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 loopback +# 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 $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 $fdw_invalid_server = "db2_fdw_invalid"; # For invalid fdw options +my $fdw_invalid_server2 = + "db2_fdw_invalid2"; # For invalid scram keys fdw options + +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_invalid_fdw_server( $node1, $db0, $fdw_invalid_server, $node2, $db2 ); +setup_fdw_server( $node1, $db0, $fdw_invalid_server2, $node2, $db2 ); + +setup_user_mapping( $node1, $db0, $fdw_server ); +setup_user_mapping( $node1, $db0, $fdw_server2 ); +setup_user_mapping( $node1, $db0, $fdw_invalid_server ); + +# 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'" ); + +unlink( $node1->data_dir . '/pg_hba.conf' ); +unlink( $node2->data_dir . '/pg_hba.conf' ); + +$node1->append_conf( + 'pg_hba.conf', qq{ +local db0 all scram-sha-256 +local db1 all scram-sha-256 +} +); +$node2->append_conf( + 'pg_hba.conf', qq{ +local db2 all scram-sha-256 +} +); + +$node1->restart; +$node2->restart; + +# End of test setup + +test_scram_keys_is_not_overwritten( $node1, $db0, $fdw_invalid_server2 ); + +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" ); + +test_fdw_auth_with_invalid_overwritten_require_auth($fdw_invalid_server); + +# Ensure that trust connections fail without superuser opt-in. +unlink( $node1->data_dir . '/pg_hba.conf' ); +unlink( $node2->data_dir . '/pg_hba.conf' ); + +$node1->append_conf( + 'pg_hba.conf', qq{ +local db0 all scram-sha-256 +local db1 all trust +} +); +$node2->append_conf( + 'pg_hba.conf', qq{ +local all all password +} +); + +$node1->restart; +$node2->restart; + +my ( $ret, $stdout, $stderr ) = $node1->psql( + $db0, + "SELECT * FROM dblink('$fdw_server', 'SELECT * FROM t') AS t(a int, b int)", + connstr => $node1->connstr($db0) . " user=$user" +); + +is( $ret, 3, 'loopback trust fails on the same cluster' ); +like( + $stderr, +qr/failed: authentication method requirement "scram-sha-256" failed: server did not complete authentication/, + 'expected error from loopback trust (same cluster)' +); + +( $ret, $stdout, $stderr ) = $node1->psql( + $db0, +"SELECT * FROM dblink('$fdw_server2', 'SELECT * FROM t2') AS t2(a int, b int)", + connstr => $node1->connstr($db0) . " user=$user" +); + +is( $ret, 3, 'loopback password fails on a different cluster' ); +like( + $stderr, +qr/authentication method requirement "scram-sha-256" failed: server requested a cleartext password/, + 'expected error from loopback password (different cluster)' +); + +# 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 test_fdw_auth_with_invalid_overwritten_require_auth { + + my ($fdw) = @_; + + my ( $ret, $stdout, $stderr ) = $node1->psql( + $db0, + "select * from dblink('$fdw', 'select * from t') as t(a int, b int)", + connstr => $node1->connstr($db0) . " user=$user" + ); + + is( $ret, 3, 'loopback trust fails when overwriting require_auth' ); + like( + $stderr, + qr/password or GSSAPI delegated credentials required/, + 'expected error when connecting to a fdw overwriting the require_auth' + ); +} + +sub test_scram_keys_is_not_overwritten { + my ( $node, $db, $fdw ) = @_; + + my ( $ret, $stdout, $stderr ) = $node->psql( + $db, +qq'CREATE USER MAPPING FOR $user SERVER $fdw OPTIONS (user \'$user\', scram_client_key \'key\');', + connstr => $node->connstr($db) . " user=$user" + ); + + is( $ret, 3, 'user mapping creation fails when using scram_client_key' ); + like( + $stderr, + qr/ERROR: invalid option "scram_client_key"/, + 'user mapping creation fails when using scram_client_key' + ); + + ( $ret, $stdout, $stderr ) = $node->psql( + $db, +qq'CREATE USER MAPPING FOR $user SERVER $fdw OPTIONS (user \'$user\', scram_server_key \'key\');', + connstr => $node->connstr($db) . " user=$user" + ); + + is( $ret, 3, 'user mapping creation fails when using scram_server_key' ); + like( + $stderr, + qr/ERROR: invalid option "scram_server_key"/, + 'user mapping creation fails when using scram_server_key' + ); +} + +sub setup_user_mapping { + my ( $node, $db, $fdw ) = @_; + + $node->safe_psql( $db, + qq'CREATE USER MAPPING FOR $user SERVER $fdw OPTIONS (user \'$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\') ' + ); + $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_invalid_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\', require_auth \'none\') ' + ); + $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_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 81f35986c88..e3b4129ae26 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 + + Foreign Data Wrapper + + + 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. + + + + The authentication with the foreign server can be via password on USER + MAPPING or using SCRAM pass-through. The + use_scram_passthrough on CREATE SERVER options controls + whether dblink_fdw will use the SCRAM pass-through + authentication to connect to the foreign server. With SCRAM pass-through + authentication, dblink_fdw 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. + + + + To use SCRAM pass-through authentication: + + + + The remote server must request SCRAM authentication. (If desired, + enforce this on the client side (FDW side) with the option + require_auth.) If another authentication method is + requested by the server, then that one will be used normally. + + + + + + The remote server can be of any PostgreSQL version that supports SCRAM. + Support for use_scram_passthrough is only required on + the client side (FDW side). + + + + + + 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.) + + + + + + The server running dblink_fdw and the remote server + must have identical SCRAM secrets (encrypted passwords) for the user being + used on dblink_fdw to authenticate on the foreign + server (same salt and iterations, not merely the same password). + + + + 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. + + + + + + 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 pass-through: SCRAM must be used going in + and out.) This is a technical requirement of the SCRAM protocol. + + + + + + + + Notes @@ -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)