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