[16+] subscription can end up in inconsistent state

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: pgsql-bugs(at)postgresql(dot)org
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: [16+] subscription can end up in inconsistent state
Date: 2023-09-10 15:45:22
Message-ID: e5892973ae2a80a1a3e0266806640dae3c428100.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs


[ Moved from -security, where it was initially posted as a precaution.
It was determined that the normal bug process is the right way to fix
this. ]

If I understand correctly, the security requirements for checking a
non-superuser's connection string are that (a) the connection string
itself specifies a password, thereby ensuring it won't come from a
PGPASSFILE; and (b) checking PQconnectionUsedPassword() after the
connection to ensure that the password was actually used for
authentication, rather than some other method.

If subpasswordrequired=true, that causes (b) to be consistently checked
for all connections.

The checking of (a) is more complicated -- it's checked at CREATE/ALTER
time, but only if the user is not a superuser. If the superuser gives
the subscription to a non-superuser, or if the subowner loses their
superuser status, then the subscription is in a weird state:
subpasswordrequired may be true even if subconninfo doesn't have a
password. That means that (a) is not being checked, and (b) is being
checked.

Repro (16+):

Publisher:
CREATE USER repl REPLICATION PASSWORD 'secret';
CREATE TABLE t(i INT);
INSERT INTO t VALUES(1);
GRANT SELECT ON t TO repl;
CREATE PUBLICATION p1 FOR TABLE t;

Subscriber (has a PGPASSFILE for user "repl"):
CREATE USER u1;
CREATE TABLE t(i INT);
ALTER TABLE t OWNER TO u1;
-- no password specified; relies on passfile
CREATE SUBSCRIPTION s1
CONNECTION 'dbname=postgres user=repl'
PUBLICATION p1 WITH (enabled=false);
ALTER SUBSCRIPTION s1 OWNER TO u1;
SELECT COUNT(*) FROM t; -- 0
\c - u1
-- still using passfile
ALTER SUBSCRIPTION s1 ENABLE;
SELECT pg_sleep(2);
SELECT COUNT(*) FROM t; -- 1
ALTER SUBSCRIPTION s1 REFRESH PUBLICATION;

It's also possible to get into this state by the superuser modifying a
non-superuser's subscription.

The purpose of the "password_required" option is to handle the case
where a subscription was formerly owned by a superuser, or has been
modified by a superuser. The docs say:

"[password_required] ... This setting is ignored when the
subscription is owned by a superuser... Only superusers can set this
value to false."
https://www.postgresql.org/docs/16/sql-createsubscription.html

So a superuser changing the owner or modifying another user's
subscription is a supported operation, and not just a superuser doing
the wrong thing. (Perhaps the docs should be more clear on this point?)

We could address this either by:

* maintaining the invariant that if subpasswordrequired=true, then
subconninfo specifies a password; or
* calling walrcv_check_conninfo() before each connection

The former might raise some compatibility concerns with 15 (or
questions about what we can do with the default for
"password_required"), so right now the latter looks like the right fix.

Regards,
Jeff Davis

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Thomas Munro 2023-09-10 22:37:34 Re: FW: query pg_stat_ssl hang 100%cpu
Previous Message PG Bug reporting form 2023-09-10 12:57:14 BUG #18103: bugs of concurrent merge into when use different join plan