Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1e10d49

Browse files
committed
Perform logical replication actions as the table owner.
Up until now, logical replication actions have been performed as the subscription owner, who will generally be a superuser. Commit cec57b1 documented hazards associated with that situation, namely, that any user who owns a table on the subscriber side could assume the privileges of the subscription owner by attaching a trigger, expression index, or some other kind of executable code to it. As a remedy, it suggested not creating configurations where users who are not fully trusted own tables on the subscriber. Although that will work, it basically precludes using logical replication in the way that people typically want to use it, namely, to replicate a database from one node to another without necessarily having any restrictions on which database users can own tables. So, instead, change logical replication to execute INSERT, UPDATE, DELETE, and TRUNCATE operations as the table owner when they are replicated. Since this involves switching the active user frequently within a session that is authenticated as the subscription user, also impose SECURITY_RESTRICTED_OPERATION restrictions on logical replication code. As an exception, if the table owner can SET ROLE to the subscription owner, these restrictions have no security value, so don't impose them in that case. Subscription owners are now required to have the ability to SET ROLE to every role that owns a table that the subscription is replicating. If they don't, replication will fail. Superusers, who normally own subscriptions, satisfy this property by default. Non-superusers users who own subscriptions will need to be granted the roles that own relevant tables. Patch by me, reviewed (but not necessarily in its entirety) by Jelte Fennema, Jeff Davis, and Noah Misch. Discussion: http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=UsYPfnOoDeFkw@mail.gmail.com
1 parent 3077324 commit 1e10d49

File tree

9 files changed

+250
-109
lines changed

9 files changed

+250
-109
lines changed

doc/src/sgml/logical-replication.sgml

+9-16
Original file line numberDiff line numberDiff line change
@@ -1729,19 +1729,6 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
17291729
<sect1 id="logical-replication-security">
17301730
<title>Security</title>
17311731

1732-
<para>
1733-
A user able to modify the schema of subscriber-side tables can execute
1734-
arbitrary code as the role which owns any subscription which modifies those tables. Limit ownership
1735-
and <literal>TRIGGER</literal> privilege on such tables to trusted roles.
1736-
Moreover, if untrusted users can create tables, use only
1737-
publications that list tables explicitly. That is to say, create a
1738-
subscription
1739-
<link linkend="sql-createpublication-for-all-tables"><literal>FOR ALL TABLES</literal></link>
1740-
or <link linkend="sql-createpublication-for-tables-in-schema"><literal>FOR TABLES IN SCHEMA</literal></link>
1741-
only when superusers trust every user permitted to create a non-temp table
1742-
on the publisher or the subscriber.
1743-
</para>
1744-
17451732
<para>
17461733
The role used for the replication connection must have
17471734
the <literal>REPLICATION</literal> attribute (or be a superuser). If the
@@ -1784,12 +1771,18 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER
17841771
</para>
17851772

17861773
<para>
1787-
To create a subscription, the user must be a superuser.
1774+
To create a subscription, the user must have the privileges of the
1775+
the <literal>pg_create_subscription</literal> role, as well as
1776+
<literal>CREATE</literal> privileges on the database.
17881777
</para>
17891778

17901779
<para>
1791-
The subscription apply process will run in the local database with the
1792-
privileges of the subscription owner.
1780+
The subscription apply process will, at a session level, run with the
1781+
privileges of the subscription owner. However, when performing an insert,
1782+
update, delete, or truncate operation on a particular table, it will switch
1783+
roles to the table owner and perform the operation with the table owner's
1784+
privileges. This means that the subscription owner needs to be able to
1785+
<literal>SET ROLE</literal> to each role that owns a replicated table.
17931786
</para>
17941787

17951788
<para>

src/backend/commands/tablecmds.c

+18-2
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
#include "utils/syscache.h"
104104
#include "utils/timestamp.h"
105105
#include "utils/typcache.h"
106+
#include "utils/usercontext.h"
106107

