public inbox for [email protected]help / color / mirror / Atom feed
Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server 5+ messages / 2 participants [nested] [flat]
* Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server @ 2026-05-18 08:01 Fujii Masao <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Fujii Masao @ 2026-05-18 08:01 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Sat, May 16, 2026 at 12:46 AM Matheus Alcantara <[email protected]> wrote: > > /* > > * Return whether SCRAM pass-through is enabled. > > * > > * If use_scram_passthrough is specified in both the foreign server > > * and the user mapping, the user mapping setting takes precedence. > > */ > > > > Sounds good, fixed. Thanks! > Yeah, good catch, this is from 3642df265d0. The problem is that > dblink_fdw_validator() was changed to call is_valid_dblink_fdw_option() > instead of is_valid_dblink_option() to make CRETE SERVER ... OPTIONS > (use_scram_passthrough '...'); works but I miss the fact that it's also > used by ALTER FOREIGN DATA WRAPPER. The new attached 0003 fix this by > only checking the fdw options when the validator context is from foreign > server or user mapping. Thanks for the patches! Neither dblink nor postgres_fdw seems to have tests checking whether options are specified at the proper object level (foreign server, user mapping, etc.). So adding the test for ALTER FOREIGN DATA WRAPPER ... (use_scram_passthrough ...) seems a bit overkill to me. Also, even if we decide to add such a test, it might be simpler to put it in sql/dblink.sql rather than as a TAP test. Thoughts? > Are you considering backporting these patches? I think that 0003 is > good, not sure about 0001 and 0002. I assume your concern is that v18 was already released with the current behavior, where the server-level use_scram_passthrough setting overrides the user-mapping-level one. Backpatching the behavior change to v18 could therefore affect existing users relying on that behavior, right? If the documentation had clearly stated that the user-mapping-level setting takes precedence, we could reasonably treat the current behavior as a bug and change it even in v18. But since there is no such documentation, I understand the hesitation. That said, I'm feeling tempted to backpatch the change to v18, because having only v18 behave differently seems odd and potentially confusing... The issue addressed by patch 0003 may have a similar concern. However, since use_scram_passthrough set on dblink_fdw currently has no effect even if there are existing users depending on that, it seems acceptable to change that behavior in v18. Is that your thinking? Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server @ 2026-05-18 14:42 Matheus Alcantara <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Matheus Alcantara @ 2026-05-18 14:42 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon May 18, 2026 at 5:01 AM -03, Fujii Masao wrote: > Thanks for the patches! > > Neither dblink nor postgres_fdw seems to have tests checking whether options > are specified at the proper object level (foreign server, user mapping, etc.). > So adding the test for ALTER FOREIGN DATA WRAPPER ... > (use_scram_passthrough ...) > seems a bit overkill to me. Also, even if we decide to add such a test, > it might be simpler to put it in sql/dblink.sql rather than as a TAP test. > Thoughts? > I think that the test is worth to have to avoid such issues in the future again, but I agree that adding as a TAP test is overkill. I've moved to sql/dblink.sql on the new attached version. Do you think that we need to add such test for postgres_fdw too? >> Are you considering backporting these patches? I think that 0003 is >> good, not sure about 0001 and 0002. > > I assume your concern is that v18 was already released with the current > behavior, where the server-level use_scram_passthrough setting overrides > the user-mapping-level one. Backpatching the behavior change to v18 could > therefore affect existing users relying on that behavior, right? > Yes, you are right, this was my main concern. > If the documentation had clearly stated that the user-mapping-level setting > takes precedence, we could reasonably treat the current behavior as a bug > and change it even in v18. But since there is no such documentation, > I understand the hesitation. > Yeah, we don't have any documentation about this on 18, but we do have for sslkey and sslcert where in postgres-fdw.sgml we have the following: sslkey and sslcert - these may appear in either or both a connection and a user mapping. If both are present, the user mapping setting overrides the connection setting. So I think that is desirable to have the same behavior for use_scram_passthrough. > That said, I'm feeling tempted to backpatch the change to v18, because having > only v18 behave differently seems odd and potentially confusing... > I agree with this. > The issue addressed by patch 0003 may have a similar concern. However, > since use_scram_passthrough set on dblink_fdw currently has no effect > even if there are existing users depending on that, it seems acceptable to > change that behavior in v18. Is that your thinking? > Yes, you are right. Even if there are users that are relying on this it's not working, so we at lest make it explicitly and avoid confusions. Thank you for reviewing the patches! -- Matheus Alcantara EDB: https://www.enterprisedb.com From b45939ac4a608a9e178de4c0b90982ddcbb0bb1b Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Thu, 14 May 2026 17:00:13 -0300 Subject: [PATCH v3 1/3] postgres_fdw: Allow user mapping to override use_scram_passthrough Previously, use_scram_passthrough was checked on the foreign server options first, which meant that if set on the server, the user mapping option would be ignored. This changes the precedence to check the user mapping option first, allowing users to override the server-level setting on a per-user basis. This is consistent with how other connection options like sslcert and sslkey are handled, where user mapping settings take precedence over server settings. Also add a test case to verify that setting use_scram_passthrough=false on a user mapping correctly disables SCRAM passthrough even when the server has it enabled. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gma... --- contrib/postgres_fdw/connection.c | 10 ++++++-- contrib/postgres_fdw/t/001_auth_scram.pl | 30 ++++++++++++++++++++++++ doc/src/sgml/postgres-fdw.sgml | 4 +++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 3d2a8d0519d..d71769296b8 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -715,12 +715,18 @@ UserMappingPasswordRequired(UserMapping *user) return true; } +/* + * Return whether SCRAM pass-through is enabled. + * + * If use_scram_passthrough is specified in both the foreign server + * and the user mapping, the user mapping setting takes precedence. + */ static bool UseScramPassthrough(ForeignServer *server, UserMapping *user) { ListCell *cell; - foreach(cell, server->options) + foreach(cell, user->options) { DefElem *def = (DefElem *) lfirst(cell); @@ -728,7 +734,7 @@ UseScramPassthrough(ForeignServer *server, UserMapping *user) return defGetBoolean(def); } - foreach(cell, user->options) + foreach(cell, server->options) { DefElem *def = (DefElem *) lfirst(cell); diff --git a/contrib/postgres_fdw/t/001_auth_scram.pl b/contrib/postgres_fdw/t/001_auth_scram.pl index 6c18db4f2c8..c4b57cd81b3 100644 --- a/contrib/postgres_fdw/t/001_auth_scram.pl +++ b/contrib/postgres_fdw/t/001_auth_scram.pl @@ -20,6 +20,7 @@ my $db1 = "db1"; # For node1 my $db2 = "db2"; # For node2 my $fdw_server = "db1_fdw"; my $fdw_server2 = "db2_fdw"; +my $fdw_server3 = "db1_fdw_override"; my $node1 = PostgreSQL::Test::Cluster->new('node1'); my $node2 = PostgreSQL::Test::Cluster->new('node2'); @@ -46,9 +47,11 @@ setup_table($node2, $db2, "t2"); $node1->safe_psql($db0, 'CREATE EXTENSION IF NOT EXISTS postgres_fdw'); setup_fdw_server($node1, $db0, $fdw_server, $node1, $db1); setup_fdw_server($node1, $db0, $fdw_server2, $node2, $db2); +setup_fdw_server($node1, $db0, $fdw_server3, $node1, $db1); setup_user_mapping($node1, $db0, $fdw_server); setup_user_mapping($node1, $db0, $fdw_server2); +setup_user_mapping($node1, $db0, $fdw_server3); # Make the user have the same SCRAM key on both servers. Forcing to have the # same iteration and salt. @@ -68,6 +71,33 @@ test_fdw_auth($node1, $db0, "t2", $fdw_server2, test_auth($node2, $db2, "t2", "SCRAM auth directly on foreign server should still succeed"); +# Test that use_scram_passthrough=false on user mapping overrides server setting +{ + my $connstr = $node1->connstr($db0) . qq' user=$user'; + + $node1->safe_psql($db0, + qq'ALTER USER MAPPING FOR $user SERVER $fdw_server3 OPTIONS(add use_scram_passthrough \'false\')', + connstr => $connstr + ); + + $node1->safe_psql( + $db0, + qq'CREATE FOREIGN TABLE override_t (g int, col2 int) SERVER $fdw_server3 OPTIONS (table_name \'t\');', + connstr => $connstr ); + $node1->safe_psql($db0, qq'GRANT SELECT ON override_t TO $user;', connstr => $connstr); + + my ($ret, $stdout, $stderr) = $node1->psql( + $db0, + qq'SELECT count(1) FROM override_t', + connstr => $connstr); + + is($ret, 3, 'SCRAM passthrough disabled on user mapping should fail'); + like( + $stderr, + qr/password/i, + 'expected password-related error when scram passthrough disabled on user mapping'); +} + SKIP: { skip "test requires Unix-domain sockets", 4 if !$use_unix_sockets; diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index b81f33732fb..b9e1b04463e 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -803,7 +803,9 @@ OPTIONS (ADD password_required 'false'); <para> This option controls whether <filename>postgres_fdw</filename> will use the SCRAM pass-through authentication to connect to the foreign - server. With SCRAM pass-through authentication, + server. It can be specified for a foreign server or a user mapping. + A user mapping setting overrides the foreign server setting. + With SCRAM pass-through authentication, <filename>postgres_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 -- 2.53.0 From 61c8f1331d83db7804a62f7856dc904723cd2baf Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Thu, 14 May 2026 17:08:24 -0300 Subject: [PATCH v3 2/3] dblink: Allow user mapping to override use_scram_passthrough Previously, use_scram_passthrough was checked on the foreign server options first, which meant that if set on the server, the user mapping option would be ignored. This changes the precedence to check the user mapping option first, allowing users to override the server-level setting on a per-user basis. This is consistent with how postgres_fdw handles this option and how other connection options like sslcert and sslkey work, where user mapping settings take precedence over server settings. Also add a test case to verify that setting use_scram_passthrough=false on a user mapping correctly disables SCRAM passthrough even when the server has it enabled. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gma... --- contrib/dblink/dblink.c | 10 ++++++++-- contrib/dblink/t/001_auth_scram.pl | 24 ++++++++++++++++++++++++ doc/src/sgml/dblink.sgml | 10 ++++++---- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index d843eee7e97..bb6fcae4974 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3230,12 +3230,18 @@ appendSCRAMKeysInfo(StringInfo buf) } +/* + * Return whether SCRAM pass-through is enabled. + * + * If use_scram_passthrough is specified in both the foreign server + * and the user mapping, the user mapping setting takes precedence. + */ static bool UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user) { ListCell *cell; - foreach(cell, foreign_server->options) + foreach(cell, user->options) { DefElem *def = lfirst(cell); @@ -3243,7 +3249,7 @@ UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user) return defGetBoolean(def); } - foreach(cell, user->options) + foreach(cell, foreign_server->options) { DefElem *def = (DefElem *) lfirst(cell); diff --git a/contrib/dblink/t/001_auth_scram.pl b/contrib/dblink/t/001_auth_scram.pl index 9558ca83b7c..b087b38e5a5 100644 --- a/contrib/dblink/t/001_auth_scram.pl +++ b/contrib/dblink/t/001_auth_scram.pl @@ -24,6 +24,7 @@ my $db1 = "db1"; # For node1 my $db2 = "db2"; # For node2 my $fdw_server = "db1_fdw"; my $fdw_server2 = "db2_fdw"; +my $fdw_server3 = "db1_fdw_override"; my $fdw_invalid_server = "db2_fdw_invalid"; # For invalid fdw options my $fdw_invalid_server2 = "db2_fdw_invalid2"; # For invalid scram keys fdw options @@ -55,10 +56,12 @@ 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_fdw_server($node1, $db0, $fdw_server3, $node1, $db1); setup_user_mapping($node1, $db0, $fdw_server); setup_user_mapping($node1, $db0, $fdw_server2); setup_user_mapping($node1, $db0, $fdw_invalid_server); +setup_user_mapping($node1, $db0, $fdw_server3); # Make the user have the same SCRAM key on both servers. Forcing to have the # same iteration and salt. @@ -96,6 +99,27 @@ test_fdw_auth($node1, $db0, "t2", $fdw_server2, test_fdw_auth_with_invalid_overwritten_require_auth($fdw_invalid_server); +# Test that use_scram_passthrough=false on user mapping overrides server setting +{ + my $connstr = $node1->connstr($db0) . qq' user=$user'; + + $node1->safe_psql($db0, + qq'ALTER USER MAPPING FOR $user SERVER $fdw_server3 OPTIONS(add use_scram_passthrough \'false\')', + connstr => $connstr + ); + + my ($ret, $stdout, $stderr) = $node1->psql( + $db0, + "select * from dblink('$fdw_server3', 'select * from t') as t(a int, b int)", + connstr => $connstr); + + is($ret, 3, 'SCRAM passthrough disabled on user mapping should fail'); + like( + $stderr, + qr/password/i, + 'expected password-related error when scram passthrough disabled on user mapping'); +} + # Ensure that trust connections fail without superuser opt-in. unlink($node1->data_dir . '/pg_hba.conf'); unlink($node2->data_dir . '/pg_hba.conf'); diff --git a/doc/src/sgml/dblink.sgml b/doc/src/sgml/dblink.sgml index dd6778d22a8..fc496b74288 100644 --- a/doc/src/sgml/dblink.sgml +++ b/doc/src/sgml/dblink.sgml @@ -154,10 +154,12 @@ dblink_connect(text connname, text connstr) returns text The foreign-data wrapper <filename>dblink_fdw</filename> has an additional Boolean option <literal>use_scram_passthrough</literal> that controls whether <filename>dblink</filename> will use the SCRAM pass-through - authentication to connect to the remote database. With SCRAM pass-through - authentication, <filename>dblink</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. + authentication to connect to the remote database. It can be specified + for a foreign server or a user mapping. A user mapping setting overrides + the foreign server setting. With SCRAM pass-through authentication, + <filename>dblink</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. See the documentation of the equivalent <link linkend="postgres-fdw-option-use-scram-passthrough"><literal>use_scram_passthrough</literal></link> option of postgres_fdw for further details and restrictions. -- 2.53.0 From bb505f0b5ee9a8b1b26cc46cdf8c0f9f3bc44a6f Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Fri, 15 May 2026 12:38:48 -0300 Subject: [PATCH v3 3/3] dblink: Reject use_scram_passthrough option on foreign data wrapper The use_scram_passthrough option only makes sense for foreign server and user mapping contexts, as it controls authentication behavior for specific connections. Previously, this option was incorrectly accepted when set via ALTER FOREIGN DATA WRAPPER OPTIONS, even though it had no effect at that level. Restrict the option validation to only accept use_scram_passthrough when the context is ForeignServerRelationId or UserMappingRelationId. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gma... --- contrib/dblink/dblink.c | 8 ++++++-- contrib/dblink/expected/dblink.out | 5 +++++ contrib/dblink/sql/dblink.sql | 4 ++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index bb6fcae4974..451c3208afe 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3115,8 +3115,12 @@ static bool is_valid_dblink_fdw_option(const PQconninfoOption *options, const char *option, Oid context) { - if (strcmp(option, "use_scram_passthrough") == 0) - return true; + /* These options are only valid for foreign server or user mapping contexts */ + if (context == ForeignServerRelationId || context == UserMappingRelationId) + { + if (strcmp(option, "use_scram_passthrough") == 0) + return true; + } return is_valid_dblink_option(options, option, context); } diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index c70c79574fd..1d2759def9e 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -1220,6 +1220,11 @@ SHOW intervalstyle; postgres (1 row) +-- Check that adding use_scram_passthrough option on an foreign data wrapper is +-- not allowed +ALTER FOREIGN DATA WRAPPER dblink_fdw OPTIONS(add use_scram_passthrough 'true'); +ERROR: invalid option "use_scram_passthrough" +HINT: There are no valid options in this context. -- Clean up GUC-setting tests SELECT dblink_disconnect('myconn'); dblink_disconnect diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql index 365b21036e8..d67a0a5992e 100644 --- a/contrib/dblink/sql/dblink.sql +++ b/contrib/dblink/sql/dblink.sql @@ -635,6 +635,10 @@ FROM dblink_fetch('myconn','error_cursor', 1) AS t(i int); SHOW datestyle; SHOW intervalstyle; +-- Check that adding use_scram_passthrough option on an foreign data wrapper is +-- not allowed +ALTER FOREIGN DATA WRAPPER dblink_fdw OPTIONS(add use_scram_passthrough 'true'); + -- Clean up GUC-setting tests SELECT dblink_disconnect('myconn'); RESET datestyle; -- 2.53.0 Attachments: [text/plain] v3-0001-postgres_fdw-Allow-user-mapping-to-override-use_s.patch (5.2K, 2-v3-0001-postgres_fdw-Allow-user-mapping-to-override-use_s.patch) download | inline diff: From b45939ac4a608a9e178de4c0b90982ddcbb0bb1b Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Thu, 14 May 2026 17:00:13 -0300 Subject: [PATCH v3 1/3] postgres_fdw: Allow user mapping to override use_scram_passthrough Previously, use_scram_passthrough was checked on the foreign server options first, which meant that if set on the server, the user mapping option would be ignored. This changes the precedence to check the user mapping option first, allowing users to override the server-level setting on a per-user basis. This is consistent with how other connection options like sslcert and sslkey are handled, where user mapping settings take precedence over server settings. Also add a test case to verify that setting use_scram_passthrough=false on a user mapping correctly disables SCRAM passthrough even when the server has it enabled. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gmail.com --- contrib/postgres_fdw/connection.c | 10 ++++++-- contrib/postgres_fdw/t/001_auth_scram.pl | 30 ++++++++++++++++++++++++ doc/src/sgml/postgres-fdw.sgml | 4 +++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 3d2a8d0519d..d71769296b8 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -715,12 +715,18 @@ UserMappingPasswordRequired(UserMapping *user) return true; } +/* + * Return whether SCRAM pass-through is enabled. + * + * If use_scram_passthrough is specified in both the foreign server + * and the user mapping, the user mapping setting takes precedence. + */ static bool UseScramPassthrough(ForeignServer *server, UserMapping *user) { ListCell *cell; - foreach(cell, server->options) + foreach(cell, user->options) { DefElem *def = (DefElem *) lfirst(cell); @@ -728,7 +734,7 @@ UseScramPassthrough(ForeignServer *server, UserMapping *user) return defGetBoolean(def); } - foreach(cell, user->options) + foreach(cell, server->options) { DefElem *def = (DefElem *) lfirst(cell); diff --git a/contrib/postgres_fdw/t/001_auth_scram.pl b/contrib/postgres_fdw/t/001_auth_scram.pl index 6c18db4f2c8..c4b57cd81b3 100644 --- a/contrib/postgres_fdw/t/001_auth_scram.pl +++ b/contrib/postgres_fdw/t/001_auth_scram.pl @@ -20,6 +20,7 @@ my $db1 = "db1"; # For node1 my $db2 = "db2"; # For node2 my $fdw_server = "db1_fdw"; my $fdw_server2 = "db2_fdw"; +my $fdw_server3 = "db1_fdw_override"; my $node1 = PostgreSQL::Test::Cluster->new('node1'); my $node2 = PostgreSQL::Test::Cluster->new('node2'); @@ -46,9 +47,11 @@ setup_table($node2, $db2, "t2"); $node1->safe_psql($db0, 'CREATE EXTENSION IF NOT EXISTS postgres_fdw'); setup_fdw_server($node1, $db0, $fdw_server, $node1, $db1); setup_fdw_server($node1, $db0, $fdw_server2, $node2, $db2); +setup_fdw_server($node1, $db0, $fdw_server3, $node1, $db1); setup_user_mapping($node1, $db0, $fdw_server); setup_user_mapping($node1, $db0, $fdw_server2); +setup_user_mapping($node1, $db0, $fdw_server3); # Make the user have the same SCRAM key on both servers. Forcing to have the # same iteration and salt. @@ -68,6 +71,33 @@ test_fdw_auth($node1, $db0, "t2", $fdw_server2, test_auth($node2, $db2, "t2", "SCRAM auth directly on foreign server should still succeed"); +# Test that use_scram_passthrough=false on user mapping overrides server setting +{ + my $connstr = $node1->connstr($db0) . qq' user=$user'; + + $node1->safe_psql($db0, + qq'ALTER USER MAPPING FOR $user SERVER $fdw_server3 OPTIONS(add use_scram_passthrough \'false\')', + connstr => $connstr + ); + + $node1->safe_psql( + $db0, + qq'CREATE FOREIGN TABLE override_t (g int, col2 int) SERVER $fdw_server3 OPTIONS (table_name \'t\');', + connstr => $connstr ); + $node1->safe_psql($db0, qq'GRANT SELECT ON override_t TO $user;', connstr => $connstr); + + my ($ret, $stdout, $stderr) = $node1->psql( + $db0, + qq'SELECT count(1) FROM override_t', + connstr => $connstr); + + is($ret, 3, 'SCRAM passthrough disabled on user mapping should fail'); + like( + $stderr, + qr/password/i, + 'expected password-related error when scram passthrough disabled on user mapping'); +} + SKIP: { skip "test requires Unix-domain sockets", 4 if !$use_unix_sockets; diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index b81f33732fb..b9e1b04463e 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -803,7 +803,9 @@ OPTIONS (ADD password_required 'false'); <para> This option controls whether <filename>postgres_fdw</filename> will use the SCRAM pass-through authentication to connect to the foreign - server. With SCRAM pass-through authentication, + server. It can be specified for a foreign server or a user mapping. + A user mapping setting overrides the foreign server setting. + With SCRAM pass-through authentication, <filename>postgres_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 -- 2.53.0 [text/plain] v3-0002-dblink-Allow-user-mapping-to-override-use_scram_p.patch (5.8K, 3-v3-0002-dblink-Allow-user-mapping-to-override-use_scram_p.patch) download | inline diff: From 61c8f1331d83db7804a62f7856dc904723cd2baf Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Thu, 14 May 2026 17:08:24 -0300 Subject: [PATCH v3 2/3] dblink: Allow user mapping to override use_scram_passthrough Previously, use_scram_passthrough was checked on the foreign server options first, which meant that if set on the server, the user mapping option would be ignored. This changes the precedence to check the user mapping option first, allowing users to override the server-level setting on a per-user basis. This is consistent with how postgres_fdw handles this option and how other connection options like sslcert and sslkey work, where user mapping settings take precedence over server settings. Also add a test case to verify that setting use_scram_passthrough=false on a user mapping correctly disables SCRAM passthrough even when the server has it enabled. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gmail.com --- contrib/dblink/dblink.c | 10 ++++++++-- contrib/dblink/t/001_auth_scram.pl | 24 ++++++++++++++++++++++++ doc/src/sgml/dblink.sgml | 10 ++++++---- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index d843eee7e97..bb6fcae4974 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3230,12 +3230,18 @@ appendSCRAMKeysInfo(StringInfo buf) } +/* + * Return whether SCRAM pass-through is enabled. + * + * If use_scram_passthrough is specified in both the foreign server + * and the user mapping, the user mapping setting takes precedence. + */ static bool UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user) { ListCell *cell; - foreach(cell, foreign_server->options) + foreach(cell, user->options) { DefElem *def = lfirst(cell); @@ -3243,7 +3249,7 @@ UseScramPassthrough(ForeignServer *foreign_server, UserMapping *user) return defGetBoolean(def); } - foreach(cell, user->options) + foreach(cell, foreign_server->options) { DefElem *def = (DefElem *) lfirst(cell); diff --git a/contrib/dblink/t/001_auth_scram.pl b/contrib/dblink/t/001_auth_scram.pl index 9558ca83b7c..b087b38e5a5 100644 --- a/contrib/dblink/t/001_auth_scram.pl +++ b/contrib/dblink/t/001_auth_scram.pl @@ -24,6 +24,7 @@ my $db1 = "db1"; # For node1 my $db2 = "db2"; # For node2 my $fdw_server = "db1_fdw"; my $fdw_server2 = "db2_fdw"; +my $fdw_server3 = "db1_fdw_override"; my $fdw_invalid_server = "db2_fdw_invalid"; # For invalid fdw options my $fdw_invalid_server2 = "db2_fdw_invalid2"; # For invalid scram keys fdw options @@ -55,10 +56,12 @@ 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_fdw_server($node1, $db0, $fdw_server3, $node1, $db1); setup_user_mapping($node1, $db0, $fdw_server); setup_user_mapping($node1, $db0, $fdw_server2); setup_user_mapping($node1, $db0, $fdw_invalid_server); +setup_user_mapping($node1, $db0, $fdw_server3); # Make the user have the same SCRAM key on both servers. Forcing to have the # same iteration and salt. @@ -96,6 +99,27 @@ test_fdw_auth($node1, $db0, "t2", $fdw_server2, test_fdw_auth_with_invalid_overwritten_require_auth($fdw_invalid_server); +# Test that use_scram_passthrough=false on user mapping overrides server setting +{ + my $connstr = $node1->connstr($db0) . qq' user=$user'; + + $node1->safe_psql($db0, + qq'ALTER USER MAPPING FOR $user SERVER $fdw_server3 OPTIONS(add use_scram_passthrough \'false\')', + connstr => $connstr + ); + + my ($ret, $stdout, $stderr) = $node1->psql( + $db0, + "select * from dblink('$fdw_server3', 'select * from t') as t(a int, b int)", + connstr => $connstr); + + is($ret, 3, 'SCRAM passthrough disabled on user mapping should fail'); + like( + $stderr, + qr/password/i, + 'expected password-related error when scram passthrough disabled on user mapping'); +} + # Ensure that trust connections fail without superuser opt-in. unlink($node1->data_dir . '/pg_hba.conf'); unlink($node2->data_dir . '/pg_hba.conf'); diff --git a/doc/src/sgml/dblink.sgml b/doc/src/sgml/dblink.sgml index dd6778d22a8..fc496b74288 100644 --- a/doc/src/sgml/dblink.sgml +++ b/doc/src/sgml/dblink.sgml @@ -154,10 +154,12 @@ dblink_connect(text connname, text connstr) returns text The foreign-data wrapper <filename>dblink_fdw</filename> has an additional Boolean option <literal>use_scram_passthrough</literal> that controls whether <filename>dblink</filename> will use the SCRAM pass-through - authentication to connect to the remote database. With SCRAM pass-through - authentication, <filename>dblink</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. + authentication to connect to the remote database. It can be specified + for a foreign server or a user mapping. A user mapping setting overrides + the foreign server setting. With SCRAM pass-through authentication, + <filename>dblink</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. See the documentation of the equivalent <link linkend="postgres-fdw-option-use-scram-passthrough"><literal>use_scram_passthrough</literal></link> option of postgres_fdw for further details and restrictions. -- 2.53.0 [text/plain] v3-0003-dblink-Reject-use_scram_passthrough-option-on-for.patch (2.9K, 4-v3-0003-dblink-Reject-use_scram_passthrough-option-on-for.patch) download | inline diff: From bb505f0b5ee9a8b1b26cc46cdf8c0f9f3bc44a6f Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Fri, 15 May 2026 12:38:48 -0300 Subject: [PATCH v3 3/3] dblink: Reject use_scram_passthrough option on foreign data wrapper The use_scram_passthrough option only makes sense for foreign server and user mapping contexts, as it controls authentication behavior for specific connections. Previously, this option was incorrectly accepted when set via ALTER FOREIGN DATA WRAPPER OPTIONS, even though it had no effect at that level. Restrict the option validation to only accept use_scram_passthrough when the context is ForeignServerRelationId or UserMappingRelationId. Reviewed-by: Fujii Masao <[email protected]> Discussion: https://www.postgresql.org/message-id/CAHGQGwEJ8rZjmbOvCicyr4vbuLio082bNTde0WNoSWaWr9wVcg%40mail.gmail.com --- contrib/dblink/dblink.c | 8 ++++++-- contrib/dblink/expected/dblink.out | 5 +++++ contrib/dblink/sql/dblink.sql | 4 ++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index bb6fcae4974..451c3208afe 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3115,8 +3115,12 @@ static bool is_valid_dblink_fdw_option(const PQconninfoOption *options, const char *option, Oid context) { - if (strcmp(option, "use_scram_passthrough") == 0) - return true; + /* These options are only valid for foreign server or user mapping contexts */ + if (context == ForeignServerRelationId || context == UserMappingRelationId) + { + if (strcmp(option, "use_scram_passthrough") == 0) + return true; + } return is_valid_dblink_option(options, option, context); } diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index c70c79574fd..1d2759def9e 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -1220,6 +1220,11 @@ SHOW intervalstyle; postgres (1 row) +-- Check that adding use_scram_passthrough option on an foreign data wrapper is +-- not allowed +ALTER FOREIGN DATA WRAPPER dblink_fdw OPTIONS(add use_scram_passthrough 'true'); +ERROR: invalid option "use_scram_passthrough" +HINT: There are no valid options in this context. -- Clean up GUC-setting tests SELECT dblink_disconnect('myconn'); dblink_disconnect diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql index 365b21036e8..d67a0a5992e 100644 --- a/contrib/dblink/sql/dblink.sql +++ b/contrib/dblink/sql/dblink.sql @@ -635,6 +635,10 @@ FROM dblink_fetch('myconn','error_cursor', 1) AS t(i int); SHOW datestyle; SHOW intervalstyle; +-- Check that adding use_scram_passthrough option on an foreign data wrapper is +-- not allowed +ALTER FOREIGN DATA WRAPPER dblink_fdw OPTIONS(add use_scram_passthrough 'true'); + -- Clean up GUC-setting tests SELECT dblink_disconnect('myconn'); RESET datestyle; -- 2.53.0 ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server @ 2026-05-23 01:58 Fujii Masao <[email protected]> parent: Matheus Alcantara <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Fujii Masao @ 2026-05-23 01:58 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, May 18, 2026 at 11:42 PM Matheus Alcantara <[email protected]> wrote: > I think that the test is worth to have to avoid such issues in the > future again, but I agree that adding as a TAP test is overkill. I've > moved to sql/dblink.sql on the new attached version. > > Do you think that we need to add such test for postgres_fdw too? No, I don't think so. In dblink, use_scram_passthrough is handled by a dblink-specific validation path, and patch 0003 fixes that specific code path. So it might be worth adding such test for dblink. In postgres_fdw, however, this option is handled by the generic option/context validation machinery, not by any special-case code. We already have tests for context-sensitive option validation there. So adding such test for postgres_fdw would be mostly redundant, I think. Thought? > Yeah, we don't have any documentation about this on 18, but we do have > for sslkey and sslcert where in postgres-fdw.sgml we have the following: > sslkey and sslcert - these may appear in either or both a connection > and a user mapping. If both are present, the user mapping setting > overrides the connection setting. > > So I think that is desirable to have the same behavior for > use_scram_passthrough. Agreed. I'm therefore thinking of backpatching patches 0001 and 0002 to v18. Even in v18, this change is very narrow. It only affects cases where use_scram_passthrough is specified at both the server and user-mapping levels with conflicting values. In such cases, the natural interpretation is that the user-mapping setting is intended to override the server-level setting; otherwise, there would be little reason to specify conflicting values. Given the limited impact, leaving v18 as the only branch with different semantics for a feature introduced in v18 seems more undesirable and potentially confusing. So I think backpatching to v18 makes sense. Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server @ 2026-05-23 16:46 Matheus Alcantara <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Matheus Alcantara @ 2026-05-23 16:46 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On 22/05/26 22:58, Fujii Masao wrote: > On Mon, May 18, 2026 at 11:42 PM Matheus Alcantara > <[email protected]> wrote: >> I think that the test is worth to have to avoid such issues in the >> future again, but I agree that adding as a TAP test is overkill. I've >> moved to sql/dblink.sql on the new attached version. >> >> Do you think that we need to add such test for postgres_fdw too? > > No, I don't think so. > > In dblink, use_scram_passthrough is handled by a dblink-specific validation > path, and patch 0003 fixes that specific code path. So it might be worth > adding such test for dblink. > > In postgres_fdw, however, this option is handled by the generic option/context > validation machinery, not by any special-case code. We already have tests for > context-sensitive option validation there. So adding such test for postgres_fdw > would be mostly redundant, I think. Thought? > Agree, it make sense. >> Yeah, we don't have any documentation about this on 18, but we do have >> for sslkey and sslcert where in postgres-fdw.sgml we have the following: >> sslkey and sslcert - these may appear in either or both a connection >> and a user mapping. If both are present, the user mapping setting >> overrides the connection setting. >> >> So I think that is desirable to have the same behavior for >> use_scram_passthrough. > > Agreed. I'm therefore thinking of backpatching patches 0001 and 0002 to v18. > > Even in v18, this change is very narrow. It only affects cases where > use_scram_passthrough is specified at both the server and user-mapping levels > with conflicting values. In such cases, the natural interpretation is that > the user-mapping setting is intended to override the server-level setting; > otherwise, there would be little reason to specify conflicting values. > > Given the limited impact, leaving v18 as the only branch with different > semantics for a feature introduced in v18 seems more undesirable and > potentially confusing. So I think backpatching to v18 makes sense. > +1 Thank you for finding the issue and reviewing the patch. -- Matheus Alcantara EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server @ 2026-05-25 16:10 Fujii Masao <[email protected]> parent: Matheus Alcantara <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Fujii Masao @ 2026-05-25 16:10 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Sun, May 24, 2026 at 1:46 AM Matheus Alcantara <[email protected]> wrote: > Thank you for finding the issue and reviewing the patch. I've pushed the patches. Thanks! Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-05-25 16:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-18 08:01 Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server Fujii Masao <[email protected]> 2026-05-18 14:42 ` Matheus Alcantara <[email protected]> 2026-05-23 01:58 ` Fujii Masao <[email protected]> 2026-05-23 16:46 ` Matheus Alcantara <[email protected]> 2026-05-25 16:10 ` Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox