From: | Martín Marqués <martin(at)2ndquadrant(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Read access for pg_monitor to pg_replication_origin_status view |
Date: | 2020-06-02 14:23:26 |
Message-ID: | CAPdiE1wTq7yNc1UN_vw2jdxNg15cKn09FJjovqRq36Nq5cFbxw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
> > $ 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.
I placed that GRANT on purpose to `pg_monitor`, separated from the
`pg_read_all_stats` role, because it doesn't match the description for
that role.
```
Read all pg_stat_* views and use various statistics related
extensions, even those normally visible only to superusers.
```
I have no problem adding it to this ROLE, but we'd have to amend the
doc for default-roles to reflect that SELECT for this view is also
granted to `pg_read_all_stats`.
> 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.
I think that the output from `pg_show_replication_origin_status()`
doesn't expose any data that `pg_read_all_stats` or `pg_monitor`
shouldn't be able to read. Removing or obfuscating `external_id`
and/or `remote_lsn` would make the view somehow pointless, in
particular for monitoring and diagnostic tools.
I'll upload new patches shortly following Michael's suggestions.
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Jürgen Purtz | 2020-06-02 15:01:31 | Re: Additional Chapter for Tutorial |
Previous Message | Dilip Kumar | 2020-06-02 14:22:48 | Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions |