Rework confusing permissions for LOCK TABLE
Started by Jeff Davisabout 3 years ago3 messages
The existing permissions for LOCK TABLE are surprising/confusing. For
instance, if you have UPDATE privileges on a table, you can lock in any
mode *except* ACCESS SHARE.
drop table x cascade;
drop user u1;
create user u1;
create table x(i int);
grant update on x to u1;
set session authorization u1;
begin;
lock table x in access exclusive mode; -- succeeds
commit;
begin;
lock table x in share mode; -- succeeds
commit;
begin;
lock table x in access share mode; -- fails
commit;
I can't think of any reason for this behavior, and I didn't find an
obvious answer in the last commits to touch that (2ad36c4e44,
fa2642438f).
Patch attached to simplify it. It uses the philosophy that, if you have
permissions to lock at a given mode, you should be able to lock at
strictly less-conflicting modes as well.
--
Jeff Davis
PostgreSQL Contributor Team - AWS
Attachments:
v1-0001-Rework-permissions-for-LOCK-TABLE.patchtext/x-patch; charset=UTF-8; name=v1-0001-Rework-permissions-for-LOCK-TABLE.patchDownload
From 53e27cfbe74f6b943fcf7969fb25ddf765219ff5 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 13 Dec 2022 17:41:55 -0800
Subject: [PATCH v1] Rework permissions for LOCK TABLE.
The prior behavior was confusing and hard to document. For instance,
if you had UPDATE privileges, you could lock a table in any lock mode
except ACCESS SHARE mode.
Now, if granted a privilege to lock at a given mode, one also has
privileges to lock at a less-conflicting mode. MAINTAIN, UPDATE,
DELETE, and TRUNCATE privileges allow any lock mode. INSERT privileges
allow ROW EXCLUSIVE (or below). SELECT privileges allow ACCESS SHARE.
---
doc/src/sgml/ref/lock.sgml | 25 ++++-----
src/backend/commands/lockcmds.c | 18 +++----
src/test/regress/expected/privileges.out | 66 ++++++++++--------------
src/test/regress/sql/privileges.sql | 61 +++++++++++-----------
4 files changed, 79 insertions(+), 91 deletions(-)
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index d9c5bf9a1d..8524182211 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -165,18 +165,19 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
<title>Notes</title>
<para>
- To lock a table, one must ordinarily have the <literal>MAINTAIN</literal>
- privilege on the table or be the table's owner, a superuser, or a role
- with privileges of the
- <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
- role. <literal>LOCK TABLE ... IN ACCESS SHARE MODE</literal> is allowed
- with <literal>SELECT</literal> privileges on the target
- table. <literal>LOCK TABLE ... IN ROW EXCLUSIVE MODE</literal> is allowed
- with <literal>INSERT</literal>, <literal>UPDATE</literal>, <literal>DELETE</literal>,
- or <literal>TRUNCATE</literal> privileges on the target table. All other
- forms of <command>LOCK</command> are allowed with
- table-level <literal>UPDATE</literal>, <literal>DELETE</literal>,
- or <literal>TRUNCATE</literal> privileges.
+ To lock a table, the user must have the right privilege for the specified
+ <replaceable class="parameter">lockmode</replaceable>, or be the table's
+ owner, a superuser, or a role with privileges of the <link
+ linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
+ role. If the user has <literal>MAINTAIN</literal>,
+ <literal>UPDATE</literal>, <literal>DELETE</literal>, or
+ <literal>TRUNCATE</literal> privileges on the table, any <replaceable
+ class="parameter">lockmode</replaceable> is permitted. If the user has
+ <literal>INSERT</literal> privileges on the table, <literal>ROW EXCLUSIVE
+ MODE</literal> (or a less-conflicting mode as described in <xref
+ linkend="explicit-locking"/>) is permitted. If a user has
+ <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
+ MODE</literal> is permitted.
</para>
<para>
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index e294efc67c..fceefafbce 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -292,16 +292,16 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)
AclResult aclresult;
AclMode aclmask;
- /* Verify adequate privilege */
- if (lockmode == AccessShareLock)
- aclmask = ACL_SELECT;
- else if (lockmode == RowExclusiveLock)
- aclmask = ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
- else
- aclmask = ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
+ /* any of these privileges permit any lock mode */
+ aclmask = ACL_MAINTAIN | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
+
+ /* SELECT privileges also permit ACCESS SHARE and below */
+ if (lockmode <= AccessShareLock)
+ aclmask |= ACL_SELECT;
- /* MAINTAIN privilege allows all lock modes */
- aclmask |= ACL_MAINTAIN;
+ /* INSERT privileges also permit ROW EXCLUSIVE and below */
+ if (lockmode <= RowExclusiveLock)
+ aclmask |= ACL_INSERT;
aclresult = pg_class_aclcheck(reloid, userid, aclmask);
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 169b364b22..58d9112ee8 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -2663,13 +2663,13 @@ CREATE TABLE lock_table (a int);
GRANT SELECT ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+COMMIT;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should fail
ERROR: permission denied for table lock_table
ROLLBACK;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
-COMMIT;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should fail
ERROR: permission denied for table lock_table
ROLLBACK;
@@ -2679,13 +2679,12 @@ REVOKE SELECT ON lock_table FROM regress_locktable_user;
GRANT INSERT ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ERROR: permission denied for table lock_table
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should fail
ERROR: permission denied for table lock_table
ROLLBACK;
@@ -2695,13 +2694,12 @@ REVOKE INSERT ON lock_table FROM regress_locktable_user;
GRANT UPDATE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ERROR: permission denied for table lock_table
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
@@ -2710,13 +2708,12 @@ REVOKE UPDATE ON lock_table FROM regress_locktable_user;
GRANT DELETE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ERROR: permission denied for table lock_table
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
@@ -2725,17 +2722,30 @@ REVOKE DELETE ON lock_table FROM regress_locktable_user;
GRANT TRUNCATE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ERROR: permission denied for table lock_table
+LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
+COMMIT;
+\c
+REVOKE TRUNCATE ON lock_table FROM regress_locktable_user;
+-- LOCK TABLE and MAINTAIN permission
+GRANT MAINTAIN ON lock_table TO regress_locktable_user;
+SET SESSION AUTHORIZATION regress_locktable_user;
+BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
ROLLBACK;
BEGIN;
+LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
+COMMIT;
+BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
-REVOKE TRUNCATE ON lock_table FROM regress_locktable_user;
+REVOKE MAINTAIN ON lock_table FROM regress_locktable_user;
-- clean up
DROP TABLE lock_table;
DROP USER regress_locktable_user;
@@ -2877,14 +2887,6 @@ REINDEX INDEX maintain_test_a_idx;
ERROR: must be owner of index maintain_test_a_idx
REINDEX SCHEMA reindex_test;
ERROR: must be owner of schema reindex_test
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-ERROR: permission denied for table maintain_test
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-ERROR: permission denied for table maintain_test
-COMMIT;
RESET ROLE;
SET ROLE regress_maintain;
VACUUM maintain_test;
@@ -2896,12 +2898,6 @@ REINDEX TABLE maintain_test;
REINDEX INDEX maintain_test_a_idx;
REINDEX SCHEMA reindex_test;
ERROR: must be owner of schema reindex_test
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-COMMIT;
RESET ROLE;
SET ROLE regress_maintain_all;
VACUUM maintain_test;
@@ -2912,12 +2908,6 @@ REFRESH MATERIALIZED VIEW refresh_test;
REINDEX TABLE maintain_test;
REINDEX INDEX maintain_test_a_idx;
REINDEX SCHEMA reindex_test;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-COMMIT;
RESET ROLE;
DROP TABLE maintain_test;
DROP MATERIALIZED VIEW refresh_test;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index b2db1c6dd5..f8efbd3061 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1698,12 +1698,12 @@ CREATE TABLE lock_table (a int);
GRANT SELECT ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
-LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should fail
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
COMMIT;
BEGIN;
+LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should fail
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should fail
ROLLBACK;
\c
@@ -1713,12 +1713,12 @@ REVOKE SELECT ON lock_table FROM regress_locktable_user;
GRANT INSERT ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should fail
ROLLBACK;
\c
@@ -1728,12 +1728,12 @@ REVOKE INSERT ON lock_table FROM regress_locktable_user;
GRANT UPDATE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
@@ -1743,12 +1743,12 @@ REVOKE UPDATE ON lock_table FROM regress_locktable_user;
GRANT DELETE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
-ROLLBACK;
-BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
@@ -1758,16 +1758,31 @@ REVOKE DELETE ON lock_table FROM regress_locktable_user;
GRANT TRUNCATE ON lock_table TO regress_locktable_user;
SET SESSION AUTHORIZATION regress_locktable_user;
BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
+ROLLBACK;
+BEGIN;
LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
COMMIT;
BEGIN;
-LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should fail
+LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
+COMMIT;
+\c
+REVOKE TRUNCATE ON lock_table FROM regress_locktable_user;
+
+-- LOCK TABLE and MAINTAIN permission
+GRANT MAINTAIN ON lock_table TO regress_locktable_user;
+SET SESSION AUTHORIZATION regress_locktable_user;
+BEGIN;
+LOCK TABLE lock_table IN ACCESS SHARE MODE; -- should pass
ROLLBACK;
BEGIN;
+LOCK TABLE lock_table IN ROW EXCLUSIVE MODE; -- should pass
+COMMIT;
+BEGIN;
LOCK TABLE lock_table IN ACCESS EXCLUSIVE MODE; -- should pass
COMMIT;
\c
-REVOKE TRUNCATE ON lock_table FROM regress_locktable_user;
+REVOKE MAINTAIN ON lock_table FROM regress_locktable_user;
-- clean up
DROP TABLE lock_table;
@@ -1875,12 +1890,6 @@ REFRESH MATERIALIZED VIEW refresh_test;
REINDEX TABLE maintain_test;
REINDEX INDEX maintain_test_a_idx;
REINDEX SCHEMA reindex_test;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-COMMIT;
RESET ROLE;
SET ROLE regress_maintain;
@@ -1892,12 +1901,6 @@ REFRESH MATERIALIZED VIEW refresh_test;
REINDEX TABLE maintain_test;
REINDEX INDEX maintain_test_a_idx;
REINDEX SCHEMA reindex_test;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-COMMIT;
RESET ROLE;
SET ROLE regress_maintain_all;
@@ -1909,12 +1912,6 @@ REFRESH MATERIALIZED VIEW refresh_test;
REINDEX TABLE maintain_test;
REINDEX INDEX maintain_test_a_idx;
REINDEX SCHEMA reindex_test;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS SHARE MODE;
-COMMIT;
-BEGIN;
-LOCK TABLE maintain_test IN ACCESS EXCLUSIVE MODE;
-COMMIT;
RESET ROLE;
DROP TABLE maintain_test;
--
2.34.1
Re: Rework confusing permissions for LOCK TABLE
On Tue, Dec 13, 2022 at 06:59:48PM -0800, Jeff Davis wrote:
I can't think of any reason for this behavior, and I didn't find an
obvious answer in the last commits to touch that (2ad36c4e44,
fa2642438f).
I can't think of a reason, either.
Patch attached to simplify it. It uses the philosophy that, if you have
permissions to lock at a given mode, you should be able to lock at
strictly less-conflicting modes as well.
+1. Your patch looks reasonable to me.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Re: Rework confusing permissions for LOCK TABLE
I filed a commitfest entry for this so that it doesn't get lost:
https://commitfest.postgresql.org/41/4093
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com