All, * Stephen Frost (sfr...@snowman.net) wrote: > * Stephen Frost (sfr...@snowman.net) wrote: > > Attached is a patch to address the pg_cancel/terminate_backend and the > > statistics info as discussed previously. It sounds like we're coming to > > And I forgot the attachment, of course. Apologies.
Updated patch attached which also changes the error messages (which
hadn't been updated in the prior versions and really should be).
Barring objections, I plan to move forward with this one and get this
relatively minor change wrapped up. As mentioned in the commit, while
this might be an arguably back-patchable change, the lack of field
complaints and the fact that it changes existing behavior mean it should
go only against master, imv.
Thanks,
Stephen
From bc1913464421921b7f846bcad332698ca6369e75 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
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 = "<backend information not available>";
- else if (!superuser() && beentry->st_userid != GetUserId())
+ else if (!has_privs_of_role(GetUserId(), beentry->st_userid))
activity = "<insufficient privilege>";
else if (*(beentry->st_activity) == '\0')
activity = "<command string not enabled>";
@@ -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
signature.asc
Description: Digital signature
