On Wed, Jan 22, 2025 at 6:10 AM Matheus Alcantara
<[email protected]> wrote:
> The attached patch enables SCRAM authentication for dblink connections when
> using dblink_fdw without requiring a plain-text password on user mapping
> properties. The implementation is very similar to what was implemented on
> postgres_fdw [0].
Not a full review, but I wanted to highlight one aspect of the patch:
> - 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);
and
> - 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);
These don't seem right to me. SCRAM passthrough should be considered
as _part_ of the connstr/security checks, but I think it should not
_bypass_ those checks. We have to enforce the use of the SCRAM
credentials on the remote for safety, similarly to GSS delegation.
(This might be a good place for `require_auth=scram-sha-256`?)
I've attached a failing test to better illustrate what I mean.
It looks like the postgres_fdw patch that landed also performs a
bypass -- I think that may need an open item to fix? CC'd Peter.
Thanks!
--Jacob
diff --git a/contrib/dblink/t/001_auth_scram.pl
b/contrib/dblink/t/001_auth_scram.pl
index 00fd6d85833..58c924f9aff 100644
--- a/contrib/dblink/t/001_auth_scram.pl
+++ b/contrib/dblink/t/001_auth_scram.pl
@@ -56,8 +56,21 @@ 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);
+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
@@ -67,6 +80,45 @@ test_fdw_auth($node1, $db0, "t", $fdw_server,
test_fdw_auth($node1, $db0, "t2", $fdw_server2,
"SCRAM auth on a different database cluster must succeed");
+# 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 db2 all trust
+});
+
+$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/password or GSSAPI delegated credentials required/,
+ '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 trust fails on a different cluster');
+like(
+ $stderr,
+ qr/password or GSSAPI delegated credentials required/,
+ 'expected error from loopback trust (different cluster)');
+
# Helper functions
sub test_fdw_auth
@@ -84,21 +136,6 @@ sub test_fdw_auth
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) = @_;
