[Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions
Mergen Imeev
imeevma at tarantool.org
Tue Dec 22 20:44:49 MSK 2020
Hi! Thank you for the review! My answer and new patch below.
On Mon, Dec 21, 2020 at 10:23:01PM +0300, Sergey Ostanevich wrote:
> Hi!
>
> Thanks for the patch!
>
> I wonder why we should have this strange naming for the function of
> backport 2_7_1 from 2_7_1 itself. Perhaps just the ‘function_access’
> is enough?
Fixed.
>
> Otherwise LGTM.
>
> Sergos
>
>
> > On 21 Dec 2020, at 13:51, Mergen Imeev via Tarantool-patches <tarantool-patches at dev.tarantool.org> wrote:
> >
> > 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
> >
>
>From 21efff563826e47ea049aff2c7215b5bac5883e6 Mon Sep 17 00:00:00 2001
Message-Id: <21efff563826e47ea049aff2c7215b5bac5883e6.1608658852.git.imeevma at gmail.com>
From: Mergen Imeev <imeevma at gmail.com>
Date: Mon, 2 Nov 2020 13:21:58 +0300
Subject: [PATCH v1 1/1] box: remove unnecessary rights from peristent
functions
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..c4a70297aad138d426a24ee4447af485e3597536 100644
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index a86a0d410..6fba260bd 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 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()
+ 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
More information about the Tarantool-patches
mailing list