[Tarantool-patches] [PATCH v1 1/1] box: remove unnecessary rights from peristent functions

[imeevma at tarantool.org]

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.

Part of tarantool/security#1
---
https://github.com/tarantool/security/issues/1
https://github.com/tarantool/tarantool/tree/imeevma/gh-security-1-lua-function-access-2_6


 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(-)

diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 8bd4f7ce24216a8bcced6aa97c20c83eb7a02c77..b9d84d7abd9dba0dd5ca72438c69cc6a5c5f0470 100644

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
 
+--------------------------------------------------------------------------------
+-- Tarantool 2.6.2
+--------------------------------------------------------------------------------
+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_6_2()
+    backport_upgrade_2_7_1_function_access()
+end
+
 --------------------------------------------------------------------------------
 
 local function get_version()
@@ -1007,6 +1036,7 @@ local function upgrade(options)
         {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, 6, 2), func = upgrade_to_2_6_2, auto = true},
     }
 
     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 = {} 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