From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pg_parameter_aclcheck() and trusted extensions
Date: Mon, 18 Jul 2022 14:56:49 -0700
Message-ID: <20220718215649.GA3699123@nathanxps13> (raw)
In-Reply-To: <20220714225735.GB3173833@nathanxps13>
References: <20220706224727.GA2158260@nathanxps13>
<YsYxEtX8zOYKf3Ru@paquier.xyz>
<2995921.1657212060@sss.pgh.pa.us>
<3126292.1657828950@sss.pgh.pa.us>
<20220714215225.GA3173833@nathanxps13>
<3159281.1657836225@sss.pgh.pa.us>
<20220714225735.GB3173833@nathanxps13>
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.comAttachments:fix_reset_all_for_gucs.patch (text/x-diff, 954B) downloaddiff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex 0328029d43..fbc1014824 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -11689,7 +11689,8 @@ validate_option_array_item(const char *name, const char *value,
* We cannot do any meaningful check on the value, so only permissions
* are useful to check.
*/
- if (superuser())+ if (superuser() ||+ pg_parameter_aclcheck(gconf->name, GetUserId(), ACL_SET) == ACLCHECK_OK)
return true;
if (skipIfNoPermissions)
return false;
@@ -11703,6 +11704,8 @@ validate_option_array_item(const char *name, const char *value,
/* ok */ ;
else if (gconf->context == PGC_SUSET && superuser())
/* ok */ ;
+ else if (pg_parameter_aclcheck(gconf->name, GetUserId(), ACL_SET) == ACLCHECK_OK)+ /* ok */ ;
else if (skipIfNoPermissions)
return false;
/* if a permissions error should be thrown, let set_config_option do it */
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: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, michael@paquier.xyz
Subject: Re: pg_parameter_aclcheck() and trusted extensions
In-Reply-To: <20220718215649.GA3699123@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox