rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Re-validate connection string in libpqrcv_connect(). 

A superuser may create a subscription with password_required=true, but
which uses a connection string without a password.

Previously, if the owner of such a subscription was changed to a
non-superuser, the non-superuser was able to utilize a password from
another source (like a password file or the PGPASSWORD environment
variable), which should not have been allowed.

This commit adds a step to re-validate the connection string before
connecting.

Reported-by: Jeff Davis
Author: Vignesh C
Reviewed-by: Peter Smith, Robert Haas, Amit Kapila
Discussion: https://www.postgresql.org/message-id/flat/e5892973ae2a80a1a3e0266806640dae3c428100.camel%40j-davis.com
Backpatch-through: 16
This commit is contained in:
Jeff Davis 2024-01-12 13:41:36 -08:00
parent a1604237a6
commit 5c31669058
3 changed files with 95 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
doc/src/sgml/ref/create_subscription.sgml
View File

@ -357,11 +357,12 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
<term><literal>password_required</literal> (<type>boolean</type>)</term>
<listitem>
<para>
Specifies whether connections to the publisher made as a result
of this subscription must use password authentication. This setting
is ignored when the subscription is owned by a superuser.
The default is <literal>true</literal>. Only superusers can set
this value to <literal>false</literal>.
If set to <literal>true</literal>, connections to the publisher made
as a result of this subscription must use password authentication
and the password must be specified as a part of the connection
string. This setting is ignored when the subscription is owned by a
superuser. The default is <literal>true</literal>. Only superusers
can set this value to <literal>false</literal>.
</para>
</listitem>
</varlistentry>

9
src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
View File

@ -137,6 +137,15 @@ libpqrcv_connect(const char *conninfo, bool logical, bool must_use_password,
const char *vals[6];
int i = 0;
/*
* Re-validate connection string. The validation already happened at DDL
* time, but the subscription owner may have changed. If we don't recheck
* with the correct must_use_password, it's possible that the connection
* will obtain the password from a different source, such as PGPASSFILE or
* PGPASSWORD.
*/
libpqrcv_check_conninfo(conninfo, must_use_password);
/*
* We use the expand_dbname parameter to process the connection string (or
* URI), and pass some extra options.

80
src/test/subscription/t/027_nosuperuser.pl
View File

@ -327,4 +327,84 @@ $node_subscriber->wait_for_log(
qr/LOG: ( [A-Z0-9]+:)? logical replication worker for subscription \"regression_sub\" will restart because the subscription owner's superuser privileges have been revoked/,
$offset);
# If the subscription connection requires a password ('password_required'
# is true) then a non-superuser must specify that password in the connection
# string.
$ENV{"PGPASSWORD"} = 'secret';
my $node_publisher1 = PostgreSQL::Test::Cluster->new('publisher1');
my $node_subscriber1 = PostgreSQL::Test::Cluster->new('subscriber1');
$node_publisher1->init(allows_streaming => 'logical');
$node_subscriber1->init;
$node_publisher1->start;
$node_subscriber1->start;
my $publisher_connstr1 =
$node_publisher1->connstr . ' user=regress_test_user dbname=postgres';
my $publisher_connstr2 =
$node_publisher1->connstr
. ' user=regress_test_user dbname=postgres password=secret';
for my $node ($node_publisher1, $node_subscriber1)
{
$node->safe_psql(
'postgres', qq(
CREATE ROLE regress_test_user PASSWORD 'secret' LOGIN REPLICATION;
GRANT CREATE ON DATABASE postgres TO regress_test_user;
GRANT PG_CREATE_SUBSCRIPTION TO regress_test_user;
));
}
$node_publisher1->safe_psql(
'postgres', qq(
SET SESSION AUTHORIZATION regress_test_user;
CREATE PUBLICATION regress_test_pub;
));
$node_subscriber1->safe_psql(
'postgres', qq(
CREATE SUBSCRIPTION regress_test_sub CONNECTION '$publisher_connstr1' PUBLICATION regress_test_pub;
));
# Wait for initial sync to finish
$node_subscriber1->wait_for_subscription_sync($node_publisher1,
'regress_test_sub');
# Setup pg_hba configuration so that logical replication connection without
# password is not allowed.
unlink($node_publisher1->data_dir . '/pg_hba.conf');
$node_publisher1->append_conf('pg_hba.conf',
qq{local all regress_test_user md5});
$node_publisher1->reload;
# Change the subscription owner to a non-superuser
$node_subscriber1->safe_psql(
'postgres', qq(
ALTER SUBSCRIPTION regress_test_sub OWNER TO regress_test_user;
));
# Non-superuser must specify password in the connection string
my ($ret, $stdout, $stderr) = $node_subscriber1->psql(
'postgres', qq(
SET SESSION AUTHORIZATION regress_test_user;
ALTER SUBSCRIPTION regress_test_sub REFRESH PUBLICATION;
));
isnt($ret, 0,
"non zero exit for subscription whose owner is a non-superuser must specify password parameter of the connection string"
);
ok( $stderr =~ m/DETAIL: Non-superusers must provide a password in the connection string./,
'subscription whose owner is a non-superuser must specify password parameter of the connection string'
);
delete $ENV{"PGPASSWORD"};
# It should succeed after including the password parameter of the connection
# string.
($ret, $stdout, $stderr) = $node_subscriber1->psql(
'postgres', qq(
SET SESSION AUTHORIZATION regress_test_user;
ALTER SUBSCRIPTION regress_test_sub CONNECTION '$publisher_connstr2';
ALTER SUBSCRIPTION regress_test_sub REFRESH PUBLICATION;
));
is($ret, 0,
"Non-superuser will be able to refresh the publication after specifying the password parameter of the connection string"
);
done_testing();