On 3/3/22 11:26, Joshua Brindle wrote:
On Thu, Feb 10, 2022 at 2:37 PM Joe Conway <m...@joeconway.com> wrote:
On 2/10/22 14:28, Nathan Bossart wrote:
> On Wed, Feb 09, 2022 at 04:39:11PM -0500, Joe Conway wrote:
>> On 2/9/22 13:13, Nathan Bossart wrote:
>>> I do wonder if users find the differences between predefined roles and role
>>> attributes confusing. INHERIT doesn't govern role attributes, but it will
>>> govern predefined roles when this patch is applied. Maybe the role
>>> attribute system should eventually be deprecated in favor of using
>>> predefined roles for everything. Or perhaps the predefined roles should be
>>> converted to role attributes.
>>
>> Yep, I was suggesting that the latter would have been preferable to me while
>> Robert seemed to prefer the former. Honestly I could be happy with either of
>> those solutions, but as I alluded to that is probably a discussion for the
>> next development cycle since I don't see us doing that big a change in this
>> one.
>
> I agree. I still think Joshua's proposed patch is a worthwhile improvement
> for v15.
+1
I am planning to get into it in detail this weekend. So far I have
really just ensured it merges cleanly and passes make world.
Rebased patch to apply to master attached.
Well longer than I planned, but finally took a closer look.
I made one minor editorial fix to Joshua's patch, rebased to current
master, and added two missing call sites that presumably are related to
recent commits for pg_basebackup.
On that last note, I did not find basebackup_to_shell.required_role
documented at all, and did not attempt to fix that.
When this thread petered out, it seemed that Robert was at least neutral
on the patch, and everyone else was +1 on applying it to master for pg15.
As such, if there are any other issues, complaints, etc., please speak
real soon now...
Thanks,
Joe
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_9icmuj0zjywwcgvi4pa...@mail.gmail.com
diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index d7d84d0..03addf1 100644
*** a/contrib/adminpack/adminpack.c
--- b/contrib/adminpack/adminpack.c
*************** convert_and_check_filename(text *arg)
*** 79,85 ****
* files on the server as the PG user, so no need to do any further checks
* here.
*/
! if (is_member_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
return filename;
/*
--- 79,85 ----
* files on the server as the PG user, so no need to do any further checks
* here.
*/
! if (has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
return filename;
/*
diff --git a/contrib/basebackup_to_shell/basebackup_to_shell.c b/contrib/basebackup_to_shell/basebackup_to_shell.c
index d82cb6d..f0ddef1 100644
*** a/contrib/basebackup_to_shell/basebackup_to_shell.c
--- b/contrib/basebackup_to_shell/basebackup_to_shell.c
*************** _PG_init(void)
*** 90,96 ****
}
/*
! * We choose to defer sanity sanity checking until shell_get_sink(), and so
* just pass the target detail through without doing anything. However, we do
* permissions checks here, before any real work has been done.
*/
--- 90,96 ----
}
/*
! * We choose to defer sanity checking until shell_get_sink(), and so
* just pass the target detail through without doing anything. However, we do
* permissions checks here, before any real work has been done.
*/
*************** shell_check_detail(char *target, char *t
*** 103,109 ****
StartTransactionCommand();
roleid = get_role_oid(shell_required_role, true);
! if (!is_member_of_role(GetUserId(), roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to use basebackup_to_shell")));
--- 103,109 ----
StartTransactionCommand();
roleid = get_role_oid(shell_required_role, true);
! if (!has_privs_of_role(GetUserId(), roleid))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to use basebackup_to_shell")));
diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index 0ac6e4e..14acdb2 100644
*** a/contrib/file_fdw/expected/file_fdw.out
--- b/contrib/file_fdw/expected/file_fdw.out
*************** ALTER FOREIGN TABLE agg_text OWNER TO re
*** 459,465 ****
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
SET ROLE regress_file_fdw_user;
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
! ERROR: only superuser or a member of the pg_read_server_files role may specify the filename option of a file_fdw foreign table
SET ROLE regress_file_fdw_superuser;
-- cleanup
RESET ROLE;
--- 459,465 ----
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
SET ROLE regress_file_fdw_user;
ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
! 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
SET ROLE regress_file_fdw_superuser;
-- cleanup
RESET ROLE;
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index db08593..4773cad 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 269,284 ****
* otherwise there'd still be a security hole.
*/
if (strcmp(def->defname, "filename") == 0 &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("only superuser or a member of the pg_read_server_files role may specify the filename option of a file_fdw foreign table")));
if (strcmp(def->defname, "program") == 0 &&
! !is_member_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("only superuser or a member of the pg_execute_server_program role may specify the program option of a file_fdw foreign table")));
filename = defGetString(def);
}
--- 269,284 ----
* otherwise there'd still be a security hole.
*/
if (strcmp(def->defname, "filename") == 0 &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! 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")));
if (strcmp(def->defname, "program") == 0 &&
! !has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! 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")));
filename = defGetString(def);
}
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 9e525a6..55786ae 100644
*** a/contrib/pg_stat_statements/pg_stat_statements.c
--- b/contrib/pg_stat_statements/pg_stat_statements.c
*************** pg_stat_statements_internal(FunctionCall
*** 1503,1510 ****
HASH_SEQ_STATUS hash_seq;
pgssEntry *entry;
! /* Superusers or members of pg_read_all_stats members are allowed */
! is_allowed_role = is_member_of_role(userid, ROLE_PG_READ_ALL_STATS);
/* hash table must exist already */
if (!pgss || !pgss_hash)
--- 1503,1510 ----
HASH_SEQ_STATUS hash_seq;
pgssEntry *entry;
! /* Superusers or roles with the privileges of pg_read_all_stats members are allowed */
! is_allowed_role = has_privs_of_role(userid, ROLE_PG_READ_ALL_STATS);
/* hash table must exist already */
if (!pgss || !pgss_hash)
diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index 713a165..1d4d496 100644
*** a/contrib/pgrowlocks/pgrowlocks.c
--- b/contrib/pgrowlocks/pgrowlocks.c
*************** pgrowlocks(PG_FUNCTION_ARGS)
*** 104,110 ****
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
ACL_SELECT);
if (aclresult != ACLCHECK_OK)
! aclresult = is_member_of_role(GetUserId(), ROLE_PG_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
--- 104,110 ----
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
ACL_SELECT);
if (aclresult != ACLCHECK_OK)
! aclresult = has_privs_of_role(GetUserId(), ROLE_PG_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml
index 0dd89be..5702456 100644
*** a/doc/src/sgml/adminpack.sgml
--- b/doc/src/sgml/adminpack.sgml
***************
*** 22,30 ****
functions in <xref linkend="functions-admin-genfile-table"/>, which
provide read-only access.)
Only files within the database cluster directory can be accessed, unless the
! user is a superuser or given one of the pg_read_server_files, or pg_write_server_files
! roles, as appropriate for the function, but either a relative or absolute path is
! allowable.
</para>
<table id="functions-adminpack-table">
--- 22,30 ----
functions in <xref linkend="functions-admin-genfile-table"/>, which
provide read-only access.)
Only files within the database cluster directory can be accessed, unless the
! user is a superuser or given privileges of one of the pg_read_server_files,
! or pg_write_server_files roles, as appropriate for the function, but either a
! relative or absolute path is allowable.
</para>
<table id="functions-adminpack-table">
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 4dc5b34..5010c63 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
*************** SCRAM-SHA-256$<replaceable><iteration
*** 10006,10013 ****
<para>
By default, the <structname>pg_backend_memory_contexts</structname> view can be
! read only by superusers or members of the <literal>pg_read_all_stats</literal>
! role.
</para>
</sect1>
--- 10006,10013 ----
<para>
By default, the <structname>pg_backend_memory_contexts</structname> view can be
! read only by superusers or roles with the privileges of the
! <literal>pg_read_all_stats</literal> role.
</para>
</sect1>
*************** SELECT * FROM pg_locks pl LEFT JOIN pg_p
*** 12448,12454 ****
<para>
Configuration file the current value was set in (null for
values set from sources other than configuration files, or when
! examined by a user who is neither a superuser or a member of
<literal>pg_read_all_settings</literal>); helpful when using
<literal>include</literal> directives in configuration files
</para></entry>
--- 12448,12454 ----
<para>
Configuration file the current value was set in (null for
values set from sources other than configuration files, or when
! examined by a user who neither is a superuser nor has privileges of
<literal>pg_read_all_settings</literal>); helpful when using
<literal>include</literal> directives in configuration files
</para></entry>
*************** SELECT * FROM pg_locks pl LEFT JOIN pg_p
*** 12461,12467 ****
<para>
Line number within the configuration file the current value was
set at (null for values set from sources other than configuration files,
! or when examined by a user who is neither a superuser or a member of
<literal>pg_read_all_settings</literal>).
</para></entry>
</row>
--- 12461,12467 ----
<para>
Line number within the configuration file the current value was
set at (null for values set from sources other than configuration files,
! or when examined by a user who neither is a superuser nor has privileges of
<literal>pg_read_all_settings</literal>).
</para></entry>
</row>
*************** SELECT * FROM pg_locks pl LEFT JOIN pg_p
*** 12837,12844 ****
<para>
By default, the <structname>pg_shmem_allocations</structname> view can be
! read only by superusers or members of the <literal>pg_read_all_stats</literal>
! role.
</para>
</sect1>
--- 12837,12844 ----
<para>
By default, the <structname>pg_shmem_allocations</structname> view can be
! read only by superusers or roles with privileges of the
! <literal>pg_read_all_stats</literal> role.
</para>
</sect1>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8a802fb..3a9d62b 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*************** SELECT collation for ('foo' COLLATE "de_
*** 25435,25441 ****
Cancels the current query of the session whose backend process has the
specified process ID. This is also allowed if the
calling role is a member of the role whose backend is being canceled or
! the calling role has been granted <literal>pg_signal_backend</literal>,
however only superusers can cancel superuser backends.
</para></entry>
</row>
--- 25435,25441 ----
Cancels the current query of the session whose backend process has the
specified process ID. This is also allowed if the
calling role is a member of the role whose backend is being canceled or
! the calling role has privileges of <literal>pg_signal_backend</literal>,
however only superusers can cancel superuser backends.
</para></entry>
</row>
*************** SELECT collation for ('foo' COLLATE "de_
*** 25508,25514 ****
Terminates the session whose backend process has the
specified process ID. This is also allowed if the calling role
is a member of the role whose backend is being terminated or the
! calling role has been granted <literal>pg_signal_backend</literal>,
however only superusers can terminate superuser backends.
</para>
<para>
--- 25508,25514 ----
Terminates the session whose backend process has the
specified process ID. This is also allowed if the calling role
is a member of the role whose backend is being terminated or the
! calling role has privileges of <literal>pg_signal_backend</literal>,
however only superusers can terminate superuser backends.
</para>
<para>
*************** postgres=# SELECT * FROM pg_walfile_name
*** 26783,26789 ****
Computes the total disk space used by the database with the specified
name or OID. To use this function, you must
have <literal>CONNECT</literal> privilege on the specified database
! (which is granted by default) or be a member of
the <literal>pg_read_all_stats</literal> role.
</para></entry>
</row>
--- 26783,26789 ----
Computes the total disk space used by the database with the specified
name or OID. To use this function, you must
have <literal>CONNECT</literal> privilege on the specified database
! (which is granted by default) or have privileges of
the <literal>pg_read_all_stats</literal> role.
</para></entry>
</row>
*************** postgres=# SELECT * FROM pg_walfile_name
*** 26913,26919 ****
Computes the total disk space used in the tablespace with the
specified name or OID. To use this function, you must
have <literal>CREATE</literal> privilege on the specified tablespace
! or be a member of the <literal>pg_read_all_stats</literal> role,
unless it is the default tablespace for the current database.
</para></entry>
</row>
--- 26913,26919 ----
Computes the total disk space used in the tablespace with the
specified name or OID. To use this function, you must
have <literal>CREATE</literal> privilege on the specified tablespace
! or have privileges of the <literal>pg_read_all_stats</literal> role,
unless it is the default tablespace for the current database.
</para></entry>
</row>
*************** SELECT pg_size_pretty(sum(pg_relation_si
*** 27392,27398 ****
a dot, directories, and other special files are excluded.
</para>
<para>
! This function is restricted to superusers and members of
the <literal>pg_monitor</literal> role by default, but other users can
be granted EXECUTE to run the function.
</para></entry>
--- 27392,27398 ----
a dot, directories, and other special files are excluded.
</para>
<para>
! This function is restricted to superusers and roles with privileges of
the <literal>pg_monitor</literal> role by default, but other users can
be granted EXECUTE to run the function.
</para></entry>
*************** SELECT pg_size_pretty(sum(pg_relation_si
*** 27416,27422 ****
are excluded.
</para>
<para>
! This function is restricted to superusers and members of
the <literal>pg_monitor</literal> role by default, but other users can
be granted EXECUTE to run the function.
</para></entry>
--- 27416,27422 ----
are excluded.
</para>
<para>
! This function is restricted to superusers and roles with privileges of
the <literal>pg_monitor</literal> role by default, but other users can
be granted EXECUTE to run the function.
</para></entry>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 35b2923..6a6b09d 100644
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
*************** postgres 27093 0.0 0.0 30096 2752
*** 280,286 ****
(sessions belonging to a role that they are a member of). In rows about
other sessions, many columns will be null. Note, however, that the
existence of a session and its general properties such as its sessions user
! and database are visible to all users. Superusers and members of the
built-in role <literal>pg_read_all_stats</literal> (see also <xref
linkend="predefined-roles"/>) can see all the information about all sessions.
</para>
--- 280,286 ----
(sessions belonging to a role that they are a member of). In rows about
other sessions, many columns will be null. Note, however, that the
existence of a session and its general properties such as its sessions user
! and database are visible to all users. Superusers and roles with privileges of
built-in role <literal>pg_read_all_stats</literal> (see also <xref
linkend="predefined-roles"/>) can see all the information about all sessions.
</para>
diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml
index e68d159..a06fd3e 100644
*** a/doc/src/sgml/pgbuffercache.sgml
--- b/doc/src/sgml/pgbuffercache.sgml
***************
*** 24,30 ****
</para>
<para>
! By default, use is restricted to superusers and members of the
<literal>pg_monitor</literal> role. Access may be granted to others
using <command>GRANT</command>.
</para>
--- 24,30 ----
</para>
<para>
! By default, use is restricted to superusers and roles with privileges of the
<literal>pg_monitor</literal> role. Access may be granted to others
using <command>GRANT</command>.
</para>
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index 1f7867d..3063885 100644
*** a/doc/src/sgml/pgfreespacemap.sgml
--- b/doc/src/sgml/pgfreespacemap.sgml
***************
*** 16,22 ****
</para>
<para>
! By default use is restricted to superusers and members of the
<literal>pg_stat_scan_tables</literal> role. Access may be granted to others
using <command>GRANT</command>.
</para>
--- 16,22 ----
</para>
<para>
! By default use is restricted to superusers and roles with privileges of the
<literal>pg_stat_scan_tables</literal> role. Access may be granted to others
using <command>GRANT</command>.
</para>
diff --git a/doc/src/sgml/pgrowlocks.sgml b/doc/src/sgml/pgrowlocks.sgml
index 392d5f1..2914bf6 100644
*** a/doc/src/sgml/pgrowlocks.sgml
--- b/doc/src/sgml/pgrowlocks.sgml
***************
*** 13,19 ****
</para>
<para>
! By default use is restricted to superusers, members of the
<literal>pg_stat_scan_tables</literal> role, and users with
<literal>SELECT</literal> permissions on the table.
</para>
--- 13,19 ----
</para>
<para>
! By default use is restricted to superusers, roles with privileges of the
<literal>pg_stat_scan_tables</literal> role, and users with
<literal>SELECT</literal> permissions on the table.
</para>
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index bc9d5bd..3a7e36b 100644
*** a/doc/src/sgml/pgstatstatements.sgml
--- b/doc/src/sgml/pgstatstatements.sgml
***************
*** 384,390 ****
</table>
<para>
! For security reasons, only superusers and members of the
<literal>pg_read_all_stats</literal> role are allowed to see the SQL text and
<structfield>queryid</structfield> of queries executed by other users.
Other users can see the statistics, however, if the view has been installed
--- 384,390 ----
</table>
<para>
! For security reasons, only superusers and roles with privileges of the
<literal>pg_read_all_stats</literal> role are allowed to see the SQL text and
<structfield>queryid</structfield> of queries executed by other users.
Other users can see the statistics, however, if the view has been installed
diff --git a/doc/src/sgml/pgvisibility.sgml b/doc/src/sgml/pgvisibility.sgml
index 7533694..8090aa5 100644
*** a/doc/src/sgml/pgvisibility.sgml
--- b/doc/src/sgml/pgvisibility.sgml
***************
*** 140,147 ****
</variablelist>
<para>
! By default, these functions are executable only by superusers and members of the
! <literal>pg_stat_scan_tables</literal> role, with the exception of
<function>pg_truncate_visibility_map(relation regclass)</function> which can only
be executed by superusers.
</para>
--- 140,147 ----
</variablelist>
<para>
! By default, these functions are executable only by superusers and roles with privileges
! of the <literal>pg_stat_scan_tables</literal> role, with the exception of
<function>pg_truncate_visibility_map(relation regclass)</function> which can only
be executed by superusers.
</para>
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 4a630b5..36baafd 100644
*** a/doc/src/sgml/ref/pg_basebackup.sgml
--- b/doc/src/sgml/ref/pg_basebackup.sgml
*************** PostgreSQL documentation
*** 237,243 ****
<literal>server:/some/path</literal>, the backup will be stored on
the machine where the server is running in the
<literal>/some/path</literal> directory. Storing a backup on the
! server requires superuser privileges or being granted the
<literal>pg_write_server_files</literal> role. If the target is set to
<literal>blackhole</literal>, the contents are discarded and not
stored anywhere. This should only be used for testing purposes, as you
--- 237,243 ----
<literal>server:/some/path</literal>, the backup will be stored on
the machine where the server is running in the
<literal>/some/path</literal> directory. Storing a backup on the
! server requires superuser privileges or having privileges of the
<literal>pg_write_server_files</literal> role. If the target is set to
<literal>blackhole</literal>, the contents are discarded and not
stored anywhere. This should only be used for testing purposes, as you
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 7da7105..7a0c897 100644
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*************** DoCopy(ParseState *pstate, const CopyStm
*** 80,105 ****
{
if (stmt->is_program)
{
! if (!is_member_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of the pg_execute_server_program role to COPY to or from an external program"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
}
else
{
! if (is_from && !is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of the pg_read_server_files role to COPY from a file"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
! if (!is_from && !is_member_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of the pg_write_server_files role to COPY to a file"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
}
--- 80,105 ----
{
if (stmt->is_program)
{
! if (!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of the pg_execute_server_program role to COPY to or from an external program"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
}
else
{
! if (is_from && !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of the pg_read_server_files role to COPY from a file"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
! if (!is_from && !has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of the pg_write_server_files role to COPY to a file"),
errhint("Anyone can COPY to stdout or from stdin. "
"psql's \\copy command also works for anyone.")));
}
diff --git a/src/backend/replication/basebackup_server.c b/src/backend/replication/basebackup_server.c
index a878629..bc16897 100644
*** a/src/backend/replication/basebackup_server.c
--- b/src/backend/replication/basebackup_server.c
*************** bbsink_server_new(bbsink *next, char *pa
*** 69,78 ****
/* Replication permission is not sufficient in this case. */
StartTransactionCommand();
! if (!is_member_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of the pg_write_server_files role to create server backup")));
CommitTransactionCommand();
/*
--- 69,78 ----
/* Replication permission is not sufficient in this case. */
StartTransactionCommand();
! if (!has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a role with privileges of the pg_write_server_files role to create server backup")));
CommitTransactionCommand();
/*
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index ceaff09..3c9411e 100644
*** a/src/backend/replication/walreceiver.c
--- b/src/backend/replication/walreceiver.c
*************** pg_stat_get_wal_receiver(PG_FUNCTION_ARG
*** 1403,1414 ****
/* Fetch values */
values[0] = Int32GetDatum(pid);
! if (!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
/*
! * Only superusers and members of pg_read_all_stats can see details.
! * Other users only get the pid value to know whether it is a WAL
! * receiver, but no details.
*/
MemSet(&nulls[1], true, sizeof(bool) * (tupdesc->natts - 1));
}
--- 1403,1414 ----
/* Fetch values */
values[0] = Int32GetDatum(pid);
! if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
/*
! * Only superusers and roles with privileges of pg_read_all_stats
! * can see details. Other users only get the pid value to know whether
! * it is a WAL receiver, but no details.
*/
MemSet(&nulls[1], true, sizeof(bool) * (tupdesc->natts - 1));
}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 2d0292a..cffb348 100644
*** a/src/backend/replication/walsender.c
--- b/src/backend/replication/walsender.c
*************** pg_stat_get_wal_senders(PG_FUNCTION_ARGS
*** 3473,3484 ****
memset(nulls, 0, sizeof(nulls));
values[0] = Int32GetDatum(pid);
! if (!is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
/*
! * Only superusers and members of pg_read_all_stats can see
! * details. Other users only get the pid value to know it's a
! * walsender, but no details.
*/
MemSet(&nulls[1], true, PG_STAT_GET_WAL_SENDERS_COLS - 1);
}
--- 3473,3484 ----
memset(nulls, 0, sizeof(nulls));
values[0] = Int32GetDatum(pid);
! if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
/*
! * Only superusers and roles with privileges of pg_read_all_stats
! * can see details. Other users only get the pid value to know
! * it's a walsender, but no details.
*/
MemSet(&nulls[1], true, PG_STAT_GET_WAL_SENDERS_COLS - 1);
}
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 0a16f81..1d3dd89 100644
*** a/src/backend/utils/adt/acl.c
--- b/src/backend/utils/adt/acl.c
*************** has_privs_of_role(Oid member, Oid role)
*** 4864,4869 ****
--- 4864,4871 ----
* Is member a member of role (directly or indirectly)?
*
* This is defined to recurse through roles regardless of rolinherit.
+ *
+ * Do not use this for privilege checking, instead use has_privs_of_role()
*/
bool
is_member_of_role(Oid member, Oid role)
*************** check_is_member_of_role(Oid member, Oid
*** 4904,4909 ****
--- 4906,4913 ----
*
* This is identical to is_member_of_role except we ignore superuser
* status.
+ *
+ * Do not use this for privilege checking, instead use has_privs_of_role()
*/
bool
is_member_of_role_nosuper(Oid member, Oid role)
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 3a2f2e1..0576764 100644
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
*************** calculate_database_size(Oid dbOid)
*** 112,123 ****
AclResult aclresult;
/*
! * User must have connect privilege for target database or be a member of
* pg_read_all_stats
*/
aclresult = pg_database_aclcheck(dbOid, GetUserId(), ACL_CONNECT);
if (aclresult != ACLCHECK_OK &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
aclcheck_error(aclresult, OBJECT_DATABASE,
get_database_name(dbOid));
--- 112,123 ----
AclResult aclresult;
/*
! * User must have connect privilege for target database or have privileges of
* pg_read_all_stats
*/
aclresult = pg_database_aclcheck(dbOid, GetUserId(), ACL_CONNECT);
if (aclresult != ACLCHECK_OK &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
aclcheck_error(aclresult, OBJECT_DATABASE,
get_database_name(dbOid));
*************** calculate_tablespace_size(Oid tblspcOid)
*** 196,207 ****
AclResult aclresult;
/*
! * User must be a member of pg_read_all_stats or have CREATE privilege for
* target tablespace, either explicitly granted or implicitly because it
* is default for current database.
*/
if (tblspcOid != MyDatabaseTableSpace &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE);
if (aclresult != ACLCHECK_OK)
--- 196,207 ----
AclResult aclresult;
/*
! * User must have privileges of pg_read_all_stats or have CREATE privilege for
* target tablespace, either explicitly granted or implicitly because it
* is default for current database.
*/
if (tblspcOid != MyDatabaseTableSpace &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS))
{
aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE);
if (aclresult != ACLCHECK_OK)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 1ed0162..88f279d 100644
*** a/src/backend/utils/adt/genfile.c
--- b/src/backend/utils/adt/genfile.c
*************** convert_and_check_filename(text *arg)
*** 59,69 ****
canonicalize_path(filename); /* filename can change length here */
/*
! * Members of the 'pg_read_server_files' role are allowed to access any
! * files on the server as the PG user, so no need to do any further checks
* here.
*/
! if (is_member_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
return filename;
/*
--- 59,69 ----
canonicalize_path(filename); /* filename can change length here */
/*
! * Roles with privleges of the 'pg_read_server_files' role are allowed to access
! * any files on the server as the PG user, so no need to do any further checks
* here.
*/
! if (has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
return filename;
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index eff45b1..ce84525 100644
*** a/src/backend/utils/adt/pgstatfuncs.c
--- b/src/backend/utils/adt/pgstatfuncs.c
***************
*** 34,40 ****
#define UINT32_ACCESS_ONCE(var) ((uint32)(*((volatile uint32 *)&(var))))
! #define HAS_PGSTAT_PERMISSIONS(role) (is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS) || has_privs_of_role(GetUserId(), role))
Datum
pg_stat_get_numscans(PG_FUNCTION_ARGS)
--- 34,40 ----
#define UINT32_ACCESS_ONCE(var) ((uint32)(*((volatile uint32 *)&(var))))
! #define HAS_PGSTAT_PERMISSIONS(role) (has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS) || has_privs_of_role(GetUserId(), role))
Datum
pg_stat_get_numscans(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e7f0a38..c3ef798 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** GetConfigOption(const char *name, bool m
*** 8215,8224 ****
return NULL;
if (restrict_privileged &&
(record->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"",
name)));
switch (record->vartype)
--- 8215,8224 ----
return NULL;
if (restrict_privileged &&
(record->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
name)));
switch (record->vartype)
*************** GetConfigOptionResetString(const char *n
*** 8262,8271 ****
record = find_option(name, false, false, ERROR);
Assert(record != NULL);
if ((record->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"",
name)));
switch (record->vartype)
--- 8262,8271 ----
record = find_option(name, false, false, ERROR);
Assert(record != NULL);
if ((record->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
name)));
switch (record->vartype)
*************** ShowAllGUCConfig(DestReceiver *dest)
*** 9537,9543 ****
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
continue;
/* assign to the values array */
--- 9537,9543 ----
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
continue;
/* assign to the values array */
*************** get_explain_guc_options(int *num)
*** 9604,9610 ****
/* return only options visible to the current user */
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
continue;
/* return only options that are different from their boot values */
--- 9604,9610 ----
/* return only options visible to the current user */
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
continue;
/* return only options that are different from their boot values */
*************** GetConfigOptionByName(const char *name,
*** 9686,9695 ****
}
if ((record->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"",
name)));
if (varname)
--- 9686,9695 ----
}
if ((record->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
name)));
if (varname)
*************** GetConfigOptionByNum(int varnum, const c
*** 9756,9762 ****
{
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
*noshow = true;
else
*noshow = false;
--- 9756,9762 ----
{
if ((conf->flags & GUC_NO_SHOW_ALL) ||
((conf->flags & GUC_SUPERUSER_ONLY) &&
! !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS)))
*noshow = true;
else
*noshow = false;
*************** GetConfigOptionByNum(int varnum, const c
*** 9951,9957 ****
* insufficiently-privileged users.
*/
if (conf->source == PGC_S_FILE &&
! is_member_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
{
values[14] = conf->sourcefile;
snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
--- 9951,9957 ----
* insufficiently-privileged users.
*/
if (conf->source == PGC_S_FILE &&
! has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_SETTINGS))
{
values[14] = conf->sourcefile;
snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index eb608fd..88b1ff8 100644
*** a/src/test/modules/unsafe_tests/expected/rolenames.out
--- b/src/test/modules/unsafe_tests/expected/rolenames.out
*************** SHOW session_preload_libraries;
*** 1077,1083 ****
SET SESSION AUTHORIZATION regress_role_nopriv;
-- fails with role not member of pg_read_all_settings
SHOW session_preload_libraries;
! ERROR: must be superuser or a member of pg_read_all_settings to examine "session_preload_libraries"
RESET SESSION AUTHORIZATION;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ROLLBACK;
--- 1077,1083 ----
SET SESSION AUTHORIZATION regress_role_nopriv;
-- fails with role not member of pg_read_all_settings
SHOW session_preload_libraries;
! ERROR: must be superuser or have privileges of pg_read_all_settings to examine "session_preload_libraries"
RESET SESSION AUTHORIZATION;
ERROR: current transaction is aborted, commands ignored until end of transaction block
ROLLBACK;
