Skip to content

Commit 1574783

Browse files
sfrostsfrost
committed
Use GRANT system to manage access to sensitive functions
Now that pg_dump will properly dump out any ACL changes made to functions which exist in pg_catalog, switch to using the GRANT system to manage access to those functions. This means removing 'if (!superuser()) ereport()' checks from the functions themselves and then REVOKEing EXECUTE right from 'public' for these functions in system_views.sql. Reviews by Alexander Korotkov, Jose Luis Tallon
1 parent 23f34fa commit 1574783

File tree

7 files changed

+81
-75
lines changed

7 files changed

+81
-75
lines changed

‎doc/src/sgml/backup.sgml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,9 @@ test ! -f /mnt/server/archivedir/00000001000000A900000065 && cp pg_xlog/
826826
</listitem>
827827
<listitem>
828828
<para>
829-
Connect to the database as a superuser and issue the command:
829+
Connect to the database as a user with rights to run pg_start_backup
830+
(superuser, or a user who has been granted EXECUTE on the function)
831+
and issue the command:
830832
<programlisting>
831833
SELECT pg_start_backup('label');
832834
</programlisting>
@@ -877,7 +879,9 @@ SELECT pg_start_backup('label', true);
877879
</listitem>
878880
<listitem>
879881
<para>
880-
Again connect to the database as a superuser, and issue the command:
882+
Again connect to the database as a user with rights to run
883+
pg_stop_backup (superuser, or a user who has been granted EXECUTE on
884+
the function), and issue the command:
881885
<programlisting>
882886
SELECT pg_stop_backup();
883887
</programlisting>

‎doc/src/sgml/func.sgml‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17339,8 +17339,9 @@ SELECT set_config('log_statement_stats', 'off', false);
1733917339
<para>
1734017340
The functions shown in <xref
1734117341
linkend="functions-admin-signal-table"> send control signals to
17342-
other server processes. Use of these functions is usually restricted
17343-
to superusers, with noted exceptions.
17342+
other server processes. Use of these functions is restricted to
17343+
superusers by default but access may be granted to others with the
17344+
<command>GRANT</command>, with noted exceptions.
1734417345
</para>
1734517346

1734617347
<table id="functions-admin-signal-table">
@@ -17490,7 +17491,7 @@ SELECT set_config('log_statement_stats', 'off', false);
1749017491
<literal><function>pg_create_restore_point(<parameter>name</> <type>text</>)</function></literal>
1749117492
</entry>
1749217493
<entry><type>pg_lsn</type></entry>
17493-
<entry>Create a named point for performing restore (restricted to superusers)</entry>
17494+
<entry>Create a named point for performing restore (restricted to superusers by default, but other users can be granted EXECUTE to run the function)</entry>
1749417495
</row>
1749517496
<row>
1749617497
<entry>
@@ -17518,21 +17519,21 @@ SELECT set_config('log_statement_stats', 'off', false);
1751817519
<literal><function>pg_start_backup(<parameter>label</> <type>text</> <optional>, <parameter>fast</> <type>boolean</> <optional>, <parameter>exclusive</> <type>boolean</> </optional></optional>)</function></literal>
1751917520
</entry>
1752017521
<entry><type>pg_lsn</type></entry>
17521-
<entry>Prepare for performing on-line backup (restricted to superusers or replication roles)</entry>
17522+
<entry>Prepare for performing on-line backup (restricted to superusers by default, but other users can be granted EXECUTE to run the function)</entry>
1752217523
</row>
1752317524
<row>
1752417525
<entry>
1752517526
<literal><function>pg_stop_backup()</function></literal>
1752617527
</entry>
1752717528
<entry><type>pg_lsn</type></entry>
17528-
<entry>Finish performing exclusive on-line backup (restricted to superusers or replication roles)</entry>
17529+
<entry>Finish performing exclusive on-line backup (restricted to superusers by default, but other users can be granted EXECUTE to run the function)</entry>
1752917530
</row>
1753017531
<row>
1753117532
<entry>
1753217533
<literal><function>pg_stop_backup(<parameter>exclusive</> <type>boolean</>)</function></literal>
1753317534
</entry>
1753417535
<entry><type>setof record</type></entry>
17535-
<entry>Finish performing exclusive or non-exclusive on-line backup (restricted to superusers or replication roles)</entry>
17536+
<entry>Finish performing exclusive or non-exclusive on-line backup (restricted to superusers by default, but other users can be granted EXECUTE to run the function)</entry>
1753617537
</row>
1753717538
<row>
1753817539
<entry>
@@ -17553,7 +17554,7 @@ SELECT set_config('log_statement_stats', 'off', false);
1755317554
<literal><function>pg_switch_xlog()</function></literal>
1755417555
</entry>
1755517556
<entry><type>pg_lsn</type></entry>
17556-
<entry>Force switch to a new transaction log file (restricted to superusers)</entry>
17557+
<entry>Force switch to a new transaction log file (restricted to superusers by default, but other users can be granted EXECUTE to run the function)</entry>
1755717558
</row>
1755817559
<row>
1755917560
<entry>
@@ -17821,15 +17822,15 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
1782117822
<literal><function>pg_xlog_replay_pause()</function></literal>
1782217823
</entry>
1782317824
<entry><type>void</type></entry>
17824-
<entry>Pauses recovery immediately (restricted to superusers).
17825+
<entry>Pauses recovery immediately (restricted to superusers by default, but other users can be granted EXECUTE to run the function).
1782517826
</entry>
1782617827
</row>
1782717828
<row>
1782817829
<entry>
1782917830
<literal><function>pg_xlog_replay_resume()</function></literal>
1783017831
</entry>
1783117832
<entry><type>void</type></entry>
17832-
<entry>Restarts recovery if it was paused (restricted to superusers).
17833+
<entry>Restarts recovery if it was paused (restricted to superusers by default, but other users can be granted EXECUTE to run the function).
1783317834
</entry>
1783417835
</row>
1783517836
</tbody>

