8000 GetUserId() changes to has_privs_of_role() · sarvex/postgres@bf03889 · GitHub
[go: up one dir, main page]
Skip to content

Commit bf03889

Browse files
sfrostsfrost
committed
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.
1 parent 12968cf commit bf03889

File tree

3 files changed

+47
-24
lines changed

3 files changed

+47
-24
lines changed

‎doc/src/sgml/func.sgml

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16328,9 +16328,9 @@ SELECT set_config('log_statement_stats', 'off', false);
1632816328
<literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
1632916329
</entry>
1633016330
<entry><type>boolean</type></entry>
16331-
<entry>Cancel a backend's current query. You can execute this against
16332-
another backend that has exactly the same role as the user calling the
16333-
function. In all other cases, you must be a superuser.
16331+
<entry>Cancel a backend's current query. This is also allowed if the
16332+
calling role is a member of the role whose backend is being cancelled,
16333+
however only superusers can cancel superuser backends.
1633416334
</entry>
1633516335
</row>
1633616336
<row>
@@ -16352,10 +16352,9 @@ SELECT set_config('log_statement_stats', 'off', false);
1635216352
<literal><function>pg_terminate_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
1635316353
</entry>
1635416354
<entry><type>boolean</type></entry>
16355-
<entry>Terminate a backend. You can execute this against
16356-
another backend that has exactly the same role as the user
16357-
calling the function. In all other cases, you must be a
16358-
superuser.
16355+
<entry>Terminate a backend. This is also allowed if the calling role
16356+
is a member of the role whose backend is being terminated, however only
16357+
superusers can terminate superuser backends.
1635916358
</entry>
1636016359
</row>
1636116360
</tbody>

‎src/backend/utils/adt/misc.c

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "utils/lsyscache.h"
3838
#include "utils/ruleutils.h"
3939
#include "tcop/tcopprot.h"
40+
#include "utils/acl.h"
4041
#include "utils/builtins.h"
4142
#include "utils/timestamp.h"
4243

@@ -81,14 +82,17 @@ current_query(PG_FUNCTION_ARGS)
8182
* role as the backend being signaled. For "dangerous" signals, an explicit
8283
* check for superuser needs to be done prior to calling this function.
8384
*
84-
* Returns 0 on success, 1 on general failure, and 2 on permission error.
85+
* Returns 0 on success, 1 on general failure, 2 on normal permission error
86+
* and 3 if the caller needs to be a superuser.
87+
*
8588
* In the event of a general failure (return code 1), a warning message will
8689
* be emitted. For permission errors, doing that is the responsibility of
8790
* the caller.
8891
*/
8992
#define SIGNAL_BACKEND_SUCCESS 0
9093
#define SIGNAL_BACKEND_ERROR 1
9194
#define SIGNAL_BACKEND_NOPERMISSION 2
95+
#define SIGNAL_BACKEND_NOSUPERUSER 3
9296
static int
9397
pg_signal_backend(int pid, int sig)
9498
{
@@ -113,7 +117,12 @@ pg_signal_backend(int pid, int sig)
113117
return SIGNAL_BACKEND_ERROR;
114118
}
115119

116-
if (!(superuser() || proc->roleId == GetUserId()))
120+
/* Only allow superusers to signal superuser-owned backends. */
121+
if (superuser_arg(proc->roleId) && !superuser())
122+
return SIGNAL_BACKEND_NOSUPERUSER;
123+
124+
/* Users can signal backends they have role membership in. */
125+
if (!has_privs_of_role(GetUserId(), proc->roleId))
117126
return SIGNAL_BACKEND_NOPERMISSION;
118127

119128
/*
@@ -141,35 +150,49 @@ pg_signal_backend(int pid, int sig)
141150
}
142151

143152
/*
144-
* Signal to cancel a backend process. This is allowed if you are superuser or
145-
* have the same role as the process being canceled.
153+
* Signal to cancel a backend process. This is allowed if you are a member of
154+
* the role whose process is being canceled.
155+
*
156+
* Note that only superusers can signal superuser-owned processes.
146157
*/
147158
Datum
148159
pg_cancel_backend(PG_FUNCTION_ARGS)
149160
{
150161
int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT);
151162

163+
if (r == SIGNAL_BACKEND_NOSUPERUSER)
164+
ereport(ERROR,
165+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
166+
(errmsg("must be a superuser to cancel superuser query"))));
167+
152168
if (r == SIGNAL_BACKEND_NOPERMISSION)
153169
ereport(ERROR,
154170
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
155-
(errmsg("must be superuser or have the same role to cancel queries running in other server processes"))));
171+
(errmsg("must be a member of the role whose query is being cancelled"))));
156172

