Re: Allow non-superuser to cancel superuser tasks.

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: "Leung, Anthony" <antholeu@amazon.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Kirill Reshke <reshkekirill@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2024-04-05T12:56:56Z
Lists: pgsql-hackers
On Fri, Apr 05, 2024 at 02:39:05PM +0900, Michael Paquier wrote:
> +	/*
> +	 * If the backend is autovacuum worker, allow user with the privileges of
> +	 * pg_signal_autovacuum role to signal the backend.
> +	 */
> +	if (pgstat_get_backend_type(GetNumberFromPGProc(proc)) == B_AUTOVAC_WORKER)
> +	{
> +		if (!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM))
> +			return SIGNAL_BACKEND_NOAUTOVACUUM;
> +	}
> 
> I was wondering why this is not done after we've checked that we have
> a superuser-owned backend, and this is giving me a pause.  @Nathan,
> why do you think we should not rely on the roleId for an autovacuum
> worker?  In core, do_autovacuum() is only called in a process without
> a role specified, and I've noticed your remark here:
> https://www.postgresql.org/message-id/20240401202146.GA2354284@nathanxps13
> It's feeling more natural here to check that we have a superuser-owned
> backend first, and then do a lookup of the process type.

I figured since there's no reason to rely on that behavior, we might as
well do a bit of future-proofing in case autovacuum workers are ever not
run as InvalidOid.  It'd be easy enough to fix this code if that ever
happened, so I'm not too worried about this.

> One thing that we should definitely not do is letting any user calling
> pg_signal_backend() know that a given PID maps to an autovacuum
> worker.  This information is hidden in pg_stat_activity.  And
> actually, doesn't the patch leak this information to all users when
> calling pg_signal_backend with random PID numbers because of the fact
> that SIGNAL_BACKEND_NOAUTOVACUUM exists?  Any role could guess which
> PIDs are used by an autovacuum worker because of the granularity
> required for the error related to pg_signal_autovacuum.

Hm.  I hadn't considered that angle.  IIUC right now they'll just get the
generic superuser error for autovacuum workers.  I don't know how concerned
to be about users distinguishing autovacuum workers from other superuser
backends, _but_ if roles with pg_signal_autovacuum can't even figure out
the PIDs for the autovacuum workers, then this feature seems kind-of
useless.  Perhaps we should allow roles with privileges of
pg_signal_autovacuum to see the autovacuum workers in pg_stat_activity.

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



Commits

  1. Look up backend type in pg_signal_backend() more cheaply.

  2. Add tap test for pg_signal_autovacuum role

  3. Introduce pg_signal_autovacuum_worker.

  4. Add a slot synchronization function.

  5. Ban role pg_signal_backend from more superuser backend types.