Re: Allow pg_read_all_stats to read pg_stat_progress_* - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: Allow pg_read_all_stats to read pg_stat_progress_*
Date
Msg-id CABUevEwEKiR-sB7suBpZcLYV=2UK3VqK+eTkXipsqwP36Fb5mQ@mail.gmail.com
Whole thread Raw
In response to Re: Allow pg_read_all_stats to read pg_stat_progress_*  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers


On Mon, Apr 20, 2020 at 4:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.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. 

--

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Allow pg_read_all_stats to read pg_stat_progress_*
Next
From: Tom Lane
Date:
Subject: Re: Poll: are people okay with function/operator table redesign?