Thread: pgsql: Allow GRANT on pg_log_backend_memory_contexts().
Allow GRANT on pg_log_backend_memory_contexts(). Remove superuser check, allowing any user granted permissions on pg_log_backend_memory_contexts() to log the memory contexts of any backend. Note that this could allow a privileged non-superuser to log the memory contexts of a superuser backend, but as discussed, that does not seem to be a problem. Reviewed-by: Nathan Bossart, Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Andres Freund Discussion: https://postgr.es/m/e5cf6684d17c8d1ef4904ae248605ccd6da03e72.camel@j-davis.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/f0b051e322d530a340e62f2ae16d99acdbcb3d05 Modified Files -------------- doc/src/sgml/func.sgml | 1 - src/backend/catalog/system_functions.sql | 2 ++ src/backend/utils/adt/mcxtfuncs.c | 14 ++++-------- src/include/catalog/catversion.h | 2 +- src/test/regress/expected/misc_functions.out | 33 ++++++++++++++++++++++++++-- src/test/regress/sql/misc_functions.sql | 26 ++++++++++++++++++++-- 6 files changed, 62 insertions(+), 16 deletions(-)
On Tue, Oct 26, 2021 at 08:41:11PM +0000, Jeff Davis wrote:
> Allow GRANT on pg_log_backend_memory_contexts().
>
> Remove superuser check, allowing any user granted permissions on
> pg_log_backend_memory_contexts() to log the memory contexts of any
> backend.
>
> Note that this could allow a privileged non-superuser to log the
> memory contexts of a superuser backend, but as discussed, that does
> not seem to be a problem.
but will not be sent to the client regardless of
<xref linkend="guc-client-min-messages"/>.
- Only superusers can request to log the memory contexts.
</para></entry>
I don't think that the documentation part of this commit is correct.
This sentence should have been changed to something like the
following:
"This function is restricted to superusers by default, but other users
can be granted EXECUTE to run the function."
Thanks,
--
Michael
Attachment
On Wed, Oct 27, 2021 at 9:45 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Tue, Oct 26, 2021 at 08:41:11PM +0000, Jeff Davis wrote: > > Allow GRANT on pg_log_backend_memory_contexts(). > > > > Remove superuser check, allowing any user granted permissions on > > pg_log_backend_memory_contexts() to log the memory contexts of any > > backend. > > > > Note that this could allow a privileged non-superuser to log the > > memory contexts of a superuser backend, but as discussed, that does > > not seem to be a problem. > > but will not be sent to the client regardless of > <xref linkend="guc-client-min-messages"/>. > - Only superusers can request to log the memory contexts. > </para></entry> > I don't think that the documentation part of this commit is correct. > This sentence should have been changed to something like the > following: > "This function is restricted to superusers by default, but other users > can be granted EXECUTE to run the function." +1 and the above statement looks good. Apart from that I have one more suggestion that I earlier made: have the function produce the error when no permissions were granted (just to ensure we have the error message covered) including has_function_privilege() case. Attached a patch with the above documentation change and the error case added. Please review it. Regards, Bharath Rupireddy.
Attachment
On Wed, Oct 27, 2021 at 10:33 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Wed, Oct 27, 2021 at 10:27:18AM +0530, Bharath Rupireddy wrote: > > +1 and the above statement looks good. Apart from that I have one more > > suggestion that I earlier made: have the function produce the error > > when no permissions were granted (just to ensure we have the error > > message covered) including has_function_privilege() case. > > has_function_privilege() makes sure of the same thing, so I think that > what Jeff has done for this part is just but fine. There is no need > for more duplication in the tests. Fair enough. Regards, Bharath Rupireddy.
On Wed, 2021-10-27 at 13:15 +0900, Michael Paquier wrote:
> I don't think that the documentation part of this commit is correct.
> This sentence should have been changed to something like the
> following:
> "This function is restricted to superusers by default, but other
> users
> can be granted EXECUTE to run the function."
Above the table of functions it already says:
"Use of these functions is restricted to superusers by default but
access may be granted to others using GRANT, with noted exceptions."
It looks like several people missed that, so perhaps we should get rid
of that statement at the top, and move it in to each function
description?
Regards,
Jeff Davis