From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DDA4B4765E0 for ; Mon, 21 Dec 2020 22:14:52 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\)) From: Sergey Ostanevich In-Reply-To: <939fc28f69a4c941845b724f493760bd8da073cb.1608550683.git.imeevma@gmail.com> Date: Mon, 21 Dec 2020 22:14:51 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <939fc28f69a4c941845b724f493760bd8da073cb.1608550683.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Hi!=20 Thanks for the patch! aaa LGTM. Sergos > On 21 Dec 2020, at 14:38, imeevma@tarantool.org wrote: >=20 > After this patch, the persistent functions "box.schema.user.info" and > "LUA" will have the same rights as the user who executed them. >=20 > The problem was that setuid was unnecessarily set. Because of this, > these functions had the same rights as the user who created them. > However, they must have the same rights as the user who used them. >=20 > Part of tarantool/security#1 > --- > https://github.com/tarantool/security/issues/1 > = https://github.com/tarantool/tarantool/tree/imeevma/gh-security-1-lua-func= tion-access-2_6 >=20 >=20 > src/box/bootstrap.snap | Bin 5976 -> 5997 bytes > src/box/lua/upgrade.lua | 30 +++++++++++++++++++++++++++++ > test/box-py/bootstrap.result | 4 ++-- > test/box/access.result | 36 +++++++++++++++++++++++++++++++++++ > test/box/access.test.lua | 15 +++++++++++++++ > 5 files changed, 83 insertions(+), 2 deletions(-) >=20 > diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap > index = 8bd4f7ce24216a8bcced6aa97c20c83eb7a02c77..b9d84d7abd9dba0dd5ca72438c69cc6a= 5c5f0470 100644 >=20 > diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua > index add791cd7..be1a0d47d 100644 > --- a/src/box/lua/upgrade.lua > +++ b/src/box/lua/upgrade.lua > @@ -971,6 +971,35 @@ local function upgrade_to_2_3_1() > create_session_settings_space() > end >=20 > = +-------------------------------------------------------------------------= ------- > +-- Tarantool 2.6.2 > = +-------------------------------------------------------------------------= ------- > +local function backport_upgrade_2_7_1_function_access() > + local _func =3D box.space._func > + local _priv =3D box.space._priv > + local datetime =3D os.date("%Y-%m-%d %H:%M:%S") > + local funcs_to_change =3D {'LUA', 'box.schema.user.info'} > + for _, name in pairs(funcs_to_change) do > + local func =3D _func.index['name']:get(name) > + if func ~=3D nil and func.setuid ~=3D 0 then > + local id =3D func.id > + log.info('remove old function "'..name..'"') > + _priv:delete({2, 'function', id}) > + _func:delete({id}) > + log.info('create function "'..name..'" with unset = setuid') > + local new_func =3D func:update({{'=3D', 4, 0}, {'=3D', = 18, datetime}, > + {'=3D', 19, datetime}}) > + _func:replace(new_func) > + log.info('grant execute on function "'..name..'" to = public') > + _priv:replace{ADMIN, PUBLIC, 'function', id, box.priv.X} > + end > + end > +end > + > +local function upgrade_to_2_6_2() > + backport_upgrade_2_7_1_function_access() > +end > + > = --------------------------------------------------------------------------= ------ >=20 > local function get_version() > @@ -1007,6 +1036,7 @@ local function upgrade(options) > {version =3D mkversion(2, 2, 1), func =3D upgrade_to_2_2_1, = auto =3D true}, > {version =3D mkversion(2, 3, 0), func =3D upgrade_to_2_3_0, = auto =3D true}, > {version =3D mkversion(2, 3, 1), func =3D upgrade_to_2_3_1, = auto =3D true}, > + {version =3D mkversion(2, 6, 2), func =3D upgrade_to_2_6_2, = auto =3D true}, > } >=20 > for _, handler in ipairs(handlers) do > diff --git a/test/box-py/bootstrap.result = b/test/box-py/bootstrap.result > index 0876e77a6..a371bb369 100644 > --- a/test/box-py/bootstrap.result > +++ b/test/box-py/bootstrap.result > @@ -4,7 +4,7 @@ box.internal.bootstrap() > box.space._schema:select{} > --- > - - ['max_id', 511] > - - ['version', 2, 3, 1] > + - ['version', 2, 6, 2] > ... > box.space._cluster:select{} > --- > @@ -167,7 +167,7 @@ box.space._user:select{} > ... > for _, v in box.space._func:pairs{} do r =3D {} table.insert(r, = v:update({{"=3D", 18, ""}, {"=3D", 19, ""}})) return r end > --- > -- - [1, 1, 'box.schema.user.info', 1, 'LUA', '', 'function', [], = 'any', 'none', 'none', > +- - [1, 1, 'box.schema.user.info', 0, 'LUA', '', 'function', [], = 'any', 'none', 'none', > false, false, true, ['LUA'], {}, '', '', ''] > ... > box.space._priv:select{} > diff --git a/test/box/access.result b/test/box/access.result > index 20b1b8b35..27e636122 100644 > --- a/test/box/access.result > +++ b/test/box/access.result > @@ -2141,3 +2141,39 @@ box.schema.user.revoke('guest', = 'read,write,execute', 'space', 'not_universe') > sp:drop() > --- > ... > +-- > +-- Make sure that the functions "LUA" and "box.schema.user.info" do = not have > +-- excess rights. > +-- > +_ =3D box.schema.func.call("LUA", "return 1") > +--- > +... > +_ =3D box.schema.func.call("LUA", "return box.space._space:count()") > +--- > +... > +_ =3D box.schema.func.call("box.schema.user.info", 0) > +--- > +... > +_ =3D box.schema.func.call("box.schema.user.info", 1) > +--- > +... > +session.su('guest') > +--- > +... > +_ =3D box.schema.func.call("LUA", "return 1") > +--- > +... > +_ =3D box.schema.func.call("LUA", "return box.space._space:count()") > +--- > +- error: Read access to space '_space' is denied for user 'guest' > +... > +_ =3D box.schema.func.call("box.schema.user.info", 0) > +--- > +... > +_ =3D box.schema.func.call("box.schema.user.info", 1) > +--- > +- error: User '1' is not found > +... > +session.su('admin') > +--- > +... > diff --git a/test/box/access.test.lua b/test/box/access.test.lua > index 3e083a383..a62f87ad8 100644 > --- a/test/box/access.test.lua > +++ b/test/box/access.test.lua > @@ -824,3 +824,18 @@ box.schema.user.grant('guest', = 'read,write,execute', 'space', 'not_universe') > box.schema.user.revoke('guest', 'read,write,execute', 'universe') > box.schema.user.revoke('guest', 'read,write,execute', 'space', = 'not_universe') > sp:drop() > + > +-- > +-- Make sure that the functions "LUA" and "box.schema.user.info" do = not have > +-- excess rights. > +-- > +_ =3D box.schema.func.call("LUA", "return 1") > +_ =3D box.schema.func.call("LUA", "return box.space._space:count()") > +_ =3D box.schema.func.call("box.schema.user.info", 0) > +_ =3D box.schema.func.call("box.schema.user.info", 1) > +session.su('guest') > +_ =3D box.schema.func.call("LUA", "return 1") > +_ =3D box.schema.func.call("LUA", "return box.space._space:count()") > +_ =3D box.schema.func.call("box.schema.user.info", 0) > +_ =3D box.schema.func.call("box.schema.user.info", 1) > +session.su('admin') > --=20 > 2.25.1 >=20