From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | "Leung, Anthony" <antholeu(at)amazon(dot)com> |
Cc: | "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Allow non-superuser to cancel superuser tasks. |
Date: | 2024-04-04 18:15:33 |
Message-ID: | 20240404181533.GA3885243@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Apr 04, 2024 at 12:30:51AM +0000, Leung, Anthony wrote:
>> if (pg_stat_is_backend_autovac_worker(proc->backendId) &&
>> !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM))
>> return SIGNAL_BACKEND_NOAUTOVACUUM;
>
> I tried to add them above the existing code. When I test it locally, a
> user without pg_signal_autovacuum will actually fail at this block
> because the user is not superuser and !OidIsValid(proc->roleId) is also
> true in the following:
Good catch.
> This is what Im planning to do - If the backend is autovacuum worker and
> the user is not superuser or has pg_signal_autovacuum role, we return the
> new value and provide the relevant error message
>
> /*
> * If the backend is autovacuum worker, allow user with privileges of the
> * pg_signal_autovacuum role to signal the backend.
> */
> if (pgstat_get_backend_type(proc->backendId) == B_AUTOVAC_WORKER)
> {
> if (!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM) || !superuser())
> return SIGNAL_BACKEND_NOAUTOVACUUM;
> }
> /*
> * Only allow superusers to signal superuser-owned backends. Any process
> * not advertising a role might have the importance of a superuser-owned
> * backend, so treat it that way.
> */
> else if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) &&
> !superuser())
> {
> return SIGNAL_BACKEND_NOSUPERUSER;
> }
> /* Users can signal backends they have role membership in. */
> else if (!has_privs_of_role(GetUserId(), proc->roleId) &&
> !has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND))
> {
> return SIGNAL_BACKEND_NOPERMISSION;
> }
There's no need for the explicit superuser() check in the
pg_signal_autovacuum section. That's built into has_privs_of_role()
already.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2024-04-04 18:23:14 | Re: Reports on obsolete Postgres versions |
Previous Message | Jacob Champion | 2024-04-04 18:12:18 | Re: WIP Incremental JSON Parser |