Hi. At Mon, 1 Jun 2020 21:41:13 -0300, Martín Marqués <mar...@2ndquadrant.com> wrote in > Hi, > > > Took me a bit longer than expected, but here is a new version, now > > with the idea of just removing the superuser() check and REVOKEing > > execution of the functions from public. At the end I grant permission > > to functions and the pg_replication_origin_status view. > > > I wonder now if I needed to GRANT execution of the functions. A grant > > on the view should be enough. > > > I'll think about it. > > Yeah, those `GRANT EXECUTE` for the 2 functions should go, as the view > which is what we want to `SELECT` from has the appropriate ACL set. > > $ git diff > diff --git a/src/backend/catalog/system_views.sql > b/src/backend/catalog/system_views.sql > index c16061f8f00..97ee72a9cfc 100644 > --- a/src/backend/catalog/system_views.sql > +++ b/src/backend/catalog/system_views.sql > @@ -1494,9 +1494,6 @@ GRANT EXECUTE ON FUNCTION > pg_ls_archive_statusdir() TO pg_monitor; > GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor; > GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor; > > -GRANT EXECUTE ON FUNCTION pg_replication_origin_progress(text, > boolean) TO pg_monitor; > -GRANT EXECUTE ON FUNCTION > pg_replication_origin_session_progress(boolean) TO pg_monitor; > - > GRANT pg_read_all_settings TO pg_monitor; > GRANT pg_read_all_stats TO pg_monitor; > GRANT pg_stat_scan_tables TO pg_monitor;
Agreed on this part. The two functions aren't needed to be granted. But, pg_show_replication_origin_status() should be allowed pg_read_all_stats, not pg_monitor. pg_monitor is just a union role of actual privileges. Another issue would be how to control output of pg_show_replication_origin_status(). Most of functions that needs pg_read_all_stats privileges are filtering sensitive columns in each row, instead of hiding the existence of rows. Maybe the view pg_replication_origin_status should show only local_id and hide other columns from non-pg_read_all_stats users. -- Kyotaro Horiguchi NTT Open Source Software Center