From bf038899965263dbc4aef2b43c8fdfe6f49b788f Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Thu, 19 Mar 2015 15:02:33 -0400 Subject: [PATCH] GetUserId() changes to has_privs_of_role() The pg_stat and pg_signal-related functions have been using GetUserId() instead of has_privs_of_role() for checking if the current user should be able to see details in pg_stat_activity or signal other processes, requiring a user to do 'SET ROLE' for inheirited roles for a permissions check, unlike other permissions checks. This patch changes that behavior to, instead, act like most other permission checks and use has_privs_of_role(), removing the 'SET ROLE' need. Documentation and error messages updated accordingly. Per discussion with Alvaro, Peter, Adam (though not using Adam's patch), and Robert. Reviewed by Jeevan Chalke. --- doc/src/sgml/func.sgml | 13 +++++----- src/backend/utils/adt/misc.c | 39 +++++++++++++++++++++++------ src/backend/utils/adt/pgstatfuncs.c | 19 +++++++------- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 5843eaa9ff..aa19e104d9 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -16328,9 +16328,9 @@ SELECT set_config('log_statement_stats', 'off', false); pg_cancel_backend(pid int) boolean - Cancel a backend's current query. You can execute this against - another backend that has exactly the same role as the user calling the - function. In all other cases, you must be a superuser. + Cancel a backend's current query. This is also allowed if the + calling role is a member of the role whose backend is being cancelled, + however only superusers can cancel superuser backends. @@ -16352,10 +16352,9 @@ SELECT set_config('log_statement_stats', 'off', false); pg_terminate_backend(pid int) boolean - Terminate a backend. You can execute this against - another backend that has exactly the same role as the user - calling the function. In all other cases, you must be a - superuser. + Terminate a backend. This is also allowed if the calling role + is a member of the role whose backend is being terminated, however only + superusers can terminate superuser backends. diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 29f7c3badf..61d609f918 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -37,6 +37,7 @@ #include "utils/lsyscache.h" #include "utils/ruleutils.h" #include "tcop/tcopprot.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/timestamp.h" @@ -81,7 +82,9 @@ current_query(PG_FUNCTION_ARGS) * role as the backend being signaled. For "dangerous" signals, an explicit * check for superuser needs to be done prior to calling this function. * - * Returns 0 on success, 1 on general failure, and 2 on permission error. + * Returns 0 on success, 1 on general failure, 2 on normal permission error + * and 3 if the caller needs to be a superuser. + * * In the event of a general failure (return code 1), a warning message will * be emitted. For permission errors, doing that is the responsibility of * the caller. @@ -89,6 +92,7 @@ current_query(PG_FUNCTION_ARGS) #define SIGNAL_BACKEND_SUCCESS 0 #define SIGNAL_BACKEND_ERROR 1 #define SIGNAL_BACKEND_NOPERMISSION 2 +#define SIGNAL_BACKEND_NOSUPERUSER 3 static int pg_signal_backend(int pid, int sig) { @@ -113,7 +117,12 @@ pg_signal_backend(int pid, int sig) return SIGNAL_BACKEND_ERROR; } - if (!(superuser() || proc->roleId == GetUserId())) + /* Only allow superusers to signal superuser-owned backends. */ + if (superuser_arg(proc->roleId) && !superuser()) + return SIGNAL_BACKEND_NOSUPERUSER; + + /* Users can signal backends they have role membership in. */ + if (!has_privs_of_role(GetUserId(), proc->roleId)) return SIGNAL_BACKEND_NOPERMISSION; /* @@ -141,35 +150,49 @@ pg_signal_backend(int pid, int sig) } /* - * Signal to cancel a backend process. This is allowed if you are superuser or - * have the same role as the process being canceled. + * Signal to cancel a backend process. This is allowed if you are a member of + * the role whose process is being canceled. + * + * Note that only superusers can signal superuser-owned processes. */ Datum pg_cancel_backend(PG_FUNCTION_ARGS) { int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT); + if (r == SIGNAL_BACKEND_NOSUPERUSER) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be a superuser to cancel superuser query")))); + if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("must be superuser or have the same role to cancel queries running in other server processes")))); + (errmsg("must be a member of the role whose query is being cancelled")))); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); } /* - * Signal to terminate a backend process. This is allowed if you are superuser - * or have the same role as the process being terminated. + * Signal to terminate a backend process. This is allowed if you are a member + * of the role whose process is being terminated. + * + * Note that only superusers can signal superuser-owned processes. */ Datum pg_terminate_backend(PG_FUNCTION_ARGS) { int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM); + if (r == SIGNAL_BACKEND_NOSUPERUSER) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be a superuser to terminate superuser process")))); + if (r == SIGNAL_BACKEND_NOPERMISSION) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("must be superuser or have the same role to terminate other server processes")))); + (errmsg("must be a member of the role whose process is being terminated")))); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 9964c5e103..78adb2d853 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -20,6 +20,7 @@ #include "libpq/ip.h" #include "miscadmin.h" #include "pgstat.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/inet.h" #include "utils/timestamp.h" @@ -675,8 +676,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) else nulls[15] = true; - /* Values only available to same user or superuser */ - if (superuser() || beentry->st_userid == GetUserId()) + /* Values only available to role member */ + if (has_privs_of_role(GetUserId(), beentry->st_userid)) { SockAddr zero_clientaddr; @@ -878,7 +879,7 @@ pg_stat_get_backend_activity(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) activity = ""; - else if (!superuser() && beentry->st_userid != GetUserId()) + else if (!has_privs_of_role(GetUserId(), beentry->st_userid)) activity = ""; else if (*(beentry->st_activity) == '\0') activity = ""; @@ -899,7 +900,7 @@ pg_stat_get_backend_waiting(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); result = beentry->st_waiting; @@ -918,7 +919,7 @@ pg_stat_get_backend_activity_start(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); result = beentry->st_activity_start_timestamp; @@ -944,7 +945,7 @@ pg_stat_get_backend_xact_start(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); result = beentry->st_xact_start_timestamp; @@ -966,7 +967,7 @@ pg_stat_get_backend_start(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); result = beentry->st_proc_start_timestamp; @@ -990,7 +991,7 @@ pg_stat_get_backend_client_addr(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); /* A zeroed client addr means we don't know */ @@ -1037,7 +1038,7 @@ pg_stat_get_backend_client_port(PG_FUNCTION_ARGS) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) PG_RETURN_NULL(); - if (!superuser() && beentry->st_userid != GetUserId()) + if (!has_privs_of_role(GetUserId(), beentry->st_userid)) PG_RETURN_NULL(); /* A zeroed client addr means we don't know */