On Tue, Apr 18, 2023 at 09:51:30AM +0800, Legs Mansion wrote: > Recently, I ran into a problem, InvokeObjectPostAlterHook was > implemented for sepgsql, sepgsql use it to determine whether to > check permissions during certain operations. But > InvokeObjectPostAlterHook doesn't handle all of the alter's > behavior, at least the table is not controlled. e.g., ALTER > TABLE... ENABLE/DISABLE ROW LEVEL SECURITY，ALTER TABLE ... DISABLE > TRIGGER, GRANT and REVOKE and so on. > Whether InvokeObjectPostAlterHook is not fully controlled? it's > a bug?
Yes, tablecmds.c has some holes and these are added when there is a ask for it, as far as I recall. In some cases, these locations can be tricky to add, so usually they require an independent analysis. For example, EnableDisableTrigger() has one AOT for the trigger itself, but not for the relation changed in tablecmds.c, as you say, anyway we should be careful with cross-dependencies. Note that 90efa2f has made the tests for OATs much easier, and there is no need to rely only on sepgsql for that. (Even if test_oat_hooks has been having some stability issues with namespace lookups because of the position on the namespace search hook.) Also, the additions of InvokeObjectPostAlterHook() are historically conservative because they create behavior changes in stable branches, meaning no backpatch. See a995b37 or 7b56584 as past examples, for example. Note that the development of PostgreSQL 16 has just finished, so now may not be the best moment to add these extra AOT calls, but these could be added in 17~ for sure at the beginning of July once the next development cycle begins. Attached would be what I think would be required to add OATs for RLS, triggers and rules, for example. There are much more of these at quick glance, still that's one step in providing more checks. Perhaps you'd like to expand this patch with more ALTER TABLE subcommands covered? -- Michael
From bb693411864399f41d93819adf8caa0f073a9a2f Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 18 Apr 2023 13:32:42 +0900
Subject: [PATCH] Add OAT calls for more flavors of ALTER TABLE
---
src/backend/commands/tablecmds.c | 12 ++
src/test/modules/test_oat_hooks/Makefile | 2 +-
.../test_oat_hooks/expected/alter_table.out | 165 ++++++++++++++++++
src/test/modules/test_oat_hooks/meson.build | 1 +
.../test_oat_hooks/sql/alter_table.sql | 48 +++++
5 files changed, 227 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/test_oat_hooks/expected/alter_table.out
create mode 100644 src/test/modules/test_oat_hooks/sql/alter_table.sql
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 343fe61115..5b6f8f0376 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14843,6 +14843,9 @@ ATExecEnableDisableTrigger(Relation rel, const char *trigname,
EnableDisableTrigger(rel, trigname, InvalidOid,
fires_when, skip_system, recurse,
lockmode);
+
+ InvokeObjectPostAlterHook(RelationRelationId,
+ RelationGetRelid(rel), 0);
}
/*
@@ -14855,6 +14858,9 @@ ATExecEnableDisableRule(Relation rel, const char *rulename,
char fires_when, LOCKMODE lockmode)
{
EnableDisableRule(rel, rulename, fires_when);
+
+ InvokeObjectPostAlterHook(RelationRelationId,
+ RelationGetRelid(rel), 0);
}
/*
@@ -16134,6 +16140,9 @@ ATExecSetRowSecurity(Relation rel, bool rls)
((Form_pg_class) GETSTRUCT(tuple))->relrowsecurity = rls;
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+ InvokeObjectPostAlterHook(RelationRelationId,
+ RelationGetRelid(rel), 0);
+
table_close(pg_class, RowExclusiveLock);
heap_freetuple(tuple);
}
@@ -16160,6 +16169,9 @@ ATExecForceNoForceRowSecurity(Relation rel, bool force_rls)
((Form_pg_class) GETSTRUCT(tuple))->relforcerowsecurity = force_rls;
CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+ InvokeObjectPostAlterHook(RelationRelationId,
+ RelationGetRelid(rel), 0);
+
table_close(pg_class, RowExclusiveLock);
heap_freetuple(tuple);
}
diff --git a/src/test/modules/test_oat_hooks/Makefile b/src/test/modules/test_oat_hooks/Makefile
index 2b5d2687f8..dcaf3a7727 100644
--- a/src/test/modules/test_oat_hooks/Makefile
+++ b/src/test/modules/test_oat_hooks/Makefile
@@ -6,7 +6,7 @@ OBJS = \
test_oat_hooks.o
PGFILEDESC = "test_oat_hooks - example use of object access hooks"
-REGRESS = test_oat_hooks
+REGRESS = test_oat_hooks alter_table
# disable installcheck for now
NO_INSTALLCHECK = 1
diff --git a/src/test/modules/test_oat_hooks/expected/alter_table.out b/src/test/modules/test_oat_hooks/expected/alter_table.out
new file mode 100644
index 0000000000..eac5071977
--- /dev/null
+++ b/src/test/modules/test_oat_hooks/expected/alter_table.out
@@ -0,0 +1,165 @@
+--
+-- OAT checks for ALTER TABLE
+--
+-- This test script fails if debug_discard_caches is enabled, because cache
+-- flushes cause extra calls of the OAT hook in recomputeNamespacePath,
+-- resulting in more NOTICE messages than are in the expected output.
+SET debug_discard_caches = 0;
+LOAD 'test_oat_hooks';
+SET test_oat_hooks.audit = true;
+NOTICE: in object_access_hook_str: superuser attempting alter (subId=0x1000, set) [test_oat_hooks.audit]
+NOTICE: in object_access_hook_str: superuser finished alter (subId=0x1000, set) [test_oat_hooks.audit]
+NOTICE: in process utility: superuser finished SET
+CREATE SCHEMA test_oat_schema;
+NOTICE: in process utility: superuser attempting CREATE SCHEMA
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE: in process utility: superuser finished CREATE SCHEMA
+CREATE TABLE test_oat_schema.test_oat_tab (c1 int, c2 text);
+NOTICE: in process utility: superuser attempting CREATE TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+LINE 1: CREATE TABLE test_oat_schema.test_oat_tab (c1 int, c2 text);
+ ^
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+LINE 1: CREATE TABLE test_oat_schema.test_oat_tab (c1 int, c2 text);
+ ^
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in object access: superuser attempting create (subId=0x0) [internal]
+NOTICE: in object access: superuser finished create (subId=0x0) [internal]
+NOTICE: in object access: superuser attempting create (subId=0x0) [internal]
+NOTICE: in object access: superuser finished create (subId=0x0) [internal]
+NOTICE: in process utility: superuser finished CREATE TABLE
+CREATE RULE test_oat_notify AS
+ ON UPDATE TO test_oat_schema.test_oat_tab
+ DO ALSO NOTIFY test_oat_tab;
+NOTICE: in process utility: superuser attempting CREATE RULE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in process utility: superuser finished CREATE RULE
+CREATE FUNCTION test_oat_schema.test_trigger()
+RETURNS trigger
+LANGUAGE plpgsql
+AS $$
+BEGIN
+ IF TG_OP = 'DELETE'
+ THEN
+ RETURN OLD;
+ ELSE
+ RETURN NEW;
+ END IF;
+END; $$;
+NOTICE: in process utility: superuser attempting CREATE FUNCTION
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in process utility: superuser finished CREATE FUNCTION
+CREATE TRIGGER test_oat_trigger BEFORE INSERT ON test_oat_schema.test_oat_tab
+ FOR EACH STATEMENT EXECUTE FUNCTION test_oat_schema.test_trigger();
+NOTICE: in process utility: superuser attempting CREATE TRIGGER
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
+NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
+NOTICE: in process utility: superuser finished CREATE TRIGGER
+-- RLS
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE ROW LEVEL SECURITY;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE ROW LEVEL SECURITY;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+ALTER TABLE test_oat_schema.test_oat_tab FORCE ROW LEVEL SECURITY;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+ALTER TABLE test_oat_schema.test_oat_tab NO FORCE ROW LEVEL SECURITY;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+-- Rules
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE RULE test_oat_notify;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE RULE test_oat_notify;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+-- Triggers
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE TRIGGER test_oat_trigger;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE TRIGGER test_oat_trigger;
+NOTICE: in process utility: superuser attempting ALTER TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser attempting alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in object access: superuser finished alter (subId=0x0) [explicit without auxiliary object]
+NOTICE: in process utility: superuser finished ALTER TABLE
+DROP TABLE test_oat_schema.test_oat_tab;
+NOTICE: in process utility: superuser attempting DROP TABLE
+NOTICE: in object access: superuser attempting namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser finished namespace search (subId=0x0) [report on violation, allowed]
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in object access: superuser attempting drop (subId=0x0) []
+NOTICE: in object access: superuser finished drop (subId=0x0) []
+NOTICE: in process utility: superuser finished DROP TABLE
diff --git a/src/test/modules/test_oat_hooks/meson.build b/src/test/modules/test_oat_hooks/meson.build
index 9c69a1eaf9..be28eb61cd 100644
--- a/src/test/modules/test_oat_hooks/meson.build
+++ b/src/test/modules/test_oat_hooks/meson.build
@@ -23,6 +23,7 @@ tests += {
'regress': {
'sql': [
'test_oat_hooks',
+ 'alter_table',
],
'regress_args': ['--no-locale', '--encoding=UTF8'],
'runningcheck': false,
diff --git a/src/test/modules/test_oat_hooks/sql/alter_table.sql b/src/test/modules/test_oat_hooks/sql/alter_table.sql
new file mode 100644
index 0000000000..3d308a47c1
--- /dev/null
+++ b/src/test/modules/test_oat_hooks/sql/alter_table.sql
@@ -0,0 +1,48 @@
+--
+-- OAT checks for ALTER TABLE
+--
+
+-- This test script fails if debug_discard_caches is enabled, because cache
+-- flushes cause extra calls of the OAT hook in recomputeNamespacePath,
+-- resulting in more NOTICE messages than are in the expected output.
+SET debug_discard_caches = 0;
+
+LOAD 'test_oat_hooks';
+SET test_oat_hooks.audit = true;
+
+CREATE SCHEMA test_oat_schema;
+CREATE TABLE test_oat_schema.test_oat_tab (c1 int, c2 text);
+CREATE RULE test_oat_notify AS
+ ON UPDATE TO test_oat_schema.test_oat_tab
+ DO ALSO NOTIFY test_oat_tab;
+
+CREATE FUNCTION test_oat_schema.test_trigger()
+RETURNS trigger
+LANGUAGE plpgsql
+AS $$
+BEGIN
+ IF TG_OP = 'DELETE'
+ THEN
+ RETURN OLD;
+ ELSE
+ RETURN NEW;
+ END IF;
+END; $$;
+CREATE TRIGGER test_oat_trigger BEFORE INSERT ON test_oat_schema.test_oat_tab
+ FOR EACH STATEMENT EXECUTE FUNCTION test_oat_schema.test_trigger();
+
+-- RLS
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE ROW LEVEL SECURITY;
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE ROW LEVEL SECURITY;
+ALTER TABLE test_oat_schema.test_oat_tab FORCE ROW LEVEL SECURITY;
+ALTER TABLE test_oat_schema.test_oat_tab NO FORCE ROW LEVEL SECURITY;
+
+-- Rules
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE RULE test_oat_notify;
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE RULE test_oat_notify;
+
+-- Triggers
+ALTER TABLE test_oat_schema.test_oat_tab DISABLE TRIGGER test_oat_trigger;
+ALTER TABLE test_oat_schema.test_oat_tab ENABLE TRIGGER test_oat_trigger;
+
+DROP TABLE test_oat_schema.test_oat_tab;
--
2.40.0
signature.asc
Description: PGP signature
