aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--doc/src/sgml/logical-replication.sgml25
-rw-r--r--src/backend/commands/tablecmds.c20
-rw-r--r--src/backend/replication/logical/worker.c22
-rw-r--r--src/backend/utils/init/Makefile3
-rw-r--r--src/backend/utils/init/meson.build3
-rw-r--r--src/backend/utils/init/usercontext.c92
-rw-r--r--src/include/commands/tablecmds.h3
-rw-r--r--src/include/utils/usercontext.h26
-rw-r--r--src/test/subscription/t/027_nosuperuser.pl165
9 files changed, 250 insertions, 109 deletions
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 10ada41d803..90f39676632 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -1730,19 +1730,6 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
<title>Security</title>
<para>
- A user able to modify the schema of subscriber-side tables can execute
- arbitrary code as the role which owns any subscription which modifies those tables. Limit ownership
- and <literal>TRIGGER</literal> privilege on such tables to trusted roles.
- Moreover, if untrusted users can create tables, use only
- publications that list tables explicitly. That is to say, create a
- subscription
- <link linkend="sql-createpublication-for-all-tables"><literal>FOR ALL TABLES</literal></link>
- or <link linkend="sql-createpublication-for-tables-in-schema"><literal>FOR TABLES IN SCHEMA</literal></link>
- only when superusers trust every user permitted to create a non-temp table
- on the publisher or the subscriber.
- </para>
-
- <para>
The role used for the replication connection must have
the <literal>REPLICATION</literal> attribute (or be a superuser). If the
role lacks <literal>SUPERUSER</literal> and <literal>BYPASSRLS</literal>,
@@ -1784,12 +1771,18 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
</para>
<para>
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must have the privileges of the
+ the <literal>pg_create_subscription</literal> role, as well as
+ <literal>CREATE</literal> privileges on the database.
</para>
<para>
- The subscription apply process will run in the local database with the
- privileges of the subscription owner.
+ The subscription apply process will, at a session level, run with the
+ privileges of the subscription owner. However, when performing an insert,
+ update, delete, or truncate operation on a particular table, it will switch
+ roles to the table owner and perform the operation with the table owner's
+ privileges. This means that the subscription owner needs to be able to
+ <literal>SET ROLE</literal> to each role that owns a replicated table.
</para>
<para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7808241d3fd..d9bbeafd82c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -103,6 +103,7 @@
#include "utils/syscache.h"
#include "utils/timestamp.h"
#include "utils/typcache.h"
+#include "utils/usercontext.h"
/*
* ON COMMIT action list
@@ -1762,7 +1763,7 @@ ExecuteTruncate(TruncateStmt *stmt)
}
ExecuteTruncateGuts(rels, relids, relids_logged,
- stmt->behavior, stmt->restart_seqs);
+ stmt->behavior, stmt->restart_seqs, false);
/* And close the rels */
foreach(cell, rels)
@@ -1790,7 +1791,8 @@ void
ExecuteTruncateGuts(List *explicit_rels,
List *relids,
List *relids_logged,
- DropBehavior behavior, bool restart_seqs)
+ DropBehavior behavior, bool restart_seqs,
+ bool run_as_table_owner)
{
List *rels;
List *seq_relids = NIL;
@@ -1929,7 +1931,14 @@ ExecuteTruncateGuts(List *explicit_rels,
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
+ UserContext ucxt;
+
+ if (run_as_table_owner)
+ SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
+ &ucxt);
ExecBSTruncateTriggers(estate, resultRelInfo);
+ if (run_as_table_owner)
+ RestoreUserContext(&ucxt);
resultRelInfo++;
}
@@ -2134,7 +2143,14 @@ ExecuteTruncateGuts(List *explicit_rels,
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
+ UserContext ucxt;
+
+ if (run_as_table_owner)
+ SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
+ &ucxt);
ExecASTruncateTriggers(estate, resultRelInfo);
+ if (run_as_table_owner)
+ RestoreUserContext(&ucxt);
resultRelInfo++;
}
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index ef2a6beb361..61009fa8cda 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -207,6 +207,7 @@
#include "utils/rls.h"
#include "utils/syscache.h"
#include "utils/timeout.h"
+#include "utils/usercontext.h"
#define NAPTIME_PER_CYCLE 1000 /* max sleep time between cycles (1s) */
@@ -2395,6 +2396,7 @@ apply_handle_insert(StringInfo s)
LogicalRepRelMapEntry *rel;
LogicalRepTupleData newtup;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
TupleTableSlot *remoteslot;
@@ -2423,6 +2425,9 @@ apply_handle_insert(StringInfo s)
return;
}
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Set relation for error callback */
apply_error_callback_arg.rel = rel;
@@ -2452,6 +2457,8 @@ apply_handle_insert(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -2530,6 +2537,7 @@ apply_handle_update(StringInfo s)
{
LogicalRepRelMapEntry *rel;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
LogicalRepTupleData oldtup;
@@ -2569,6 +2577,9 @@ apply_handle_update(StringInfo s)
/* Check if we can do the update. */
check_relation_updatable(rel);
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Initialize the executor state. */
edata = create_edata_for_relation(rel);
estate = edata->estate;
@@ -2619,6 +2630,8 @@ apply_handle_update(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -2702,6 +2715,7 @@ apply_handle_delete(StringInfo s)
LogicalRepRelMapEntry *rel;
LogicalRepTupleData oldtup;
LogicalRepRelId relid;
+ UserContext ucxt;
ApplyExecutionData *edata;
EState *estate;
TupleTableSlot *remoteslot;
@@ -2736,6 +2750,9 @@ apply_handle_delete(StringInfo s)
/* Check if we can do the delete. */
check_relation_updatable(rel);
+ /* Make sure that any user-supplied code runs as the table owner. */
+ SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
+
/* Initialize the executor state. */
edata = create_edata_for_relation(rel);
estate = edata->estate;
@@ -2761,6 +2778,8 @@ apply_handle_delete(StringInfo s)
/* Reset relation for error callback */
apply_error_callback_arg.rel = NULL;
+ RestoreUserContext(&ucxt);
+
logicalrep_rel_close(rel, NoLock);
end_replication_step();
@@ -3211,7 +3230,8 @@ apply_handle_truncate(StringInfo s)
relids,
relids_logged,
DROP_RESTRICT,
- restart_seqs);
+ restart_seqs,
+ true);
foreach(lc, remote_rels)
{
LogicalRepRelMapEntry *rel = lfirst(lc);
diff --git a/src/backend/utils/init/Makefile b/src/backend/utils/init/Makefile
index 362569393b5..18c947464f4 100644
--- a/src/backend/utils/init/Makefile
+++ b/src/backend/utils/init/Makefile
@@ -15,6 +15,7 @@ include $(top_builddir)/src/Makefile.global
OBJS = \
globals.o \
miscinit.o \
- postinit.o
+ postinit.o \
+ usercontext.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/utils/init/meson.build b/src/backend/utils/init/meson.build
index b5ab154055e..186be1381d8 100644
--- a/src/backend/utils/init/meson.build
+++ b/src/backend/utils/init/meson.build
@@ -3,4 +3,5 @@
backend_sources += files(
'globals.c',
'miscinit.c',
- 'postinit.c')
+ 'postinit.c',
+ 'usercontext.c')
diff --git a/src/backend/utils/init/usercontext.c b/src/backend/utils/init/usercontext.c
new file mode 100644
index 00000000000..88a7e55478e
--- /dev/null
+++ b/src/backend/utils/init/usercontext.c
@@ -0,0 +1,92 @@
+/*-------------------------------------------------------------------------
+ *
+ * usercontext.c
+ * Convenience functions for running code as a different database user.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/utils/init/usercontext.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "miscadmin.h"
+#include "utils/acl.h"
+#include "utils/guc.h"
+#include "utils/usercontext.h"
+
+/*
+ * Temporarily switch to a new user ID.
+ *
+ * If the current user doesn't have permission to SET ROLE to the new user,
+ * an ERROR occurs.
+ *
+ * If the new user doesn't have permission to SET ROLE to the current user,
+ * SECURITY_RESTRICTED_OPERATION is imposed and a new GUC nest level is
+ * created so that any settings changes can be rolled back.
+ */
+void
+SwitchToUntrustedUser(Oid userid, UserContext *context)
+{
+ /* Get the current user ID and security context. */
+ GetUserIdAndSecContext(&context->save_userid,
+ &context->save_sec_context);
+
+ /* Check that we have sufficient privileges to assume the target role. */
+ if (!member_can_set_role(context->save_userid, userid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("role \"%s\" cannot SET ROLE to \"%s\"",
+ GetUserNameFromId(context->save_userid, false),
+ GetUserNameFromId(userid, false))));
+
+ /*
+ * Try to prevent the user to which we're switching from assuming the
+ * privileges of the current user, unless they can SET ROLE to that user
+ * anyway.
+ */
+ if (member_can_set_role(userid, context->save_userid))
+ {
+ /*
+ * Each user can SET ROLE to the other, so there's no point in
+ * imposing any security restrictions. Just let the user do whatever
+ * they want.
+ */
+ SetUserIdAndSecContext(userid, context->save_sec_context);
+ context->save_nestlevel = -1;
+ }
+ else
+ {
+ int sec_context = context->save_sec_context;
+
+ /*
+ * This user can SET ROLE to the target user, but not the other way
+ * around, so protect ourselves against the target user by setting
+ * SECURITY_RESTRICTED_OPERATION to prevent certain changes to the
+ * session state. Also set up a new GUC nest level, so that we can roll
+ * back any GUC changes that may be made by code running as the target
+ * user, inasmuch as they could be malicious.
+ */
+ sec_context |= SECURITY_RESTRICTED_OPERATION;
+ SetUserIdAndSecContext(userid, sec_context);
+ context->save_nestlevel = NewGUCNestLevel();
+ }
+}
+
+/*
+ * Switch back to the original user ID.
+ *
+ * If we created a new GUC nest level, also role back any changes that were
+ * made within it.
+ */
+void
+RestoreUserContext(UserContext *context)
+{
+ if (context->save_nestlevel != -1)
+ AtEOXact_GUC(false, context->save_nestlevel);
+ SetUserIdAndSecContext(context->save_userid, context->save_sec_context);
+}
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index e7c2b91a583..17b94049371 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -60,7 +60,8 @@ extern void ExecuteTruncateGuts(List *explicit_rels,
List *relids,
List *relids_logged,
DropBehavior behavior,
- bool restart_seqs);
+ bool restart_seqs,
+ bool run_as_table_owner);
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
diff --git a/src/include/utils/usercontext.h b/src/include/utils/usercontext.h
new file mode 100644
index 00000000000..a8195c194de
--- /dev/null
+++ b/src/include/utils/usercontext.h
@@ -0,0 +1,26 @@
+/*-------------------------------------------------------------------------
+ *
+ * usercontext.h
+ * Convenience functions for running code as a different database user.
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef USERCONTEXT_H
+#define USERCONTEXT_H
+
+/*
+ * When temporarily changing to run as a different user, this structure
+ * holds the details needed to restore the original state.
+ */
+typedef struct UserContext
+{
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
+} UserContext;
+
+/* Function prototypes. */
+extern void SwitchToUntrustedUser(Oid userid, UserContext *context);
+extern void RestoreUserContext(UserContext *context);
+
+#endif /* USERCONTEXT_H */
diff --git a/src/test/subscription/t/027_nosuperuser.pl b/src/test/subscription/t/027_nosuperuser.pl
index e770e0615c9..8a7e79cacac 100644
--- a/src/test/subscription/t/027_nosuperuser.pl
+++ b/src/test/subscription/t/027_nosuperuser.pl
@@ -76,22 +76,6 @@ sub grant_superuser
ALTER ROLE $role SUPERUSER));
}
-sub revoke_bypassrls
-{
- my ($role) = @_;
- $node_subscriber->safe_psql(
- 'postgres', qq(
- ALTER ROLE $role NOBYPASSRLS));
-}
-
-sub grant_bypassrls
-{
- my ($role) = @_;
- $node_subscriber->safe_psql(
- 'postgres', qq(
- ALTER ROLE $role BYPASSRLS));
-}
-
# Create publisher and subscriber nodes with schemas owned and published by
# "regress_alice" but subscribed and replicated by different role
# "regress_admin". For partitioned tables, layout the partitions differently
@@ -177,59 +161,32 @@ expect_failure(
2,
5,
7,
- qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ qr/ERROR: ( [A-Z0-9]+:)? role "regress_admin" cannot SET ROLE to "regress_alice"/msi,
"non-superuser admin fails to replicate update");
grant_superuser("regress_admin");
expect_replication("alice.unpartitioned", 2, 7, 9,
"admin with restored superuser privilege replicates update");
-# Grant INSERT, UPDATE, DELETE privileges on the target tables to
-# "regress_admin" so that superuser privileges are not necessary for
-# replication.
-#
-# Note that UPDATE and DELETE also require SELECT privileges, which
-# will be granted in subsequent test.
-#
+# Privileges on the target role suffice for non-superuser replication.
$node_subscriber->safe_psql(
'postgres', qq(
ALTER ROLE regress_admin NOSUPERUSER;
-SET SESSION AUTHORIZATION regress_alice;
-GRANT INSERT,UPDATE,DELETE ON
- alice.unpartitioned,
- alice.hashpart, alice.hashpart_a, alice.hashpart_b
- TO regress_admin;
-REVOKE SELECT ON alice.unpartitioned FROM regress_admin;
+GRANT regress_alice TO regress_admin;
));
publish_insert("alice.unpartitioned", 11);
expect_replication("alice.unpartitioned", 3, 7, 11,
- "nosuperuser admin with INSERT privileges can replicate into unpartitioned"
+ "nosuperuser admin with privileges on role can replicate INSERT into unpartitioned"
);
publish_update("alice.unpartitioned", 7 => 13);
-expect_failure(
- "alice.unpartitioned",
- 3,
- 7,
- 11,
- qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
- "non-superuser admin without SELECT privileges fails to replicate update"
+expect_replication("alice.unpartitioned", 3, 9, 13,
+ "nosuperuser admin with privileges on role can replicate UPDATE into unpartitioned"
);
-# Now grant SELECT
-#
-$node_subscriber->safe_psql(
- 'postgres', qq(
-SET SESSION AUTHORIZATION regress_alice;
-GRANT SELECT ON
- alice.unpartitioned,
- alice.hashpart, alice.hashpart_a, alice.hashpart_b
- TO regress_admin;
-));
-
publish_delete("alice.unpartitioned", 9);
expect_replication("alice.unpartitioned", 2, 11, 13,
- "nosuperuser admin with all table privileges can replicate into unpartitioned"
+ "nosuperuser admin with privileges on role can replicate DELETE into unpartitioned"
);
# Test partitioning
@@ -240,80 +197,114 @@ publish_insert("alice.hashpart", 103);
publish_update("alice.hashpart", 102 => 120);
publish_delete("alice.hashpart", 101);
expect_replication("alice.hashpart", 2, 103, 120,
- "nosuperuser admin with all table privileges can replicate into hashpart"
+ "nosuperuser admin with privileges on role can replicate into hashpart"
);
-
-# Enable RLS on the target table and check that "regress_admin" can
-# only replicate into it when superuser or bypassrls.
-#
+# Force RLS on the target table and check that replication fails.
$node_subscriber->safe_psql(
'postgres', qq(
SET SESSION AUTHORIZATION regress_alice;
ALTER TABLE alice.unpartitioned ENABLE ROW LEVEL SECURITY;
+ALTER TABLE alice.unpartitioned FORCE ROW LEVEL SECURITY;
));
-revoke_superuser("regress_admin");
publish_insert("alice.unpartitioned", 15);
expect_failure(
"alice.unpartitioned",
2,
11,
13,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin fails to replicate insert into rls enabled table");
-grant_superuser("regress_admin");
+ qr/ERROR: ( [A-Z0-9]+:)? user "regress_alice" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
+ "replication of insert into table with forced rls fails");
+
+# Since replication acts as the table owner, replication will succeed if we don't force it.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned NO FORCE ROW LEVEL SECURITY;
+));
expect_replication("alice.unpartitioned", 3, 11, 15,
- "admin with restored superuser privilege replicates insert into rls enabled unpartitioned"
+ "non-superuser admin can replicate insert if rls is not forced"
);
-revoke_superuser("regress_admin");
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned FORCE ROW LEVEL SECURITY;
+));
publish_update("alice.unpartitioned", 11 => 17);
expect_failure(
"alice.unpartitioned",
3,
11,
15,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin fails to replicate update into rls enabled unpartitioned"
+ qr/ERROR: ( [A-Z0-9]+:)? user "regress_alice" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
+ "replication of update into table with forced rls fails"
);
-
-grant_bypassrls("regress_admin");
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+ALTER TABLE alice.unpartitioned NO FORCE ROW LEVEL SECURITY;
+));
expect_replication("alice.unpartitioned", 3, 13, 17,
- "admin with bypassrls replicates update into rls enabled unpartitioned");
+ "non-superuser admin can replicate update if rls is not forced");
-revoke_bypassrls("regress_admin");
-publish_delete("alice.unpartitioned", 13);
+# Remove some of alice's privileges on her own table. Then replication should fail.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+REVOKE SELECT, INSERT ON alice.unpartitioned FROM regress_alice;
+));
+publish_insert("alice.unpartitioned", 19);
expect_failure(
"alice.unpartitioned",
3,
13,
17,
- qr/ERROR: ( [A-Z0-9]+:)? user "regress_admin" cannot replicate into relation with row-level security enabled: "unpartitioned\w*"/msi,
- "non-superuser admin without bypassrls fails to replicate delete into rls enabled unpartitioned"
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of insert fails if table owner lacks insert permission"
);
-grant_bypassrls("regress_admin");
-expect_replication("alice.unpartitioned", 2, 15, 17,
- "admin with bypassrls replicates delete into rls enabled unpartitioned");
-grant_superuser("regress_admin");
-# Alter the subscription owner to "regress_alice". She has neither superuser
-# nor bypassrls, but as the table owner should be able to replicate.
-#
+# alice needs INSERT but not SELECT to replicate an INSERT.
$node_subscriber->safe_psql(
'postgres', qq(
-ALTER SUBSCRIPTION admin_sub DISABLE;
-ALTER ROLE regress_alice SUPERUSER;
-ALTER SUBSCRIPTION admin_sub OWNER TO regress_alice;
-ALTER ROLE regress_alice NOSUPERUSER;
-ALTER SUBSCRIPTION admin_sub ENABLE;
+GRANT INSERT ON alice.unpartitioned TO regress_alice;
));
+expect_replication("alice.unpartitioned", 4, 13, 19,
+ "restoring insert permission permits replication to continue");
-publish_insert("alice.unpartitioned", 23);
-publish_update("alice.unpartitioned", 15 => 25);
-publish_delete("alice.unpartitioned", 17);
-expect_replication("alice.unpartitioned", 2, 23, 25,
- "nosuperuser nobypassrls table owner can replicate delete into unpartitioned despite rls"
+# Now let's try an UPDATE and a DELETE.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+REVOKE UPDATE, DELETE ON alice.unpartitioned FROM regress_alice;
+));
+publish_update("alice.unpartitioned", 13 => 21);
+publish_delete("alice.unpartitioned", 15);
+expect_failure(
+ "alice.unpartitioned",
+ 4,
+ 13,
+ 19,
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of update/delete fails if table owner lacks corresponding permission"
);
+# Restoring UPDATE and DELETE is insufficient.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+GRANT UPDATE, DELETE ON alice.unpartitioned TO regress_alice;
+));
+expect_failure(
+ "alice.unpartitioned",
+ 4,
+ 13,
+ 19,
+ qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+ "replication of update/delete fails if table owner lacks SELECT permission"
+);
+
+# alice needs INSERT but not SELECT to replicate an INSERT.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+GRANT SELECT ON alice.unpartitioned TO regress_alice;
+));
+expect_replication("alice.unpartitioned", 3, 17, 21,
+ "restoring SELECT permission permits replication to continue");
+
done_testing();