Horton

v1-0001-Add-permission-check-for-MERGE-SPLIT-partition-op.patch

application/octet-stream

Download
Filename: v1-0001-Add-permission-check-for-MERGE-SPLIT-partition-op.patch
Type: application/octet-stream
Part: 1
Message: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v1-0001
Subject: Add permission check for MERGE/SPLIT partition operations
File+−
src/backend/parser/parse_utilcmd.c 5 0
src/test/regress/expected/partition_merge.out 29 0
src/test/regress/expected/partition_split.out 29 0
src/test/regress/sql/partition_merge.sql 33 0
src/test/regress/sql/partition_split.sql 33 0 
From 02e3ca736fdc7425ccec294a9bc5fae8b82bc673 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Wed, 8 May 2024 17:30:09 +0300
Subject: [PATCH v1 1/2] Add permission check for MERGE/SPLIT partition
 operations

Currently, we check only owner permission for the parent table before
MERGE/SPLIT partition operations.  This leads to a security hole when users
can get access to the data of partitions without permission.  This commit
fixes this problem by requiring owner permission on all the partitions
involved.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/0520c72e-8d97-245e-53f9-173beca2ab2e%40gmail.com
Author: Dmitry Koval, Alexander Korotkov
---
 src/backend/parser/parse_utilcmd.c            |  5 +++
 src/test/regress/expected/partition_merge.out | 29 ++++++++++++++++
 src/test/regress/expected/partition_split.out | 29 ++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 33 +++++++++++++++++++
 src/test/regress/sql/partition_split.sql      | 33 +++++++++++++++++++
 5 files changed, 129 insertions(+)

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 6520bf9baa5..0598e897d90 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3456,6 +3456,11 @@ checkPartition(Relation rel, Oid partRelOid)
 						RelationGetRelationName(partRel),
 						RelationGetRelationName(rel))));
 
+	/* Permissions checks */
+	if (!object_ownercheck(RelationRelationId, RelationGetRelid(partRel), GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(partRel->rd_rel->relkind),
+					   RelationGetRelationName(partRel));
+
 	relation_close(partRel, AccessShareLock);
 }
 
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 52e5c3ce0da..076264c88eb 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -881,6 +881,35 @@ ORDER BY c.relname;
 
 DROP TABLE t;
 DROP ACCESS METHOD partitions_merge_heap;
+-- Test permission checks.  The user needs to own the parent table and all
+-- the merging partitions to do the merge.
+CREATE ROLE regress_partition_merge_alice;
+CREATE ROLE regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_alice;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  must be owner of table t
+RESET SESSION AUTHORIZATION;
+ALTER TABLE t OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  must be owner of table tp_0_1
+RESET SESSION AUTHORIZATION;
+ALTER TABLE tp_0_1 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  must be owner of table tp_1_2
+RESET SESSION AUTHORIZATION;
+ALTER TABLE tp_1_2 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+RESET SESSION AUTHORIZATION;
+DROP TABLE t;
+DROP ROLE regress_partition_merge_alice;
+DROP ROLE regress_partition_merge_bob;
 RESET search_path;
 --
 DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 641e1acc3d7..74e19d250e9 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1514,6 +1514,35 @@ ORDER BY c.relname;
 
 DROP TABLE t;
 DROP ACCESS METHOD partition_split_heap;
+-- Test permission checks.  The user needs to own the parent table and the
+-- the partition to split to do the split.
+CREATE ROLE regress_partition_merge_alice;
+CREATE ROLE regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_alice;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+ERROR:  must be owner of table t
+RESET SESSION AUTHORIZATION;
+ALTER TABLE t OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+ERROR:  must be owner of table tp_0_2
+RESET SESSION AUTHORIZATION;
+ALTER TABLE tp_0_2 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+RESET SESSION AUTHORIZATION;
+DROP TABLE t;
+DROP ROLE regress_partition_merge_alice;
+DROP ROLE regress_partition_merge_bob;
 --
 DROP SCHEMA partition_split_schema;
 DROP SCHEMA partition_split_schema2;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 84a3462205a..9bfeb0d7ef3 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -549,6 +549,39 @@ ORDER BY c.relname;
 DROP TABLE t;
 DROP ACCESS METHOD partitions_merge_heap;
 
+-- Test permission checks.  The user needs to own the parent table and all
+-- the merging partitions to do the merge.
+CREATE ROLE regress_partition_merge_alice;
+CREATE ROLE regress_partition_merge_bob;
+
+SET SESSION AUTHORIZATION regress_partition_merge_alice;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+RESET SESSION AUTHORIZATION;
+
+ALTER TABLE t OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+RESET SESSION AUTHORIZATION;
+
+ALTER TABLE tp_0_1 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+RESET SESSION AUTHORIZATION;
+
+ALTER TABLE tp_1_2 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+RESET SESSION AUTHORIZATION;
+
+DROP TABLE t;
+DROP ROLE regress_partition_merge_alice;
+DROP ROLE regress_partition_merge_bob;
+
 RESET search_path;
 
 --
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index d2c687c41ba..bb52514b7ed 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -894,6 +894,39 @@ ORDER BY c.relname;
 DROP TABLE t;
 DROP ACCESS METHOD partition_split_heap;
 
+-- Test permission checks.  The user needs to own the parent table and the
+-- the partition to split to do the split.
+CREATE ROLE regress_partition_merge_alice;
+CREATE ROLE regress_partition_merge_bob;
+
+SET SESSION AUTHORIZATION regress_partition_merge_alice;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+RESET SESSION AUTHORIZATION;
+
+ALTER TABLE t OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+RESET SESSION AUTHORIZATION;
+
+ALTER TABLE tp_0_2 OWNER TO regress_partition_merge_bob;
+SET SESSION AUTHORIZATION regress_partition_merge_bob;
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+RESET SESSION AUTHORIZATION;
+
+DROP TABLE t;
+DROP ROLE regress_partition_merge_alice;
+DROP ROLE regress_partition_merge_bob;
+
 
 --
 DROP SCHEMA partition_split_schema;
-- 
2.39.3 (Apple Git-145)