Re: Allow pg_read_all_stats to read pg_stat_progress_*

On Mon, Apr 20, 2020 at 4:12 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > Ugh. That doesn't make it correct though.. We really should be using
> > has_privs_of_role() for these cases (and that goes for all of the
> > default role cases- some of which are correct and others are not, it
> > seems).
>
> I have a different concern about this patch: while reading statistical
> values is fine, do we REALLY want pg_read_all_stats to enable
> pg_stat_get_activity(), ie viewing other sessions' command strings?
> That opens security considerations that don't seem to me to be covered
> by the description of the role.
>

It already did allow that, and that's fully documented.

The patch only adds the ability to get at it through functions, but not
through views. (And the pg_stat_progress_* views).

The pg_stat_activity change is only:
@@ -669,8 +671,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
nulls[16] = true;

/* Values only available to role member or
pg_read_all_stats */
- if (has_privs_of_role(GetUserId(), beentry->st_userid) ||
- is_member_of_role(GetUserId(),
DEFAULT_ROLE_READ_ALL_STATS))
+ if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid))
{
SockAddr zero_clientaddr;
char *clipped_activity;

Which moves the check into the macro, but doesn't change how it works.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>