[tarantool-patches] [security 2/3] security: Refactor reads from systems spaces

[Ilya Markov <imarkov@tarantool.org>]

Replace reads from systems spaces with reads from corresponding
system views.

After this patch some error messages are changed:
   * Accessing to objects that are not accessible for current user
   raises the error claiming these objects don't exists.
   * Attempt to add in transaction such methods as object create, drop
   raises an multi-engine transaction error instead of multi-statement
   transaction error.

In scope of #3250
---
 src/box/lua/schema.lua      | 91 ++++++++++++++++++++++++++-------------------
 test/box/access.result      | 46 ++++++++++++-----------
 test/box/access.test.lua    | 25 ++++++-------
 test/box/access_bin.result  |  8 ++--
 test/box/access_misc.result |  2 +-
 test/box/on_replace.result  |  8 ++--
 test/box/role.result        |  2 +-
 test/box/transaction.result |  6 +--
 8 files changed, 103 insertions(+), 85 deletions(-)

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 30c6bc6..5496acb 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -102,12 +102,12 @@ ffi.cdef[[
 ]]
 
 local function user_or_role_resolve(user)
-    local _user = box.space[box.schema.USER_ID]
+    local _vuser = box.space[box.schema.VUSER_ID]
     local tuple
     if type(user) == 'string' then
-        tuple = _user.index.name:get{user}
+        tuple = _vuser.index.name:get{user}
     else
-        tuple = _user:get{user}
+        tuple = _vuser:get{user}
     end
     if tuple == nil then
         return nil
@@ -116,12 +116,12 @@ local function user_or_role_resolve(user)
 end
 
 local function role_resolve(name_or_id)
-    local _user = box.space[box.schema.USER_ID]
+    local _vuser = box.space[box.schema.VUSER_ID]
     local tuple
     if type(name_or_id) == 'string' then
-        tuple = _user.index.name:get{name_or_id}
+        tuple = _vuser.index.name:get{name_or_id}
     elseif type(name_or_id) ~= 'nil' then
-        tuple = _user:get{name_or_id}
+        tuple = _vuser:get{name_or_id}
     end
     if tuple == nil or tuple[4] ~= 'role' then
         return nil
@@ -131,12 +131,12 @@ local function role_resolve(name_or_id)
 end
 
 local function user_resolve(name_or_id)
-    local _user = box.space[box.schema.USER_ID]
+    local _vuser = box.space[box.schema.VUSER_ID]
     local tuple
     if type(name_or_id) == 'string' then
-        tuple = _user.index.name:get{name_or_id}
+        tuple = _vuser.index.name:get{name_or_id}
     elseif type(name_or_id) ~= 'nil' then
-        tuple = _user:get{name_or_id}
+        tuple = _vuser:get{name_or_id}
     end
     if tuple == nil or tuple[4] ~= 'user' then
         return nil
@@ -146,12 +146,12 @@ local function user_resolve(name_or_id)
 end
 
 local function sequence_resolve(name_or_id)
-    local _sequence = box.space[box.schema.SEQUENCE_ID]
+    local _vsequence = box.space[box.schema.VSEQUENCE_ID]
     local tuple
     if type(name_or_id) == 'string' then
-        tuple = _sequence.index.name:get{name_or_id}
+        tuple = _vsequence.index.name:get{name_or_id}
     elseif type(name_or_id) ~= 'nil' then
-        tuple = _sequence:get{name_or_id}
+        tuple = _vsequence:get{name_or_id}
     end
     if tuple ~= nil then
         return tuple[1], tuple
@@ -1368,7 +1368,19 @@ function box.schema.space.bless(space)
 -- primary key and returns it back to the user
     space_mt.auto_increment = function(space, tuple)
         check_space_arg(space, 'auto_increment')
-        local max_tuple = check_primary_index(space):max()
+        local euid = box.session.euid()
+        -- HACK: habe to call box.session.su here,
+        -- as checking max requires 'READ' access to space, though auto_increment
+        -- is a 'WRITE' operation
+        local max_tuple
+        if euid ~= 1 then
+            box.session.su("admin")
+            max_tuple = check_primary_index(space):max()
+            box.session.su(euid)
+        else
+            max_tuple = check_primary_index(space):max()
+        end
+
         local max = 0
         if max_tuple ~= nil then
             max = max_tuple[1]
@@ -1686,12 +1698,12 @@ local function object_resolve(object_type, object_name)
         return space.id
     end
     if object_type == 'function' then
