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

Commit 6198420

Browse files
committed
Use has_privs_for_roles for predefined role checks
Generally if a role is granted membership to another role with NOINHERIT they must use SET ROLE to access the privileges of that role, however with predefined roles the membership and privilege is conflated. Fix that by replacing is_member_of_role with has_privs_for_role for predefined roles. Patch does not remove is_member_of_role from acl.h, but it does add a warning not to use that function for privilege checking. Not backpatched based on hackers list discussion. Author: Joshua Brindle Reviewed-by: Stephen Frost, Nathan Bossart, Joe Conway Discussion: https://postgr.es/m/flat/CAGB+Vh4Zv_TvKt2tv3QNS6tUM_F_9icmuj0zjywwcgVi4PAhFA@mail.gmail.com
1 parent 79de984 commit 6198420

File tree

23 files changed

+68
-64
lines changed

23 files changed

+68
-64
lines changed

contrib/adminpack/adminpack.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ convert_and_check_filename(text *arg)
7979
* files on the server as the PG user, so no need to do any further checks
8080
* here.
8181
*/
82-
if (is_member_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
82+
if (has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
8383
return filename;
8484

8585
/*

contrib/file_fdw/expected/file_fdw.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ ALTER FOREIGN TABLE agg_text OWNER TO regress_file_fdw_user;
459459
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
460460
SET ROLE regress_file_fdw_user;
461461
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
462-
ERROR: only superuser or a member of the pg_read_server_files role may specify the filename option of a file_fdw foreign table
462+
ERROR: only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table
463463
SET ROLE regress_file_fdw_superuser;
464464
-- cleanup
465465
RESET ROLE;

contrib/file_fdw/file_fdw.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -269,16 +269,16 @@ file_fdw_validator(PG_FUNCTION_ARGS)
269269
* otherwise there'd still be a security hole.
270270
*/
271271
if (strcmp(def->defname, "filename") == 0 &&
272-
!is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
272+
!has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
273273
ereport(ERROR,
274274
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
275-
errmsg("only superuser or a member of the pg_read_server_files role may specify the filename option of a file_fdw foreign table")));
275+
errmsg("only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table")));
276276

277277
if (strcmp(def->defname, "program") == 0 &&
278-
!is_member_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
278+
!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
279279
ereport(ERROR,
280280
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
281-
errmsg("only superuser or a member of the pg_execute_server_program role may specify the program option of a file_fdw foreign table")));
281+
errmsg("only superuser or a role with privileges of the pg_execute_server_program role may specify the program option of a file_fdw foreign table")));
282282

283283
filename = defGetString(def);
284284
}

contrib/pg_stat_statements/pg_stat_statements.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1503,8 +1503,8 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
15031503
HASH_SEQ_STATUS hash_seq;
15041504
pgssEntry *entry;
15051505

1506-
/* Superusers or members of pg_read_all_stats members are allowed */
1507-
is_allowed_role = is_member_of_role(userid, ROLE_PG_READ_ALL_STATS);
1506+
/* Superusers or roles with the privileges of pg_read_all_stats members are allowed */
1507+
is_allowed_role = has_privs_of_role(userid, ROLE_PG_READ_ALL_STATS);
15081508

15091509
/* hash table must exist already */
15101510
if (!pgss || !pgss_hash)

contrib/pgrowlocks/pgrowlocks.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pgrowlocks(PG_FUNCTION_ARGS)
104104
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
105105
ACL_SELECT);
106106
if (aclresult != ACLCHECK_OK)
107-
aclresult = is_member_of_role(GetUserId(), ROLE_PG_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
107+
aclresult = has_privs_of_role(GetUserId(), ROLE_PG_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
108108

109109
if (aclresult != ACLCHECK_OK)
110110
aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),

doc/src/sgml/adminpack.sgml

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
functions in <xref linkend="functions-admin-genfile-table"/>, which
2323
provide read-only access.)
2424
Only files within the database cluster directory can be accessed, unless the
25-
user is a superuser or given one of the pg_read_server_files, or pg_write_server_files
26-
roles, as appropriate for the function, but either a relative or absolute path is
27-
allowable.
25+
user is a superuser or given privileges of one of the pg_read_server_files,
26+
or pg_write_server_files roles, as appropriate for the function, but either a
27+
relative or absolute path is allowable.
2828
</para>
2929

3030
<table id="functions-adminpack-table">

