From 038dbd373d138d6bcd5d275868772274361dcfc9 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 18 Sep 2026 17:31:49 +0900 Subject: [PATCH 1/2] Fix RI fast-path permission checks The fast path required table-level SELECT on the referenced table, rejecting checks that the SPI path allows with column-level grants. It also omitted the UPDATE privilege required by FOR KEY SHARE. When table privileges do not suffice, use ExecCheckOneRelPerms() with the referenced key columns as selectedCols and an empty updatedCols. This accepts SELECT on all referenced columns and UPDATE on any column, the same privileges the executor would require for the SELECT ... FOR KEY SHARE the SPI path runs. Keep the table-privilege check as a shortcut that avoids constructing a column bitmap in the usual case. Add missing regression test coverage for the fixed cases. Reported-by: Nikolay Samokhvalov Author: Nikolay Samokhvalov Co-authored-by: Amit Langote Discussion: https://www.postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq%2BnOrDq1j__A%40mail.gmail.com Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 47 +++++++++++++----- src/test/regress/expected/foreign_key.out | 55 ++++++++++++++++++++- src/test/regress/sql/foreign_key.sql | 59 ++++++++++++++++++++++- 3 files changed, 146 insertions(+), 15 deletions(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 43f40172b67..3196d971939 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -395,7 +395,8 @@ static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap, static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo); static bool ri_check_fastpath_index(RI_ConstraintInfo *riinfo, Relation pk_rel, Relation idx_rel); -static void ri_CheckPermissions(Relation query_rel); +static void ri_CheckPermissions(const RI_ConstraintInfo *riinfo, + Relation query_rel); static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, int nkeys, TupleTableSlot *new_slot); static void build_index_scankeys(const RI_ConstraintInfo *riinfo, @@ -2939,7 +2940,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, saved_sec_context | SECURITY_LOCAL_USERID_CHANGE | SECURITY_NOFORCE_RLS); - ri_CheckPermissions(pk_rel); + ri_CheckPermissions(riinfo, pk_rel); /* * Begin the scan under the switched user id, so that any access method @@ -3106,7 +3107,7 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, * albeit checked once per flush rather than once per row, like in * ri_FastPathCheck(). */ - ri_CheckPermissions(pk_rel); + ri_CheckPermissions(riinfo, pk_rel); /* * Begin the scan under the switched user id, so that any access method @@ -3572,13 +3573,16 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo, /* * ri_CheckPermissions - * Check that the current user has permissions to look into the schema of - * and SELECT from 'query_rel' + * Check permissions for the SELECT ... FOR KEY SHARE used by the SPI + * path, as the referenced table's owner. */ static void -ri_CheckPermissions(Relation query_rel) +ri_CheckPermissions(const RI_ConstraintInfo *riinfo, Relation query_rel) { AclResult aclresult; + AclMode requiredPerms = ACL_SELECT | ACL_SELECT_FOR_UPDATE; + RTEPermissionInfo *perminfo; + bool result; /* USAGE on schema. */ aclresult = object_aclcheck(NamespaceRelationId, @@ -3588,11 +3592,32 @@ ri_CheckPermissions(Relation query_rel) aclcheck_error(aclresult, OBJECT_SCHEMA, get_namespace_name(RelationGetNamespace(query_rel))); - /* SELECT on relation. */ - aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(), - ACL_SELECT); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_TABLE, + /* Avoid building the column bitmap when table privileges suffice. */ + if (pg_class_aclmask(RelationGetRelid(query_rel), GetUserId(), + requiredPerms, ACLMASK_ALL) == requiredPerms) + return; + + /* + * SELECT is needed only on the referenced key columns. FOR KEY SHARE + * also needs UPDATE privilege, which may be granted on any column. Use + * the executor's checks for both, leaving updatedCols empty as the SPI + * query does. + */ + perminfo = makeNode(RTEPermissionInfo); + perminfo->relid = RelationGetRelid(query_rel); + perminfo->requiredPerms = requiredPerms; + for (int i = 0; i < riinfo->nkeys; i++) + { + int attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber; + + perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno); + } + + result = ExecCheckOneRelPerms(perminfo); + bms_free(perminfo->selectedCols); + pfree(perminfo); + if (!result) + aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE, RelationGetRelationName(query_rel)); } diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 8d81240f1c6..c54a9895b55 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -402,17 +402,68 @@ CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE, ftest2 int ); INSERT INTO PKTABLE VALUES (1, 'Test1'); INSERT INTO PKTABLE VALUES (2, 'Test2'); INSERT INTO PKTABLE VALUES (3, 'Test3'); --- Grant usage on PKTABLE to user regress_foreign_key_user +-- Grant SELECT on PKTABLE to user regress_foreign_key_user CREATE USER regress_foreign_key_user NOLOGIN; GRANT SELECT ON PKTABLE TO regress_foreign_key_user; ALTER TABLE PKTABLE OWNER to regress_foreign_key_user; -- Inserting into FKTABLE should work INSERT INTO FKTABLE VALUES (3, 5); --- Revoke usage on PKTABLE from user regress_foreign_key_user +-- Revoke SELECT on PKTABLE from user regress_foreign_key_user REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user; -- Inserting into FKTABLE should fail INSERT INTO FKTABLE VALUES (2, 6); ERROR: permission denied for table pktable +-- SELECT on the referenced key column is enough, without SELECT on ptest2. +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); +-- SELECT on an unrelated column does not suffice. +REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails +ERROR: permission denied for table pktable +REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; +-- FOR KEY SHARE also requires UPDATE privilege. +REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails +ERROR: permission denied for table pktable +-- UPDATE on any column suffices, even one that the check does not read. +GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); +-- Table-level SELECT can be combined with column-level UPDATE. +GRANT SELECT ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); +REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails +ERROR: permission denied for table pktable +DROP TABLE FKTABLE; +DROP TABLE PKTABLE; +-- Check all referenced columns, including when index and FK order differ. +CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int, + PRIMARY KEY (ptest2, ptest1) ); +CREATE TABLE FKTABLE ( ftest1 int, ftest2 int ); +INSERT INTO PKTABLE VALUES ('unused', 1, 2); +INSERT INTO FKTABLE VALUES (1, 2); +ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk + FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID; +ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user; +ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user; +REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; +-- Lack of SELECT on FKTABLE forces validation to check each row. +REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user; +SET ROLE regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails +ERROR: permission denied for table pktable +GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user; +-- Per-row validation also requires UPDATE privilege. +REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails +ERROR: permission denied for table pktable +-- UPDATE on the unrelated column is enough. +GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; +RESET ROLE; DROP TABLE FKTABLE; DROP TABLE PKTABLE; DROP USER regress_foreign_key_user; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 184d9efdc97..c013d1f8834 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -286,7 +286,7 @@ INSERT INTO PKTABLE VALUES (1, 'Test1'); INSERT INTO PKTABLE VALUES (2, 'Test2'); INSERT INTO PKTABLE VALUES (3, 'Test3'); --- Grant usage on PKTABLE to user regress_foreign_key_user +-- Grant SELECT on PKTABLE to user regress_foreign_key_user CREATE USER regress_foreign_key_user NOLOGIN; GRANT SELECT ON PKTABLE TO regress_foreign_key_user; @@ -295,12 +295,67 @@ ALTER TABLE PKTABLE OWNER to regress_foreign_key_user; -- Inserting into FKTABLE should work INSERT INTO FKTABLE VALUES (3, 5); --- Revoke usage on PKTABLE from user regress_foreign_key_user +-- Revoke SELECT on PKTABLE from user regress_foreign_key_user REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user; -- Inserting into FKTABLE should fail INSERT INTO FKTABLE VALUES (2, 6); +-- SELECT on the referenced key column is enough, without SELECT on ptest2. +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); + +-- SELECT on an unrelated column does not suffice. +REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails +REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; + +-- FOR KEY SHARE also requires UPDATE privilege. +REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails + +-- UPDATE on any column suffices, even one that the check does not read. +GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); + +-- Table-level SELECT can be combined with column-level UPDATE. +GRANT SELECT ON PKTABLE TO regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); +REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user; +INSERT INTO FKTABLE VALUES (2, 6); -- fails + +DROP TABLE FKTABLE; +DROP TABLE PKTABLE; + +-- Check all referenced columns, including when index and FK order differ. +CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int, + PRIMARY KEY (ptest2, ptest1) ); +CREATE TABLE FKTABLE ( ftest1 int, ftest2 int ); +INSERT INTO PKTABLE VALUES ('unused', 1, 2); +INSERT INTO FKTABLE VALUES (1, 2); +ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk + FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID; +ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user; +ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user; +REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user; +GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user; + +-- Lack of SELECT on FKTABLE forces validation to check each row. +REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user; +SET ROLE regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails +GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user; + +-- Per-row validation also requires UPDATE privilege. +REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails +-- UPDATE on the unrelated column is enough. +GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user; +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; +RESET ROLE; + DROP TABLE FKTABLE; DROP TABLE PKTABLE; -- 2.47.3