Hi On Tue, Mar 28, 2017 at 2:22 PM, Stephen Frost <sfr...@snowman.net> wrote: > Dave, > > * Dave Page (dp...@pgadmin.org) wrote: >> OK, so before I start hacking again, here's a proposal based on my >> understanding of folks comments, and so open questions. If I can get >> agreement and answers, I'll be able to break out vi again without >> (hopefully) too many more revisions: >> >> pg_read_all_stats: Will have C-coded access to pg_stats views and >> pg_*_size that are currently hard-coded to superuser > > Not quite sure what you mean here by 'C-coded access to pg_stats > *views*', but I'm guessing that isn't exactly what you meant since I'm > sure we aren't going to change how view permissions are done in this > patch. I take it you mean access to the functions under the views? If > so, I believe this is correct.
Right, sorry I wasn't clear. >> pg_read_all_settings: Will have C-coded access to read GUCs that are >> currently hard-coded to the superuser > > Right. > >> pg_monitor: Will include pg_read_all_stats and pg_read_all_settings, >> and all explicitly GRANTable rights, e.g. in contrib modules. > > Right. > >> Patch to be rebased on Simon's updated version. > > Right. > >> Questions: >> >> - pg_stat_statements has a hard-coded superuser check. Shall I remove >> that, and include REVOKE ALL FROM ALL and then GRANT to pg_monitor? > > pg_stat_statements shouldn't have ever had that superuser check and it > shouldn't have ever used '==' for the user check, it should have been > using 'has_privs_of_role()' from the start, which means that the > superuser check isn't necessary. > > I don't think we should remove that check, nor should we REVOKE EXECUTE > from public for the function. We *should* add a hard-coded role check > to allow another role which isn't a member of the role whose query it is > to view the query. That shouldn't be pg_monitor, of course (as > discussed). I don't think pg_read_all_stats or pg_read_all_settings > really covers this case either- this is more like pg_read_all_queries > and should also be used for pg_stat_activity. OK, so essentially what I did, except s/pg_read_all_stats/pg_read_all_queries ? > That would then be granted to pg_monitor. > >> - pgrowlocks has hard-coded access to superuser and users with SELECT >> on the table being examined. How should this be handled? > > I don't see any hard-coded superuser check? There is a > pg_class_aclcheck() for SELECT rights on the table. I like the idea of > having another way to grant access to run this function on a table > besides giving SELECT rights on the entire table to the user. This > would fall under the mandate of the role described in your next bullet, > in my view. > >> - Stephen suggested a separate role for functions that can lock >> tables. Is this still desired, or shall we just grant access to >> pg_monitor (I think the latter is fine)? > > Right, I was thinking something like pg_stat_all_tables or > pg_stat_scan_tables or similar. We would add that (perhaps also with a > SELECT check like pgrowlocks has) for the other functions like > pgstattuple and pg_freespacemap and pg_visibility. So pgstattuple, pg_sfreespacemap, pg_visibility and pgrowlocks to be allowed access from members of pg_stat_scan_tables, which in turn is granted to pg_monitor? Thanks! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers