public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Leung, Anthony <[email protected]>
Cc: Andrey M. Borodin <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Allow non-superuser to cancel superuser tasks.
Date: Thu, 4 Apr 2024 13:15:33 -0500
Message-ID: <20240404181533.GA3885243@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <20240226151047.GA1872389@nathanxps13>
	<CALdSSPiQPuuQpOkF7x0g2QkA5eE-3xXt7hiJFvShV1bHKDvf8w@mail.gmail.com>
	<CALdSSPjvW+Us39bwDfXNXR1RE+rG3dxRvKyzMT0go_4mYtjUGQ@mail.gmail.com>
	<20240227192322.GB2432025@nathanxps13>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<20240401202146.GA2354284@nathanxps13>
	<[email protected]>
	<[email protected]>

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






view thread (19+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Allow non-superuser to cancel superuser tasks.
  In-Reply-To: <20240404181533.GA3885243@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox