From d907bd0543aa63e59653d7345840bed0f8b3a83b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 3 Nov 2020 15:41:32 -0500 Subject: [PATCH] Allow users with BYPASSRLS to alter their own passwords. The intention in commit 491c029db was to require superuserness to change the BYPASSRLS property, but the actual effect of the coding in AlterRole() was to require superuserness to change anything at all about a BYPASSRLS role. Other properties of a BYPASSRLS role should be changeable under the same rules as for a normal role, though. Fix that, and also take care of some documentation omissions related to BYPASSRLS and REPLICATION role properties. Tom Lane and Stephen Frost, per bug report from Wolfgang Walther. Back-patch to all supported branches. Discussion: https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de --- doc/src/sgml/ref/alter_role.sgml | 4 +++- doc/src/sgml/ref/create_role.sgml | 11 +++++++++-- src/backend/commands/user.c | 10 ++++++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.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 { role_specification | A Attributes not mentioned in the command retain their previous settings. Database superusers can change any of these settings for any role. Roles having CREATEROLE privilege can change any of these - settings, but only for non-superuser and non-replication roles. + settings except SUPERUSER, REPLICATION, + and BYPASSRLS; but only for non-superuser and + non-replication roles. Ordinary roles can only change their own password. diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index d23133945d..73b30d7b23 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, NOREPLICATION is the default. + You must be a superuser to create a new role having the + REPLICATION attribute. @@ -192,11 +194,16 @@ in sync when changing the above synopsis! These clauses determine whether a role bypasses every row-level security (RLS) policy. NOBYPASSRLS is the default. + You must be a superuser to create a new role having + the BYPASSRLS attribute. + + + Note that pg_dump will set row_security to OFF 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. + permissions, an error will be returned. However, superusers and the + owner of the table being dumped always bypass RLS. diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 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 &&