From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Amit Langote <amitlangote09@gmail.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: table partitioning and access privileges
Date: Mon, 27 Jan 2020 11:19:04 +0900
Message-ID: <0ff3cecc-20f4-4ee4-fbfc-601a6a5a9eab@oss.nttdata.com> (raw)
In-Reply-To: <3546b691-b744-12c9-fd2f-7dc31c3c030e@oss.nttdata.com>
References: <CAHGQGwFHdSvifhJE+-GSNqUHSfbiKxaeQQ7HGcYz6SC2n_oDcg@mail.gmail.com>
<6218.1577388354@sss.pgh.pa.us>
<CA+HiwqGCQLKizA+Voyw3GjRv0q8t9adS2YFGmw6GzAbRb+yazg@mail.gmail.com>
<CAHGQGwE+GauyG7POtRfRwwthAGwTjPQYdFHR97+LzA4RHGnJxA@mail.gmail.com>
<CA+HiwqF55uZK6znXq7=qP-LS5HQkcjGEvCHxGUjYhJGq=z9nDg@mail.gmail.com>
<3546b691-b744-12c9-fd2f-7dc31c3c030e@oss.nttdata.com>
On 2020/01/23 22:14, Fujii Masao wrote:
>
>
> On 2020/01/22 16:54, Amit Langote wrote:
>> Fujii-san,
>>
>> Thanks for taking a look.
>>
>> On Fri, Jan 10, 2020 at 10:29 AM Fujii Masao <masao.fujii@gmail.com>
>> wrote:
>>> On Tue, Jan 7, 2020 at 5:15 PM Amit Langote <amitlangote09@gmail.com>
>>> wrote:
>>>> I tend to agree that TRUNCATE's permission model for inheritance
>>>> should be consistent with that for the other commands. How about the
>>>> attached patch toward that end?
>>>
>>> Thanks for the patch!
>>>
>>> The patch basically looks good to me.
>>>
>>> +GRANT SELECT (f1, fz), UPDATE (fz) ON atestc TO regress_priv_user2;
>>> +REVOKE TRUNCATE ON atestc FROM regress_priv_user2;
>>>
>>> These seem not to be necessary for the test.
>>
>> You're right. Removed in the attached updated patch.
>
> Thanks for updating the patch! Barring any objection,
> I will commit this fix and backport it to all supported versions.
Attached are the back-port versions of the patches.
- patch for master and v12
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg12-13.patch
- patch for v11
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg11.patch
- patch for v10
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg10.patch
- patch for v9.6
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg96.patch
- patch for v9.5 and v9.4
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg94-95.patch
The patch for master branch separates truncate_check_activity() into two
functions, but in v11 or before, truncate_check_activity() didn't exist and
its code was in truncate_check_rel(). So I had to write the back-port
version
of the patch for the previous versions and separate truncate_check_rel()
into three functions, i.e., truncate_check_rel(),
truncate_check_activity() and
truncate_check_perms().
Also the names of users that the regression test for privileges use were
different between PostgreSQL versions. This is another reason
why I had to write several back-port versions of the patches.
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
Attachments:download all (5 files, .tar.gz)0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg10.patch (text/plain, 6.0K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 1a005760d8..204d25aeb3 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -291,7 +291,9 @@ struct DropRelationCallbackState
#define child_dependency_type(child_is_partition) \
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
-static void truncate_check_rel(Relation rel);+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);+static void truncate_check_activity(Relation rel);
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
bool is_partition, List **supOids, List **supconstr,
int *supOidCount);
@@ -1242,7 +1244,11 @@ ExecuteTruncate(TruncateStmt *stmt)
heap_close(rel, lockmode);
continue;
}
- truncate_check_rel(rel);++ truncate_check_rel(myrelid, rel->rd_rel);+ truncate_check_perms(myrelid, rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
@@ -1278,7 +1284,15 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
- truncate_check_rel(rel);+ /*+ * Inherited TRUNCATE commands perform access+ * permission checks on the parent table only.+ * So we skip checking the children's permissions+ * and don't call truncate_check_perms() here.+ */+ truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
}
@@ -1317,7 +1331,9 @@ ExecuteTruncate(TruncateStmt *stmt)
ereport(NOTICE,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
- truncate_check_rel(rel);+ truncate_check_rel(relid, rel->rd_rel);+ truncate_check_perms(relid, rel->rd_rel);+ truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
}
@@ -1530,35 +1546,50 @@ ExecuteTruncate(TruncateStmt *stmt)
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
*/
static void
-truncate_check_rel(Relation rel)+truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;+ char *relname = NameStr(reltuple->relname);
/*
* Only allow truncate on regular tables and partitioned tables (although,
* the latter are only being included here for the following checks; no
* physical truncation will occur in their case.)
*/
- if (rel->rd_rel->relkind != RELKIND_RELATION &&- rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)+ if (reltuple->relkind != RELKIND_RELATION &&+ reltuple->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",- RelationGetRelationName(rel))));-- /* Permissions checks */- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),- ACL_TRUNCATE);- if (aclresult != ACLCHECK_OK)- aclcheck_error(aclresult, ACL_KIND_CLASS,- RelationGetRelationName(rel));+ errmsg("\"%s\" is not a table", relname)));- if (!allowSystemTableMods && IsSystemRelation(rel))+ if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
- RelationGetRelationName(rel))));+ relname)));+}++/*+ * Check that current user has the permission to truncate given relation.+ */+static void+truncate_check_perms(Oid relid, Form_pg_class reltuple)+{+ char *relname = NameStr(reltuple->relname);+ AclResult aclresult;+ /* Permissions checks */+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);+ if (aclresult != ACLCHECK_OK)+ aclcheck_error(aclresult, ACL_KIND_CLASS, relname);+}++/*+ * Set of extra sanity checks to check if a given relation is safe to+ * truncate.+ */+static void+truncate_check_activity(Relation rel)+{
/*
* Don't allow truncate on temp tables of other backends ... their local
* buffer manager is not going to cope.
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.outindex dacc984505..e242137549 100644--- a/src/test/regress/expected/privileges.out+++ b/src/test/regress/expected/privileges.out@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
-----
(0 rows)
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_user1;+REVOKE ALL ON atestc FROM regress_user2;+GRANT ALL ON atestp1 TO regress_user2;+SET SESSION AUTHORIZATION regress_user2;+SELECT f2 FROM atestp1; -- ok+ f2 +----+(0 rows)++SELECT f2 FROM atestc; -- fail+ERROR: permission denied for relation atestc+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+ERROR: permission denied for relation atestc+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+ERROR: permission denied for relation atestc+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+ERROR: permission denied for relation atestc
-- privileges on functions, languages
-- switch to superuser
\c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sqlindex 4263315a2d..973dde8143 100644--- a/src/test/regress/sql/privileges.sql+++ b/src/test/regress/sql/privileges.sql@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
SELECT atestp2 FROM atestp2; -- ok
SELECT oid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_user1;+REVOKE ALL ON atestc FROM regress_user2;+GRANT ALL ON atestp1 TO regress_user2;+SET SESSION AUTHORIZATION regress_user2;+SELECT f2 FROM atestp1; -- ok+SELECT f2 FROM atestc; -- fail+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+
-- privileges on functions, languages
-- switch to superuser
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg11.patch (text/plain, 6.3K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 38386fb9cf..9b0dc3fd10 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -300,7 +300,9 @@ struct DropRelationCallbackState
#define child_dependency_type(child_is_partition) \
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
-static void truncate_check_rel(Relation rel);+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);+static void truncate_check_activity(Relation rel);
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
bool is_partition, List **supOids, List **supconstr,
int *supOidCount);
@@ -1381,7 +1383,11 @@ ExecuteTruncate(TruncateStmt *stmt)
heap_close(rel, lockmode);
continue;
}
- truncate_check_rel(rel);++ truncate_check_rel(myrelid, rel->rd_rel);+ truncate_check_perms(myrelid, rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
/* Log this relation only if needed for logical decoding */
@@ -1420,7 +1426,15 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
- truncate_check_rel(rel);+ /*+ * Inherited TRUNCATE commands perform access+ * permission checks on the parent table only.+ * So we skip checking the children's permissions+ * and don't call truncate_check_perms() here.+ */+ truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
/* Log this relation only if needed for logical decoding */
@@ -1503,7 +1517,9 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
ereport(NOTICE,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
- truncate_check_rel(rel);+ truncate_check_rel(relid, rel->rd_rel);+ truncate_check_perms(relid, rel->rd_rel);+ truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
/* Log this relation only if needed for logical decoding */
@@ -1759,35 +1775,51 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
*/
static void
-truncate_check_rel(Relation rel)+truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;+ char *relname = NameStr(reltuple->relname);
/*
* Only allow truncate on regular tables and partitioned tables (although,
* the latter are only being included here for the following checks; no
* physical truncation will occur in their case.)
*/
- if (rel->rd_rel->relkind != RELKIND_RELATION &&- rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)+ if (reltuple->relkind != RELKIND_RELATION &&+ reltuple->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",- RelationGetRelationName(rel))));-- /* Permissions checks */- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),- ACL_TRUNCATE);- if (aclresult != ACLCHECK_OK)- aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),- RelationGetRelationName(rel));+ errmsg("\"%s\" is not a table", relname)));- if (!allowSystemTableMods && IsSystemRelation(rel))+ if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
- RelationGetRelationName(rel))));+ relname)));+}++/*+ * Check that current user has the permission to truncate given relation.+ */+static void+truncate_check_perms(Oid relid, Form_pg_class reltuple)+{+ char *relname = NameStr(reltuple->relname);+ AclResult aclresult;+ /* Permissions checks */+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);+ if (aclresult != ACLCHECK_OK)+ aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),+ relname);+}++/*+ * Set of extra sanity checks to check if a given relation is safe to+ * truncate.+ */+static void+truncate_check_activity(Relation rel)+{
/*
* Don't allow truncate on temp tables of other backends ... their local
* buffer manager is not going to cope.
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.outindex a8346e1717..6221601c5d 100644--- a/src/test/regress/expected/privileges.out+++ b/src/test/regress/expected/privileges.out@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
-----
(0 rows)
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_priv_user1;+REVOKE ALL ON atestc FROM regress_priv_user2;+GRANT ALL ON atestp1 TO regress_priv_user2;+SET SESSION AUTHORIZATION regress_priv_user2;+SELECT f2 FROM atestp1; -- ok+ f2 +----+(0 rows)++SELECT f2 FROM atestc; -- fail+ERROR: permission denied for table atestc+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+ERROR: permission denied for table atestc+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+ERROR: permission denied for table atestc+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+ERROR: permission denied for table atestc
-- privileges on functions, languages
-- switch to superuser
\c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sqlindex c1e42d1be2..c25157b32d 100644--- a/src/test/regress/sql/privileges.sql+++ b/src/test/regress/sql/privileges.sql@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
SELECT atestp2 FROM atestp2; -- ok
SELECT oid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_priv_user1;+REVOKE ALL ON atestc FROM regress_priv_user2;+GRANT ALL ON atestp1 TO regress_priv_user2;+SET SESSION AUTHORIZATION regress_priv_user2;+SELECT f2 FROM atestp1; -- ok+SELECT f2 FROM atestc; -- fail+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+
-- privileges on functions, languages
-- switch to superuser
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg12-13.patch (text/plain, 5.0K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 7c23968f2d..9afdbc5d53 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -304,6 +304,7 @@ struct DropRelationCallbackState
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
static void truncate_check_rel(Oid relid, Form_pg_class reltuple);
+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);
static void truncate_check_activity(Relation rel);
static void RangeVarCallbackForTruncate(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
@@ -1615,6 +1616,12 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
+ /*+ * Inherited TRUNCATE commands perform access+ * permission checks on the parent table only.+ * So we skip checking the children's permissions+ * and don't call truncate_check_perms() here.+ */
truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);
truncate_check_activity(rel);
@@ -1701,6 +1708,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
truncate_check_rel(relid, rel->rd_rel);
+ truncate_check_perms(relid, rel->rd_rel);
truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
@@ -1951,7 +1959,6 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
static void
truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;
char *relname = NameStr(reltuple->relname);
/*
@@ -1965,12 +1972,6 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a table", relname)));
- /* Permissions checks */- aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);- if (aclresult != ACLCHECK_OK)- aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),- relname);-
if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1980,6 +1981,22 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
InvokeObjectTruncateHook(relid);
}
+/*+ * Check that current user has the permission to truncate given relation.+ */+static void+truncate_check_perms(Oid relid, Form_pg_class reltuple)+{+ char *relname = NameStr(reltuple->relname);+ AclResult aclresult;++ /* Permissions checks */+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);+ if (aclresult != ACLCHECK_OK)+ aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind),+ relname);+}+
/*
* Set of extra sanity checks to check if a given relation is safe to
* truncate. This is split with truncate_check_rel() as
@@ -15287,6 +15304,7 @@ RangeVarCallbackForTruncate(const RangeVar *relation,
elog(ERROR, "cache lookup failed for relation %u", relId);
truncate_check_rel(relId, (Form_pg_class) GETSTRUCT(tuple));
+ truncate_check_perms(relId, (Form_pg_class) GETSTRUCT(tuple));
ReleaseSysCache(tuple);
}
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.outindex 0ddbd8e89f..bc8e198097 100644--- a/src/test/regress/expected/privileges.out+++ b/src/test/regress/expected/privileges.out@@ -695,6 +695,27 @@ SELECT tableoid FROM atestp2; -- ok
----------
(0 rows)
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_priv_user1;+REVOKE ALL ON atestc FROM regress_priv_user2;+GRANT ALL ON atestp1 TO regress_priv_user2;+SET SESSION AUTHORIZATION regress_priv_user2;+SELECT f2 FROM atestp1; -- ok+ f2 +----+(0 rows)++SELECT f2 FROM atestc; -- fail+ERROR: permission denied for table atestc+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+ERROR: permission denied for table atestc+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+ERROR: permission denied for table atestc+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+ERROR: permission denied for table atestc
-- privileges on functions, languages
-- switch to superuser
\c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sqlindex f15d1f3773..dfe2603fe2 100644--- a/src/test/regress/sql/privileges.sql+++ b/src/test/regress/sql/privileges.sql@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
SELECT atestp2 FROM atestp2; -- ok
SELECT tableoid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_priv_user1;+REVOKE ALL ON atestc FROM regress_priv_user2;+GRANT ALL ON atestp1 TO regress_priv_user2;+SET SESSION AUTHORIZATION regress_priv_user2;+SELECT f2 FROM atestp1; -- ok+SELECT f2 FROM atestc; -- fail+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+
-- privileges on functions, languages
-- switch to superuser
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg94-95.patch (text/plain, 5.7K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 7fcc3db14c..5f46aaa9da 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -269,7 +269,9 @@ struct DropRelationCallbackState
#define ATT_COMPOSITE_TYPE 0x0010
#define ATT_FOREIGN_TABLE 0x0020
-static void truncate_check_rel(Relation rel);+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);+static void truncate_check_activity(Relation rel);
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
List **supOids, List **supconstr, int *supOidCount);
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
@@ -1046,7 +1048,11 @@ ExecuteTruncate(TruncateStmt *stmt)
heap_close(rel, lockmode);
continue;
}
- truncate_check_rel(rel);++ truncate_check_rel(myrelid, rel->rd_rel);+ truncate_check_perms(myrelid, rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
@@ -1082,7 +1088,15 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
- truncate_check_rel(rel);+ /*+ * Inherited TRUNCATE commands perform access+ * permission checks on the parent table only.+ * So we skip checking the children's permissions+ * and don't call truncate_check_perms() here.+ */+ truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
}
@@ -1116,7 +1130,9 @@ ExecuteTruncate(TruncateStmt *stmt)
ereport(NOTICE,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
- truncate_check_rel(rel);+ truncate_check_rel(relid, rel->rd_rel);+ truncate_check_perms(relid, rel->rd_rel);+ truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
}
@@ -1324,30 +1340,45 @@ ExecuteTruncate(TruncateStmt *stmt)
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
*/
static void
-truncate_check_rel(Relation rel)+truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;+ char *relname = NameStr(reltuple->relname);
/* Only allow truncate on regular tables */
- if (rel->rd_rel->relkind != RELKIND_RELATION)+ if (reltuple->relkind != RELKIND_RELATION)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",- RelationGetRelationName(rel))));+ errmsg("\"%s\" is not a table", relname)));- /* Permissions checks */- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),- ACL_TRUNCATE);- if (aclresult != ACLCHECK_OK)- aclcheck_error(aclresult, ACL_KIND_CLASS,- RelationGetRelationName(rel));-- if (!allowSystemTableMods && IsSystemRelation(rel))+ if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
- RelationGetRelationName(rel))));+ relname)));+}+/*+ * Check that current user has the permission to truncate given relation.+ */+static void+truncate_check_perms(Oid relid, Form_pg_class reltuple)+{+ char *relname = NameStr(reltuple->relname);+ AclResult aclresult;++ /* Permissions checks */+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);+ if (aclresult != ACLCHECK_OK)+ aclcheck_error(aclresult, ACL_KIND_CLASS, relname);+}++/*+ * Set of extra sanity checks to check if a given relation is safe to+ * truncate.+ */+static void+truncate_check_activity(Relation rel)+{
/*
* Don't allow truncate on temp tables of other backends ... their local
* buffer manager is not going to cope.
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.outindex 2d92698c4d..41645ad6cb 100644--- a/src/test/regress/expected/privileges.out+++ b/src/test/regress/expected/privileges.out@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
-----
(0 rows)
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regressuser1;+REVOKE ALL ON atestc FROM regressuser2;+GRANT ALL ON atestp1 TO regressuser2;+SET SESSION AUTHORIZATION regressuser2;+SELECT f2 FROM atestp1; -- ok+ f2 +----+(0 rows)++SELECT f2 FROM atestc; -- fail+ERROR: permission denied for relation atestc+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+ERROR: permission denied for relation atestc+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+ERROR: permission denied for relation atestc+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+ERROR: permission denied for relation atestc
-- privileges on functions, languages
-- switch to superuser
\c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sqlindex 3ad2e39930..33955d559c 100644--- a/src/test/regress/sql/privileges.sql+++ b/src/test/regress/sql/privileges.sql@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
SELECT atestp2 FROM atestp2; -- ok
SELECT oid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regressuser1;+REVOKE ALL ON atestc FROM regressuser2;+GRANT ALL ON atestp1 TO regressuser2;+SET SESSION AUTHORIZATION regressuser2;+SELECT f2 FROM atestp1; -- ok+SELECT f2 FROM atestc; -- fail+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+
-- privileges on functions, languages
-- switch to superuser
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg96.patch (text/plain, 5.7K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 9c6ca7ff9c..31bc431d9d 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -271,7 +271,9 @@ struct DropRelationCallbackState
#define ATT_COMPOSITE_TYPE 0x0010
#define ATT_FOREIGN_TABLE 0x0020
-static void truncate_check_rel(Relation rel);+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);+static void truncate_check_activity(Relation rel);
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
List **supOids, List **supconstr, int *supOidCount);
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
@@ -1050,7 +1052,11 @@ ExecuteTruncate(TruncateStmt *stmt)
heap_close(rel, lockmode);
continue;
}
- truncate_check_rel(rel);++ truncate_check_rel(myrelid, rel->rd_rel);+ truncate_check_perms(myrelid, rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);
@@ -1086,7 +1092,15 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
- truncate_check_rel(rel);+ /*+ * Inherited TRUNCATE commands perform access+ * permission checks on the parent table only.+ * So we skip checking the children's permissions+ * and don't call truncate_check_perms() here.+ */+ truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);+ truncate_check_activity(rel);+
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
}
@@ -1120,7 +1134,9 @@ ExecuteTruncate(TruncateStmt *stmt)
ereport(NOTICE,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
- truncate_check_rel(rel);+ truncate_check_rel(relid, rel->rd_rel);+ truncate_check_perms(relid, rel->rd_rel);+ truncate_check_activity(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, relid);
}
@@ -1328,30 +1344,45 @@ ExecuteTruncate(TruncateStmt *stmt)
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
*/
static void
-truncate_check_rel(Relation rel)+truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;+ char *relname = NameStr(reltuple->relname);
/* Only allow truncate on regular tables */
- if (rel->rd_rel->relkind != RELKIND_RELATION)+ if (reltuple->relkind != RELKIND_RELATION)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",- RelationGetRelationName(rel))));+ errmsg("\"%s\" is not a table", relname)));- /* Permissions checks */- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),- ACL_TRUNCATE);- if (aclresult != ACLCHECK_OK)- aclcheck_error(aclresult, ACL_KIND_CLASS,- RelationGetRelationName(rel));-- if (!allowSystemTableMods && IsSystemRelation(rel))+ if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
- RelationGetRelationName(rel))));+ relname)));+}+/*+ * Check that current user has the permission to truncate given relation.+ */+static void+truncate_check_perms(Oid relid, Form_pg_class reltuple)+{+ char *relname = NameStr(reltuple->relname);+ AclResult aclresult;++ /* Permissions checks */+ aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);+ if (aclresult != ACLCHECK_OK)+ aclcheck_error(aclresult, ACL_KIND_CLASS, relname);+}++/*+ * Set of extra sanity checks to check if a given relation is safe to+ * truncate.+ */+static void+truncate_check_activity(Relation rel)+{
/*
* Don't allow truncate on temp tables of other backends ... their local
* buffer manager is not going to cope.
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.outindex b46af7c5e6..34f1e74f6a 100644--- a/src/test/regress/expected/privileges.out+++ b/src/test/regress/expected/privileges.out@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
-----
(0 rows)
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_user1;+REVOKE ALL ON atestc FROM regress_user2;+GRANT ALL ON atestp1 TO regress_user2;+SET SESSION AUTHORIZATION regress_user2;+SELECT f2 FROM atestp1; -- ok+ f2 +----+(0 rows)++SELECT f2 FROM atestc; -- fail+ERROR: permission denied for relation atestc+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+ERROR: permission denied for relation atestc+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+ERROR: permission denied for relation atestc+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+ERROR: permission denied for relation atestc
-- privileges on functions, languages
-- switch to superuser
\c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sqlindex c7d7347091..c802f190d5 100644--- a/src/test/regress/sql/privileges.sql+++ b/src/test/regress/sql/privileges.sql@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
SELECT atestp2 FROM atestp2; -- ok
SELECT oid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent+SET SESSION AUTHORIZATION regress_user1;+REVOKE ALL ON atestc FROM regress_user2;+GRANT ALL ON atestp1 TO regress_user2;+SET SESSION AUTHORIZATION regress_user2;+SELECT f2 FROM atestp1; -- ok+SELECT f2 FROM atestc; -- fail+DELETE FROM atestp1; -- ok+DELETE FROM atestc; -- fail+UPDATE atestp1 SET f1 = 1; -- ok+UPDATE atestc SET f1 = 1; -- fail+TRUNCATE atestp1; -- ok+TRUNCATE atestc; -- fail+
-- privileges on functions, languages
-- switch to superuser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, amitlangote09@gmail.com, masao.fujii@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: table partitioning and access privileges
In-Reply-To: <0ff3cecc-20f4-4ee4-fbfc-601a6a5a9eab@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox