Proposal: Allow user with pg_monitor role to call pg_stat_reset* functions

Hi,

On 2022-11-21 00:16:20 -0800, sirisha chamarthi wrote:

At present, calling pg_stat_reset* functions requires super user access
unless explicitly grant execute permission on those. In this thread, I am
proposing to grant execute on them to users with pg_monitor role
permissions. This comes handy to the monitoring users (part of pg_monitor
role) to capture the stats fresh and analyze. Do you see any concerns with
this approach?

I think the common assumption is that a monitoring role cannot modify
the system, but this would change that. Normally a monitoring tool
should be able to figure out what changed in stats by comparing values
across time, rather than resetting stats.

Greetings,

Andres Freund

On Mon, Nov 21, 2022 at 3:45 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-11-21 00:16:20 -0800, sirisha chamarthi wrote:

At present, calling pg_stat_reset* functions requires super user access
unless explicitly grant execute permission on those. In this thread, I am
proposing to grant execute on them to users with pg_monitor role
permissions. This comes handy to the monitoring users (part of pg_monitor
role) to capture the stats fresh and analyze. Do you see any concerns with
this approach?

I think the common assumption is that a monitoring role cannot modify
the system, but this would change that. Normally a monitoring tool
should be able to figure out what changed in stats by comparing values
across time, rather than resetting stats.

+1.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Nov 21, 2022 at 03:47:38PM -0500, Robert Haas wrote:

On Mon, Nov 21, 2022 at 3:45 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-11-21 00:16:20 -0800, sirisha chamarthi wrote:

At present, calling pg_stat_reset* functions requires super user access
unless explicitly grant execute permission on those. In this thread, I am
proposing to grant execute on them to users with pg_monitor role
permissions. This comes handy to the monitoring users (part of pg_monitor
role) to capture the stats fresh and analyze. Do you see any concerns with
this approach?

I think the common assumption is that a monitoring role cannot modify
the system, but this would change that. Normally a monitoring tool
should be able to figure out what changed in stats by comparing values
across time, rather than resetting stats.

+1.

This can have negative impact for autovacuum, so +1 too.

On Tue, Nov 22, 2022 at 2:15 AM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2022-11-21 00:16:20 -0800, sirisha chamarthi wrote:

At present, calling pg_stat_reset* functions requires super user access
unless explicitly grant execute permission on those. In this thread, I am
proposing to grant execute on them to users with pg_monitor role
permissions. This comes handy to the monitoring users (part of pg_monitor
role) to capture the stats fresh and analyze. Do you see any concerns with
this approach?

I think the common assumption is that a monitoring role cannot modify
the system, but this would change that. Normally a monitoring tool
should be able to figure out what changed in stats by comparing values
across time, rather than resetting stats.

+1.

A bit more info: AFAICS, there's no explicit if (!superuser()) {
error;} checks in any of pg_stat_reset* functions, which means, one
can still grant execute permissions on them to anyone (any predefined
roles, non-superusers or other superusers) via a superuser outside of
postgres, if that's the use case [1]-- -- The default permissions for functions mean that anyone can execute them. -- A number of functions shouldn't be executable by just anyone, but rather -- than use explicit 'superuser()' checks in those functions, we use the GRANT -- system to REVOKE access to those functions at initdb time. Administrators -- can later change who can access these functions, or leave them as only -- available to superuser / cluster owner, if they choose. -- [2]postgres=# create role foo with nosuperuser; CREATE ROLE postgres=# set role foo; SET postgres=> select pg_stat_reset(); ERROR: permission denied for function pg_stat_reset postgres=> reset role; RESET postgres=# grant execute on function pg_stat_reset() to foo; GRANT postgres=# set role foo; SET postgres=> select pg_stat_reset(); pg_stat_reset ---------------. That's the flexibility
postgres provides for some of the system functions but not all. Most
of the extension functions and some core functions pg_nextoid,
pg_stop_making_pinned_objects, pg_rotate_logfile,
pg_import_system_collations, pg_cancel_backend, pg_terminate_backend,
pg_read_file still have such explicit if (!superuser()) { error;}
checks. I'm not sure if it's the right time to remove such explicit
checks and move to explicit GRANT-REVOKE system.

FWIW, here's a recent commit f0b051e322d530a340e62f2ae16d99acdbcb3d05.

[1]: -- -- The default permissions for functions mean that anyone can execute them. -- A number of functions shouldn't be executable by just anyone, but rather -- than use explicit 'superuser()' checks in those functions, we use the GRANT -- system to REVOKE access to those functions at initdb time. Administrators -- can later change who can access these functions, or leave them as only -- available to superuser / cluster owner, if they choose. --
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
-- than use explicit 'superuser()' checks in those functions, we use the GRANT
-- system to REVOKE access to those functions at initdb time. Administrators
-- can later change who can access these functions, or leave them as only
-- available to superuser / cluster owner, if they choose.
--

[2]: postgres=# create role foo with nosuperuser; CREATE ROLE postgres=# set role foo; SET postgres=> select pg_stat_reset(); ERROR: permission denied for function pg_stat_reset postgres=> reset role; RESET postgres=# grant execute on function pg_stat_reset() to foo; GRANT postgres=# set role foo; SET postgres=> select pg_stat_reset(); pg_stat_reset ---------------
postgres=# create role foo with nosuperuser;
CREATE ROLE
postgres=# set role foo;
SET
postgres=> select pg_stat_reset();
ERROR: permission denied for function pg_stat_reset
postgres=> reset role;
RESET
postgres=# grant execute on function pg_stat_reset() to foo;
GRANT
postgres=# set role foo;
SET
postgres=> select pg_stat_reset();
pg_stat_reset
---------------

(1 row)

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com