[tarantool-patches] Re: [PATCH v2 2/9] box: fix _trigger and _ck_constraint access check
Kirill Shcherbatov
kshcherbatov at tarantool.org
Tue Mar 26 13:59:21 MSK 2019
> Why did you check only alter privilege? On drop we should check
> drop privilege, on alter - alter privilege and on creation - creation
> privilege.
Hi! Here I check that we have alter privilege on corresponding "space" because replace
in _trigger, _fk_constraint (and in further patches _ck_constaint) cause space *
object change.
This is very similar with access checks for _index space.
As for _trigger, _fk_constraint regular access check everything is ok already.
Look:
tarantool> box.schema.user.grant('guest', 'write', 'space', '_trigger')
---
...
tarantool> c.space._trigger:replace({'TR1', 512, {sql='CREATE TRIGGER tr1 AFTER DELETE ON t5 BEGIN DELETE FROM t5; END;'}})
---
- error: Alter access to space 'T5' is denied for user 'guest'
...
> Please, add tests on these cases.
Done.
=========================================================================
Due to the fact that the insert in _fk_constraint and _trigger
leads to a change in object space * need to check the right to
change corresponding space before creating triggers or foreign
key.
---
src/box/alter.cc | 17 +++++++++++++++++
test/box/net.box.result | 32 ++++++++++++++++++++++++++++++++
test/box/net.box.test.lua | 12 ++++++++++++
3 files changed, 61 insertions(+)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 080a72b9f..fb668aa4c 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3514,6 +3514,11 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
uint32_t space_id =
tuple_field_u32_xc(old_tuple,
BOX_TRIGGER_FIELD_SPACE_ID);
+ struct space *space = space_by_id(space_id);
+ assert(space != NULL);
+ access_check_ddl(space->def->name, space->def->id,
+ space->def->uid, SC_SPACE, PRIV_A);
+
char *trigger_name =
(char *)region_alloc_xc(&fiber()->gc,
trigger_name_len + 1);
@@ -3567,6 +3572,10 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
"trigger space_id does not match the value "
"resolved on AST building from SQL");
}
+ struct space *space = space_by_id(space_id);
+ assert(space != NULL);
+ access_check_ddl(space->def->name, space->def->id,
+ space->def->uid, SC_SPACE, PRIV_A);
struct sql_trigger *old_trigger;
if (sql_trigger_replace(trigger_name,
@@ -3897,6 +3906,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
fk_def->name,
"referencing space can't be VIEW");
}
+ access_check_ddl(child_space->def->name, child_space->def->id,
+ child_space->def->uid, SC_SPACE, PRIV_A);
struct space *parent_space =
space_cache_find_xc(fk_def->parent_id);
if (parent_space->def->opts.is_view) {
@@ -3904,6 +3915,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
fk_def->name,
"referenced space can't be VIEW");
}
+ access_check_ddl(parent_space->def->name, parent_space->def->id,
+ parent_space->def->uid, SC_SPACE, PRIV_A);
/*
* FIXME: until SQL triggers are completely
* integrated into server (i.e. we are able to
@@ -4031,6 +4044,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
space_cache_find_xc(fk_def->child_id);
struct space *parent_space =
space_cache_find_xc(fk_def->parent_id);
+ access_check_ddl(child_space->def->name, child_space->def->id,
+ child_space->def->uid, SC_SPACE, PRIV_A);
+ access_check_ddl(parent_space->def->name, parent_space->def->id,
+ parent_space->def->uid, SC_SPACE, PRIV_A);
struct fk_constraint *old_fk=
fk_constraint_remove(&child_space->child_fk_constraint,
fk_def->name);
diff --git a/test/box/net.box.result b/test/box/net.box.result
index aecaf9436..f488bc030 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3526,3 +3526,35 @@ s:drop()
box.cfg{readahead = readahead}
---
...
+-- Test alter privilege for space that is modified by insertion in
+-- _trigger, _ck_constraint space.
+box.sql.execute("CREATE TABLE t5(x INT primary key, y INT, CHECK( x + y < 2 ));")
+---
+...
+box.sql.execute("CREATE TRIGGER tr1 AFTER DELETE ON t5 BEGIN DELETE FROM t5; END;")
+---
+...
+box.schema.user.grant('guest','create,read,write','universe')
+---
+...
+c = net.connect(box.cfg.listen)
+---
+...
+c.space._trigger:replace({'TR1', box.space.T5.id, {sql='CREATE TRIGGER tr1 AFTER DELETE ON t5 BEGIN DELETE FROM t5; END;'}})
+---
+- error: Alter access to space 'T5' is denied for user 'guest'
+...
+c:execute('CREATE TABLE t6(x INT PRIMARY KEY REFERENCES t5 ON DELETE RESTRICT);')
+---
+- error: 'Failed to execute SQL statement: Alter access to space ''T5'' is denied
+ for user ''guest'''
+...
+c:close()
+---
+...
+box.schema.user.revoke('guest','create,read,write','universe')
+---
+...
+box.space.T5:drop()
+---
+...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 04d6c1903..074c1a0fe 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1435,3 +1435,15 @@ test_run:wait_log('default', 'readahead limit is reached', 1024, 0.1)
s:drop()
box.cfg{readahead = readahead}
+
+-- Test alter privilege for space that is modified by insertion in
+-- _trigger, _ck_constraint space.
+box.sql.execute("CREATE TABLE t5(x INT primary key, y INT, CHECK( x + y < 2 ));")
+box.sql.execute("CREATE TRIGGER tr1 AFTER DELETE ON t5 BEGIN DELETE FROM t5; END;")
+box.schema.user.grant('guest','create,read,write','universe')
+c = net.connect(box.cfg.listen)
+c.space._trigger:replace({'TR1', box.space.T5.id, {sql='CREATE TRIGGER tr1 AFTER DELETE ON t5 BEGIN DELETE FROM t5; END;'}})
+c:execute('CREATE TABLE t6(x INT PRIMARY KEY REFERENCES t5 ON DELETE RESTRICT);')
+c:close()
+box.schema.user.revoke('guest','create,read,write','universe')
+box.space.T5:drop()
--
2.21.0
More information about the Tarantool-patches
mailing list