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.
This commit is contained in:
Stephen Frost 2015-03-19 15:02:33 -04:00
parent 12968cf408
commit bf03889996
3 changed files with 47 additions and 24 deletions

View File

@ -16328,9 +16328,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal> <literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
</entry> </entry>
<entry><type>boolean</type></entry> <entry><type>boolean</type></entry>
<entry>Cancel a backend's current query. You can execute this against <entry>Cancel a backend's current query. This is also allowed if the
another backend that has exactly the same role as the user calling the calling role is a member of the role whose backend is being cancelled,
function. In all other cases, you must be a superuser. however only superusers can cancel superuser backends.
</entry> </entry>
</row> </row>
<row> <row>
@ -16352,10 +16352,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<literal><function>pg_terminate_backend(<parameter>pid</parameter> <type>int</>)</function></literal> <literal><function>pg_terminate_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
</entry> </entry>
<entry><type>boolean</type></entry> <entry><type>boolean</type></entry>
<entry>Terminate a backend. You can execute this against <entry>Terminate a backend. This is also allowed if the calling role
another backend that has exactly the same role as the user is a member of the role whose backend is being terminated, however only
calling the function. In all other cases, you must be a superusers can terminate superuser backends.
superuser.
</entry> </entry>
</row> </row>
</tbody> </tbody>

View File

@ -37,6 +37,7 @@
#include "utils/lsyscache.h" #include "utils/lsyscache.h"
#include "utils/ruleutils.h" #include "utils/ruleutils.h"
#include "tcop/tcopprot.h" #include "tcop/tcopprot.h"
#include "utils/acl.h"
#include "utils/builtins.h" #include "utils/builtins.h"
#include "utils/timestamp.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 * role as the backend being signaled. For "dangerous" signals, an explicit
* check for superuser needs to be done prior to calling this function. * 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 * 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 * be emitted. For permission errors, doing that is the responsibility of
* the caller. * the caller.
@ -89,6 +92,7 @@ current_query(PG_FUNCTION_ARGS)
#define SIGNAL_BACKEND_SUCCESS 0 #define SIGNAL_BACKEND_SUCCESS 0
#define SIGNAL_BACKEND_ERROR 1 #define SIGNAL_BACKEND_ERROR 1
#define SIGNAL_BACKEND_NOPERMISSION 2 #define SIGNAL_BACKEND_NOPERMISSION 2
#define SIGNAL_BACKEND_NOSUPERUSER 3
static int static int
pg_signal_backend(int pid, int sig) pg_signal_backend(int pid, int sig)
{ {
@ -113,7 +117,12 @@ pg_signal_backend(int pid, int sig)
return SIGNAL_BACKEND_ERROR; 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; 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 * Signal to cancel a backend process. This is allowed if you are a member of
* have the same role as the process being canceled. * the role whose process is being canceled.
*
* Note that only superusers can signal superuser-owned processes.
*/ */
Datum Datum
pg_cancel_backend(PG_FUNCTION_ARGS) pg_cancel_backend(PG_FUNCTION_ARGS)
{ {
int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT); 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) if (r == SIGNAL_BACKEND_NOPERMISSION)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (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); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
} }
/* /*
* Signal to terminate a backend process. This is allowed if you are superuser * Signal to terminate a backend process. This is allowed if you are a member
* or have the same role as the process being terminated. * of the role whose process is being terminated.
*
* Note that only superusers can signal superuser-owned processes.
*/ */
Datum Datum
pg_terminate_backend(PG_FUNCTION_ARGS) pg_terminate_backend(PG_FUNCTION_ARGS)
{ {
int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM); 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) if (r == SIGNAL_BACKEND_NOPERMISSION)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (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); PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
} }

View File

@ -20,6 +20,7 @@
#include "libpq/ip.h" #include "libpq/ip.h"
#include "miscadmin.h" #include "miscadmin.h"
#include "pgstat.h" #include "pgstat.h"
#include "utils/acl.h"
#include "utils/builtins.h" #include "utils/builtins.h"
#include "utils/inet.h" #include "utils/inet.h"
#include "utils/timestamp.h" #include "utils/timestamp.h"
@ -675,8 +676,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
else else
nulls[15] = true; nulls[15] = true;
/* Values only available to same user or superuser */ /* Values only available to role member */
if (superuser() || beentry->st_userid == GetUserId()) if (has_privs_of_role(GetUserId(), beentry->st_userid))
{ {
SockAddr zero_clientaddr; SockAddr zero_clientaddr;
@ -878,7 +879,7 @@ pg_stat_get_backend_activity(PG_FUNCTION_ARGS)
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
activity = "<backend information not available>"; 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>"; activity = "<insufficient privilege>";
else if (*(beentry->st_activity) == '\0') else if (*(beentry->st_activity) == '\0')
activity = "<command string not enabled>"; activity = "<command string not enabled>";
@ -899,7 +900,7 @@ pg_stat_get_backend_waiting(PG_FUNCTION_ARGS)
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
result = beentry->st_waiting; 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) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
result = beentry->st_activity_start_timestamp; 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) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
result = beentry->st_xact_start_timestamp; 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) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
result = beentry->st_proc_start_timestamp; 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) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
/* A zeroed client addr means we don't know */ /* 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) if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
PG_RETURN_NULL(); PG_RETURN_NULL();
if (!superuser() && beentry->st_userid != GetUserId()) if (!has_privs_of_role(GetUserId(), beentry->st_userid))
PG_RETURN_NULL(); PG_RETURN_NULL();
/* A zeroed client addr means we don't know */ /* A zeroed client addr means we don't know */