[PATCH] allow pg_current_logfile() execution under pg_monitor role

Started by Pavlo Golubabout 2 years ago11 messageshackers
Jump to latest
#1Pavlo Golub
pavlo.golub@cybertec.at

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+4-1
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Pavlo Golub (#1)
Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#3Pavlo Golub
pavlo.golub@cybertec.at
In reply to: Nathan Bossart (#2)
Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Pavlo Golub (#3)
Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#4)
Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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+41-2
#6Daniel Gustafsson
daniel@yesql.se
In reply to: Nathan Bossart (#5)
Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#7Nathan Bossart
nathandbossart@gmail.com
In reply to: Daniel Gustafsson (#6)
Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

On Wed, Feb 14, 2024 at 08:59:06AM +0100, Daniel Gustafsson wrote:

LGTM.

Committed.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#8Pavlo Golub
pavlo.golub@cybertec.at
In reply to: Nathan Bossart (#5)
Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavlo Golub (#8)
Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role

"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

#10Pavlo Golub
pavlo.golub@cybertec.at
In reply to: Tom Lane (#9)
Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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

#11Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#9)
Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role

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