Re: pg_parameter_aclcheck() and trusted extensions

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_parameter_aclcheck() and trusted extensions
Date: 2022-07-18 21:56:49
Message-ID: 20220718215649.GA3699123@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 14, 2022 at 03:57:35PM -0700, Nathan Bossart wrote:
> However, ALTER ROLE RESET ALL will be blocked, while resetting only the
> individual GUC will go through.
>
> postgres=> ALTER ROLE other RESET ALL;
> ALTER ROLE
> postgres=> SELECT * FROM pg_db_role_setting;
> setdatabase | setrole | setconfig
> -------------+---------+-------------------------
> 0 | 16385 | {zero_damaged_pages=on}
> (1 row)
>
> postgres=> ALTER ROLE other RESET zero_damaged_pages;
> ALTER ROLE
> postgres=> SELECT * FROM pg_db_role_setting;
> setdatabase | setrole | setconfig
> -------------+---------+-----------
> (0 rows)
>
> I think this is because GUCArrayReset() is the only caller of
> validate_option_array_item() that sets skipIfNoPermissions to true. The
> others fall through to set_config_option(), which does a
> pg_parameter_aclcheck(). So, you are right.

Here's a small patch that seems to fix this case. However, I wonder if a
better way to fix this is to provide a way to stop set_config_option() from
throwing errors (e.g., setting elevel to -1). That way, we could remove
the manual permissions checks in favor of always using the real ones, which
might help prevent similar bugs in the future.

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

Attachment Content-Type Size
fix_reset_all_for_gucs.patch text/x-diff 955 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-07-18 22:09:38 Re: Use -fvisibility=hidden for shared libraries
Previous Message Martin Kalcher 2022-07-18 21:48:55 Re: [PATCH] Introduce array_shuffle() and array_sample()