Thread: [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)
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
Attachment
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
Not that I'm aware of at the moment. This one was found by chance.Are there any otherfunctions that pg_monitor ought to have privileges for?
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
Attachment
> 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
Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role
From
Pavlo Golub
Date:
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.
regards, tom lane
Re: Re[2]: [PATCH] allow pg_current_logfile() execution under pg_monitor role
From
Nathan Bossart
Date:
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