-        local _func = box.space[box.schema.FUNC_ID]
+        local _vfunc = box.space[box.schema.VFUNC_ID]
         local func
         if type(object_name) == 'string' then
-            func = _func.index.name:get{object_name}
+            func = _vfunc.index.name:get{object_name}
         else
-            func = _func:get{object_name}
+            func = _vfunc:get{object_name}
         end
         if func then
             return func[1]
@@ -1707,12 +1719,12 @@ local function object_resolve(object_type, object_name)
         return seq
     end
     if object_type == 'role' then
-        local _user = box.space[box.schema.USER_ID]
+        local _vuser = box.space[box.schema.VUSER_ID]
         local role
         if type(object_name) == 'string' then
-            role = _user.index.name:get{object_name}
+            role = _vuser.index.name:get{object_name}
         else
-            role = _user:get{object_name}
+            role = _vuser:get{object_name}
         end
         if role and role[4] == 'role' then
             return role[1]
@@ -1730,13 +1742,13 @@ local function object_name(object_type, object_id)
     end
     local space
     if object_type == 'space' then
-        space = box.space._space
+        space = box.space._vspace
     elseif object_type == 'sequence' then
         space = box.space._sequence
     elseif object_type == 'function' then
-        space = box.space._func
+        space = box.space._vfunc
     elseif object_type == 'role' or object_type == 'user' then
-        space = box.space._user
+        space = box.space._vuser
     else
         box.error(box.error.UNKNOWN_SCHEMA_OBJECT, object_type)
     end
@@ -1750,7 +1762,8 @@ box.schema.func.create = function(name, opts)
                               if_not_exists = 'boolean',
                               language = 'string'})
     local _func = box.space[box.schema.FUNC_ID]
-    local func = _func.index.name:get{name}
+    local _vfunc = box.space[box.schema.VFUNC_ID]
+    local func = _vfunc.index.name:get{name}
     if func then
         if not opts.if_not_exists then
             box.error(box.error.FUNCTION_EXISTS, name)
@@ -1767,12 +1780,13 @@ box.schema.func.drop = function(name, opts)
     opts = opts or {}
     check_param_table(opts, { if_exists = 'boolean' })
     local _func = box.space[box.schema.FUNC_ID]
+    local _vfunc = box.space[box.schema.VFUNC_ID]
     local fid
     local tuple
     if type(name) == 'string' then
-        tuple = _func.index.name:get{name}
+        tuple = _vfunc.index.name:get{name}
     else
-        tuple = _func:get{name}
+        tuple = _vfunc:get{name}
     end
     if tuple then
         fid = tuple[1]
@@ -1788,12 +1802,12 @@ box.schema.func.drop = function(name, opts)
 end
 
 function box.schema.func.exists(name_or_id)
-    local _func = box.space[box.schema.FUNC_ID]
+    local _vfunc = box.space[box.schema.VFUNC_ID]
     local tuple = nil
     if type(name_or_id) == 'string' then
-        tuple = _func.index.name:get{name_or_id}
+        tuple = _vfunc.index.name:get{name_or_id}
     elseif type(name_or_id) == 'number' then
-        tuple = _func:get{name_or_id}
+        tuple = _vfunc:get{name_or_id}
     end
     return tuple ~= nil
 end
