From e5528f708fcd75135a46146da97d338d5c763843 Mon Sep 17 00:00:00 2001 From: Ajin Cherian Date: Mon, 15 May 2023 04:34:40 -0400 Subject: [PATCH v2] Fix issue where the "copy" command does not adhere to the run_as_owner setting. Make sure that "copy" command used during tablesync also respects the run_as_owner setting. --- src/backend/replication/logical/tablesync.c | 14 +++++ src/test/subscription/t/033_run_as_table_owner.pl | 64 +++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 0c71ae9..a8d3c47 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -120,6 +120,7 @@ #include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/syscache.h" +#include "utils/usercontext.h" static bool table_states_valid = false; static List *table_states_not_ready = NIL; @@ -1253,6 +1254,8 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) char originname[NAMEDATALEN]; RepOriginId originid; bool must_use_password; + UserContext ucxt; + bool run_as_owner; /* Check the state of the table synchronization. */ StartTransactionCommand(); @@ -1456,6 +1459,14 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) originname))); } + /* + * Make sure that the copy command runs as the table owner, unless + * the user has opted out of that behaviour. + */ + run_as_owner = MySubscription->runasowner; + if (!run_as_owner) + SwitchToUntrustedUser(rel->rd_rel->relowner, &ucxt); + /* Now do the initial data copy */ PushActiveSnapshot(GetTransactionSnapshot()); copy_table(rel); @@ -1469,6 +1480,9 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos) res->err))); walrcv_clear_result(res); + if(!run_as_owner) + RestoreUserContext(&ucxt); + table_close(rel, NoLock); /* Make the copy visible. */ diff --git a/src/test/subscription/t/033_run_as_table_owner.pl b/src/test/subscription/t/033_run_as_table_owner.pl index cabc8a7..91a95f8 100644 --- a/src/test/subscription/t/033_run_as_table_owner.pl +++ b/src/test/subscription/t/033_run_as_table_owner.pl @@ -68,6 +68,14 @@ sub revoke_superuser ALTER ROLE $role NOSUPERUSER)); } +sub grant_superuser +{ + my ($role) = @_; + $node_subscriber->safe_psql( + 'postgres', qq( + ALTER ROLE $role SUPERUSER)); +} + # 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 @@ -201,4 +209,60 @@ expect_replication("alice.unpartitioned", 3, 7, 13, "with INHERIT but not SET ROLE can replicate" ); +$node_subscriber->safe_psql( + 'postgres', qq( + DROP SUBSCRIPTION admin_sub; +)); + +grant_superuser("regress_admin"); + +# When a SUBSCRIPTION is created with run_as_owner as false, +# then replication runs as the owner of the table, and should not have +# permission to access tables that are not accessible to the owner of the +# table being replicated. +# Create a table owned by regress_admin which regress_alice does not +# have permission to access. +$node_subscriber->safe_psql( + 'postgres', qq( +SET SESSION AUTHORIZATION regress_admin; +CREATE TABLE admin_audit (i integer); +)); + +# Create a trigger on table alice.unpartitioned that writes +# to a table that regress_alice does not have permission. +$node_subscriber->safe_psql( + 'postgres', qq( +SET SESSION AUTHORIZATION regress_alice; +CREATE OR REPLACE FUNCTION alice.alice_audit() +RETURNS trigger AS +\$\$ + BEGIN + insert into public.admin_audit values(2); + RETURN NEW; + END; +\$\$ +LANGUAGE 'plpgsql'; +CREATE TRIGGER ALICE_TRIGGER AFTER INSERT ON alice.unpartitioned FOR EACH ROW +EXECUTE PROCEDURE alice.alice_audit(); +ALTER TABLE alice.unpartitioned ENABLE ALWAYS TRIGGER ALICE_TRIGGER; +)); + +# Create subscription with run_as_owner = false. +$node_subscriber->safe_psql( + 'postgres', qq( +SET SESSION AUTHORIZATION regress_admin; +CREATE SUBSCRIPTION admin_sub CONNECTION '$publisher_connstr' PUBLICATION alice + WITH (run_as_owner = false, password_required = false); +)); + +# This should fail in the copy stage of the tablesync. +expect_failure( + "admin_audit", + 0, + '', + '', + qr/ERROR: ( [A-Z0-9]+:)? permission denied for table admin_audit/msi, + "with run_as_owner false cannot access table without permission" +); + done_testing(); -- 1.8.3.1