summaryrefslogtreecommitdiff
path: root/src/test
diff options
context:
space:
mode:
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>2021-02-08 11:01:51 +0200
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>2021-02-08 11:01:55 +0200
commit8e56684d54d44ba4ed737d5847d31fba6fb13763 (patch)
tree45755445ea059c3dbfa9da83e7ee4e9334e5f883 /src/test
parentb4199a94946388c60586b44ad82e77755e1543f4 (diff)
Fix permission checks on constraint violation errors on partitions.
If a cross-partition UPDATE violates a constraint on the target partition, and the columns in the new partition are in different physical order than in the parent, the error message can reveal columns that the user does not have SELECT permission on. A similar bug was fixed earlier in commit 804b6b6db4. The cause of the bug is that the callers of the ExecBuildSlotValueDescription() function got confused when constructing the list of modified columns. If the tuple was routed from a parent, we converted the tuple to the parent's format, but the list of modified columns was grabbed directly from the child's RTE entry. ExecUpdateLockMode() had a similar issue. That lead to confusion on which columns are key columns, leading to wrong tuple lock being taken on tables referenced by foreign keys, when a row is updated with INSERT ON CONFLICT UPDATE. A new isolation test is added for that corner case. With this patch, the ri_RangeTableIndex field is no longer set for partitions that don't have an entry in the range table. Previously, it was set to the RTE entry of the parent relation, but that was confusing. NOTE: This modifies the ResultRelInfo struct, replacing the ri_PartitionRoot field with ri_RootResultRelInfo. That's a bit risky to backpatch, because it breaks any extensions accessing the field. The change that ri_RangeTableIndex is not set for partitions could potentially break extensions, too. The ResultRelInfos are visible to FDWs at least, and this patch required small changes to postgres_fdw. Nevertheless, this seem like the least bad option. I don't think these fields widely used in extensions; I don't think there are FDWs out there that uses the FDW "direct update" API, other than postgres_fdw. If there is, you will get a compilation error, so hopefully it is caught quickly. Backpatch to 11, where support for both cross-partition UPDATEs, and unique indexes on partitioned tables, were added. Reviewed-by: Amit Langote Security: CVE-2021-3393
Diffstat (limited to 'src/test')
-rw-r--r--src/test/isolation/expected/tuplelock-partition.out20
-rw-r--r--src/test/isolation/isolation_schedule1
-rw-r--r--src/test/isolation/specs/tuplelock-partition.spec32
-rw-r--r--src/test/regress/expected/privileges.out39
-rw-r--r--src/test/regress/sql/privileges.sql38
5 files changed, 130 insertions, 0 deletions
diff --git a/src/test/isolation/expected/tuplelock-partition.out b/src/test/isolation/expected/tuplelock-partition.out
new file mode 100644
index 00000000000..dd6d37c577a
--- /dev/null
+++ b/src/test/isolation/expected/tuplelock-partition.out
@@ -0,0 +1,20 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1update_nokey s2locktuple s1c
+step s1b: BEGIN;
+step s1update_nokey: INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET col1 = 'x', col2 = 'y';
+step s2locktuple: SELECT * FROM parttab FOR KEY SHARE;
+col1 key col2
+
+a 1 b
+step s1c: COMMIT;
+
+starting permutation: s1b s1update_key s2locktuple s1c
+step s1b: BEGIN;
+step s1update_key: INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET key=1;
+step s2locktuple: SELECT * FROM parttab FOR KEY SHARE; <waiting ...>
+step s1c: COMMIT;
+step s2locktuple: <... completed>
+col1 key col2
+
+a 1 b
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index a4144127add..9ec39730ebc 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -53,6 +53,7 @@ test: propagate-lock-delete
test: tuplelock-conflict
test: tuplelock-update
test: tuplelock-upgrade-no-deadlock
+test: tuplelock-partition
test: freeze-the-dead
test: nowait
test: nowait-2
diff --git a/src/test/isolation/specs/tuplelock-partition.spec b/src/test/isolation/specs/tuplelock-partition.spec
new file mode 100644
index 00000000000..9a585cb1615
--- /dev/null
+++ b/src/test/isolation/specs/tuplelock-partition.spec
@@ -0,0 +1,32 @@
+# Test tuple locking on INSERT ON CONFLICT UPDATE on a partitioned table.
+
+setup
+{
+ DROP TABLE IF EXISTS parttab;
+ CREATE TABLE parttab (col1 text, key INTEGER PRIMARY KEY, col2 text) PARTITION BY LIST (key);
+ CREATE TABLE parttab1 (key INTEGER PRIMARY KEY, col1 text, col2 text);
+ CREATE TABLE parttab2 (key INTEGER PRIMARY KEY, col1 text, col2 text);
+ ALTER TABLE parttab ATTACH PARTITION parttab1 FOR VALUES IN (1);
+ ALTER TABLE parttab ATTACH PARTITION parttab2 FOR VALUES IN (2);
+ INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b');
+}
+
+teardown
+{
+ DROP TABLE parttab;
+}
+
+session "s1"
+step "s1b" { BEGIN; }
+step "s1update_nokey" { INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET col1 = 'x', col2 = 'y'; }
+step "s1update_key" { INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET key=1; }
+step "s1c" { COMMIT; }
+
+session "s2"
+step "s2locktuple" { SELECT * FROM parttab FOR KEY SHARE; }
+
+# INSERT ON CONFLICT UPDATE, performs an UPDATE on non-key columns
+permutation "s1b" "s1update_nokey" "s2locktuple" "s1c"
+
+# INSERT ON CONFLICT UPDATE, performs an UPDATE on key column
+permutation "s1b" "s1update_key" "s2locktuple" "s1c"
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 5e5f98ac68a..dac1d07e92b 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -616,6 +616,45 @@ ERROR: new row for relation "t1" violates check constraint "t1_c3_check"
DETAIL: Failing row contains (c1, c3) = (1, 10).
SET SESSION AUTHORIZATION regress_priv_user1;
DROP TABLE t1;
+-- check error reporting with column privs on a partitioned table
+CREATE TABLE errtst(a text, b text NOT NULL, c text, secret1 text, secret2 text) PARTITION BY LIST (a);
+CREATE TABLE errtst_part_1(secret2 text, c text, a text, b text NOT NULL, secret1 text);
+CREATE TABLE errtst_part_2(secret1 text, secret2 text, a text, c text, b text NOT NULL);
+ALTER TABLE errtst ATTACH PARTITION errtst_part_1 FOR VALUES IN ('aaa');
+ALTER TABLE errtst ATTACH PARTITION errtst_part_2 FOR VALUES IN ('aaaa');
+GRANT SELECT (a, b, c) ON TABLE errtst TO regress_priv_user2;
+GRANT UPDATE (a, b, c) ON TABLE errtst TO regress_priv_user2;
+GRANT INSERT (a, b, c) ON TABLE errtst TO regress_priv_user2;
+INSERT INTO errtst_part_1 (a, b, c, secret1, secret2)
+VALUES ('aaa', 'bbb', 'ccc', 'the body', 'is in the attic');
+SET SESSION AUTHORIZATION regress_priv_user2;
+-- Perform a few updates that violate the NOT NULL constraint. Make sure
+-- the error messages don't leak the secret fields.
+-- simple insert.
+INSERT INTO errtst (a, b) VALUES ('aaa', NULL);
+ERROR: null value in column "b" of relation "errtst_part_1" violates not-null constraint
+DETAIL: Failing row contains (a, b, c) = (aaa, null, null).
+-- simple update.
+UPDATE errtst SET b = NULL;
+ERROR: null value in column "b" of relation "errtst_part_1" violates not-null constraint
+DETAIL: Failing row contains (b) = (null).
+-- partitioning key is updated, doesn't move the row.
+UPDATE errtst SET a = 'aaa', b = NULL;
+ERROR: null value in column "b" of relation "errtst_part_1" violates not-null constraint
+DETAIL: Failing row contains (a, b, c) = (aaa, null, ccc).
+-- row is moved to another partition.
+UPDATE errtst SET a = 'aaaa', b = NULL;
+ERROR: null value in column "b" of relation "errtst_part_2" violates not-null constraint
+DETAIL: Failing row contains (a, b, c) = (aaaa, null, ccc).
+-- row is moved to another partition. This differs from the previous case in
+-- that the new partition is excluded by constraint exclusion, so its
+-- ResultRelInfo is not created at ExecInitModifyTable, but needs to be
+-- constructed on the fly when the updated tuple is routed to it.
+UPDATE errtst SET a = 'aaaa', b = NULL WHERE a = 'aaa';
+ERROR: null value in column "b" of relation "errtst_part_2" violates not-null constraint
+DETAIL: Failing row contains (a, b, c) = (aaaa, null, ccc).
+SET SESSION AUTHORIZATION regress_priv_user1;
+DROP TABLE errtst;
-- test column-level privileges when involved with DELETE
SET SESSION AUTHORIZATION regress_priv_user1;
ALTER TABLE atest6 ADD COLUMN three integer;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index fff76e0bd08..407150baf3a 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -401,6 +401,44 @@ UPDATE t1 SET c3 = 10; -- fail, but see columns with SELECT rights, or being mod
SET SESSION AUTHORIZATION regress_priv_user1;
DROP TABLE t1;
+-- check error reporting with column privs on a partitioned table
+CREATE TABLE errtst(a text, b text NOT NULL, c text, secret1 text, secret2 text) PARTITION BY LIST (a);
+CREATE TABLE errtst_part_1(secret2 text, c text, a text, b text NOT NULL, secret1 text);
+CREATE TABLE errtst_part_2(secret1 text, secret2 text, a text, c text, b text NOT NULL);
+
+ALTER TABLE errtst ATTACH PARTITION errtst_part_1 FOR VALUES IN ('aaa');
+ALTER TABLE errtst ATTACH PARTITION errtst_part_2 FOR VALUES IN ('aaaa');
+
+GRANT SELECT (a, b, c) ON TABLE errtst TO regress_priv_user2;
+GRANT UPDATE (a, b, c) ON TABLE errtst TO regress_priv_user2;
+GRANT INSERT (a, b, c) ON TABLE errtst TO regress_priv_user2;
+
+INSERT INTO errtst_part_1 (a, b, c, secret1, secret2)
+VALUES ('aaa', 'bbb', 'ccc', 'the body', 'is in the attic');
+
+SET SESSION AUTHORIZATION regress_priv_user2;
+
+-- Perform a few updates that violate the NOT NULL constraint. Make sure
+-- the error messages don't leak the secret fields.
+
+-- simple insert.
+INSERT INTO errtst (a, b) VALUES ('aaa', NULL);
+-- simple update.
+UPDATE errtst SET b = NULL;
+-- partitioning key is updated, doesn't move the row.
+UPDATE errtst SET a = 'aaa', b = NULL;
+-- row is moved to another partition.
+UPDATE errtst SET a = 'aaaa', b = NULL;
+
+-- row is moved to another partition. This differs from the previous case in
+-- that the new partition is excluded by constraint exclusion, so its
+-- ResultRelInfo is not created at ExecInitModifyTable, but needs to be
+-- constructed on the fly when the updated tuple is routed to it.
+UPDATE errtst SET a = 'aaaa', b = NULL WHERE a = 'aaa';
+
+SET SESSION AUTHORIZATION regress_priv_user1;
+DROP TABLE errtst;
+
-- test column-level privileges when involved with DELETE
SET SESSION AUTHORIZATION regress_priv_user1;
ALTER TABLE atest6 ADD COLUMN three integer;