@@ -1948,8 +1962,9 @@ local function grant(uid, name, privilege, object_type,
         options.grantor = user_or_role_resolve(options.grantor)
     end
     local _priv = box.space[box.schema.PRIV_ID]
+    local _vpriv = box.space[box.schema.VPRIV_ID]
     -- add the granted privilege to the current set
-    local tuple = _priv:get{uid, object_type, oid}
+    local tuple = _vpriv:get{uid, object_type, oid}
     local old_privilege
     if tuple ~= nil then
         old_privilege = tuple[5]
@@ -1984,7 +1999,8 @@ local function revoke(uid, name, privilege, object_type, object_name, options)
     options = options or {}
     local oid = object_resolve(object_type, object_name)
     local _priv = box.space[box.schema.PRIV_ID]
-    local tuple = _priv:get{uid, object_type, oid}
+    local _vpriv = box.space[box.schema.VPRIV_ID]
+    local tuple = _vpriv:get{uid, object_type, oid}
     -- system privileges of admin and guest can't be revoked
     if tuple == nil then
         if options.if_exists then
@@ -2013,17 +2029,17 @@ end
 
 local function drop(uid, opts)
     -- recursive delete of user data
-    local _priv = box.space[box.schema.PRIV_ID]
-    local spaces = box.space[box.schema.SPACE_ID].index.owner:select{uid}
+    local _vpriv = box.space[box.schema.VPRIV_ID]
+    local spaces = box.space[box.schema.VSPACE_ID].index.owner:select{uid}
     for k, tuple in pairs(spaces) do
         box.space[tuple[1]]:drop()
     end
-    local funcs = box.space[box.schema.FUNC_ID].index.owner:select{uid}
+    local funcs = box.space[box.schema.VFUNC_ID].index.owner:select{uid}
     for k, tuple in pairs(funcs) do
         box.schema.func.drop(tuple[1])
     end
     -- if this is a role, revoke this role from whoever it was granted to
-    local grants = _priv.index.object:select{'role', uid}
+    local grants = _vpriv.index.object:select{'role', uid}
     for k, tuple in pairs(grants) do
         revoke(tuple[2], tuple[2], uid)
     end
@@ -2034,11 +2050,11 @@ local function drop(uid, opts)
     -- xxx: hack, we have to revoke session and usage privileges
     -- of a user using a setuid function in absence of create/drop
     -- privileges and grant option
-    if box.space._user:get{uid}[4] == 'user' then
+    if box.space._vuser:get{uid}[4] == 'user' then
         box.session.su('admin', box.schema.user.revoke, uid,
                        'session,usage', 'universe', nil, {if_exists = true})
     end
-    local privs = _priv.index.primary:select{uid}
+    local privs = _vpriv.index.primary:select{uid}
     for k, tuple in pairs(privs) do
         revoke(uid, uid, tuple[5], tuple[3], tuple[4])
     end
@@ -2095,8 +2111,7 @@ box.schema.user.drop = function(name, opts)
 end
 
 local function info(id)
-    local _priv = box.space._priv
-    local _user = box.space._priv
+    local _priv = box.space._vpriv
     local privs = {}
     for _, v in pairs(_priv:select{id}) do
         table.insert(
diff --git a/test/box/access.result b/test/box/access.result
index 191857f..2b28f07 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -386,7 +386,8 @@ session.su('grantee')
 -- fails - can't suicide - ask the creator to kill you
 box.schema.user.drop('grantee')
 ---
-- error: Read access to space '_user' is denied for user 'grantee'
+- error: 'Failed to drop user or role ''grantee'': the user is active in the current
+    session'
 ...
 session.su('grantor')
 ---
@@ -471,7 +472,7 @@ session.su('user1')
 -- permission denied
 box.schema.user.passwd('admin', 'xxx')
 ---
-- error: Read access to space '_user' is denied for user 'user1'
+- error: User 'admin' is not found
 ...
 session.su('admin')
 ---
@@ -1048,7 +1049,7 @@ session.su("test1")
 ...
 box.schema.user.disable("test")
 ---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
 ...
 session.su("admin")
 ---
@@ -1080,7 +1081,7 @@ session.su("test1")
 ...
 box.schema.user.grant("test", "usage", "universe")
 ---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
 ...
 session.su('admin')
 ---
@@ -1173,11 +1174,11 @@ box.schema.space.create('test')
 ...
 box.schema.user.create('test')
 ---
-- error: Read access to space '_user' is denied for user 'guest'
+- error: Write access to space '_user' is denied for user 'guest'
 ...
 box.schema.func.create('test')
 ---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
 ...
 box.session.su('admin')
 ---
@@ -1367,18 +1368,24 @@ box.schema.user.grant("tester", "read,execute", "universe")
 ---
 ...
 -- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
+box.session.su("tester")
+---
+...
+box.schema.space.create("test_space")
 ---
 - error: Write access to space '_schema' is denied for user 'tester'
 ...
-box.session.su("tester", box.schema.user.create, 'test_user')
+box.schema.user.create('test_user')
 ---
 - error: Write access to space '_user' is denied for user 'tester'
 ...
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.schema.func.create('test_func')
 ---
 - error: Write access to space '_func' is denied for user 'tester'
 ...
+box.session.su("admin")
+---
+...
 --
 -- FIXME 2.0: we still need to grant 'write' on universe
 -- explicitly since we still use process_rw to write to system
@@ -1387,24 +1394,27 @@ box.session.su("tester", box.schema.func.create, 'test_func')
 box.schema.user.grant("tester", "create,write", "universe")
 ---
 ...
+box.session.su("tester")
+---
+...
 -- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
+s1 = box.schema.space.create("test_space")
 ---
 ...
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
+_ = box.schema.user.create('test_user')
 ---
 ...
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+_ = box.schema.func.create('test_func')
 ---
 ...
 -- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
+s1:drop()
 ---
 ...
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
+box.schema.user.drop('test_user')
 ---
 ...
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+box.schema.func.drop('test_func')
 ---
 ...
 -- failed alter
@@ -1414,12 +1424,6 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
 -- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
 -- failed drop
 -- box.session.su("tester", s.drop, s)
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
----
-...
 box.schema.user.drop("test")
 ---
 - error: Revoke access to role 'public' is denied for user 'tester'
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 7e880a0..100dad5 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -514,9 +514,11 @@ f = box.schema.func.create("test")
 box.schema.user.grant("tester", "read,execute", "universe")
 
 -- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
-box.session.su("tester", box.schema.user.create, 'test_user')
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.session.su("tester")
+box.schema.space.create("test_space")
+box.schema.user.create('test_user')
+box.schema.func.create('test_func')
+box.session.su("admin")
 
 --
 -- FIXME 2.0: we still need to grant 'write' on universe
@@ -524,15 +526,16 @@ box.session.su("tester", box.schema.func.create, 'test_func')
 -- tables from ddl
 --
 box.schema.user.grant("tester", "create,write", "universe")
+box.session.su("tester")
 -- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+s1 = box.schema.space.create("test_space")
+_ = box.schema.user.create('test_user')
+_ = box.schema.func.create('test_func')
 
 -- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+s1:drop()
+box.schema.user.drop('test_user')
+box.schema.func.drop('test_func')
 
 -- failed alter
 -- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
@@ -544,10 +547,6 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
 -- failed drop
 -- box.session.su("tester", s.drop, s)
 
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
 box.schema.user.drop("test")
 box.session.su("admin")
 
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index b81279c..d093c75 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -55,14 +55,14 @@ c = remote.connect(box.cfg.listen)
 ...
 c:call("setuid_func")
 ---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
 ...
 session.su('guest')
 ---
 ...
 setuid_func()
 ---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
 ...
 session.su('admin')
 ---
@@ -85,7 +85,7 @@ session.su('guest')
 ...
 setuid_func()
 ---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
 ...
 session.su('admin')
 ---
@@ -122,7 +122,7 @@ session.su('guest')
 ...
 setuid_func()
 ---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
 ...
 session.su('admin')
 ---
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 3a56a4c..abc0be7 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -177,7 +177,7 @@ gs = box.schema.space.create('guest_space')
 ...
 box.schema.func.create('guest_func')
 ---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
 ...
 session.su('admin')
 ---
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index d6158dc..095a0e2 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -478,14 +478,14 @@ t = s:on_replace(function () box.schema.user.create('newu') end, t)
 ...
 s:replace({3, 4})
 ---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 t = s:on_replace(function () box.schema.role.create('newr') end, t)
 ---
 ...
 s:replace({4, 5})
 ---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 t = s:on_replace(function () s:drop() end, t)
 ---
@@ -499,14 +499,14 @@ t = s:on_replace(function () box.schema.func.create('newf') end, t)
 ...
 s:replace({6, 7})
 ---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 t = s:on_replace(function () box.schema.user.grant('guest', 'read,write', 'space', 'test_on_repl_ddl') end, t)
 ---
 ...
 s:replace({7, 8})
 ---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 t = s:on_replace(function () s:rename('newname') end, t)
 ---
diff --git a/test/box/role.result b/test/box/role.result
index 736ec85..806cea9 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -662,7 +662,7 @@ box.session.su('john')
 -- error
 box.schema.user.grant('grantee', 'role')
 ---
-- error: Read access to space '_user' is denied for user 'john'
+- error: User 'grantee' is not found
 ...
 --
 box.session.su('admin')
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 2a4b3b2..3ab5205 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -69,21 +69,21 @@ box.rollback();
 ...
 box.begin() box.schema.func.create('test');
 ---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 box.rollback();
 ---
 ...
 box.begin() box.schema.user.create('test');
 ---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 box.rollback();
 ---
 ...
 box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
 ---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
 ...
 box.rollback();
 ---
-- 
2.7.4