From: Tom Lane <tgl@sss.pgh.pa.us>
To: Wolfgang Walther <walther@technowledgy.de>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: User with BYPASSRLS privilege can't change password
Date: Tue, 03 Nov 2020 13:51:52 -0500
Message-ID: <961302.1604429512@sss.pgh.pa.us> (raw)
In-Reply-To: <be39bd26-4024-0bf5-eb28-31984a887265@technowledgy.de>
References: <a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de>
<923005.1604420579@sss.pgh.pa.us>
<956994.1604425451@sss.pgh.pa.us>
<3e771893-1ed1-97e7-6c4a-7b0d00992103@technowledgy.de>
<958492.1604427541@sss.pgh.pa.us>
<be39bd26-4024-0bf5-eb28-31984a887265@technowledgy.de>
Wolfgang Walther <walther@technowledgy.de> writes:
> Tom Lane:
>> so AFAICS it's impossible to get there. If it isn't impossible,
>> we have a much bigger hole with respect to issuper.
> Yes, you're right. I read the || as &&. And also missed the ! in else if
> (!have_createrole_privilege()) btw. :)
Actually the right way to deal with this potential confusion is to
add a comment, as below. I fixed the docs too.
> I guess the error message "must be superuser to alter replication users"
> led me on the wrong path. I would be more precise as "must be superuser
> to alter replication users or change replication attribute" to cover the
> change-non-replication-to-replication user case, I think. The same thing
> for superusers.
I'd be okay with changing the error text in HEAD, but less so in the back
branches, since that'd cause thrashing of translatable strings.
regards, tom lane
Attachments:fix-bypassrls-privilege-check-2.patch (text/x-diff, 2.1K) downloaddiff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgmlindex aef30521bc..5aa5648ae7 100644--- a/doc/src/sgml/ref/alter_role.sgml+++ b/doc/src/sgml/ref/alter_role.sgml@@ -71,7 +71,9 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
Attributes not mentioned in the command retain their previous settings.
Database superusers can change any of these settings for any role.
Roles having <literal>CREATEROLE</literal> privilege can change any of these
- settings, but only for non-superuser and non-replication roles.+ settings except <literal>SUPERUSER</literal>, <literal>REPLICATION</literal>,+ and <literal>BYPASSRLS</literal>; but only for non-superuser and+ non-replication roles.
Ordinary roles can only change their own password.
</para>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.cindex 9ce9a66921..293e7e4c0c 100644--- a/src/backend/commands/user.c+++ b/src/backend/commands/user.c@@ -709,8 +709,10 @@ AlterRole(AlterRoleStmt *stmt)
roleid = authform->oid;
/*
- * To mess with a superuser you gotta be superuser; else you need- * createrole, or just want to change your own password+ * To mess with a superuser or replication role in any way you gotta be+ * superuser. We also insist on superuser to change the BYPASSRLS+ * property. Otherwise, if you don't have createrole, you're only allowed+ * to change your own password.
*/
if (authform->rolsuper || issuper >= 0)
{
@@ -726,7 +728,7 @@ AlterRole(AlterRoleStmt *stmt)
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to alter replication users")));
}
- else if (authform->rolbypassrls || bypassrls >= 0)+ else if (bypassrls >= 0)
{
if (!superuser())
ereport(ERROR,
@@ -735,11 +737,11 @@ AlterRole(AlterRoleStmt *stmt)
}
else if (!have_createrole_privilege())
{
+ /* We already checked issuper, isreplication, and bypassrls */
if (!(inherit < 0 &&
createrole < 0 &&
createdb < 0 &&
canlogin < 0 &&
- isreplication < 0 &&
!dconnlimit &&
!rolemembers &&
!validUntil &&
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, walther@technowledgy.de, sfrost@snowman.net, pgsql-bugs@lists.postgresql.org
Subject: Re: User with BYPASSRLS privilege can't change password
In-Reply-To: <961302.1604429512@sss.pgh.pa.us>
* 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