doc/src/sgml/catalogs.sgml

+6-6
Original file line numberDiff line numberDiff line change
@@ -10044,8 +10044,8 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
1004410044

1004510045
<para>
1004610046
By default, the <structname>pg_backend_memory_contexts</structname> view can be
10047-
read only by superusers or members of the <literal>pg_read_all_stats</literal>
10048-
role.
10047+
read only by superusers or roles with the privileges of the
10048+
<literal>pg_read_all_stats</literal> role.
1004910049
</para>
1005010050
</sect1>
1005110051

@@ -12552,7 +12552,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
1255212552
<para>
1255312553
Configuration file the current value was set in (null for
1255412554
values set from sources other than configuration files, or when
12555-
examined by a user who is neither a superuser or a member of
12555+
examined by a user who neither is a superuser nor has privileges of
1255612556
<literal>pg_read_all_settings</literal>); helpful when using
1255712557
<literal>include</literal> directives in configuration files
1255812558
</para></entry>
@@ -12565,7 +12565,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
1256512565
<para>
1256612566
Line number within the configuration file the current value was
1256712567
set at (null for values set from sources other than configuration files,
12568-
or when examined by a user who is neither a superuser or a member of
12568+
or when examined by a user who neither is a superuser nor has privileges of
1256912569
<literal>pg_read_all_settings</literal>).
1257012570
</para></entry>
1257112571
</row>
@@ -12941,8 +12941,8 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
1294112941

1294212942
<para>
1294312943
By default, the <structname>pg_shmem_allocations</structname> view can be
12944-
read only by superusers or members of the <literal>pg_read_all_stats</literal>
12945-
role.
12944+
read only by superusers or roles with privileges of the
12945+
<literal>pg_read_all_stats</literal> role.
1294612946
</para>
1294712947
</sect1>
1294812948

doc/src/sgml/func.sgml

+6-6
Original file line numberDiff line numberDiff line change
@@ -25435,7 +25435,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
2543525435
Cancels the current query of the session whose backend process has the
2543625436
specified process ID. This is also allowed if the
2543725437
calling role is a member of the role whose backend is being canceled or
25438-
the calling role has been granted <literal>pg_signal_backend</literal>,
25438+
the calling role has privileges of <literal>pg_signal_backend</literal>,
2543925439
however only superusers can cancel superuser backends.
2544025440
</para></entry>
2544125441
</row>
@@ -25508,7 +25508,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
2550825508
Terminates the session whose backend process has the
2550925509
specified process ID. This is also allowed if the calling role
2551025510
is a member of the role whose backend is being terminated or the
25511-
calling role has been granted <literal>pg_signal_backend</literal>,
25511+
calling role has privileges of <literal>pg_signal_backend</literal>,
2551225512
however only superusers can terminate superuser backends.
2551325513
</para>
2551425514
<para>
@@ -26783,7 +26783,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
2678326783
Computes the total disk space used by the database with the specified
2678426784
name or OID. To use this function, you must
2678526785
have <literal>CONNECT</literal> privilege on the specified database
26786-
(which is granted by default) or be a member of
26786+
(which is granted by default) or have privileges of
2678726787
the <literal>pg_read_all_stats</literal> role.
2678826788
</para></entry>
2678926789
</row>
@@ -26913,7 +26913,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
2691326913
Computes the total disk space used in the tablespace with the
2691426914
specified name or OID. To use this function, you must
2691526915
have <literal>CREATE</literal> privilege on the specified tablespace
26916-
or be a member of the <literal>pg_read_all_stats</literal> role,
26916+
or have privileges of the <literal>pg_read_all_stats</literal> role,
2691726917
unless it is the default tablespace for the current database.
2691826918
</para></entry>
2691926919
</row>
@@ -27392,7 +27392,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
2739227392
a dot, directories, and other special files are excluded.
2739327393
</para>
2739427394
<para>
27395-
This function is restricted to superusers and members of
27395+
This function is restricted to superusers and roles with privileges of
2739627396
the <literal>pg_monitor</literal> role by default, but other users can
2739727397
be granted EXECUTE to run the function.
2739827398
</para></entry>
@@ -27416,7 +27416,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
2741627416
are excluded.
2741727417
</para>
2741827418
<para>
27419-
This function is restricted to superusers and members of
27419+
This function is restricted to superusers and roles with privileges of
2742027420
the <literal>pg_monitor</literal> role by default, but other users can
2742127421
be granted EXECUTE to run the function.
2742227422
</para></entry>

