From bc1913464421921b7f846bcad332698ca6369e75 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Sat, 29 Nov 2014 14:53:08 -0500 Subject: [PATCH] GetUserId() Cleanup for pg_stat and pg_signal 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 see details in pg_stat_activity or signal other processes. This is arguably a bug in existing branches, but as it's a user-visible change and we haven't gotten complaints about it, it's probably best to just do it in master. Per discussion with Alvaro, Peter and Adam (though not using Adam's patch). --- src/backend/utils/adt/misc.c | 31 +++++++++++++++++++++++-------- src/backend/utils/adt/pgstatfuncs.c | 19 ++++++++++--------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 67539ec..f3dca1f 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" @@ -113,8 +114,16 @@ pg_signal_backend(int pid, int sig) return SIGNAL_BACKEND_ERROR; } - if (!(superuser() || proc->roleId == GetUserId())) - return SIGNAL_BACKEND_NOPERMISSION; + /* Only allow superusers to signal superuser-owned backends. */ + if (superuser_arg(proc->roleId) && !superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("permission denied to send signal to other process"), + errdetail("You must be a superuser to signal processes owned by superusers.")))); + + /* Users can signal backends they have role membership in. */ + if (!has_privs_of_role(GetUserId(), proc->roleId)) + return SIGNAL_BACKEND_NOPERMISSION; /* * Can the process we just validated above end, followed by the pid being @@ -141,8 +150,10 @@ 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) @@ -152,14 +163,17 @@ pg_cancel_backend(PG_FUNCTION_ARGS) 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("permission denied to cancel query"), + errdetail("You must be a member of the role whose query you are cancelling.")))); 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) @@ -169,7 +183,8 @@ pg_terminate_backend(PG_FUNCTION_ARGS) 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("permission denied to terminate server process"), + errdetail("You must be a member of the role whose process you are terminating.")))); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS); } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index d621a68..3663e93 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" @@ -674,8 +675,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; @@ -877,7 +878,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 = ""; @@ -898,7 +899,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; @@ -917,7 +918,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; @@ -943,7 +944,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; @@ -965,7 +966,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; @@ -989,7 +990,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 */ @@ -1036,7 +1037,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 */ -- 1.9.1