pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedallow_system_table_mods and DROP RULE
3+ messages / 2 participants
[nested] [flat]
* allow_system_table_mods and DROP RULE
@ 2019-12-18 08:56 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-12-18 15:53 ` Re: allow_system_table_mods and DROP RULE Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Peter Eisentraut @ 2019-12-18 08:56 UTC (permalink / raw)
To: pgsql-hackers
As a curious omission, DROP RULE does not check allow_system_table_mods.
Creating and renaming a rule does, and also creating, renaming, and
dropping a trigger does. The impact of this is probably nil in
practice, but for consistency we should probably add that. The patch is
pretty simple.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0ceb620b17b197b1effa96a46e35b6c03942ca7a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 18 Dec 2019 09:52:10 +0100
Subject: [PATCH] Disallow dropping rules on system tables by default
This was previously not covered by allow_system_table_mods, but now it
is. The impact in practice is probably low, but this makes it
consistent with most other DDL commands.
---
src/backend/rewrite/rewriteRemove.c | 8 ++++++++
.../unsafe_tests/expected/alter_system_table.out | 11 ++++++++++-
.../modules/unsafe_tests/sql/alter_system_table.sql | 10 +++++++++-
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/backend/rewrite/rewriteRemove.c b/src/backend/rewrite/rewriteRemove.c
index c5e2aed58d..bb98b6936a 100644
--- a/src/backend/rewrite/rewriteRemove.c
+++ b/src/backend/rewrite/rewriteRemove.c
@@ -18,6 +18,7 @@
#include "access/htup_details.h"
#include "access/sysattr.h"
#include "access/table.h"
+#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
@@ -28,6 +29,7 @@
#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/lsyscache.h"
+#include "utils/rel.h"
#include "utils/syscache.h"
/*
@@ -72,6 +74,12 @@ RemoveRewriteRuleById(Oid ruleOid)
eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
event_relation = table_open(eventRelationOid, AccessExclusiveLock);
+ if (!allowSystemTableMods && IsSystemRelation(event_relation))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(event_relation))));
+
/*
* Now delete the pg_rewrite tuple for the rule
*/
diff --git a/src/test/modules/unsafe_tests/expected/alter_system_table.out b/src/test/modules/unsafe_tests/expected/alter_system_table.out
index ca7eabe9bb..ecd1505cdc 100644
--- a/src/test/modules/unsafe_tests/expected/alter_system_table.out
+++ b/src/test/modules/unsafe_tests/expected/alter_system_table.out
@@ -81,7 +81,16 @@ CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
ERROR: permission denied: "pg_description" is a system catalog
ALTER RULE r1 ON pg_description RENAME TO r2;
ERROR: permission denied: "pg_description" is a system catalog
---DROP RULE r2 ON pg_description;
+-- now make one to test dropping:
+SET allow_system_table_mods TO on;
+CREATE RULE r2 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+RESET allow_system_table_mods;
+DROP RULE r2 ON pg_description;
+ERROR: permission denied: "pg_description" is a system catalog
+-- cleanup:
+SET allow_system_table_mods TO on;
+DROP RULE r2 ON pg_description;
+RESET allow_system_table_mods;
SET allow_system_table_mods = on;
-- create new table in pg_catalog
BEGIN;
diff --git a/src/test/modules/unsafe_tests/sql/alter_system_table.sql b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
index 44cb3c7148..5663570d31 100644
--- a/src/test/modules/unsafe_tests/sql/alter_system_table.sql
+++ b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
@@ -79,7 +79,15 @@ CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
-- rules
CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
ALTER RULE r1 ON pg_description RENAME TO r2;
---DROP RULE r2 ON pg_description;
+-- now make one to test dropping:
+SET allow_system_table_mods TO on;
+CREATE RULE r2 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+RESET allow_system_table_mods;
+DROP RULE r2 ON pg_description;
+-- cleanup:
+SET allow_system_table_mods TO on;
+DROP RULE r2 ON pg_description;
+RESET allow_system_table_mods;
SET allow_system_table_mods = on;
--
2.24.1
Attachments:
[text/plain] 0001-Disallow-dropping-rules-on-system-tables-by-default.patch (3.7K, ../../ee9df1af-c0d8-7c82-5be7-39ce4e3b0a9d@2ndquadrant.com/2-0001-Disallow-dropping-rules-on-system-tables-by-default.patch)
download | inline diff:
From 0ceb620b17b197b1effa96a46e35b6c03942ca7a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 18 Dec 2019 09:52:10 +0100
Subject: [PATCH] Disallow dropping rules on system tables by default
This was previously not covered by allow_system_table_mods, but now it
is. The impact in practice is probably low, but this makes it
consistent with most other DDL commands.
---
src/backend/rewrite/rewriteRemove.c | 8 ++++++++
.../unsafe_tests/expected/alter_system_table.out | 11 ++++++++++-
.../modules/unsafe_tests/sql/alter_system_table.sql | 10 +++++++++-
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/backend/rewrite/rewriteRemove.c b/src/backend/rewrite/rewriteRemove.c
index c5e2aed58d..bb98b6936a 100644
--- a/src/backend/rewrite/rewriteRemove.c
+++ b/src/backend/rewrite/rewriteRemove.c
@@ -18,6 +18,7 @@
#include "access/htup_details.h"
#include "access/sysattr.h"
#include "access/table.h"
+#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
@@ -28,6 +29,7 @@
#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/lsyscache.h"
+#include "utils/rel.h"
#include "utils/syscache.h"
/*
@@ -72,6 +74,12 @@ RemoveRewriteRuleById(Oid ruleOid)
eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
event_relation = table_open(eventRelationOid, AccessExclusiveLock);
+ if (!allowSystemTableMods && IsSystemRelation(event_relation))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(event_relation))));
+
/*
* Now delete the pg_rewrite tuple for the rule
*/
diff --git a/src/test/modules/unsafe_tests/expected/alter_system_table.out b/src/test/modules/unsafe_tests/expected/alter_system_table.out
index ca7eabe9bb..ecd1505cdc 100644
--- a/src/test/modules/unsafe_tests/expected/alter_system_table.out
+++ b/src/test/modules/unsafe_tests/expected/alter_system_table.out
@@ -81,7 +81,16 @@ CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
ERROR: permission denied: "pg_description" is a system catalog
ALTER RULE r1 ON pg_description RENAME TO r2;
ERROR: permission denied: "pg_description" is a system catalog
---DROP RULE r2 ON pg_description;
+-- now make one to test dropping:
+SET allow_system_table_mods TO on;
+CREATE RULE r2 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+RESET allow_system_table_mods;
+DROP RULE r2 ON pg_description;
+ERROR: permission denied: "pg_description" is a system catalog
+-- cleanup:
+SET allow_system_table_mods TO on;
+DROP RULE r2 ON pg_description;
+RESET allow_system_table_mods;
SET allow_system_table_mods = on;
-- create new table in pg_catalog
BEGIN;
diff --git a/src/test/modules/unsafe_tests/sql/alter_system_table.sql b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
index 44cb3c7148..5663570d31 100644
--- a/src/test/modules/unsafe_tests/sql/alter_system_table.sql
+++ b/src/test/modules/unsafe_tests/sql/alter_system_table.sql
@@ -79,7 +79,15 @@ CREATE TRIGGER t1 BEFORE INSERT ON pg_description EXECUTE FUNCTION tf1();
-- rules
CREATE RULE r1 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
ALTER RULE r1 ON pg_description RENAME TO r2;
---DROP RULE r2 ON pg_description;
+-- now make one to test dropping:
+SET allow_system_table_mods TO on;
+CREATE RULE r2 AS ON INSERT TO pg_description DO INSTEAD NOTHING;
+RESET allow_system_table_mods;
+DROP RULE r2 ON pg_description;
+-- cleanup:
+SET allow_system_table_mods TO on;
+DROP RULE r2 ON pg_description;
+RESET allow_system_table_mods;
SET allow_system_table_mods = on;
--
2.24.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: allow_system_table_mods and DROP RULE
2019-12-18 08:56 allow_system_table_mods and DROP RULE Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
@ 2019-12-18 15:53 ` Robert Haas <robertmhaas@gmail.com>
2019-12-20 07:33 ` Re: allow_system_table_mods and DROP RULE Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 1 reply; 3+ messages in thread
From: Robert Haas @ 2019-12-18 15:53 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
On Wed, Dec 18, 2019 at 3:56 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> As a curious omission, DROP RULE does not check allow_system_table_mods.
> Creating and renaming a rule does, and also creating, renaming, and
> dropping a trigger does. The impact of this is probably nil in
> practice, but for consistency we should probably add that. The patch is
> pretty simple.
+1. LGTM.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: allow_system_table_mods and DROP RULE
2019-12-18 08:56 allow_system_table_mods and DROP RULE Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-12-18 15:53 ` Re: allow_system_table_mods and DROP RULE Robert Haas <robertmhaas@gmail.com>
@ 2019-12-20 07:33 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 0 replies; 3+ messages in thread
From: Peter Eisentraut @ 2019-12-20 07:33 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
On 2019-12-18 16:53, Robert Haas wrote:
> On Wed, Dec 18, 2019 at 3:56 AM Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> As a curious omission, DROP RULE does not check allow_system_table_mods.
>> Creating and renaming a rule does, and also creating, renaming, and
>> dropping a trigger does. The impact of this is probably nil in
>> practice, but for consistency we should probably add that. The patch is
>> pretty simple.
>
> +1. LGTM.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2019-12-20 07:33 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 08:56 allow_system_table_mods and DROP RULE Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-12-18 15:53 ` Robert Haas <robertmhaas@gmail.com>
2019-12-20 07:33 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
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