v2-0001-Handle-non-superuser-subscription-owners-sensibly.patch
application/octet-stream
Filename: v2-0001-Handle-non-superuser-subscription-owners-sensibly.patch
Type: application/octet-stream
Part: 0
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: format-patch
Series: patch v2-0001
Subject: Handle non-superuser subscription owners sensibly
| File | + | − |
|---|---|---|
| doc/src/sgml/ref/alter_subscription.sgml | 9 | 2 |
| src/backend/commands/subscriptioncmds.c | 20 | 0 |
| src/test/regress/expected/subscription.out | 51 | 0 |
| src/test/regress/sql/subscription.sql | 35 | 0 |
From 6225780bd7e4282ec47e153c19ef59fc10648c17 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Sun, 26 Sep 2021 18:21:26 -0700
Subject: [PATCH v2 1/3] Handle non-superuser subscription owners sensibly
Stop pretending that subscriptions are always owned by superusers
and instead fix security problems that arise as a consequence of
them not being superuser. Specifically, disallow a non-superuser
changing the connection, the list of publications, or the options
for their subscription.
The prior behavior violated the principle of least surprise,
allowing a non-superuser in possession of a subscription to change
all aspects of the subscription, connecting it to a different server
and subscribing a different set of publications, effectively
amounting to a non-superuser creating a new subscription.
The new behavior restricts the non-superuser owner to enabling,
disabling, refreshing, renaming, and further assigning ownership of
the subscription.
---
doc/src/sgml/ref/alter_subscription.sgml | 11 ++++-
src/backend/commands/subscriptioncmds.c | 20 +++++++++
src/test/regress/expected/subscription.out | 51 ++++++++++++++++++++++
src/test/regress/sql/subscription.sql | 35 +++++++++++++++
4 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 0b027cc346..b1fd73f776 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -47,8 +47,6 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
You must own the subscription to use <command>ALTER SUBSCRIPTION</command>.
To alter the owner, you must also be a direct or indirect member of the
new owning role. The new owner has to be a superuser.
- (Currently, all subscription owners must be superusers, so the owner checks
- will be bypassed in practice. But this might change in the future.)
</para>
<para>
@@ -99,6 +97,9 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
<xref linkend="sql-createsubscription"/>. See there for more
information.
</para>
+ <para>
+ Only superusers may change the connection property.
+ </para>
</listitem>
</varlistentry>
@@ -139,6 +140,9 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
<literal>REFRESH PUBLICATION</literal> may be specified, to control the
implicit refresh operation.
</para>
+ <para>
+ Only superusers may change the list of subscribed publications.
+ </para>
</listitem>
</varlistentry>
@@ -204,6 +208,9 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
<literal>binary</literal>, and
<literal>streaming</literal>.
</para>
+ <para>
+ Only superusers may alter subscription parameters.
+ </para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index c47ba26369..6a5d192128 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -868,6 +868,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
{
case ALTER_SUBSCRIPTION_OPTIONS:
{
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter the options for a subscription")));
+
supported_opts = (SUBOPT_SLOT_NAME |
SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY |
SUBOPT_STREAMING);
@@ -946,6 +951,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
}
case ALTER_SUBSCRIPTION_CONNECTION:
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter the connection for a subscription")));
+
/* Load the library providing us libpq calls. */
load_file("libpqwalreceiver", false);
/* Check the connection info string. */
@@ -959,6 +969,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
case ALTER_SUBSCRIPTION_SET_PUBLICATION:
{
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter publications for a subscription")));
+
supported_opts = SUBOPT_COPY_DATA | SUBOPT_REFRESH;
parse_subscription_options(pstate, stmt->options,
supported_opts, &opts);
@@ -1006,6 +1021,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
List *publist;
bool isadd = stmt->kind == ALTER_SUBSCRIPTION_ADD_PUBLICATION;
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to alter publications for a subscription")));
+
supported_opts = SUBOPT_REFRESH | SUBOPT_COPY_DATA;
parse_subscription_options(pstate, stmt->options,
supported_opts, &opts);
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 15a1ac6398..a05e6f1795 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -144,6 +144,57 @@ HINT: The owner of a subscription must be a superuser.
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
+-- revoke superuser from new owner
+ALTER ROLE regress_subscription_user2 NOSUPERUSER;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+-- fail - non-superuser owner cannot change connection parameter
+ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=somethingelse';
+ERROR: must be superuser to alter the connection for a subscription
+-- fail - non-superuser owner cannot alter the publications list
+ALTER SUBSCRIPTION regress_testsub ADD PUBLICATION somepub;
+ERROR: must be superuser to alter publications for a subscription
+ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION otherpub;
+ERROR: must be superuser to alter publications for a subscription
+ALTER SUBSCRIPTION regress_testsub SET PUBLICATION somepub, otherpub;
+ERROR: must be superuser to alter publications for a subscription
+-- fail - non-superuser owner cannot change subscription parameters
+ALTER SUBSCRIPTION regress_testsub SET (copy_data = true);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (copy_data = false);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (create_slot = true);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (create_slot = false);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'somethingelse');
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = on);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = off);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = local);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = remote_write);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = remote_apply);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (binary = on);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (binary = off);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (connect = on);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (connect = off);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (streaming = on);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (streaming = off);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (two_phase = on);
+ERROR: must be superuser to alter the options for a subscription
+ALTER SUBSCRIPTION regress_testsub SET (two_phase = off);
+ERROR: must be superuser to alter the options for a subscription
+SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - cannot do DROP SUBSCRIPTION inside transaction block with slot name
BEGIN;
DROP SUBSCRIPTION regress_testsub;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 7faa935a2a..8f66709441 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -105,6 +105,41 @@ ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
+-- revoke superuser from new owner
+ALTER ROLE regress_subscription_user2 NOSUPERUSER;
+
+SET SESSION AUTHORIZATION regress_subscription_user2;
+
+-- fail - non-superuser owner cannot change connection parameter
+ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=somethingelse';
+
+-- fail - non-superuser owner cannot alter the publications list
+ALTER SUBSCRIPTION regress_testsub ADD PUBLICATION somepub;
+ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION otherpub;
+ALTER SUBSCRIPTION regress_testsub SET PUBLICATION somepub, otherpub;
+
+-- fail - non-superuser owner cannot change subscription parameters
+ALTER SUBSCRIPTION regress_testsub SET (copy_data = true);
+ALTER SUBSCRIPTION regress_testsub SET (copy_data = false);
+ALTER SUBSCRIPTION regress_testsub SET (create_slot = true);
+ALTER SUBSCRIPTION regress_testsub SET (create_slot = false);
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'somethingelse');
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = on);
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = off);
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = local);
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = remote_write);
+ALTER SUBSCRIPTION regress_testsub SET (synchronous_commit = remote_apply);
+ALTER SUBSCRIPTION regress_testsub SET (binary = on);
+ALTER SUBSCRIPTION regress_testsub SET (binary = off);
+ALTER SUBSCRIPTION regress_testsub SET (connect = on);
+ALTER SUBSCRIPTION regress_testsub SET (connect = off);
+ALTER SUBSCRIPTION regress_testsub SET (streaming = on);
+ALTER SUBSCRIPTION regress_testsub SET (streaming = off);
+ALTER SUBSCRIPTION regress_testsub SET (two_phase = on);
+ALTER SUBSCRIPTION regress_testsub SET (two_phase = off);
+
+SET SESSION AUTHORIZATION 'regress_subscription_user';
+
-- fail - cannot do DROP SUBSCRIPTION inside transaction block with slot name
BEGIN;
DROP SUBSCRIPTION regress_testsub;
--
2.21.1 (Apple Git-122.3)