Обсуждение: docs: mention "pg_read_all_stats" in "track_activities" description
Hi Regarding the visibility of query information, the description for "track_activities" [1] says: > Note that even when enabled, this information is not visible to all users, > only to superusers and the user owning the session being reported on, so it > should not represent a security risk. [1] https://www.postgresql.org/docs/current/runtime-config-statistics.html#GUC-TRACK-ACTIVITIES It seems reasonable to mention here that the information is also visible to members of "pg_read_all_stats", similar to what is done in the pg_stat_statements docs [2]. [2] https://www.postgresql.org/docs/current/pgstatstatements.html#PGSTATSTATEMENTS-COLUMNS Suggested wording: > Note that even when enabled, this information is only visible to superusers, > members of the <literal>pg_read_all_stats</literal> role and the user owning > the session being reported on, so it should not represent a security risk. Patch (for HEAD) with suggested wording attached; the change should IMO be applied all the way back to v10 (though as-is the patch only applies to HEAD, can provide others if needed). Regards Ian Barwick -- EnterpriseDB: https://www.enterprisedb.com
Вложения
On Fri, May 20, 2022 at 03:17:29PM +0900, Ian Lawrence Barwick wrote: > It seems reasonable to mention here that the information is also visible to > members of "pg_read_all_stats", similar to what is done in the > pg_stat_statements > docs [2]. > > [2] https://www.postgresql.org/docs/current/pgstatstatements.html#PGSTATSTATEMENTS-COLUMNS > > Suggested wording: > >> Note that even when enabled, this information is only visible to superusers, >> members of the <literal>pg_read_all_stats</literal> role and the user owning >> the session being reported on, so it should not represent a security risk. > > Patch (for HEAD) with suggested wording attached; the change should > IMO be applied > all the way back to v10 (though as-is the patch only applies to HEAD, > can provide > others if needed). LGTM -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Fri, May 20, 2022 at 04:08:37PM -0700, Nathan Bossart wrote: > LGTM Indeed, it is a good idea to add this information. Will apply and backpatch accordingly. -- Michael
Вложения
On Sat, May 21, 2022 at 12:28:58PM +0900, Michael Paquier wrote: > Indeed, it is a good idea to add this information. Will apply and > backpatch accordingly. Sorry, I should've noticed this yesterday. This should probably follow 6198420's example and say "roles with privileges of the pg_read_all_stats role" instead of "members of the pg_read_all_stats role." Also, I think we should mention that this information is visible to roles with privileges of the session user being reported on, too. Patch attached. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Вложения
On Sat, May 21, 2022 at 11:57:43AM -0700, Nathan Bossart wrote:
> Sorry, I should've noticed this yesterday. This should probably follow
> 6198420's example and say "roles with privileges of the pg_read_all_stats
> role" instead of "members of the pg_read_all_stats role."
Yes, I saw that, but that sounds pretty much the same to me, while we
mention membership of a role in other places. I don't mind tweaking
that more, FWIW, while we are on it.
> Also, I think we
> should mention that this information is visible to roles with privileges of
> the session user being reported on, too. Patch attached.
default. Note that even when enabled, this information is only
- visible to superusers, members of the
- <literal>pg_read_all_stats</literal> role and the user owning the
- session being reported on, so it should not represent a security risk.
- Only superusers and users with the appropriate <literal>SET</literal>
- privilege can change this setting.
+ visible to superusers, roles with privileges of the
+ <literal>pg_read_all_stats</literal> role, and roles with privileges of
+ the user owning the session being reported on, so it should not
+ represent a security risk. Only superusers and users with the
+ appropriate <literal>SET</literal> privilege can change this setting.
Regarding the fact that a user can see its own information, the last
part of the description would be right, still a bit confusing perhaps
when it comes to one's own information?
--
Michael
Вложения
On Sun, May 22, 2022 at 09:59:47AM +0900, Michael Paquier wrote:
> + visible to superusers, roles with privileges of the
> + <literal>pg_read_all_stats</literal> role, and roles with privileges of
> + the user owning the session being reported on, so it should not
> + represent a security risk. Only superusers and users with the
> + appropriate <literal>SET</literal> privilege can change this setting.
>
> Regarding the fact that a user can see its own information, the last
> part of the description would be right, still a bit confusing perhaps
> when it comes to one's own information?
Yeah, this crossed my mind. I thought that "superusers, roles with
privileges of the pg_read_all_stats_role, roles with privileges of the user
owning the session being reported on, and the user owning the session being
reported on" might be too long-winded and redundant. But I see your point
that it might be a bit confusing. Perhaps it could be trimmed down to
something like this:
... superusers, roles with privileges of the pg_read_all_stats role,
and roles with privileges of the user owning the session being reported
on (including the session owner).
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Sun, May 22, 2022 at 01:26:08PM -0700, Nathan Bossart wrote: > Yeah, this crossed my mind. I thought that "superusers, roles with > privileges of the pg_read_all_stats_role, roles with privileges of the user > owning the session being reported on, and the user owning the session being > reported on" might be too long-winded and redundant. But I see your point > that it might be a bit confusing. Perhaps it could be trimmed down to > something like this: > > ... superusers, roles with privileges of the pg_read_all_stats role, > and roles with privileges of the user owning the session being reported > on (including the session owner). Yeah, that sounds better to me. monitoring.sgml has a different way of wording what looks like the same thing for pg_stat_xact_*_tables: "Ordinary users can only see all the information about their own sessions (sessions belonging to a role that they are a member of)". So you could say instead something like: this information is only visible to superusers, roles with privileges of the pg_read_all_stats role, and the user owning the sessionS being reported on (including sessions belonging to a role that they are a member of). -- Michael
Вложения
On Mon, May 23, 2022 at 08:53:24AM +0900, Michael Paquier wrote: > On Sun, May 22, 2022 at 01:26:08PM -0700, Nathan Bossart wrote: >> ... superusers, roles with privileges of the pg_read_all_stats role, >> and roles with privileges of the user owning the session being reported >> on (including the session owner). > > Yeah, that sounds better to me. monitoring.sgml has a different way > of wording what looks like the same thing for pg_stat_xact_*_tables: > "Ordinary users can only see all the information about their own > sessions (sessions belonging to a role that they are a member of)". > > So you could say instead something like: this information is only > visible to superusers, roles with privileges of the pg_read_all_stats > role, and the user owning the sessionS being reported on (including > sessions belonging to a role that they are a member of). I think we need to be careful about saying "member of" when we really mean "roles with privileges of." Unless I am mistaken, role membership alone is not sufficient for viewing this information. You also need to inherit the role's privileges via INHERIT. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Mon, May 23, 2022 at 09:41:42AM -0700, Nathan Bossart wrote: > I think we need to be careful about saying "member of" when we really mean > "roles with privileges of." Unless I am mistaken, role membership alone is > not sufficient for viewing this information. You also need to inherit the > role's privileges via INHERIT. Good point. So this would give, to be exact: "This information is only visible to superusers, roles with privileges of the pg_read_all_stats role, and and the user owning the sessionS being reported on (including sessions belonging to a role they have the privileges of)." Opinions? -- Michael
Вложения
On Wed, May 25, 2022 at 01:04:04PM +0900, Michael Paquier wrote: > Good point. So this would give, to be exact: > "This information is only visible to superusers, roles with privileges > of the pg_read_all_stats role, and and the user owning the sessionS > being reported on (including sessions belonging to a role they have > the privileges of)." Nathan, Ian, if you think that this could be worded better, please feel free to let me know. Thanks. -- Michael
Вложения
On Sat, May 28, 2022 at 05:50:35PM +0900, Michael Paquier wrote: > On Wed, May 25, 2022 at 01:04:04PM +0900, Michael Paquier wrote: >> Good point. So this would give, to be exact: >> "This information is only visible to superusers, roles with privileges >> of the pg_read_all_stats role, and and the user owning the sessionS >> being reported on (including sessions belonging to a role they have >> the privileges of)." > > Nathan, Ian, if you think that this could be worded better, please > feel free to let me know. Thanks. Sorry, I missed this one earlier. I'm okay with something along those lines. I'm still trying to think of ways to make the last part a little clearer, but I don't have any ideas beyond what we've discussed upthread. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Sat, May 28, 2022 at 06:10:31AM -0700, Nathan Bossart wrote: > Sorry, I missed this one earlier. I'm okay with something along those > lines. I'm still trying to think of ways to make the last part a little > clearer, but I don't have any ideas beyond what we've discussed upthread. Okay. I have used the wording of upthread then. Thanks! -- Michael
Вложения
Re: docs: mention "pg_read_all_stats" in "track_activities" description
От
Ian Lawrence Barwick
Дата:
Hi Apologies for the delayed response, was caught up in a minor life diversion over the past couple of weeks. 2022年5月21日(土) 12:29 Michael Paquier <michael@paquier.xyz>: > > On Fri, May 20, 2022 at 04:08:37PM -0700, Nathan Bossart wrote: > > LGTM > > Indeed, it is a good idea to add this information. Will apply and > backpatch accordingly. Thanks! 2022年5月30日(月) 11:34 Michael Paquier <michael@paquier.xyz>: > > On Sat, May 28, 2022 at 06:10:31AM -0700, Nathan Bossart wrote: > > Sorry, I missed this one earlier. I'm okay with something along those > > lines. I'm still trying to think of ways to make the last part a little > > clearer, but I don't have any ideas beyond what we've discussed upthread. > > Okay. I have used the wording of upthread then. Thanks! A little late to the party, but as an alternative suggestion for the last part: "... and users who either own the session being reported on, or who have privileges of the role to which the session belongs," so the whole sentence would read: Note that even when enabled, this information is only visible to superusers, roles with privileges of the pg_read_all_stats role, and users who either own the session being reported on or who have privileges of the role to which the session belongs, so it should not represent a security risk. or with some parentheses to break it up a little: Note that even when enabled, this information is only visible to superusers, roles with privileges of the pg_read_all_stats role, and users who either own the session being reported on (or who have privileges of the role to which the session belongs), so it should not represent a security risk. I'm not sure if it really improves on the latest committed change, so just a suggestion. Regards Ian Barwick
On Tue, Jun 07, 2022 at 10:08:21PM +0900, Ian Lawrence Barwick wrote: > A little late to the party, but as an alternative suggestion for the last > part: > > "... and users who either own the session being reported on, or who have > privileges of the role to which the session belongs," > > so the whole sentence would read: > > Note that even when enabled, this information is only visible to superusers, > roles with privileges of the pg_read_all_stats role, and users who either own > the session being reported on or who have privileges of the role to which the > session belongs, so it should not represent a security risk. This seems clearer to me. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com