From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id F120B2AC28 for ; Tue, 26 Mar 2019 06:59:24 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bP0fXXKT9AuN for ; Tue, 26 Mar 2019 06:59:24 -0400 (EDT) Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 57C3B2AC24 for ; Tue, 26 Mar 2019 06:59:24 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 2/9] box: fix _trigger and _ck_constraint access check References: <3226bb82065bbaea578a909f5382571bbe01fa3b.1548838034.git.kshcherbatov@tarantool.org> <29CE984E-A6A6-4E1D-AFFD-721477BD73B1@tarantool.org> From: Kirill Shcherbatov Message-ID: <21a029d4-90de-48a8-8999-8baed0a7a76d@tarantool.org> Date: Tue, 26 Mar 2019 13:59:21 +0300 MIME-Version: 1.0 In-Reply-To: <29CE984E-A6A6-4E1D-AFFD-721477BD73B1@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, "n.pettik" > 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