107108
/*
108109
* ON COMMIT action list
@@ -1762,7 +1763,7 @@ ExecuteTruncate(TruncateStmt *stmt)
17621763
}
17631764

17641765
ExecuteTruncateGuts(rels, relids, relids_logged,
1765-
stmt->behavior, stmt->restart_seqs);
1766+
stmt->behavior, stmt->restart_seqs, false);
17661767

17671768
/* And close the rels */
17681769
foreach(cell, rels)
@@ -1790,7 +1791,8 @@ void
17901791
ExecuteTruncateGuts(List *explicit_rels,
17911792
List *relids,
17921793
List *relids_logged,
1793-
DropBehavior behavior, bool restart_seqs)
1794+
DropBehavior behavior, bool restart_seqs,
1795+
bool run_as_table_owner)
17941796
{
17951797
List *rels;
17961798
List *seq_relids = NIL;
@@ -1929,7 +1931,14 @@ ExecuteTruncateGuts(List *explicit_rels,
19291931
resultRelInfo = resultRelInfos;
19301932
foreach(cell, rels)
19311933
{
1934+
UserContext ucxt;
1935+
1936+
if (run_as_table_owner)
1937+
SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
1938+
&ucxt);
19321939
ExecBSTruncateTriggers(estate, resultRelInfo);
1940+
if (run_as_table_owner)
1941+
RestoreUserContext(&ucxt);
19331942
resultRelInfo++;
19341943
}
19351944

@@ -2134,7 +2143,14 @@ ExecuteTruncateGuts(List *explicit_rels,
21342143
resultRelInfo = resultRelInfos;
21352144
foreach(cell, rels)
21362145
{
2146+
UserContext ucxt;
2147+
2148+
if (run_as_table_owner)
2149+
SwitchToUntrustedUser(resultRelInfo->ri_RelationDesc->rd_rel->relowner,
2150+
&ucxt);
21372151
ExecASTruncateTriggers(estate, resultRelInfo);
2152+
if (run_as_table_owner)
2153+
RestoreUserContext(&ucxt);
21382154
resultRelInfo++;
21392155
}
21402156

src/backend/replication/logical/worker.c

+21-1
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@
207207
#include "utils/rls.h"
208208
#include "utils/syscache.h"
209209
#include "utils/timeout.h"
210+
#include "utils/usercontext.h"
210211

211212
#define NAPTIME_PER_CYCLE 1000 /* max sleep time between cycles (1s) */
212213

@@ -2395,6 +2396,7 @@ apply_handle_insert(StringInfo s)
23952396
LogicalRepRelMapEntry *rel;
23962397
LogicalRepTupleData newtup;
23972398
LogicalRepRelId relid;
2399+
UserContext ucxt;
23982400
ApplyExecutionData *edata;
23992401
EState *estate;
24002402
TupleTableSlot *remoteslot;
@@ -2423,6 +2425,9 @@ apply_handle_insert(StringInfo s)
24232425
return;
24242426
}
24252427

2428+
/* Make sure that any user-supplied code runs as the table owner. */
2429+
SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
2430+
24262431
/* Set relation for error callback */
24272432
apply_error_callback_arg.rel = rel;
24282433

@@ -2452,6 +2457,8 @@ apply_handle_insert(StringInfo s)
24522457
/* Reset relation for error callback */
24532458
apply_error_callback_arg.rel = NULL;
24542459

2460+
RestoreUserContext(&ucxt);
2461+
24552462
logicalrep_rel_close(rel, NoLock);
24562463

24572464
end_replication_step();
@@ -2530,6 +2537,7 @@ apply_handle_update(StringInfo s)
25302537
{
25312538
LogicalRepRelMapEntry *rel;
25322539
LogicalRepRelId relid;
2540+
UserContext ucxt;
25332541
ApplyExecutionData *edata;
25342542
EState *estate;
25352543
LogicalRepTupleData oldtup;
@@ -2569,6 +2577,9 @@ apply_handle_update(StringInfo s)
25692577
/* Check if we can do the update. */
25702578
check_relation_updatable(rel);
25712579

2580+
/* Make sure that any user-supplied code runs as the table owner. */
2581+
SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
2582+
25722583
/* Initialize the executor state. */
25732584
edata = create_edata_for_relation(rel);
25742585
estate = edata->estate;
@@ -2619,6 +2630,8 @@ apply_handle_update(StringInfo s)
26192630
/* Reset relation for error callback */
26202631
apply_error_callback_arg.rel = NULL;
26212632

2633+
RestoreUserContext(&ucxt);
2634+
26222635
logicalrep_rel_close(rel, NoLock);
26232636

26242637
end_replication_step();
@@ -2702,6 +2715,7 @@ apply_handle_delete(StringInfo s)
27022715
LogicalRepRelMapEntry *rel;
27032716
LogicalRepTupleData oldtup;
27042717
LogicalRepRelId relid;
2718+
UserContext ucxt;
27052719
ApplyExecutionData *edata;
27062720
EState *estate;
27072721
TupleTableSlot *remoteslot;
@@ -2736,6 +2750,9 @@ apply_handle_delete(StringInfo s)
27362750
/* Check if we can do the delete. */
27372751
check_relation_updatable(rel);
27382752

2753+
/* Make sure that any user-supplied code runs as the table owner. */
2754+
SwitchToUntrustedUser(rel->localrel->rd_rel->relowner, &ucxt);
2755+
27392756
/* Initialize the executor state. */
27402757
edata = create_edata_for_relation(rel);
27412758
estate = edata->estate;
@@ -2761,6 +2778,8 @@ apply_handle_delete(StringInfo s)
27612778
/* Reset relation for error callback */
27622779
apply_error_callback_arg.rel = NULL;
27632780

2781+
RestoreUserContext(&ucxt);
2782+
27642783
logicalrep_rel_close(rel, NoLock);
27652784

27662785
end_replication_step();
@@ -3211,7 +3230,8 @@ apply_handle_truncate(StringInfo s)
32113230
relids,
32123231
relids_logged,
32133232
DROP_RESTRICT,
3214-
restart_seqs);
3233+
restart_seqs,
3234+
true);
32153235
foreach(lc, remote_rels)
32163236
{
32173237
LogicalRepRelMapEntry *rel = lfirst(lc);

src/backend/utils/init/Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ include $(top_builddir)/src/Makefile.global
1515
OBJS = \
1616
globals.o \
1717
miscinit.o \
18-
postinit.o
18+
postinit.o \
19+
usercontext.o
1920

2021
include $(top_srcdir)/src/backend/common.mk

src/backend/utils/init/meson.build

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
backend_sources += files(
44
'globals.c',
55
'miscinit.c',
6-
'postinit.c')
6+
'postinit.c',
7+
'usercontext.c')

