Re: Allow non-superuser to cancel superuser tasks.

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

In response to

Responses

Browse pgsql-hackers by date

  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