Re: Allow pg_read_all_stats to read pg_stat_progress_*

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.

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