‎doc/src/sgml/monitoring.sgml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2289,7 +2289,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
22892289
<entry><type>void</type></entry>
22902290
<entry>
22912291
Reset all statistics counters for the current database to zero
2292-
(requires superuser privileges)
2292+
(requires superuser privileges by default, but EXECUTE for this
2293+
function can be granted to others.)
22932294
</entry>
22942295
</row>
22952296

@@ -2298,7 +2299,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
22982299
<entry><type>void</type></entry>
22992300
<entry>
23002301
Reset some cluster-wide statistics counters to zero, depending on the
2301-
argument (requires superuser privileges).
2302+
argument (requires superuser privileges by default, but EXECUTE for
2303+
this functiion can be granted to others).
23022304
Calling <literal>pg_stat_reset_shared('bgwriter')</> will zero all the
23032305
counters shown in the <structname>pg_stat_bgwriter</> view.
23042306
Calling <literal>pg_stat_reset_shared('archiver')</> will zero all the
@@ -2311,7 +2313,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
23112313
<entry><type>void</type></entry>
23122314
<entry>
23132315
Reset statistics for a single table or index in the current database to
2314-
zero (requires superuser privileges)
2316+
zero (requires superuser privileges by default, but EXECUTE for this
2317+
function can be granted to others)
23152318
</entry>
23162319
</row>
23172320

@@ -2320,7 +2323,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
23202323
<entry><type>void</type></entry>
23212324
<entry>
23222325
Reset statistics for a single function in the current database to
2323-
zero (requires superuser privileges)
2326+
zero (requires superuser privileges by default, but EXECUTE for this
2327+
function can be granted to others)
23242328
</entry>
23252329
</row>
23262330
</tbody>

‎src/backend/access/transam/xlogfuncs.c‎

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ nonexclusive_base_backup_cleanup(int code, Datum arg)
6666
* contains the user-supplied label string (typically this would be used
6767
* to tell where the backup dump will be stored) and the starting time and
6868
* starting WAL location for the dump.
69+
*
70+
* Permission checking for this function is managed through the normal
71+
* GRANT system.
6972
*/
7073
Datum
7174
pg_start_backup(PG_FUNCTION_ARGS)
@@ -79,11 +82,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
7982

8083
backupidstr = text_to_cstring(backupid);
8184

