Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps
 (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <pgsql-bugs-owner+archive@lists.postgresql.org>)
	id 1ka2WV-0004t8-F2
	for pgsql-bugs@arkaria.postgresql.org; Tue, 03 Nov 2020 20:03:19 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <pgsql-bugs-owner+archive@lists.postgresql.org>)
	id 1ka2Vx-0000TP-6t
	for pgsql-bugs@arkaria.postgresql.org; Tue, 03 Nov 2020 20:02:45 +0000
Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29])
	by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1ka2Vx-0000TI-0b
	for pgsql-bugs@lists.postgresql.org; Tue, 03 Nov 2020 20:02:45 +0000
Received: from tamriel.snowman.net ([2001:470:e38f::11])
	by magus.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1ka2Vt-00023D-Nj
	for pgsql-bugs@lists.postgresql.org; Tue, 03 Nov 2020 20:02:43 +0000
Received: by tamriel.snowman.net (Postfix, from userid 1000)
	id 395795F799; Tue,  3 Nov 2020 15:02:38 -0500 (EST)
Date: Tue, 3 Nov 2020 15:02:38 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Wolfgang Walther <walther@technowledgy.de>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: User with BYPASSRLS privilege can't change password
Message-ID: <20201103200237.GY16415@tamriel.snowman.net>
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>
 <961302.1604429512@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="EBUAyYbgySJLQCwI"
Content-Disposition: inline
In-Reply-To: <961302.1604429512@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
List-Id: <pgsql-bugs.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-bugs@lists.postgresql.org>
List-Owner: <mailto:pgsql-bugs-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-bugs>
Precedence: bulk


--EBUAyYbgySJLQCwI
Content-Type: multipart/mixed; boundary="38WTx9r8F1BUWUep"
Content-Disposition: inline


--38WTx9r8F1BUWUep
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> 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.
>=20
> > Yes, you're right. I read the || as &&. And also missed the ! in else i=
f=20
> > (!have_createrole_privilege()) btw. :)
>=20
> Actually the right way to deal with this potential confusion is to
> add a comment, as below.  I fixed the docs too.
>=20
> > I guess the error message "must be superuser to alter replication users=
"=20
> > led me on the wrong path. I would be more precise as "must be superuser=
=20
> > to alter replication users or change replication attribute" to cover th=
e=20
> > change-non-replication-to-replication user case, I think. The same thin=
g=20
> > for superusers.
>=20
> 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.

I tend to agree with that.

> diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_ro=
le.sgml
> index 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=3D"parameter">role_spec=
ification</replaceable> | A
>     Attributes not mentioned in the command retain their previous setting=
s.
>     Database superusers can change any of these settings for any role.
>     Roles having <literal>CREATEROLE</literal> privilege can change any o=
f these
> -   settings, but only for non-superuser and non-replication roles.
> +   settings except <literal>SUPERUSER</literal>, <literal>REPLICATION</l=
iteral>,
> +   and <literal>BYPASSRLS</literal>; but only for non-superuser and
> +   non-replication roles.
>     Ordinary roles can only change their own password.
>    </para>

This looks good, though should we also mention that you have to be a
superuser to set BYPASSRLS on a role in the CREATE ROLE documentation
too..?  The error message there could also be improved, it looks like.
Also, we don't seem to document in CREATE ROLE that you have to be a
superuser to create a REPLICATION role either, even though the code
clearly says that.

Attached is a patch for all of those.  As mentioned, the error message
probably only makes sense to change in HEAD, though improving the
documentation could be back-patched.

Thanks,

Stephen

--38WTx9r8F1BUWUep
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="cleanup_bypassrls_v1.patch"

diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index d23133945d..0d9c430789 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -181,6 +181,8 @@ in sync when changing the above synopsis!
         highly privileged role, and should only be used on roles actually
         used for replication. If not specified,
         <literal>NOREPLICATION</literal> is the default.
+        You must be a superuser to create a new role having the
+        <literal>REPLICATION</literal> attribute set.
        </para>
       </listitem>
      </varlistentry>
@@ -196,7 +198,8 @@ in sync when changing the above synopsis!
         <literal>OFF</literal> by default, to ensure all contents of a table are
         dumped out.  If the user running pg_dump does not have appropriate
         permissions, an error will be returned.  The superuser and owner of the
-        table being dumped always bypass RLS.
+        table being dumped always bypass RLS.  You must be a superuser to create
+        a new role having the <literal>BYPASSRLS</literal> attribute set.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 9ce9a66921..4a93b68ee3 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -305,7 +305,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
 		if (!superuser())
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-					 errmsg("must be superuser to change bypassrls attribute")));
+					 errmsg("must be superuser to create bypassrls users")));
 	}
 	else
 	{

--38WTx9r8F1BUWUep--

--EBUAyYbgySJLQCwI
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJfobddAAoJEO1sijiDR2RVFSEP/2M3canG5ipXWVKkJ+AjFxcy
8ixDczQ+DvZy9JKrH9ua07gaUnJ8MtHAgP1WKzJPv2z0VnT9WbL0oRsmmuBwQu9P
myByaML9XvFy5NaA0Maq9CsBWpqAEmJMtCQYSywpm2rUQK7ymT//qCZ3JhesPlJW
9CFSXYuI0eTlrguyloxIfEnTBPmAt+wqGpjZ8zG3qFwdAUsby7XdWnQtdSL4fsP6
v4ajt/BoqhVKq77+QGhtQQ4GC/JD4JlHs9pgMiavG/QLimPR43IBL+Cx5xRJ7dpc
MgeHscx5f5jnerMgq8cDZ1bAA2Yj+ybSSNeMZhuiLHFMKoQfI89egW0royQQorDa
UtoIFxu4MXwzFiiHkWyRP1CwOA2q88KNm7amGC4+f0S+7gqE6AKiYJ02mlF7f+MP
LwXR5WdBweuTxrpGVAyu1s2eJhnSDy98Socxy5LHL0KVvHqEMgvzfosFQHAqIJhC
mojJ+VDPKx0Pkfz4dcpgQDbqJ+0+a/0dkcJSmlBD4ruGY+Svj2J3GTo8CG1wRB+V
fHIl8cPzQ4eBwV+P/U/ff8DN8mQr8IXHx83SCytqldoKk/99u6wkHb2XxivtFwSZ
vE+YHuKiHSs7rfntjqJZoj6Cbzx+ZDccgFMEODo8gZPFKgONG/qJq3NPQfRZ8OLr
sBvfD0UfWrJUEDQkHfQR
=jm+Y
-----END PGP SIGNATURE-----

--EBUAyYbgySJLQCwI--