[PATCH] allow pg_current_logfile() execution under pg_monitor role
Hello,
The patch attached fixes an oversight/inconsistency of disallowing the
pg_monitor system role to execute pg_current_logfile([text]).
pgwatch3=# create user joe;
CREATE ROLE
pgwatch3=# set role joe;
SET
pgwatch3=> select pg_current_logfile();
ERROR: permission denied for function pg_current_logfile
pgwatch3=> reset role;
RESET
pgwatch3=# grant pg_monitor to joe;
GRANT ROLE
pgwatch3=# set role joe;
SET
pgwatch3=> select pg_current_logfile();
ERROR: permission denied for function pg_current_logfile
pgwatch3=> select * FROM pg_ls_logdir();
name | size | modification
----------------------------------+----------+------------------------
postgresql-2024-02-08_130906.log | 652 | 2024-02-08 13:10:04+01
(5 rows)
Best regards,
Pavlo Golub
Attachments:
0001-allow-pg_current_logfile-execution-under-pg_monitor-.patchapplication/octet-stream; name=0001-allow-pg_current_logfile-execution-under-pg_monitor-.patchDownload
From 9142cfd2c9c7a21a7b73a4c85ff662dade3c41e2 Mon Sep 17 00:00:00 2001
From: Pavlo Golub <pavlo.golub@gmail.com>
Date: Fri, 9 Feb 2024 15:42:43 +0100
Subject: [PATCH] allow pg_current_logfile() execution under pg_monitor role
---
src/backend/catalog/system_functions.sql | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 346cfb98a0..37b8977670 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -761,6 +761,10 @@ REVOKE EXECUTE ON FUNCTION pg_ls_replslotdir(text) FROM PUBLIC;
-- We also set up some things as accessible to standard roles.
--
+GRANT EXECUTE ON FUNCTION pg_current_logfile() TO pg_monitor;
+
+GRANT EXECUTE ON FUNCTION pg_current_logfile(text) TO pg_monitor;
+
GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
--
2.34.1
On Fri, Feb 09, 2024 at 04:01:58PM +0100, Pavlo Golub wrote:
The patch attached fixes an oversight/inconsistency of disallowing the
pg_monitor system role to execute pg_current_logfile([text]).
I think this is reasonable. We allow pg_monitor to execute functions like
pg_ls_logdir(), so it doesn't seem like much of a stretch to expect it to
have privileges for pg_current_logfile(), too. Are there any other
functions that pg_monitor ought to have privileges for?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Are there any other
functions that pg_monitor ought to have privileges for?
Not that I'm aware of at the moment. This one was found by chance.
Kind regards,
Pavlo Golub
On Mon, Feb 12, 2024 at 12:27:54PM +0000, Pavlo Golub wrote:
Are there any other
functions that pg_monitor ought to have privileges for?Not that I'm aware of at the moment. This one was found by chance.
Okay. I'll plan on committing this in the next few days.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
Okay. I'll plan on committing this in the next few days.
Here is what I have staged for commit.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Allow-pg_monitor-to-execute-pg_current_logfile.patchtext/x-diff; charset=us-asciiDownload
From bfe542c5d7b3c981e75ac6551abb34fbdf646eea Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 13 Feb 2024 15:12:36 -0600
Subject: [PATCH v2 1/1] Allow pg_monitor to execute pg_current_logfile().
We allow roles with privileges of pg_monitor to execute functions
like pg_ls_logdir(), so it seems natural that such roles would also
be able to execute this function.
Bumps catversion.
Co-authored-by: Pavlo Golub
Discussion: https://postgr.es/m/CAK7ymcLmEYWyQkiCZ64WC-HCzXAB0omM%3DYpj9B3rXe8vUAFMqw%40mail.gmail.com
---
doc/src/sgml/func.sgml | 5 +++++
src/backend/catalog/system_functions.sql | 4 ++++
src/include/catalog/catversion.h | 2 +-
src/test/regress/expected/misc_functions.out | 20 ++++++++++++++++++++
src/test/regress/sql/misc_functions.sql | 11 +++++++++++
5 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 11d537b341..c4e5b4967e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23735,6 +23735,11 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
<xref linkend="guc-log-destination"/>.
The result reflects the contents of
the <filename>current_logfiles</filename> file.
+ </para>
+ <para>
+ This function is restricted to superusers and roles with privileges of
+ the <literal>pg_monitor</literal> role by default, but other users can
+ be granted EXECUTE to run the function.
</para></entry>
</row>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 346cfb98a0..fe2bb50f46 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -777,6 +777,10 @@ GRANT EXECUTE ON FUNCTION pg_ls_logicalmapdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_replslotdir(text) TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_current_logfile() TO pg_monitor;
+
+GRANT EXECUTE ON FUNCTION pg_current_logfile(text) TO pg_monitor;
+
GRANT pg_read_all_settings TO pg_monitor;
GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9fc8ac9290..80a4c19565 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202401301
+#define CATALOG_VERSION_NO 202402131
#endif
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 7c15477104..d5f61dfad9 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -683,3 +683,23 @@ SELECT gist_stratnum_identity(18::smallint);
18
(1 row)
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege
+------------------------
+ f
+(1 row)
+
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege
+------------------------
+ t
+(1 row)
+
+DROP ROLE regress_current_logfile;
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 851dad90f4..928b04db7f 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -254,3 +254,14 @@ FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size - 1),
-- test stratnum support functions
SELECT gist_stratnum_identity(3::smallint);
SELECT gist_stratnum_identity(18::smallint);
+
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+ 'pg_current_logfile()', 'EXECUTE');
+DROP ROLE regress_current_logfile;
--
2.25.1
On 13 Feb 2024, at 22:29, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
Okay. I'll plan on committing this in the next few days.
Here is what I have staged for commit.
LGTM.
--
Daniel Gustafsson
On Wed, Feb 14, 2024 at 08:59:06AM +0100, Daniel Gustafsson wrote:
LGTM.
Committed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
Okay. I'll plan on committing this in the next few days.
Here is what I have staged for commit.
Oh, thanks! I forgot, indeed, to update docs and catalog version! My
bad!
In my defense, I was trying to find tests but I missed
regress/sql/misc_functions.sql somehow.
Now I will know. Thanks again!
Best regards,
Pavlo Golub
"Pavlo Golub" <pavlo.golub@cybertec.at> writes:
Oh, thanks! I forgot, indeed, to update docs and catalog version! My
bad!
Docs, yes, but don't include catversion bumps in submitted patches.
They'll just lead to merge problems when somebody else changes the
current catversion. We rely on the committer to remember to do this
(which is an imperfect system, but...)
regards, tom lane
On Wed, Feb 14, 2024, 19:45 Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Pavlo Golub" <pavlo.golub@cybertec.at> writes:
Oh, thanks! I forgot, indeed, to update docs and catalog version! My
bad!Docs, yes, but don't include catversion bumps in submitted patches.
They'll just lead to merge problems when somebody else changes the
current catversion. We rely on the committer to remember to do this
(which is an imperfect system, but...)
Thanks for the clarification.
Show quoted text
regards, tom lane
On Wed, Feb 14, 2024 at 01:45:31PM -0500, Tom Lane wrote:
"Pavlo Golub" <pavlo.golub@cybertec.at> writes:
Oh, thanks! I forgot, indeed, to update docs and catalog version! My
bad!Docs, yes, but don't include catversion bumps in submitted patches.
They'll just lead to merge problems when somebody else changes the
current catversion. We rely on the committer to remember to do this
(which is an imperfect system, but...)
+1, I only included it in the patch I posted so that I didn't forget it...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com