82-
if (!superuser() && !has_rolreplication(GetUserId()))
83-
ereport(ERROR,
84-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
85-
errmsg("must be superuser or replication role to run a backup")));
86-
8785
if (exclusive_backup_running || nonexclusive_backup_running)
8886
ereport(ERROR,
8987
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -142,17 +140,15 @@ pg_start_backup(PG_FUNCTION_ARGS)
142140
* Note: this version is only called to stop an exclusive backup. The function
143141
* pg_stop_backup_v2 (overloaded as pg_stop_backup in SQL) is called to
144142
* stop non-exclusive backups.
143+
*
144+
* Permission checking for this function is managed through the normal
145+
* GRANT system.
145146
*/
146147
Datum
147148
pg_stop_backup(PG_FUNCTION_ARGS)
148149
{
149150
XLogRecPtr stoppoint;
150151

151-
if (!superuser() && !has_rolreplication(GetUserId()))
152-
ereport(ERROR,
153-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
154-
(errmsg("must be superuser or replication role to run a backup"))));
155-
156152
if (nonexclusive_backup_running)
157153
ereport(ERROR,
158154
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -179,6 +175,9 @@ pg_stop_backup(PG_FUNCTION_ARGS)
179175
* Works the same as pg_stop_backup, except for non-exclusive backups it returns
180176
* the backup label and tablespace map files as text fields in as part of the
181177
* resultset.
178+
*
179+
* Permission checking for this function is managed through the normal
180+
* GRANT system.
182181
*/
183182
Datum
184183
pg_stop_backup_v2(PG_FUNCTION_ARGS)
@@ -205,11 +204,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
205204
errmsg("materialize mode required, but it is not " \
206205
"allowed in this context")));
207206

208-
if (!superuser() && !has_rolreplication(GetUserId()))
209-
ereport(ERROR,
210-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
211-
(errmsg("must be superuser or replication role to run a backup"))));
212-
213207
/* Build a tuple descriptor for our result type */
214208
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
215209
elog(ERROR, "return type must be a row type");
@@ -285,17 +279,15 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
285279

