Re: should we allow users with a predefined role to access pg_backend_memory_contexts view and pg_log_backend_memory_contexts function?

On Fri, Oct 8, 2021 at 12:27 AM Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
>
> On 10/7/21, 10:42 AM, "Bharath Rupireddy" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > In a typical production environment, the user (not necessarily a
> > superuser) sometimes wants to analyze the memory usage via
> > pg_backend_memory_contexts view or pg_log_backend_memory_contexts
> > function which are accessible to only superusers. Isn't it better to
> > allow non-superusers with an appropriate predefined role (I'm thinking
> > of pg_monitor) to access them?
>
> It looks like this was discussed previously [0]. From the description
> of pg_monitor [1], I think it's definitely arguable that this view and
> function should be accessible by roles that are members of pg_monitor.
>
> The pg_monitor, pg_read_all_settings, pg_read_all_stats and
> pg_stat_scan_tables roles are intended to allow administrators
> to easily configure a role for the purpose of monitoring the
> database server. They grant a set of common privileges
> allowing the role to read various useful configuration
> settings, statistics and other system information normally
> restricted to superusers.

Hm.

> AFAICT the current permissions were chosen as a safe default, but
> maybe it can be revisited. The view and function appear to only
> reveal high level information about the memory contexts in use (e.g.,
> name, size, amount used), so I'm not seeing any obvious reason why
> they should remain superuser-only. pg_log_backend_memory_contexts()
> directly affects the server log, which might be a bit beyond what
> pg_monitor should be able to do. My currently thinking is that we
> should give pg_monitor access to pg_backend_memory_contexts (and maybe
> even pg_shmem_allocations).

pg_shmem_allocations is also a good candidate.

> However, one interesting thing I see is
> that there is no mention of any predefined roles in system_views.sql.
> Instead, the convention seems to be to add hard-coded checks for
> predefined roles in the backing functions. I don't know if that's a
> hard and fast rule, but I do see that predefined roles are given
> special privileges in system_functions.sql.

There are two things: 1) We revoke the permissions for nonsuperuser in
system_views.sql with the below
REVOKE ALL ON pg_shmem_allocations FROM PUBLIC;
REVOKE EXECUTE ON FUNCTION pg_get_shmem_allocations() FROM PUBLIC;
REVOKE ALL ON pg_backend_memory_contexts FROM PUBLIC;
REVOKE EXECUTE ON FUNCTION pg_get_backend_memory_contexts() FROM PUBLIC;

2) We don't revoke any permissions in the system_views.sql, but we
have the following kind checks in the underlying function:
/*
* Only superusers or members of pg_monitor can <<see the details>>.
*/
if (!superuser() || !is_member_of_role(GetUserId(), ROLE_PG_MONITOR))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be a superuser or a member of the pg_monitor role to
<<see the details>>")));

I think we can remove the below revoke statements from
system_views.sql and place the checks shown at (2) in the underlying
functions pg_get_shmem_allocations, pg_get_backend_memory_contexts,
also in pg_log_backend_memory_contexts.

REVOKE ALL ON pg_shmem_allocations FROM PUBLIC;
REVOKE EXECUTE ON FUNCTION pg_get_shmem_allocations() FROM PUBLIC;
REVOKE ALL ON pg_backend_memory_contexts FROM PUBLIC;
REVOKE EXECUTE ON FUNCTION pg_get_backend_memory_contexts() FROM PUBLIC;

Thoughts?

Regards,
Bharath Rupireddy.