src/backend/utils/init/usercontext.c

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* usercontext.c
4+
* Convenience functions for running code as a different database user.
5+
*
6+
* Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
7+
* Portions Copyright (c) 1994, Regents of the University of California
8+
*
9+
*
10+
* IDENTIFICATION
11+
* src/backend/utils/init/usercontext.c
12+
*
13+
*-------------------------------------------------------------------------
14+
*/
15+
#include "postgres.h"
16+
17+
#include "miscadmin.h"
18+
#include "utils/acl.h"
19+
#include "utils/guc.h"
20+
#include "utils/usercontext.h"
21+
22+
/*
23+
* Temporarily switch to a new user ID.
24+
*
25+
* If the current user doesn't have permission to SET ROLE to the new user,
26+
* an ERROR occurs.
27+
*
28+
* If the new user doesn't have permission to SET ROLE to the current user,
29+
* SECURITY_RESTRICTED_OPERATION is imposed and a new GUC nest level is
30+
* created so that any settings changes can be rolled back.
31+
*/
32+
void
33+
SwitchToUntrustedUser(Oid userid, UserContext *context)
34+
{
35+
/* Get the current user ID and security context. */
36+
GetUserIdAndSecContext(&context->save_userid,
37+
&context->save_sec_context);
38+
39+
/* Check that we have sufficient privileges to assume the target role. */
40+
if (!member_can_set_role(context->save_userid, userid))
41+
ereport(ERROR,
42+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
43+
errmsg("role \"%s\" cannot SET ROLE to \"%s\"",
44+
GetUserNameFromId(context->save_userid, false),
45+
GetUserNameFromId(userid, false))));
46+
47+
/*
48+
* Try to prevent the user to which we're switching from assuming the
49+
* privileges of the current user, unless they can SET ROLE to that user
50+
* anyway.
51+
*/
52+
if (member_can_set_role(userid, context->save_userid))
53+
{
54+
/*
55+
* Each user can SET ROLE to the other, so there's no point in
56+
* imposing any security restrictions. Just let the user do whatever
57+
* they want.
58+
*/
59+
SetUserIdAndSecContext(userid, context->save_sec_context);
60+
context->save_nestlevel = -1;
61+
}
62+
else
63+
{
64+
int sec_context = context->save_sec_context;
65+
66+
/*
67+
* This user can SET ROLE to the target user, but not the other way
68+
* around, so protect ourselves against the target user by setting
69+
* SECURITY_RESTRICTED_OPERATION to prevent certain changes to the
70+
* session state. Also set up a new GUC nest level, so that we can roll
71+
* back any GUC changes that may be made by code running as the target
72+
* user, inasmuch as they could be malicious.
73+
*/
74+
sec_context |= SECURITY_RESTRICTED_OPERATION;
75+
SetUserIdAndSecContext(userid, sec_context);
76+
context->save_nestlevel = NewGUCNestLevel();
77+
}
78+
}
79+
80+
/*
81+
* Switch back to the original user ID.
82+
*
83+
* If we created a new GUC nest level, also role back any changes that were
84+
* made within it.
85+
*/
86+
void
87+
RestoreUserContext(UserContext *context)
88+
{
89+
if (context->save_nestlevel != -1)
90+
AtEOXact_GUC(false, context->save_nestlevel);
91+
SetUserIdAndSecContext(context->save_userid, context->save_sec_context);
92+
}

src/include/commands/tablecmds.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ extern void ExecuteTruncateGuts(List *explicit_rels,
6060
List *relids,
6161
List *relids_logged,
6262
DropBehavior behavior,
63-
bool restart_seqs);
63+
bool restart_seqs,
64+
bool run_as_table_owner);
6465

6566
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
6667

src/include/utils/usercontext.h

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* usercontext.h
4+
* Convenience functions for running code as a different database user.
5+
*
6+
*-------------------------------------------------------------------------
7+
*/
8+
#ifndef USERCONTEXT_H
9+
#define USERCONTEXT_H
10+
11+
/*
12+
* When temporarily changing to run as a different user, this structure
13+
* holds the details needed to restore the original state.
14+
*/
15+
typedef struct UserContext
16+
{
17+
Oid save_userid;
18+
int save_sec_context;
19+
int save_nestlevel;
20+
} UserContext;
21+
22+
/* Function prototypes. */
23+
extern void SwitchToUntrustedUser(Oid userid, UserContext *context);
24+
extern void RestoreUserContext(UserContext *context);
25+
26+
#endif /* USERCONTEXT_H */

0 commit comments

Comments
 (0)