286280
/*
287281
* pg_switch_xlog: switch to next xlog file
282+
*
283+
* Permission checking for this function is managed through the normal
284+
* GRANT system.
288285
*/
289286
Datum
290287
pg_switch_xlog(PG_FUNCTION_ARGS)
291288
{
292289
XLogRecPtr switchpoint;
293290

294-
if (!superuser())
295-
ereport(ERROR,
296-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
297-
(errmsg("must be superuser to switch transaction log files"))));
298-
299291
if (RecoveryInProgress())
300292
ereport(ERROR,
301293
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -312,6 +304,9 @@ pg_switch_xlog(PG_FUNCTION_ARGS)
312304

313305
/*
314306
* pg_create_restore_point: a named point for restore
307+
*
308+
* Permission checking for this function is managed through the normal
309+
* GRANT system.
315310
*/
316311
Datum
317312
pg_create_restore_point(PG_FUNCTION_ARGS)
@@ -320,11 +315,6 @@ pg_create_restore_point(PG_FUNCTION_ARGS)
320315
char *restore_name_str;
321316
XLogRecPtr restorepoint;
322317

323-
if (!superuser())
324-
ereport(ERROR,
325-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
326-
(errmsg("must be superuser to create a restore point"))));
327-
328318
if (RecoveryInProgress())
329319
ereport(ERROR,
330320
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -546,15 +536,13 @@ pg_xlogfile_name(PG_FUNCTION_ARGS)
546536

547537
/*
548538
* pg_xlog_replay_pause - pause recovery now
539+
*
540+
* Permission checking for this function is managed through the normal
541+
* GRANT system.
549542
*/
550543
Datum
551544
pg_xlog_replay_pause(PG_FUNCTION_ARGS)
552545
{
553-
if (!superuser())
554-
ereport(ERROR,
555-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
556-
(errmsg("must be superuser to control recovery"))));
557-
558546
if (!RecoveryInProgress())
559547
ereport(ERROR,
560548
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -568,15 +556,13 @@ pg_xlog_replay_pause(PG_FUNCTION_ARGS)
568556

569557
/*
570558
* pg_xlog_replay_resume - resume recovery now
559+
*
560+
* Permission checking for this function is managed through the normal
561+
* GRANT system.
571562
*/
572563
Datum
573564
pg_xlog_replay_resume(PG_FUNCTION_ARGS)
574565
{
575-
if (!superuser())
576-
ereport(ERROR,
577-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
578-
(errmsg("must be superuser to control recovery"))));
579-
580566
if (!RecoveryInProgress())
581567
ereport(ERROR,
582568
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

‎src/backend/catalog/system_views.sql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,3 +1005,24 @@ RETURNS jsonb
10051005
LANGUAGE INTERNAL
10061006
STRICT IMMUTABLE
10071007
AS 'jsonb_insert';
1008+
1009+
-- The default permissions for functions mean that anyone can execute them.
1010+
-- A number of functions shouldn't be executable by just anyone, but rather
1011+
-- than use explicit 'superuser()' checks in those functions, we use the GRANT
1012+
-- system to REVOKE access to those functions at initdb time. Administrators
1013+
-- can later change who can access these functions, or leave them as only
1014+
-- available to superuser / cluster owner, if they choose.
1015+
REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
1016+
REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
1017+
REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean) FROM public;
1018+
REVOKE EXECUTE ON FUNCTION pg_create_restore_point(text) FROM public;
1019+
REVOKE EXECUTE ON FUNCTION pg_switch_xlog() FROM public;
1020+
REVOKE EXECUTE ON FUNCTION pg_xlog_replay_pause() FROM public;
1021+
REVOKE EXECUTE ON FUNCTION pg_xlog_replay_resume() FROM public;
1022+
REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public;
1023+
REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public;
1024+
1025+
REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public;
1026+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
1027+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
1028+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;

‎src/backend/postmaster/pgstat.c‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,9 @@ pgstat_drop_relation(Oid relid)
12171217
* pgstat_reset_counters() -
12181218
*
12191219
* Tell the statistics collector to reset counters for our database.
1220+
*
1221+
* Permission checking for this function is managed through the normal
1222+
* GRANT system.
12201223
* ----------
12211224
*/
12221225
void
@@ -1227,11 +1230,6 @@ pgstat_reset_counters(void)
12271230
if (pgStatSock == PGINVALID_SOCKET)
12281231
return;
12291232

1230-
if (!superuser())
1231-
ereport(ERROR,
1232-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1233-
errmsg("must be superuser to reset statistics counters")));
1234-
12351233
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_RESETCOUNTER);
12361234
msg.m_databaseid = MyDatabaseId;
12371235
pgstat_send(&msg, sizeof(msg));
@@ -1241,6 +1239,9 @@ pgstat_reset_counters(void)
12411239
* pgstat_reset_shared_counters() -
12421240
*
12431241
* Tell the statistics collector to reset cluster-wide shared counters.
1242+
*
1243+
* Permission checking for this function is managed through the normal
1244+
* GRANT system.
12441245
* ----------
12451246
*/
12461247
void
@@ -1251,11 +1252,6 @@ pgstat_reset_shared_counters(const char *target)
12511252
if (pgStatSock == PGINVALID_SOCKET)
12521253
return;
12531254

1254-
if (!superuser())
1255-
ereport(ERROR,
1256-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1257-
errmsg("must be superuser to reset statistics counters")));
1258-
12591255
if (strcmp(target, "archiver") == 0)
12601256
msg.m_resettarget = RESET_ARCHIVER;
12611257
else if (strcmp(target, "bgwriter") == 0)
@@ -1274,6 +1270,9 @@ pgstat_reset_shared_counters(const char *target)
12741270
* pgstat_reset_single_counter() -
12751271
*
12761272
* Tell the statistics collector to reset a single counter.
1273+
*
1274+
* Permission checking for this function is managed through the normal
1275+
* GRANT system.
12771276
* ----------
12781277
*/
12791278
void
@@ -1284,11 +1283,6 @@ pgstat_reset_single_counter(Oid objoid, PgStat_Single_Reset_Type type)
12841283
if (pgStatSock == PGINVALID_SOCKET)
12851284
return;
12861285

1287-
if (!superuser())
1288-
ereport(ERROR,
1289-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1290-
errmsg("must be superuser to reset statistics counters")));
1291-
12921286
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_RESETSINGLECOUNTER);
12931287
msg.m_databaseid = MyDatabaseId;
12941288
msg.m_resettype = type;

0 commit comments

Comments
 (0)