doc/src/sgml/monitoring.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
280280
(sessions belonging to a role that they are a member of). In rows about
281281
other sessions, many columns will be null. Note, however, that the
282282
existence of a session and its general properties such as its sessions user
283-
and database are visible to all users. Superusers and members of the
283+
and database are visible to all users. Superusers and roles with privileges of
284284
built-in role <literal>pg_read_all_stats</literal> (see also <xref
285285
linkend="predefined-roles"/>) can see all the information about all sessions.
286286
</para>

doc/src/sgml/pgbuffercache.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
</para>
2525

2626
<para>
27-
By default, use is restricted to superusers and members of the
27+
By default, use is restricted to superusers and roles with privileges of the
2828
<literal>pg_monitor</literal> role. Access may be granted to others
2929
using <command>GRANT</command>.
3030
</para>

doc/src/sgml/pgfreespacemap.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
</para>
1717

1818
<para>
19-
By default use is restricted to superusers and members of the
19+
By default use is restricted to superusers and roles with privileges of the
2020
<literal>pg_stat_scan_tables</literal> role. Access may be granted to others
2121
using <command>GRANT</command>.
2222
</para>

doc/src/sgml/pgrowlocks.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
</para>
1414

1515
<para>
16-
By default use is restricted to superusers, members of the
16+
By default use is restricted to superusers, roles with privileges of the
1717
<literal>pg_stat_scan_tables</literal> role, and users with
1818
<literal>SELECT</literal> permissions on the table.
1919
</para>

doc/src/sgml/pgstatstatements.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@
384384
</table>
385385

386386
<para>
387-
For security reasons, only superusers and members of the
387+
For security reasons, only superusers and roles with privileges of the
388388
<literal>pg_read_all_stats</literal> role are allowed to see the SQL text and
389389
<structfield>queryid</structfield> of queries executed by other users.
390390
Other users can see the statistics, however, if the view has been installed

doc/src/sgml/pgvisibility.sgml

+2-2
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@
140140
</variablelist>
141141

142142
<para>
143-
By default, these functions are executable only by superusers and members of the
144-
<literal>pg_stat_scan_tables</literal> role, with the exception of
143+
By default, these functions are executable only by superusers and roles with privileges
144+
of the <literal>pg_stat_scan_tables</literal> role, with the exception of
145145
<function>pg_truncate_visibility_map(relation regclass)</function> which can only
146146
be executed by superusers.
147147
</para>

