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

On Sat, Oct 9, 2021 at 12:45 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> Greetings,
>
> * Bossart, Nathan (bossartn(at)amazon(dot)com) wrote:
> > On 10/8/21, 12:01 AM, "Bharath Rupireddy" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > > 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?
> >
> > This approach would add a restriction that a role must have SUPERUSER
> > or be a member of pg_monitor to use the views/functions. I think
> > there is value in allowing any role to use them (if granted the proper
> > privileges). In any case, users may already depend on being able to
> > do that.
> >
> > Instead, I think we should just grant privileges to pg_monitor. I've
> > attached a (basically untested) patch to demonstrate what I'm
> > thinking.
>
> I'm not necessarily against this, but I will point out that we've stayed
> away, so far, from explicitly GRANT'ing privileges to pg_monitor itself,
> intending that to be a role which just combines privileges of certain
> other predefined roles together.
>
> I would think that these would fall under "pg_read_all_stats", in
> particular, which is explicitly documented as: Read all pg_stat_* views
> and use various statistics related extensions, even those normally
> visible only to superusers.
>
> (the last bit being particularly relevant in this case)

+1. I will prepare the patch with the pg_read_all_stats role.

Regards,
Bharath Rupireddy.