Hi On Thu, Mar 23, 2017 at 12:23 PM, Stephen Frost <sfr...@snowman.net> wrote: > >> diff --git a/contrib/pg_stat_statements/pg_stat_statements.c >> b/contrib/pg_stat_statements/pg_stat_statements.c >> index 221ac98d4a..cec94d5896 100644 >> --- a/contrib/pg_stat_statements/pg_stat_statements.c >> +++ b/contrib/pg_stat_statements/pg_stat_statements.c >> @@ -1386,7 +1387,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, >> MemoryContext per_query_ctx; >> MemoryContext oldcontext; >> Oid userid = GetUserId(); >> - bool is_superuser = superuser(); >> + bool is_superuser = false; >> char *qbuffer = NULL; >> Size qbuffer_size = 0; >> Size extent = 0; >> @@ -1394,6 +1395,9 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, >> HASH_SEQ_STATUS hash_seq; >> pgssEntry *entry; >> >> + /* For the purposes of this module, we consider pg_read_all_stats >> members to be superusers */ >> + is_superuser = (superuser() || is_member_of_role(GetUserId(), >> DEFAULT_ROLE_READ_ALL_STATS)); > > Whoahhhh, let's *not* set an 'is_superuser' flag for a case where the > user is not, actually, a superuser. This should be 'allowed_role' or > something along those lines, I'm thinking.
Fixed. >> diff --git a/contrib/pg_visibility/Makefile b/contrib/pg_visibility/Makefile >> index bc42944426..21d787ddf7 100644 >> --- a/contrib/pg_visibility/Makefile >> +++ b/contrib/pg_visibility/Makefile >> @@ -4,7 +4,8 @@ MODULE_big = pg_visibility >> OBJS = pg_visibility.o $(WIN32RES) >> >> EXTENSION = pg_visibility >> -DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql >> +DATA = pg_visibility--1.1.sql pg_visibility--1.1--1.2.sql \ >> + pg_visibility--1.0--1.1.sql >> PGFILEDESC = "pg_visibility - page visibility information" >> >> REGRESS = pg_visibility > > Missing file here also. Yeah, I forgot to add this and the others. Sorry about - fixed now. >> diff --git a/contrib/pgrowlocks/pgrowlocks.c >> b/contrib/pgrowlocks/pgrowlocks.c >> index db9e0349a0..c9194d8c0d 100644 >> --- a/contrib/pgrowlocks/pgrowlocks.c >> +++ b/contrib/pgrowlocks/pgrowlocks.c >> @@ -28,6 +28,7 @@ >> #include "access/relscan.h" >> #include "access/xact.h" >> #include "catalog/namespace.h" >> +#include "catalog/pg_authid.h" >> #include "funcapi.h" >> #include "miscadmin.h" >> #include "storage/bufmgr.h" >> @@ -98,9 +99,11 @@ pgrowlocks(PG_FUNCTION_ARGS) >> relrv = >> makeRangeVarFromNameList(textToQualifiedNameList(relname)); >> rel = heap_openrv(relrv, AccessShareLock); >> >> - /* check permissions: must have SELECT on table */ >> - aclresult = pg_class_aclcheck(RelationGetRelid(rel), >> GetUserId(), >> - >> ACL_SELECT); >> + /* check permissions: must have SELECT on table or be in >> pg_read_all_stats */ >> + aclresult = (pg_class_aclcheck(RelationGetRelid(rel), >> GetUserId(), >> + >> ACL_SELECT) || >> + is_member_of_role(GetUserId(), >> DEFAULT_ROLE_READ_ALL_STATS)); >> + >> if (aclresult != ACLCHECK_OK) >> aclcheck_error(aclresult, ACL_KIND_CLASS, >> >> RelationGetRelationName(rel)); > > Doesn't this go beyond just pg_stat_X, but actually allows running > pgrowlocks against any relation? That seems like it should be > independent, though it actually fits the "global-read-metadata" role > case that has been discussed previously as being what pg_visibility, > pgstattuple, and other similar contrib modules need. I don't have a > great name for that role, but it seems different to me from > 'pg_read_all_stats', though I don't hold that position very strongly as > I can see an argument that these kind of are stats. That was the way I was leaning, on the grounds that the distinction between two separate roles might end up being confusing for users. It's simple enough to change if that's the consencus, and we can come up with a suitable name. > I see you did change pgstattuple too, and perhaps these are the changes > you intended for pg_visibility too? In any case, if so, we need to make > it clear in the docs that pg_read_all_stats grants access to more than > just pg_stat_X. The pg_read_all_stats role docs have: Read all pg_stat_* views and use various statistics related extensions, even those normally visible only to superusers. And then I updated the docs for each contrib module to note where pg_read_all_stats can use functionality. >> diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml >> index df0435c3f0..5472ff76a7 100644 >> --- a/doc/src/sgml/catalogs.sgml >> +++ b/doc/src/sgml/catalogs.sgml >> @@ -10027,15 +10027,17 @@ SELECT * FROM pg_locks pl LEFT JOIN >> pg_prepared_xacts ppx >> <entry><type>text</type></entry> >> <entry>Configuration file the current value was set in (null for >> values set from sources other than configuration files, or when >> - examined by a non-superuser); >> - helpful when using <literal>include</> directives in configuration >> files</entry> >> + examined by a user who is neither a superuser or a member of >> + <literal>pg_read_all_settings</literal>); helpful when using >> + <literal>include</> directives in configuration files</entry> >> </row> >> <row> >> <entry><structfield>sourceline</structfield></entry> >> <entry><type>integer</type></entry> >> <entry>Line number within the configuration file the current value was >> set at (null for values set from sources other than configuration >> files, >> - or when examined by a non-superuser) >> + or when examined by a user who is neither a superuser or a member of >> + <literal>pg_read_all_settings</literal>). >> </entry> >> </row> >> <row> > > We should really figure out a way to link back to the default roles > section when we refer to roles from other places. Not sure what the > best way to do that off-hand is though. Yeah, I have no idea about that. >> diff --git a/doc/src/sgml/pgbuffercache.sgml >> b/doc/src/sgml/pgbuffercache.sgml >> index b261a4dbe0..9d1faefdd6 100644 >> --- a/doc/src/sgml/pgbuffercache.sgml >> +++ b/doc/src/sgml/pgbuffercache.sgml >> @@ -24,8 +24,9 @@ >> </para> >> >> <para> >> - By default public access is revoked from both of these, just in case there >> - are security issues lurking. >> + By default use is restricted to superusers and members of the >> + <literal>pg_read_all_stats</literal> role, just in case there are security >> + issues lurking. >> </para> >> >> <sect2> >> diff --git a/doc/src/sgml/pgfreespacemap.sgml >> b/doc/src/sgml/pgfreespacemap.sgml >> index f2f99d571e..f4f0e08473 100644 >> --- a/doc/src/sgml/pgfreespacemap.sgml >> +++ b/doc/src/sgml/pgfreespacemap.sgml >> @@ -16,8 +16,9 @@ >> </para> >> >> <para> >> - By default public access is revoked from the functions, just in case >> - there are security issues lurking. >> + By default use is restricted to superusers and members of the >> + <literal>pg_read_all_stats</literal> role, just in case there are security >> + issues lurking. >> </para> >> >> <sect2> > > These also look like cases where we might want to think about if we > should have a dedicated role which, essentially, allows locking and > scanning of all relations. > > In particular, I can certainly imagine admins who wish to GRANT out the > rights to view other queries in pg_stat_activity to another admin and > maybe even allow them to run pg_terminate_backend, or pg_cancel_query, > but not allow them to scan every relation in the database. Easy enough to add - thoughts on the name though? I'm struggling to come up with anything sane. >> diff --git a/src/backend/replication/walreceiver.c >> b/src/backend/replication/walreceiver.c >> index 31c567b37e..8e5c8c507c 100644 >> --- a/src/backend/replication/walreceiver.c >> +++ b/src/backend/replication/walreceiver.c >> @@ -50,6 +50,7 @@ >> #include "access/timeline.h" >> #include "access/transam.h" >> #include "access/xlog_internal.h" >> +#include "catalog/pg_authid.h" >> #include "catalog/pg_type.h" >> #include "funcapi.h" >> #include "libpq/pqformat.h" >> @@ -1396,7 +1397,7 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS) >> /* Fetch values */ >> values[0] = Int32GetDatum(walrcv->pid); >> >> - if (!superuser()) >> + if (!superuser() && !is_member_of_role(GetUserId(), >> DEFAULT_ROLE_MONITOR)) >> { >> /* >> * Only superusers can see details. Other users only get the >> pid value > > Shouldn't that be DEFAULT_ROLE_READ_ALL_STATS, not MONITOR? Ooops, fixed. >> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c >> index 4feb26aa7a..d1da580c9c 100644 >> --- a/src/backend/utils/misc/guc.c >> +++ b/src/backend/utils/misc/guc.c >> @@ -34,6 +34,7 @@ >> #include "access/xact.h" >> #include "access/xlog_internal.h" >> #include "catalog/namespace.h" >> +#include "catalog/pg_authid.h" >> #include "commands/async.h" >> #include "commands/prepare.h" >> #include "commands/user.h" >> @@ -6677,10 +6678,12 @@ GetConfigOption(const char *name, bool missing_ok, >> bool restrict_superuser) >> } >> if (restrict_superuser && >> (record->flags & GUC_SUPERUSER_ONLY) && >> - !superuser()) >> + !superuser() && >> + !is_member_of_role(GetUserId(), >> DEFAULT_ROLE_READ_ALL_SETTINGS)) > > Seems like having a variable to indicate if the caller should be able to > read all these settings or not might be nice to have, so we have one > place that figures out the privileges question. Those checks are not all identical, and they're in different functions so a variable wouldn't work (I assume you're not suggesting I add a global :-p ). I could push part of each check out into a separate function, but it doesn't seem like it would really add to the readability to me. > A few comments where that variable is set wouldn't be bad either. > > Also, I'm thinking the pg_monitor documentation really needs to be > expanded and made very clear. I'll think a bit more on just how to try > and phrase that, but the description included certainly seemed > inadequate. I've added some additional text below the table. > That's it for a quick review. If we can get these changes done and > there aren't any more questions or concerns, I'm happy to continue > working to move this forward. I definitely see the usefulness of these > changes and it'd be great to have an answer to the oft-asked question of > how to do proper monitoring with a non-superuser role. Thanks - updated patch attached. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
diff --git a/contrib/pg_buffercache/Makefile b/contrib/pg_buffercache/Makefile index 497dbeb229..18f7a87452 100644 --- a/contrib/pg_buffercache/Makefile +++ b/contrib/pg_buffercache/Makefile @@ -4,8 +4,9 @@ MODULE_big = pg_buffercache OBJS = pg_buffercache_pages.o $(WIN32RES) EXTENSION = pg_buffercache -DATA = pg_buffercache--1.2.sql pg_buffercache--1.1--1.2.sql \ - pg_buffercache--1.0--1.1.sql pg_buffercache--unpackaged--1.0.sql +DATA = pg_buffercache--1.2.sql pg_buffercache--1.2--1.3.sql \ + pg_buffercache--1.1--1.2.sql pg_buffercache--1.0--1.1.sql \ + pg_buffercache--unpackaged--1.0.sql PGFILEDESC = "pg_buffercache - monitoring of shared buffer cache in real-time" ifdef USE_PGXS diff --git a/contrib/pg_buffercache/pg_buffercache.control b/contrib/pg_buffercache/pg_buffercache.control index a4d664f3fa..8c060ae9ab 100644 --- a/contrib/pg_buffercache/pg_buffercache.control +++ b/contrib/pg_buffercache/pg_buffercache.control @@ -1,5 +1,5 @@ # pg_buffercache extension comment = 'examine the shared buffer cache' -default_version = '1.2' +default_version = '1.3' module_pathname = '$libdir/pg_buffercache' relocatable = true diff --git a/contrib/pg_freespacemap/Makefile b/contrib/pg_freespacemap/Makefile index 7bc0e9555d..0a2f000ec6 100644 --- a/contrib/pg_freespacemap/Makefile +++ b/contrib/pg_freespacemap/Makefile @@ -4,8 +4,8 @@ MODULE_big = pg_freespacemap OBJS = pg_freespacemap.o $(WIN32RES) EXTENSION = pg_freespacemap -DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.0--1.1.sql \ - pg_freespacemap--unpackaged--1.0.sql +DATA = pg_freespacemap--1.1.sql pg_freespacemap--1.1--1.2.sql \ + pg_freespacemap--1.0--1.1.sql pg_freespacemap--unpackaged--1.0.sql PGFILEDESC = "pg_freespacemap - monitoring of free space map" ifdef USE_PGXS diff --git a/contrib/pg_freespacemap/pg_freespacemap.control b/contrib/pg_freespacemap/pg_freespacemap.control index 764db30d18..ac8fc5050a 100644 --- a/contrib/pg_freespacemap/pg_freespacemap.control +++ b/contrib/pg_freespacemap/pg_freespacemap.control @@ -1,5 +1,5 @@ # pg_freespacemap extension comment = 'examine the free space map (FSM)' -default_version = '1.1' +default_version = '1.2' module_pathname = '$libdir/pg_freespacemap' relocatable = true diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile index 298951a5f5..39b368b70e 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -4,9 +4,10 @@ MODULE_big = pg_stat_statements OBJS = pg_stat_statements.o $(WIN32RES) EXTENSION = pg_stat_statements -DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.3--1.4.sql \ - pg_stat_statements--1.2--1.3.sql pg_stat_statements--1.1--1.2.sql \ - pg_stat_statements--1.0--1.1.sql pg_stat_statements--unpackaged--1.0.sql +DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.4--1.5.sql \ + pg_stat_statements--1.3--1.4.sql pg_stat_statements--1.2--1.3.sql \ + pg_stat_statements--1.1--1.2.sql pg_stat_statements--1.0--1.1.sql \ + pg_stat_statements--unpackaged--1.0.sql PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements" LDFLAGS_SL += $(filter -lm, $(LIBS)) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 221ac98d4a..3d2dabdce4 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -62,6 +62,7 @@ #include <unistd.h> #include "access/hash.h" +#include "catalog/pg_authid.h" #include "executor/instrument.h" #include "funcapi.h" #include "mb/pg_wchar.h" @@ -1386,7 +1387,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, MemoryContext per_query_ctx; MemoryContext oldcontext; Oid userid = GetUserId(); - bool is_superuser = superuser(); + bool is_allowed_role = false; char *qbuffer = NULL; Size qbuffer_size = 0; Size extent = 0; @@ -1394,6 +1395,9 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, HASH_SEQ_STATUS hash_seq; pgssEntry *entry; + /* For the purposes of this module, we consider pg_read_all_stats members to be superusers */ + is_allowed_role = (superuser() || is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS)); + /* hash table must exist already */ if (!pgss || !pgss_hash) ereport(ERROR, @@ -1536,7 +1540,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, values[i++] = ObjectIdGetDatum(entry->key.userid); values[i++] = ObjectIdGetDatum(entry->key.dbid); - if (is_superuser || entry->key.userid == userid) + if (is_allowed_role || entry->key.userid == userid) { if (api_version >= PGSS_V1_2) values[i++] = Int64GetDatumFast(queryid); diff --git a/contrib/pg_stat_statements/pg_stat_statements.control b/contrib/pg_stat_statements/pg_stat_statements.control index 24038f56b1..193fcdfafa 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.control +++ b/contrib/pg_stat_statements/pg_stat_statements.control @@ -1,5 +1,5 @@ # pg_stat_statements extension comment = 'track execution statistics of all SQL statements executed' -default_version = '1.4' +default_version = '1.5' module_pathname = '$libdir/pg_stat_statements' relocatable = true diff --git a/contrib/pg_visibility/Makefile b/contrib/pg_visibility/Makefile index bc42944426..21d787ddf7 100644 --- a/contrib/pg_visibility/Makefile +++ b/contrib/pg_visibility/Makefile @@ -4,7 +4,8 @@ MODULE_big = pg_visibility OBJS = pg_visibility.o $(WIN32RES) EXTENSION = pg_visibility -DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql +DATA = pg_visibility--1.1.sql pg_visibility--1.1--1.2.sql \ + pg_visibility--1.0--1.1.sql PGFILEDESC = "pg_visibility - page visibility information" REGRESS = pg_visibility diff --git a/contrib/pg_visibility/pg_visibility.control b/contrib/pg_visibility/pg_visibility.control index f93ed0176e..3cffa08b01 100644 --- a/contrib/pg_visibility/pg_visibility.control +++ b/contrib/pg_visibility/pg_visibility.control @@ -1,5 +1,5 @@ # pg_visibility extension comment = 'examine the visibility map (VM) and page-level visibility info' -default_version = '1.1' +default_version = '1.2' module_pathname = '$libdir/pg_visibility' relocatable = true diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c index db9e0349a0..c9194d8c0d 100644 --- a/contrib/pgrowlocks/pgrowlocks.c +++ b/contrib/pgrowlocks/pgrowlocks.c @@ -28,6 +28,7 @@ #include "access/relscan.h" #include "access/xact.h" #include "catalog/namespace.h" +#include "catalog/pg_authid.h" #include "funcapi.h" #include "miscadmin.h" #include "storage/bufmgr.h" @@ -98,9 +99,11 @@ pgrowlocks(PG_FUNCTION_ARGS) relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); rel = heap_openrv(relrv, AccessShareLock); - /* check permissions: must have SELECT on table */ - aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(), - ACL_SELECT); + /* check permissions: must have SELECT on table or be in pg_read_all_stats */ + aclresult = (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(), + ACL_SELECT) || + is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS)); + if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, ACL_KIND_CLASS, RelationGetRelationName(rel)); diff --git a/contrib/pgstattuple/pgstattuple--1.4--1.5.sql b/contrib/pgstattuple/pgstattuple--1.4--1.5.sql index 84e112e1c2..a6143fcafe 100644 --- a/contrib/pgstattuple/pgstattuple--1.4--1.5.sql +++ b/contrib/pgstattuple/pgstattuple--1.4--1.5.sql @@ -17,6 +17,7 @@ AS 'MODULE_PATHNAME', 'pgstattuple_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstattuple(text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstattuple(text) TO pg_read_all_stats; CREATE OR REPLACE FUNCTION pgstatindex(IN relname text, OUT version INT, @@ -33,6 +34,7 @@ AS 'MODULE_PATHNAME', 'pgstatindex_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstatindex(text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstatindex(text) TO pg_read_all_stats; CREATE OR REPLACE FUNCTION pg_relpages(IN relname text) RETURNS BIGINT @@ -40,6 +42,7 @@ AS 'MODULE_PATHNAME', 'pg_relpages_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pg_relpages(text) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_relpages(text) TO pg_read_all_stats; /* New stuff in 1.1 begins here */ @@ -51,6 +54,7 @@ AS 'MODULE_PATHNAME', 'pgstatginindex_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstatginindex(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstatginindex(regclass) TO pg_read_all_stats; /* New stuff in 1.2 begins here */ @@ -68,6 +72,7 @@ AS 'MODULE_PATHNAME', 'pgstattuplebyid_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstattuple(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstattuple(regclass) TO pg_read_all_stats; CREATE OR REPLACE FUNCTION pgstatindex(IN relname regclass, OUT version INT, @@ -84,6 +89,7 @@ AS 'MODULE_PATHNAME', 'pgstatindexbyid_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstatindex(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstatindex(regclass) TO pg_read_all_stats; CREATE OR REPLACE FUNCTION pg_relpages(IN relname regclass) RETURNS BIGINT @@ -91,6 +97,7 @@ AS 'MODULE_PATHNAME', 'pg_relpagesbyid_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pg_relpages(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_relpages(regclass) TO pg_read_all_stats; /* New stuff in 1.3 begins here */ @@ -109,6 +116,7 @@ AS 'MODULE_PATHNAME', 'pgstattuple_approx_v1_5' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstattuple_approx(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstattuple_approx(regclass) TO pg_read_all_stats; /* New stuff in 1.5 begins here */ @@ -125,3 +133,4 @@ AS 'MODULE_PATHNAME', 'pgstathashindex' LANGUAGE C STRICT PARALLEL SAFE; REVOKE EXECUTE ON FUNCTION pgstathashindex(regclass) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pgstathashindex(regclass) TO pg_read_all_stats; diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index df0435c3f0..5472ff76a7 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -10027,15 +10027,17 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx <entry><type>text</type></entry> <entry>Configuration file the current value was set in (null for values set from sources other than configuration files, or when - examined by a non-superuser); - helpful when using <literal>include</> directives in configuration files</entry> + examined by a user who is neither a superuser or a member of + <literal>pg_read_all_settings</literal>); helpful when using + <literal>include</> directives in configuration files</entry> </row> <row> <entry><structfield>sourceline</structfield></entry> <entry><type>integer</type></entry> <entry>Line number within the configuration file the current value was set at (null for values set from sources other than configuration files, - or when examined by a non-superuser) + or when examined by a user who is neither a superuser or a member of + <literal>pg_read_all_settings</literal>). </entry> </row> <row> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 4dc30caccb..b32d1ce17c 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -19370,9 +19370,11 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); accept the OID or name of a database or tablespace, and return the total disk space used therein. To use <function>pg_database_size</function>, you must have <literal>CONNECT</> permission on the specified database - (which is granted by default). To use <function>pg_tablespace_size</>, - you must have <literal>CREATE</> permission on the specified tablespace, - unless it is the default tablespace for the current database. + (which is granted by default), or be a member of the <literal>pg_read_all_stats</> + role. To use <function>pg_tablespace_size</>, you must have + <literal>CREATE</> permission on the specified tablespace, or be a member + of the <literal>pg_read_all_stats</> role unless it is the default tablespace for + the current database. </para> <para> @@ -19681,7 +19683,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); <entry><type>setof record</type></entry> <entry> List the name, size, and last modification time of files in the log - directory. Access may be granted to non-superuser roles. + directory. Access is granted to members of the <literal>pg_monitor</> + role and may be granted to other non-superuser roles. </entry> </row> <row> @@ -19691,7 +19694,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); <entry><type>setof record</type></entry> <entry> List the name, size, and last modification time of files in the WAL - directory. Access may be granted to non-superuser roles. + directory.Access is granted to members of the <literal>pg_monitor</> + role and may be granted to other non-superuser roles. </entry> </row> <row> @@ -19752,8 +19756,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); <para> <function>pg_ls_logdir</> returns the name, size, and last modified time (mtime) of each file in the log directory. By default, only superusers - can use this function, but access may be granted to others using - <command>GRANT</command>. + and members of the <literal>pg_monitor</> role can use this function, but + access may be granted to others using <command>GRANT</command>. </para> <indexterm> @@ -19762,8 +19766,9 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); <para> <function>pg_ls_waldir</> returns the name, size, and last modified time (mtime) of each file in the write ahead log (WAL) directory. By - default only superusers can use this function, but access may be granted - to others using <command>GRANT</command>. + default only superusers and members of the <literal>pg_monitor</> role + can use this function, but access may be granted to others using + <command>GRANT</command>. </para> <indexterm> diff --git a/doc/src/sgml/pgbuffercache.sgml b/doc/src/sgml/pgbuffercache.sgml index b261a4dbe0..9d1faefdd6 100644 --- a/doc/src/sgml/pgbuffercache.sgml +++ b/doc/src/sgml/pgbuffercache.sgml @@ -24,8 +24,9 @@ </para> <para> - By default public access is revoked from both of these, just in case there - are security issues lurking. + By default use is restricted to superusers and members of the + <literal>pg_read_all_stats</literal> role, just in case there are security + issues lurking. </para> <sect2> diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml index f2f99d571e..f4f0e08473 100644 --- a/doc/src/sgml/pgfreespacemap.sgml +++ b/doc/src/sgml/pgfreespacemap.sgml @@ -16,8 +16,9 @@ </para> <para> - By default public access is revoked from the functions, just in case - there are security issues lurking. + By default use is restricted to superusers and members of the + <literal>pg_read_all_stats</literal> role, just in case there are security + issues lurking. </para> <sect2> diff --git a/doc/src/sgml/pgrowlocks.sgml b/doc/src/sgml/pgrowlocks.sgml index d73511579c..5f167ebd6a 100644 --- a/doc/src/sgml/pgrowlocks.sgml +++ b/doc/src/sgml/pgrowlocks.sgml @@ -12,6 +12,13 @@ locking information for a specified table. </para> + <para> + By default use is restricted to superusers, members of the + <literal>pg_read_all_stats</literal> role, and users with + <literal>SELECT</literal> permissions on the table. + </para> + + <sect2> <title>Overview</title> diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index d4f09fc2a3..694f3db417 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -226,10 +226,11 @@ </table> <para> - For security reasons, non-superusers are not allowed to see the SQL - text or <structfield>queryid</structfield> of queries executed by other users. - They can see the statistics, however, if the view has been installed in their - database. + For security reasons, only superusers and members of the + <literal>pg_read_all_stats<literal> role are allowed to see the SQL text and + <structfield>queryid</structfield> of queries executed by other users. + Other users can see the statistics, however, if the view has been installed + in their database. </para> <para> diff --git a/doc/src/sgml/pgstattuple.sgml b/doc/src/sgml/pgstattuple.sgml index 62b1a6f479..abd4214859 100644 --- a/doc/src/sgml/pgstattuple.sgml +++ b/doc/src/sgml/pgstattuple.sgml @@ -16,7 +16,8 @@ As these functions return detailed page-level information, only the superuser has EXECUTE privileges on them upon installation. After the functions have been installed, users may issue <command>GRANT</command> commands to change - the privileges on the functions to allow non-superusers to execute them. See + the privileges on the functions to allow non-superusers to execute them. Members + of the <literal>pg_read_all_stats</literal> role are granted access by default. See the description of the <xref linkend="sql-grant"> command for specifics. </para> diff --git a/doc/src/sgml/pgvisibility.sgml b/doc/src/sgml/pgvisibility.sgml index fd486696fc..d7b0a448f3 100644 --- a/doc/src/sgml/pgvisibility.sgml +++ b/doc/src/sgml/pgvisibility.sgml @@ -140,7 +140,10 @@ </variablelist> <para> - By default, these functions are executable only by superusers. + By default, these functions are executable only by superusers and members of the + <literal>pg_read_all_stats</literal> role, with the exception of + <function>pg_truncate_visibility_map(relation regclass)</function> which can only + be executed by superusers. </para> </sect2> diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 7eaefe58c2..db2b2f4592 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -516,6 +516,22 @@ DROP ROLE doomed_role; </thead> <tbody> <row> + <entry>pg_monitor</entry> + <entry>Read/execute various monitoring views and functions. + This role is a member of <literal>pg_read_all_settings</literal> and + <literal>pg_read_all_stats</literal>.</entry> + </row> + <row> + <entry>pg_read_all_settings</entry> + <entry>Read all configuration variables, even those normally visible only to + superusers.</entry> + </row> + <row> + <entry>pg_read_all_stats</entry> + <entry>Read all pg_stat_* views and use various statistics related extensions, + even those normally visible only to superusers.</entry> + </row> + <row> <entry>pg_signal_backend</entry> <entry>Send signals to other backends (eg: cancel query, terminate).</entry> </row> @@ -524,6 +540,20 @@ DROP ROLE doomed_role; </table> <para> + The <literal>pg_monitor</literal>, <literal>pg_read_all_settings</literal> and + <literal>pg_read_all_stats</literal> roles are intended to allow administrators + to easily configure a role for the purpose of monitoring the database server + by granting a set of common privileges allowing the role to read various useful + configuration settings, statistics and other system information normally restricted + to superusers. + </para> + + <para> + Care should be taken when granting these roles to ensure they are only used where + needed to perform the desired monitoring. + </para> + + <para> Administrators can grant access to these roles to users using the GRANT command: diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index c2b0bedc1d..d6ca10f2b1 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1111,3 +1111,8 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM publ REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public; +GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor; +GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor; + +GRANT pg_read_all_settings TO pg_monitor; +GRANT pg_read_all_stats TO pg_monitor; diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 31c567b37e..a61c565593 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -50,6 +50,7 @@ #include "access/timeline.h" #include "access/transam.h" #include "access/xlog_internal.h" +#include "catalog/pg_authid.h" #include "catalog/pg_type.h" #include "funcapi.h" #include "libpq/pqformat.h" @@ -1396,7 +1397,7 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS) /* Fetch values */ values[0] = Int32GetDatum(walrcv->pid); - if (!superuser()) + if (!superuser() && !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS)) { /* * Only superusers can see details. Other users only get the pid value diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 58923912eb..6d56638208 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "catalog/catalog.h" #include "catalog/namespace.h" +#include "catalog/pg_authid.h" #include "catalog/pg_tablespace.h" #include "commands/dbcommands.h" #include "commands/tablespace.h" @@ -88,11 +89,17 @@ calculate_database_size(Oid dbOid) char pathname[MAXPGPATH]; AclResult aclresult; - /* User must have connect privilege for target database */ + /* + * User must have connect privilege for target database + * or be a member of pg_read_all_stats + */ aclresult = pg_database_aclcheck(dbOid, GetUserId(), ACL_CONNECT); - if (aclresult != ACLCHECK_OK) + if (aclresult != ACLCHECK_OK && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS)) + { aclcheck_error(aclresult, ACL_KIND_DATABASE, get_database_name(dbOid)); + } /* Shared storage in pg_global is not counted */ @@ -172,11 +179,12 @@ calculate_tablespace_size(Oid tblspcOid) AclResult aclresult; /* - * User must have CREATE privilege for target tablespace, either - * explicitly granted or implicitly because it is default for current - * database. + * User must be a member of pg_read_all_stats or have CREATE privilege for + * target tablespace, either explicitly granted or implicitly because + * it is default for current database. */ - if (tblspcOid != MyDatabaseTableSpace) + if (tblspcOid != MyDatabaseTableSpace && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS)) { aclresult = pg_tablespace_aclcheck(tblspcOid, GetUserId(), ACL_CREATE); if (aclresult != ACLCHECK_OK) diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index a987d0d621..6bec301450 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "catalog/pg_authid.h" #include "catalog/pg_type.h" #include "common/ip.h" #include "funcapi.h" @@ -648,8 +649,9 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) nulls[18] = nulls[19] = nulls[20] = nulls[21] = nulls[22] = true; } - /* Values only available to role member */ - if (has_privs_of_role(GetUserId(), beentry->st_userid)) + /* 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)) { SockAddr zero_clientaddr; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 4feb26aa7a..d1da580c9c 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -34,6 +34,7 @@ #include "access/xact.h" #include "access/xlog_internal.h" #include "catalog/namespace.h" +#include "catalog/pg_authid.h" #include "commands/async.h" #include "commands/prepare.h" #include "commands/user.h" @@ -6677,10 +6678,12 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser) } if (restrict_superuser && (record->flags & GUC_SUPERUSER_ONLY) && - !superuser()) + !superuser() && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to examine \"%s\"", name))); + errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"", + name))); switch (record->vartype) { @@ -6725,10 +6728,13 @@ GetConfigOptionResetString(const char *name) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("unrecognized configuration parameter \"%s\"", name))); - if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser()) + if ((record->flags & GUC_SUPERUSER_ONLY) && + !superuser() && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to examine \"%s\"", name))); + errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"", + name))); switch (record->vartype) { @@ -8015,10 +8021,13 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok) errmsg("unrecognized configuration parameter \"%s\"", name))); } - if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser()) + if ((record->flags & GUC_SUPERUSER_ONLY) && + !superuser() && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to examine \"%s\"", name))); + errmsg("must be superuser or a member of pg_read_all_settings to examine \"%s\"", + name))); if (varname) *varname = record->name; @@ -8044,7 +8053,9 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow) if (noshow) { if ((conf->flags & GUC_NO_SHOW_ALL) || - ((conf->flags & GUC_SUPERUSER_ONLY) && !superuser())) + ((conf->flags & GUC_SUPERUSER_ONLY) && + !superuser() && + !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))) *noshow = true; else *noshow = false; diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index def71edaa8..46b098c84e 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -99,10 +99,16 @@ typedef FormData_pg_authid *Form_pg_authid; * ---------------- */ DATA(insert OID = 10 ( "POSTGRES" t t t t t t t -1 _null_ _null_)); +DATA(insert OID = 3355 ( "pg_monitor" f t f f f f f -1 _null_ _null_)); +DATA(insert OID = 3356 ( "pg_read_all_settings" f t f f f f f -1 _null_ _null_)); +DATA(insert OID = 3357 ( "pg_read_all_stats" f t f f f f f -1 _null_ _null_)); DATA(insert OID = 4200 ( "pg_signal_backend" f t f f f f f -1 _null_ _null_)); #define BOOTSTRAP_SUPERUSERID 10 +#define DEFAULT_ROLE_MONITOR 3355 +#define DEFAULT_ROLE_READ_ALL_SETTINGS 3356 +#define DEFAULT_ROLE_READ_ALL_STATS 3357 #define DEFAULT_ROLE_SIGNAL_BACKENDID 4200 #endif /* PG_AUTHID_H */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers