docs: mention "pg_read_all_stats" in "track_activities" description

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

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