Re: Read access for pg_monitor to pg_replication_origin_status view

On Thu, 4 Jun 2020 at 21:17, Martín Marqués <martin@2ndquadrant.com> wrote:
>
> Hi Kyotaro-san,
>
> > Sorry for not mentioning it at that time, but about the following diff:
> >
> > +GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;
> >
> > system_views.sql already has a REVOKE command on the view. We should
> > put the above just below the REVOKE command.
> >
> > I'm not sure where to put the GRANT on
> > pg_show_replication_origin_status(), but maybe it also should be at
> > the same place.
>
> Yes, I agree that it makes the revoking/granting easier to read if
> it's grouped by objects, or groups of objects.
>
> Done.
>
> > In the previous comment, one point I meant is that the "to the
> > superuser" should be "to superusers", because a PostgreSQL server
> > (cluster) can define multiple superusers. Another is that "permitted
> > to other users by using the GRANT command." might be obscure for
> > users. In this regard I found a more specific description in the same
> > file:
>
> OK, now I understand what you were saying. :-)
>
> >   Computes the total disk space used by the database with the specified
> >   name or OID.  To use this function, you must
> >   have <literal>CONNECT</literal> privilege on the specified database
> >   (which is granted by default) or be a member of
> >   the <literal>pg_read_all_stats</literal> role.
> >
> > So, as the result it would be like the following: (Note that, as you
> > know, I'm not good at this kind of task..)
> >
> >   Use of functions for replication origin is restricted to superusers.
> >   Use for these functions may be permitted to other users by granting
> >   <literal>EXECUTE<literal> privilege on the functions.
> >
> > And in regard to the view, granting privileges on both the view and
> > function to individual user is not practical so we should mention only
> > granting pg_read_all_stats to users, like the attached patch.
>
> I did some re-writing of the doc, which is pretty close to what you
> proposed above.
>
> > By the way, the attachements of your mail are out-of-order. I'm not
> > sure that that does something bad, though.
>
> That's likely Gmail giving them random order when you attach multiple
> files all at once.
>
> New patches attached.
>

I've looked at these patches and have one question:

 REVOKE ALL ON pg_replication_origin_status FROM public;

+GRANT SELECT ON pg_replication_origin_status TO pg_read_all_stats;

+REVOKE EXECUTE ON FUNCTION pg_show_replication_origin_status() FROM public;
+
+GRANT EXECUTE ON FUNCTION pg_show_replication_origin_status() TO
pg_read_all_stats;

I thought that this patch has pg_replication_origin_status view behave
like other pg_stat_* views in terms of privileges but it's slightly
different. For instance, since we grant all privileges on
pg_stat_replication to public by default, the only user who either is
a member of pg_read_all_stats or is superuser can see all values but
other users not having such privileges also can access that view and
see the part of statistics. On the other hand, with this patch, we
allow only user who either is a member of pg_read_all_stats or is
superuser to access pg_replication_origin_status view. Other users
cannot even access to that view. Is there any reason why we grant
select privilege to only pg_read_all_stats? I wonder if we can have
pg_replication_origin_status accessible by public and filter some
column data in pg_show_replication_origin_status() that we don't want
to show to users who neither a member of pg_read_all_stats nor
superuser.

There is a typo in 0001 patch:

+--
+-- Permision to execute Replication Origin functions should be
revoked from public
+--

s/Permision/Permission/

Regards,

--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services