src/backend/commands/copy.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -80,26 +80,26 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
8080
{
8181
if (stmt->is_program)
8282
{
83-
if (!is_member_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
83+
if (!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
8484
ereport(ERROR,
8585
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
86-
errmsg("must be superuser or a member of the pg_execute_server_program role to COPY to or from an external program"),
86+
errmsg("must be superuser or have privileges of the pg_execute_server_program role to COPY to or from an external program"),
8787
errhint("Anyone can COPY to stdout or from stdin. "
8888
"psql's \\copy command also works for anyone.")));
8989
}
9090
else
9191
{
92-
if (is_from && !is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
92+
if (is_from && !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
9393
ereport(ERROR,
9494
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
95-
errmsg("must be superuser or a member of the pg_read_server_files role to COPY from a file"),
95+
errmsg("must be superuser or have privileges of the pg_read_server_files role to COPY from a file"),
9696
errhint("Anyone can COPY to stdout or from stdin. "
9797
"psql's \\copy command also works for anyone.")));
9898

99-
if (!is_from && !is_member_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
99+
if (!is_from && !has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
100100
ereport(ERROR,
101101
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
102-
errmsg("must be superuser or a member of the pg_write_server_files role to COPY to a file"),
102+
errmsg("must be superuser or have privileges of the pg_write_server_files role to COPY to a file"),
103103
errhint("Anyone can COPY to stdout or from stdin. "
104104
"psql's \\copy command also works for anyone.")));
105105
}

src/backend/replication/walreceiver.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -1403,12 +1403,12 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
14031403
/* Fetch values */
14041404
values[0] = Int32GetDatum(pid);
14051405

1406-
if (!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
1406+
if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
14071407
{
14081408
/*
1409-
* Only superusers and members of pg_read_all_stats can see details.
1410-
* Other users only get the pid value to know whether it is a WAL
1411-
* receiver, but no details.
1409+
* Only superusers and roles with privileges of pg_read_all_stats
1410+
* can see details. Other users only get the pid value to know whether
1411+
* it is a WAL receiver, but no details.
14121412
*/
14131413
MemSet(&nulls[1], true, sizeof(bool) * (tupdesc->natts - 1));
14141414
}

src/backend/replication/walsender.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -3473,12 +3473,12 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
34733473
memset(nulls, 0, sizeof(nulls));
34743474
values[0] = Int32GetDatum(pid);
34753475

3476-
if (!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
3476+
if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
34773477
{
34783478
/*
3479-
* Only superusers and members of pg_read_all_stats can see
3480-
* details. Other users only get the pid value to know it's a
3481-
* walsender, but no details.
3479+
* Only superusers and roles with privileges of pg_read_all_stats
3480+
* can see details. Other users only get the pid value to know
3481+
* it's a walsender, but no details.
34823482
*/
34833483
MemSet(&nulls[1], true, PG_STAT_GET_WAL_SENDERS_COLS - 1);
34843484
}

src/backend/utils/adt/acl.c

+4
Original file line numberDiff line numberDiff line change
@@ -4859,6 +4859,8 @@ has_privs_of_role(Oid member, Oid role)
48594859
* Is member a member of role (directly or indirectly)?
48604860
*
48614861
* This is defined to recurse through roles regardless of rolinherit.
4862+
*
4863+
* Do not use this for privilege checking, instead use has_privs_of_role()
48624864
*/
48634865
bool
48644866
is_member_of_role(Oid member, Oid role)
@@ -4899,6 +4901,8 @@ check_is_member_of_role(Oid member, Oid role)
48994901
*
49004902
* This is identical to is_member_of_role except we ignore superuser
49014903
* status.
4904+
*
4905+
* Do not use this for privilege checking, instead use has_privs_of_role()
49024906
*/
49034907
bool
49044908
is_member_of_role_nosuper(Oid member, Oid role)

src/backend/utils/adt/dbsize.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ calculate_database_size(Oid dbOid)
112112
AclResult aclresult;
113113

114114
/*
115-
* User must have connect privilege for target database or be a member of
115+
* User must have connect privilege for target database or have privileges of
116116
* pg_read_all_stats
117117
*/
118118
aclresult = pg_database_aclcheck(dbOid, GetUserId(), ACL_CONNECT);
119119
if (aclresult != ACLCHECK_OK &&
120-
!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
120+
!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
121121
{
122122
aclcheck_error(aclresult, OBJECT_DATABASE,
123123
get_database_name(dbOid));
@@ -196,12 +196,12 @@ calculate_tablespace_size(Oid tblspcOid)
196196
AclResult aclresult;
197197

198198
/*
199-
* User must be a member of pg_read_all_stats or have CREATE privilege for
199+
* User must have privileges of pg_read_all_stats or have CREATE privilege for
200200
* target tablespace, either explicitly granted or implicitly because it
201201
* is default for current database.
202202
*/
203203
if (tblspcOid != MyDatabaseTableSpace &&
204-
!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
204+
!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
205205
{
206206
aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE);
207207
if (aclresult != ACLCHECK_OK)

src/backend/utils/adt/genfile.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ convert_and_check_filename(text *arg)
5959
canonicalize_path(filename); /* filename can change length here */
6060

6161
/*
62-
* Members of the 'pg_read_server_files' role are allowed to access any
63-
* files on the server as the PG user, so no need to do any further checks
62+
* Roles with privleges of the 'pg_read_server_files' role are allowed to access
63+
* any files on the server as the PG user, so no need to do any further checks
6464
* here.
6565
*/
66-
if (is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
66+
if (has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
6767
return filename;
6868

6969
/*

src/backend/utils/adt/pgstatfuncs.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
#define UINT32_ACCESS_ONCE(var) ((uint32)(*((volatile uint32 *)&(var))))
3636

37-
#define HAS_PGSTAT_PERMISSIONS(role) (is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS) || has_privs_of_role(GetUserId(), role))
37+
#define HAS_PGSTAT_PERMISSIONS(role) (has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS) || has_privs_of_role(GetUserId(), role))
3838

3939
Datum
4040
pg_stat_get_numscans(PG_FUNCTION_ARGS)

0 commit comments

Comments
 (0)