Fix permissions check on pg_stat_get_wal_senders
Fix permissions check on pg_stat_get_wal_senders
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 introduced the
possibility for the pg_read_all_stats to have access to all pg_stat_*
views.
In the discussion, the pg_stat_replication and pg_stat_wal_receiver
views were also considered to be part of that, however
pg_stat_get_wal_senders was somehow not part of that commit, that
seems an oversight.
1: /messages/by-id/CA+OCxoyYxO+Jmzv2Micj4uAaQdAi6nq0w25BPQgLLxsrvTmREw@mail.gmail.com%5C
regards,
Feike Steenbergen
Attachments:
0001-Fix-permissions-check-on-pg_stat_get_wal_senders.patchapplication/octet-stream; name=0001-Fix-permissions-check-on-pg_stat_get_wal_senders.patchDownload
From 540c9481ec1bf1fd31482b14ec59547a0073b79d Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feike.steenbergen@adyen.com>
Date: Thu, 21 Dec 2017 10:08:04 +0000
Subject: [PATCH 1/1] Fix permissions check on pg_stat_get_wal_senders
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 introduced the
possibility for the pg_read_all_stats to have access to all pg_stat_*
views.
In the discussion, the pg_stat_replication and pg_stat_wal_receiver
views were also considered to be part of that, however
pg_stat_get_wal_senders was somehow not part of that commit, that
seems an oversight.
1: https://www.postgresql.org/message-id/CA%2BOCxoyYxO%2BJmzv2Micj4uAaQdAi6nq0w25BPQgLLxsrvTmREw%40mail.gmail.com
---
src/backend/replication/walreceiver.c | 4 ++--
src/backend/replication/walsender.c | 7 ++++---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index fe4e085938..643e47133a 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1442,8 +1442,8 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
{
/*
- * Only superusers can see details. Other users only get the pid value
- * to know whether it is a WAL receiver, but no details.
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value to know it's a walsender, but no details.
*/
MemSet(&nulls[1], true, sizeof(bool) * (tupdesc->natts - 1));
}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 6a252fcf45..490293eaa7 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -56,6 +56,7 @@
#include "access/xlog_internal.h"
#include "access/xlogutils.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
@@ -3236,11 +3237,11 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
memset(nulls, 0, sizeof(nulls));
values[0] = Int32GetDatum(pid);
- if (!superuser())
+ if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
{
/*
- * Only superusers can see details. Other users only get the pid
- * value to know it's a walsender, but no details.
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value to know it's a walsender, but no details.
*/
MemSet(&nulls[1], true, PG_STAT_GET_WAL_SENDERS_COLS - 1);
}
--
2.14.3 (Apple Git-98)
On Thu, Dec 21, 2017 at 7:30 PM, Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:
Fix permissions check on pg_stat_get_wal_senders
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 introduced the
possibility for the pg_read_all_stats to have access to all pg_stat_*
views.
In the discussion, the pg_stat_replication and pg_stat_wal_receiver
views were also considered to be part of that, however
pg_stat_get_wal_senders was somehow not part of that commit, that
seems an oversight.1: /messages/by-id/CA+OCxoyYxO+Jmzv2Micj4uAaQdAi6nq0w25BPQgLLxsrvTmREw@mail.gmail.com%5C
Yes, that's a bug, albeit a minor one.
- * Only superusers can see details. Other users only get the pid value
- * to know whether it is a WAL receiver, but no details.
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value to know it's a
walsender, but no details.
You mean a WAL receiver here, not a WAL sender.
--
MichaelOn 21 December 2017 at 14:11, Michael Paquier <michael.paquier@gmail.com> wrote:
You mean a WAL receiver here, not a WAL sender.
Fixed, thanks
Attachments:
0002-Fix-permissions-check-on-pg_stat_get_wal_senders.patchapplication/octet-stream; name=0002-Fix-permissions-check-on-pg_stat_get_wal_senders.patchDownload
From 355eb0bff3c8ffd14a4c0f7c39ad81330aff6a89 Mon Sep 17 00:00:00 2001
From: Feike Steenbergen <feike.steenbergen@adyen.com>
Date: Thu, 21 Dec 2017 10:08:04 +0000
Subject: [PATCH 1/1] Fix permissions check on pg_stat_get_wal_senders
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 introduced the
possibility for the pg_read_all_stats to have access to all pg_stat_*
views.
In the discussion, the pg_stat_replication and pg_stat_wal_receiver
views were also considered to be part of that, however
pg_stat_get_wal_senders was somehow not part of that commit, that
seems an oversight.
1: https://www.postgresql.org/message-id/CA%2BOCxoyYxO%2BJmzv2Micj4uAaQdAi6nq0w25BPQgLLxsrvTmREw%40mail.gmail.com
---
src/backend/replication/walreceiver.c | 3 ++-
src/backend/replication/walsender.c | 8 +++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index fe4e085938..85803500b1 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1442,7 +1442,8 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
{
/*
- * Only superusers can see details. Other users only get the pid value
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value
* to know whether it is a WAL receiver, but no details.
*/
MemSet(&nulls[1], true, sizeof(bool) * (tupdesc->natts - 1));
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 6a252fcf45..fd3b47cda0 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -56,6 +56,7 @@
#include "access/xlog_internal.h"
#include "access/xlogutils.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
@@ -3236,11 +3237,12 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
memset(nulls, 0, sizeof(nulls));
values[0] = Int32GetDatum(pid);
- if (!superuser())
+ if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
{
/*
- * Only superusers can see details. Other users only get the pid
- * value to know it's a walsender, but no details.
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value to know it's a walsender,
+ * but no details.
*/
MemSet(&nulls[1], true, PG_STAT_GET_WAL_SENDERS_COLS - 1);
}
--
2.14.3 (Apple Git-98)
On Fri, Dec 22, 2017 at 07:49:34AM +0100, Feike Steenbergen wrote:
On 21 December 2017 at 14:11, Michael Paquier <michael.paquier@gmail.com> wrote:
You mean a WAL receiver here, not a WAL sender.
Fixed, thanks
[nit]
/*
- * Only superusers can see details. Other users only get the pid value
+ * Only superusers and members of pg_read_all_stats can see details.
+ * Other users only get the pid value
* to know whether it is a WAL receiver, but no details.
*/Incorrect comment format.
[/nit]
Committers run pgindent on each patch before committing anyway, and what
you are proposing here looks good to me, so I am marking that as ready for
committer. Simon, as the original committer of 25fff407, could you look
at what is proposed here?
--
Michael
On 23 December 2017 at 10:56, Michael Paquier <michael.paquier@gmail.com> wrote:
On Fri, Dec 22, 2017 at 07:49:34AM +0100, Feike Steenbergen wrote:
On 21 December 2017 at 14:11, Michael Paquier <michael.paquier@gmail.com> wrote:
You mean a WAL receiver here, not a WAL sender.
Fixed, thanks
[nit] /* - * Only superusers can see details. Other users only get the pid value + * Only superusers and members of pg_read_all_stats can see details. + * Other users only get the pid value * to know whether it is a WAL receiver, but no details. */Incorrect comment format.
[/nit]Committers run pgindent on each patch before committing anyway, and what
you are proposing here looks good to me, so I am marking that as ready for
committer. Simon, as the original committer of 25fff407, could you look
at what is proposed here?
Yup, I got this.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 5 January 2018 at 15:19, Simon Riggs <simon@2ndquadrant.com> wrote:
Yup, I got this.
I saw the commit in the master branch but not in the REL_10_STABLE branch,
I'm totally not up-to-date with the backpatching process, but I was wondering
if it still needs to be added to REL_10_STABLE
regards,
Feike
On 24 January 2018 at 13:15, Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:
On 5 January 2018 at 15:19, Simon Riggs <simon@2ndquadrant.com> wrote:
Yup, I got this.
I saw the commit in the master branch but not in the REL_10_STABLE branch,
I'm totally not up-to-date with the backpatching process, but I was wondering
if it still needs to be added to REL_10_STABLE
Yep, as the commit message said: "then later backpatch to 10". Will do.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services