From: imeevma@tarantool.org To: s.ostanevich@corp.mail.ru Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions Date: Mon, 21 Dec 2020 13:51:01 +0300 [thread overview] Message-ID: <2cc8e87a9cf72e326e0eacec99eb127a90b3f5e1.1608547708.git.imeevma@gmail.com> (raw) After this patch, the persistent functions "box.schema.user.info" and "LUA" will have the same rights as the user who executed them. 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. Fixes tarantool/security#1 --- https://github.com/tarantool/security/issues/1 https://github.com/tarantool/tarantool/tree/imeevma/gh-security-1-lua-function-access @ChangeLog - The functions "LUA" and "box.schema.user.info" now have the same rights as the user who executes them. (gh-security-1). src/box/bootstrap.snap | Bin 5976 -> 5991 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(-) diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 8bd4f7ce24216a8bcced6aa97c20c83eb7a02c77..9e7a4e3291accfc387b315f85be9b01f443048fd 100644 diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index a86a0d410..09cd015f0 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 +-------------------------------------------------------------------------------- +-- Tarantool 2.7.1 +-------------------------------------------------------------------------------- +local function backport_upgrade_2_7_1_function_access() + local _func = box.space._func + local _priv = box.space._priv + local datetime = os.date("%Y-%m-%d %H:%M:%S") + local funcs_to_change = {'LUA', 'box.schema.user.info'} + for _, name in pairs(funcs_to_change) do + local func = _func.index['name']:get(name) + if func ~= nil and func.setuid ~= 0 then + local id = 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 = func:update({{'=', 4, 0}, {'=', 18, datetime}, + {'=', 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_7_1() + backport_upgrade_2_7_1_function_access() +end + -------------------------------------------------------------------------------- local handlers = { @@ -985,6 +1014,7 @@ local handlers = { {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true}, {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true}, {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true}, + {version = mkversion(2, 7, 1), func = upgrade_to_2_7_1, auto = true}, } -- Schema version of the snapshot. diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 0876e77a6..ed7accea3 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, 7, 1] ... box.space._cluster:select{} --- @@ -167,7 +167,7 @@ box.space._user:select{} ... for _, v in box.space._func:pairs{} do r = {} table.insert(r, v:update({{"=", 18, ""}, {"=", 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. +-- +_ = box.schema.func.call("LUA", "return 1") +--- +... +_ = box.schema.func.call("LUA", "return box.space._space:count()") +--- +... +_ = box.schema.func.call("box.schema.user.info", 0) +--- +... +_ = box.schema.func.call("box.schema.user.info", 1) +--- +... +session.su('guest') +--- +... +_ = box.schema.func.call("LUA", "return 1") +--- +... +_ = box.schema.func.call("LUA", "return box.space._space:count()") +--- +- error: Read access to space '_space' is denied for user 'guest' +... +_ = box.schema.func.call("box.schema.user.info", 0) +--- +... +_ = 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. +-- +_ = box.schema.func.call("LUA", "return 1") +_ = box.schema.func.call("LUA", "return box.space._space:count()") +_ = box.schema.func.call("box.schema.user.info", 0) +_ = box.schema.func.call("box.schema.user.info", 1) +session.su('guest') +_ = box.schema.func.call("LUA", "return 1") +_ = box.schema.func.call("LUA", "return box.space._space:count()") +_ = box.schema.func.call("box.schema.user.info", 0) +_ = box.schema.func.call("box.schema.user.info", 1) +session.su('admin') -- 2.25.1
next reply other threads:[~2020-12-21 10:51 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-21 10:51 imeevma [this message] 2020-12-21 19:23 ` Sergey Ostanevich 2020-12-22 17:44 ` Mergen Imeev 2020-12-23 9:44 ` Sergey Ostanevich 2020-12-23 12:58 ` Kirill Yukhin -- strict thread matches above, loose matches on Subject: below -- 2020-12-22 7:56 imeevma 2020-12-22 8:13 ` Sergey Ostanevich 2020-12-21 11:38 imeevma 2020-12-21 19:14 ` Sergey Ostanevich 2020-11-03 0:03 imeevma 2020-11-11 21:48 ` Vladislav Shpilevoy 2020-12-03 9:54 ` Mergen Imeev 2020-12-04 23:10 ` Vladislav Shpilevoy 2020-12-16 20:23 ` Mergen Imeev 2020-12-20 14:05 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2cc8e87a9cf72e326e0eacec99eb127a90b3f5e1.1608547708.git.imeevma@gmail.com \ --to=imeevma@tarantool.org \ --cc=s.ostanevich@corp.mail.ru \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox