Horton

Thread

Commits

  1. doc: Reword description of roles able to view track_activities's info

  2. doc: Mention pg_read_all_stats in description of track_activities

  1. docs: mention "pg_read_all_stats" in "track_activities" description

    Ian Lawrence Barwick <barwick@gmail.com> — 2022-05-20T06:17:29Z

    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

  2. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-05-20T23:08:37Z

    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

  3. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-21T03:28:58Z

    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

  4. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-05-21T18:57:43Z

    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

  5. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-22T00:59:47Z

    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

  6. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-05-22T20:26:08Z

    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

  7. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-22T23:53:24Z

    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

  8. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-05-23T16:41:42Z

    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

  9. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-25T04:04:04Z

    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

  10. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-28T08:50:35Z

    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

  11. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-05-28T13:10:31Z

    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

  12. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Michael Paquier <michael@paquier.xyz> — 2022-05-30T02:34:10Z

    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

  13. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Ian Lawrence Barwick <barwick@gmail.com> — 2022-06-07T13:08:21Z

    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

  14. Re: docs: mention "pg_read_all_stats" in "track_activities" description

    Nathan Bossart <nathandbossart@gmail.com> — 2022-06-15T17:39:25Z

    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