Hi!
> Currently, user with pg_subscription_users can create subscription into any
> system table, can't they?
> We certainly need to change it to more secure way.
No, you can't add system tables to publication. In new patch i add privileges
checks on target table, non superuser can't create/refresh subscription if he
don't have INSERT, UPDATE, DELETE and TRUNCATE privileges.
--------
Efimkin Evgeny
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..c2c7241084 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -201,9 +201,10 @@
<para>
Subscriptions are dumped by <command>pg_dump</command> if the current user
- is a superuser. Otherwise a warning is written and subscriptions are
- skipped, because non-superusers cannot read all subscription information
- from the <structname>pg_subscription</structname> catalog.
+ is a superuser or member of role pg_subscription_users, other users should
+ use <literal>no-subscriptions</literal> because non-superusers cannot read
+ all subscription information from the <structname>pg_subscription</structname>
+ catalog.
</para>
<para>
@@ -522,7 +523,10 @@
</para>
<para>
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must be a superuser or be member of role
+ <literal>pg_subscription_users</literal> and have <literal>INSERT </literal>,
+ <literal>UPDATE</literal>, <literal>DELETE</literal>, <literal>TRUNCATE</literal>
+ privileges on target table.
</para>
<para>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 6106244d32..9c23a6280d 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -556,6 +556,10 @@ DROP ROLE doomed_role;
<literal>pg_read_all_stats</literal> and
<literal>pg_stat_scan_tables</literal>.</entry>
</row>
+ <row>
+ <entry>pg_subscription_users</entry>
+ <entry>Allow create/drop subscriptions and be owner of subscription. Read pg_subscription</entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -579,6 +583,12 @@ DROP ROLE doomed_role;
other system information normally restricted to superusers.
</para>
+ <para>
+ The <literal>pg_subscription_users</literal> role are intended to allow
+ administrators trusted, but non-superuser, which are able to create/drop subscription.
+ </para>
+
+
<para>
Care should be taken when granting these roles to ensure they are only used where
needed and with the understanding that these roles grant access to privileged
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index afee2838cc..5483a65376 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -242,12 +242,16 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
XLogRecPtr sublsn)
{
Relation rel;
+ Relation target_rel;
HeapTuple tup;
bool nulls[Natts_pg_subscription_rel];
Datum values[Natts_pg_subscription_rel];
+ AclResult aclresult;
+ AclMode aclmask;
LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock);
+
rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);
/* Try finding existing mapping. */
@@ -258,6 +262,14 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
elog(ERROR, "subscription table %u in subscription %u already exists",
relid, subid);
+ /* Check permission on target table. */
+ aclmask = ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
+ target_rel = table_open(relid, NoLock);
+ aclresult = pg_class_aclcheck(RelationGetRelid(target_rel), GetUserId(), aclmask);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(target_rel->rd_rel->relkind),
+ RelationGetRelationName(target_rel));
+
/* Form the tuple. */
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
@@ -278,6 +290,7 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
/* Cleanup. */
table_close(rel, NoLock);
+ table_close(target_rel, NoLock);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index d962648bc5..08e58295bd 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -939,9 +939,12 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname, subowner, subenabled,
+ subslotname, subsynccommit, subpublications)
ON pg_subscription TO public;
+GRANT SELECT (subconninfo)
+ ON pg_subscription TO pg_subscription_users;
--
-- We have a few function definitions in here, too.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index a60a15193a..e1555ed6eb 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -26,6 +26,7 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/objectaddress.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
@@ -342,10 +343,10 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
if (create_slot)
PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)");
- if (!superuser())
+ if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_SUBSCRIPTION_USERS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to create subscriptions"))));
+ (errmsg("must be member of role \"pg_subscription_users\" to create subscriptions"))));
rel = table_open(SubscriptionRelationId, RowExclusiveLock);
@@ -1035,12 +1036,12 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
NameStr(form->subname));
/* New owner must be a superuser */
- if (!superuser_arg(newOwnerId))
+ if (!is_member_of_role(newOwnerId, DEFAULT_ROLE_SUBSCRIPTION_USERS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to change owner of subscription \"%s\"",
NameStr(form->subname)),
- errhint("The owner of a subscription must be a superuser.")));
+ errhint("The owner of a subscription must be member of role \"pg_subscription_users\".")));
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, &tup->t_self, tup);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4c98ae4d7f..8751801b89 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4111,7 +4111,7 @@ getSubscriptions(Archive *fout)
if (dopt->no_subscriptions || fout->remoteVersion < 100000)
return;
- if (!is_superuser(fout))
+ if (!is_superuser(fout) && fout->remoteVersion < 120000)
{
int n;
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index c21f97adcf..8f613ac9b3 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -55,6 +55,11 @@
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '4572', oid_symbol => 'DEFAULT_ROLE_SUBSCRIPTION_USERS',
+ rolname => 'pg_subscription_users', rolsuper => 'f', rolinherit => 't',
+ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+ rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+ rolpassword => '_null_', rolvaliduntil => '_null_' },
{ oid => '4200', oid_symbol => 'DEFAULT_ROLE_SIGNAL_BACKENDID',
rolname => 'pg_signal_backend', rolsuper => 'f', rolinherit => 't',
rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 4fcbf7efe9..9b48030ae1 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -4,6 +4,8 @@
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
CREATE ROLE regress_subscription_user2;
CREATE ROLE regress_subscription_user_dummy LOGIN NOSUPERUSER;
+CREATE ROLE regress_subscription_user3 LOGIN NOSUPERUSER;
+GRANT pg_subscription_users to regress_subscription_user3;
SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - no publications
CREATE SUBSCRIPTION testsub CONNECTION 'foo';
@@ -40,10 +42,10 @@ SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
-- fail - name already exists
CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
ERROR: subscription "testsub" already exists
--- fail - must be superuser
+-- fail - must be member of role "pg_subscription_users"
SET SESSION AUTHORIZATION 'regress_subscription_user2';
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
-ERROR: must be superuser to create subscriptions
+ERROR: must be member of role "pg_subscription_users" to create subscriptions
SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - invalid option combinations
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false, copy_data = true);
@@ -70,6 +72,12 @@ ALTER SUBSCRIPTION testsub3 ENABLE;
ERROR: cannot enable subscription that does not have a slot name
ALTER SUBSCRIPTION testsub3 REFRESH PUBLICATION;
ERROR: ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions
+-- ok - member of pg_subcription_users
+SET SESSION AUTHORIZATION 'regress_subscription_user3';
+CREATE SUBSCRIPTION testsub4 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (slot_name = NONE, connect = false);
+WARNING: tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
+DROP SUBSCRIPTION testsub4;
+SET SESSION AUTHORIZATION 'regress_subscription_user';
DROP SUBSCRIPTION testsub3;
-- fail - invalid connection string
ALTER SUBSCRIPTION testsub CONNECTION 'foobar';
@@ -134,10 +142,10 @@ HINT: Available values: local, remote_write, remote_apply, on, off.
-- rename back to keep the rest simple
ALTER SUBSCRIPTION testsub_foo RENAME TO testsub;
--- fail - new owner must be superuser
+-- fail - new owner must be member of role "pg_subscription_users"
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
ERROR: permission denied to change owner of subscription "testsub"
-HINT: The owner of a subscription must be a superuser.
+HINT: The owner of a subscription must be member of role "pg_subscription_users".
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
@@ -159,3 +167,5 @@ RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user2;
DROP ROLE regress_subscription_user_dummy;
+REVOKE pg_subscription_users FROM regress_subscription_user3;
+DROP ROLE regress_subscription_user3;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 36fa1bbac8..09440a9ce5 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -5,6 +5,8 @@
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
CREATE ROLE regress_subscription_user2;
CREATE ROLE regress_subscription_user_dummy LOGIN NOSUPERUSER;
+CREATE ROLE regress_subscription_user3 LOGIN NOSUPERUSER;
+GRANT pg_subscription_users to regress_subscription_user3;
SET SESSION AUTHORIZATION 'regress_subscription_user';
-- fail - no publications
@@ -33,7 +35,7 @@ SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
-- fail - name already exists
CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (connect = false);
--- fail - must be superuser
+-- fail - must be member of role "pg_subscription_users"
SET SESSION AUTHORIZATION 'regress_subscription_user2';
CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (connect = false);
SET SESSION AUTHORIZATION 'regress_subscription_user';
@@ -53,6 +55,11 @@ CREATE SUBSCRIPTION testsub3 CONNECTION 'dbname=doesnotexist' PUBLICATION testpu
-- fail
ALTER SUBSCRIPTION testsub3 ENABLE;
ALTER SUBSCRIPTION testsub3 REFRESH PUBLICATION;
+-- ok - member of pg_subcription_users
+SET SESSION AUTHORIZATION 'regress_subscription_user3';
+CREATE SUBSCRIPTION testsub4 CONNECTION 'dbname=doesnotexist' PUBLICATION foo WITH (slot_name = NONE, connect = false);
+DROP SUBSCRIPTION testsub4;
+SET SESSION AUTHORIZATION 'regress_subscription_user';
DROP SUBSCRIPTION testsub3;
@@ -96,7 +103,7 @@ ALTER SUBSCRIPTION testsub_foo SET (synchronous_commit = foobar);
-- rename back to keep the rest simple
ALTER SUBSCRIPTION testsub_foo RENAME TO testsub;
--- fail - new owner must be superuser
+-- fail - new owner must be member of role "pg_subscription_users"
ALTER SUBSCRIPTION testsub OWNER TO regress_subscription_user2;
ALTER ROLE regress_subscription_user2 SUPERUSER;
-- now it works
@@ -121,3 +128,5 @@ RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user2;
DROP ROLE regress_subscription_user_dummy;
+REVOKE pg_subscription_users FROM regress_subscription_user3;
+DROP ROLE regress_subscription_user3;
\ No newline at end of file