157173
PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
158174
}
159175

160176
/*
161-
* Signal to terminate a backend process. This is allowed if you are superuser
162-
* or have the same role as the process being terminated.
177+
* Signal to terminate a backend process. This is allowed if you are a member
178+
* of the role whose process is being terminated.
179+
*
180+
* Note that only superusers can signal superuser-owned processes.
163181
*/
164182
Datum
165183
pg_terminate_backend(PG_FUNCTION_ARGS)
166184
{
167185
int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM);
168186

187+
if (r == SIGNAL_BACKEND_NOSUPERUSER)
188+
ereport(ERROR,
189+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
190+
(errmsg("must be a superuser to terminate superuser process"))));
191+
169192
if (r == SIGNAL_BACKEND_NOPERMISSION)
170193
ereport(ERROR,
171194
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
172-
(errmsg("must be superuser or have the same role to terminate other server processes"))));
195+
(errmsg("must be a member of the role whose process is being terminated"))));
173196

174197
PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
175198
}

‎src/backend/utils/adt/pgstatfuncs.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "libpq/ip.h"
2121
#include "miscadmin.h"
2222
#include "pgstat.h"
23+
#include "utils/acl.h"
2324
#include "utils/builtins.h"
2425
#include "utils/inet.h"
2526
#include "utils/timestamp.h"
@@ -675,8 +676,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
675676
else
676677
nulls[15] = true;
677678

678-
/* Values only available to same user or superuser */
679-
if (superuser() || beentry->st_userid == GetUserId())
679+
/* Values only available to role member */
680+
if (has_privs_of_role(GetUserId(), beentry->st_userid))
680681
{
681682
SockAddr zero_clientaddr;
682683

@@ -878,7 +879,7 @@ pg_stat_get_backend_activity(PG_FUNCTION_ARGS)
878879

879880
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
880881
activity = "<backend information not available>";
881-
else if (!superuser() && beentry->st_userid != GetUserId())
882+
else if (!has_privs_of_role(GetUserId(), beentry->st_userid))
882883
activity = "<insufficient privilege>";
883884
else if (*(beentry->st_activity) == '\0')
884885
activity = "<command string not enabled>";
@@ -899,7 +900,7 @@ pg_stat_get_backend_waiting(PG_FUNCTION_ARGS)
899900
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
900901
PG_RETURN_NULL();
901902

902-
if (!superuser() && beentry->st_userid != GetUserId())
903+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
903904
PG_RETURN_NULL();
904905

905906
result = beentry->st_waiting;
@@ -918,7 +919,7 @@ pg_stat_get_backend_activity_start(PG_FUNCTION_ARGS)
918919
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
919920
PG_RETURN_NULL();
920921

921-
if (!superuser() && beentry->st_userid != GetUserId())
922+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
922923
PG_RETURN_NULL();
923924

924925
result = beentry->st_activity_start_timestamp;
@@ -944,7 +945,7 @@ pg_stat_get_backend_xact_start(PG_FUNCTION_ARGS)
944945
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
945946
PG_RETURN_NULL();
946947

947-
if (!superuser() && beentry->st_userid != GetUserId())
948+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
948949
PG_RETURN_NULL();
949950

950951
result = beentry->st_xact_start_timestamp;
@@ -966,7 +967,7 @@ pg_stat_get_backend_start(PG_FUNCTION_ARGS)
966967
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
967968
PG_RETURN_NULL();
968969

969-
if (!superuser() && beentry->st_userid != GetUserId())
970+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
970971
PG_RETURN_NULL();
971972

972973
result = beentry->st_proc_start_timestamp;
@@ -990,7 +991,7 @@ pg_stat_get_backend_client_addr(PG_FUNCTION_ARGS)
990991
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
991992
PG_RETURN_NULL();
992993

993-
if (!superuser() && beentry->st_userid != GetUserId())
994+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
994995
PG_RETURN_NULL();
995996

996997
/* A zeroed client addr means we don't know */
@@ -1037,7 +1038,7 @@ pg_stat_get_backend_client_port(PG_FUNCTION_ARGS)
10371038
if ((beentry = pgstat_fetch_stat_beentry(beid)) == NULL)
10381039
PG_RETURN_NULL();
10391040

1040-
if (!superuser() && beentry->st_userid != GetUserId())
1041+
if (!has_privs_of_role(GetUserId(), beentry->st_userid))
10411042
PG_RETURN_NULL();
10421043

10431044
/* A zeroed client addr means we don't know */

0 commit comments

Comments
 (0)
0