pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedSHOW ALL does not honor pg_read_all_settings membership
13+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2018-06-08 0c8910a0ca a25c207b2f Teach SHOW ALL to honor pg_read_all_settings membership
* SHOW ALL does not honor pg_read_all_settings membership
@ 2018-03-01 15:22 Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 2 replies; 13+ messages in thread
From: Laurenz Albe @ 2018-03-01 15:22 UTC (permalink / raw)
To: pgsql-hackers
I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.
Patch attached; I think this should be backpatched.
Yours,
Laurenz Albe
Attachments:
0001-Teach-SHOW-ALL-to-honor-pg_read_all_settings-members.patch (text/x-patch, 1.1K) download
From f3a92a6ecc8dc116a6843c1bd76aa5e2a97785ff Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Thu, 1 Mar 2018 16:18:04 +0100
Subject: [PATCH] Teach SHOW ALL to honor pg_read_all_settings membership
This was an oversight in commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212.
---
src/backend/utils/misc/guc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 87ba67661a..a8c8330dde 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8012,7 +8012,6 @@ ShowGUCConfigOption(const char *name, DestReceiver *dest)
static void
ShowAllGUCConfig(DestReceiver *dest)
{
- bool am_superuser = superuser();
int i;
TupOutputState *tstate;
TupleDesc tupdesc;
@@ -8037,7 +8036,8 @@ ShowAllGUCConfig(DestReceiver *dest)
char *setting;
if ((conf->flags & GUC_NO_SHOW_ALL) ||
- ((conf->flags & GUC_SUPERUSER_ONLY) && !am_superuser))
+ ((conf->flags & GUC_SUPERUSER_ONLY) &&
+ !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)))
continue;
/* assign to the values array */
--
2.14.3
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-04-16 12:32 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
1 sibling, 1 reply; 13+ messages in thread
From: Laurenz Albe @ 2018-04-16 12:32 UTC (permalink / raw)
To: pgsql-hackers; +Cc: simon@2ndquadrant.com
On Thu, 2018-03-01 at 16:22 +0100, I wrote:
> I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
> to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.
>
> Patch attached; I think this should be backpatched.
Now that the dust from the last commitfest is settling, I'll make a second
attempt to attract attention for this small bug fix.
The original commit was Simon's.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-04-17 03:28 Michael Paquier <michael@paquier.xyz>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 2 replies; 13+ messages in thread
From: Michael Paquier @ 2018-04-17 03:28 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-hackers; simon@2ndquadrant.com
On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
> Now that the dust from the last commitfest is settling, I'll make a second
> attempt to attract attention for this small bug fix.
>
> The original commit was Simon's.
Thanks for the ping.
This was new as of v10, so this cannot be listed as an open item still I
have added that under the section for older bugs, because you are right
as far as I can see.
GetConfigOption is wrong by the way, as restrict_superuser means that
all members of the group pg_read_all_settings can read
GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
least needs a fix, the variable ought to be renamed as well.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-04-20 11:21 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 13+ messages in thread
From: Laurenz Albe @ 2018-04-20 11:21 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers; simon@2ndquadrant.com
Michael Paquier wrote:
> On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
> > Now that the dust from the last commitfest is settling, I'll make a second
> > attempt to attract attention for this small bug fix.
> >
> > The original commit was Simon's.
>
> Thanks for the ping.
>
> This was new as of v10, so this cannot be listed as an open item still I
> have added that under the section for older bugs, because you are right
> as far as I can see.
>
> GetConfigOption is wrong by the way, as restrict_superuser means that
> all members of the group pg_read_all_settings can read
> GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
> least needs a fix, the variable ought to be renamed as well.
Thanks for the review!
I agree; here is a patch for that.
Yours,
Laurenz Albe
Attachments:
0002-Fix-comments-and-a-parameter-name.patch (text/x-patch, 2.6K) download
From cfe04ef974dba324c47510b854587de6c33e39a1 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 20 Apr 2018 13:13:20 +0200
Subject: [PATCH] Fix comments and a parameter name
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 changed the semantics
of GetConfigOption, but did not update the comment.
Also, the parameter name should better be changed.
Noted by Michael Paquier.
---
src/backend/utils/misc/guc.c | 10 +++++-----
src/include/utils/guc.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7acec5ea21..b5501fd949 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6869,15 +6869,15 @@ SetConfigOption(const char *name, const char *value,
* this cannot be distinguished from a string variable with a NULL value!),
* otherwise throw an ereport and don't return.
*
- * If restrict_superuser is true, we also enforce that only superusers can
- * see GUC_SUPERUSER_ONLY variables. This should only be passed as true
- * in user-driven calls.
+ * If restrict_privileged is true, we also enforce that only superusers and
+ * members of the pg_read_all_settings role can see GUC_SUPERUSER_ONLY
+ * variables. This should only be passed as true in user-driven calls.
*
* The string is *not* allocated for modification and is really only
* valid until the next call to configuration related functions.
*/
const char *
-GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)
+GetConfigOption(const char *name, bool missing_ok, bool restrict_privileged)
{
struct config_generic *record;
static char buffer[256];
@@ -6892,7 +6892,7 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)
errmsg("unrecognized configuration parameter \"%s\"",
name)));
}
- if (restrict_superuser &&
+ if (restrict_privileged &&
(record->flags & GUC_SUPERUSER_ONLY) &&
!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))
ereport(ERROR,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 3d13a33b94..f462eabe59 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -347,7 +347,7 @@ extern void DefineCustomEnumVariable(
extern void EmitWarningsOnPlaceholders(const char *className);
extern const char *GetConfigOption(const char *name, bool missing_ok,
- bool restrict_superuser);
+ bool restrict_privileged);
extern const char *GetConfigOptionResetString(const char *name);
extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void ProcessConfigFile(GucContext context);
--
2.14.3
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-04-20 13:58 Michael Paquier <michael@paquier.xyz>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 0 replies; 13+ messages in thread
From: Michael Paquier @ 2018-04-20 13:58 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-hackers; simon@2ndquadrant.com
On Fri, Apr 20, 2018 at 01:21:46PM +0200, Laurenz Albe wrote:
> I agree; here is a patch for that.
Thanks for taking care of that as well this looks fine to me. Note to
committer: this needs to be merged with the first patch actually fixing
the SHOW ALL issue.
All the four core callers of GetConfigOption() actually do not use
restrict_superuser set to true... Modules may use this option, so of
course let's not remove it.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-05-31 18:05 Simon Riggs <simon@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 13+ messages in thread
From: Simon Riggs @ 2018-05-31 18:05 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
On 17 April 2018 at 04:28, Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
>> Now that the dust from the last commitfest is settling, I'll make a second
>> attempt to attract attention for this small bug fix.
>>
>> The original commit was Simon's.
>
> Thanks for the ping.
>
> This was new as of v10, so this cannot be listed as an open item still I
> have added that under the section for older bugs, because you are right
> as far as I can see.
OK, agreed, its a bug.
Any objections to backpatch to v10?
> GetConfigOption is wrong by the way, as restrict_superuser means that
> all members of the group pg_read_all_settings can read
> GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
> least needs a fix, the variable ought to be renamed as well.
OK
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-05-31 18:42 Michael Paquier <michael@paquier.xyz>
parent: Simon Riggs <simon@2ndquadrant.com>
0 siblings, 1 reply; 13+ messages in thread
From: Michael Paquier @ 2018-05-31 18:42 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
On Thu, May 31, 2018 at 07:05:58PM +0100, Simon Riggs wrote:
> Any objections to backpatch to v10?
A backpatch is acceptable in my opinion.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-07 21:23 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 13+ messages in thread
From: Alvaro Herrera @ 2018-06-07 21:23 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
On 2018-May-31, Michael Paquier wrote:
> On Thu, May 31, 2018 at 07:05:58PM +0100, Simon Riggs wrote:
> > Any objections to backpatch to v10?
>
> A backpatch is acceptable in my opinion.
Agreed on backpatching.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-08 19:08 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 13+ messages in thread
From: Alvaro Herrera @ 2018-06-08 19:08 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
BTW a further bug here seems to be that "select * from pg_settings()"
does not show the source file/line for members of the role, which seems
to be documented to occur.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-08 19:13 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 13+ messages in thread
From: Alvaro Herrera @ 2018-06-08 19:13 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
On 2018-Jun-08, Alvaro Herrera wrote:
> BTW a further bug here seems to be that "select * from pg_settings()"
> does not show the source file/line for members of the role, which seems
> to be documented to occur.
And I think this fixes it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-fix-sourcefile-line-display-for-pg_read_all_settings.patch (text/plain, 944B) download
From c71a14bdac838efc09d01b60383fa6e631606c57 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 8 Jun 2018 15:12:58 -0400
Subject: [PATCH] fix sourcefile/line display for pg_read_all_settings role
---
src/backend/utils/misc/guc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 274034d88b..e84d56dae6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8595,7 +8595,8 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow)
* security reasons, we don't show source file/line number for
* non-superusers.
*/
- if (conf->source == PGC_S_FILE && superuser())
+ if (conf->source == PGC_S_FILE &&
+ is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))
{
values[14] = conf->sourcefile;
snprintf(buffer, sizeof(buffer), "%d", conf->sourceline);
--
2.11.0
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-08 20:35 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
1 sibling, 1 reply; 13+ messages in thread
From: Alvaro Herrera @ 2018-06-08 20:35 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-hackers
On 2018-Mar-01, Laurenz Albe wrote:
> I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
> to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.
>
> Patch attached; I think this should be backpatched.
Done, with the further fixes downthread. Thanks!
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-11 01:38 Michael Paquier <michael@paquier.xyz>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 13+ messages in thread
From: Michael Paquier @ 2018-06-11 01:38 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-hackers
On Fri, Jun 08, 2018 at 03:13:57PM -0400, Alvaro Herrera wrote:
> And I think this fixes it.
- if (conf->source == PGC_S_FILE && superuser())
+ if (conf->source == PGC_S_FILE &&
+ is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))
Thanks Alvaro! This bit in GetConfigOptionByNum() looks fine to me.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: SHOW ALL does not honor pg_read_all_settings membership
@ 2018-06-11 06:28 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 13+ messages in thread
From: Laurenz Albe @ 2018-06-11 06:28 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: pgsql-hackers
Alvaro Herrera wrote:
> On 2018-Mar-01, Laurenz Albe wrote:
>
> > I noticed that commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 forgot
> > to teach SHOW ALL to show all GUCs when the user belongs to pg_read_all_settings.
> >
> > Patch attached; I think this should be backpatched.
>
> Done, with the further fixes downthread. Thanks!
Thank you!
Laurenz Albe
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2018-06-11 06:28 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-03-01 15:22 SHOW ALL does not honor pg_read_all_settings membership Laurenz Albe <laurenz.albe@cybertec.at>
2018-04-16 12:32 ` Laurenz Albe <laurenz.albe@cybertec.at>
2018-04-17 03:28 ` Michael Paquier <michael@paquier.xyz>
2018-04-20 11:21 ` Laurenz Albe <laurenz.albe@cybertec.at>
2018-04-20 13:58 ` Michael Paquier <michael@paquier.xyz>
2018-05-31 18:05 ` Simon Riggs <simon@2ndquadrant.com>
2018-05-31 18:42 ` Michael Paquier <michael@paquier.xyz>
2018-06-07 21:23 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2018-06-08 19:08 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2018-06-08 19:13 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2018-06-11 01:38 ` Michael Paquier <michael@paquier.xyz>
2018-06-08 20:35 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2018-06-11 06:28 ` Laurenz Albe <laurenz.albe@cybertec.at>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox