* [tarantool-patches] [security 0/2] System spaces access control lists @ 2018-05-16 12:37 Ilya Markov 2018-05-16 12:37 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov 2018-05-16 12:37 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov 0 siblings, 2 replies; 8+ messages in thread From: Ilya Markov @ 2018-05-16 12:37 UTC (permalink / raw) To: georgy; +Cc: kostja, tarantool-patches Ilya Markov (1): security: Refactor reads from systems spaces imarkov (1): security: Refactor system space access checks branch: gh-3250-system-space-access src/box/alter.cc | 280 ++++++++++++++++++++++++--------------- src/box/lua/schema.lua | 109 +++++++-------- src/box/schema.cc | 105 +++++++++++++++ src/box/schema.h | 14 ++ src/box/space.c | 3 +- src/box/space.h | 17 ++- src/box/sysview_engine.c | 1 + src/box/sysview_index.c | 86 +++++++----- src/box/user.h | 7 + test/box/access.result | 213 ++++++++++++++++++++++++----- test/box/access.test.lua | 92 +++++++++---- test/box/access_bin.result | 16 ++- test/box/access_bin.test.lua | 4 +- test/box/access_misc.result | 46 +++++-- test/box/access_misc.test.lua | 14 +- test/box/access_sysview.result | 11 +- test/box/access_sysview.test.lua | 6 +- test/box/ddl.result | 4 + test/box/ddl.test.lua | 2 + test/box/net.box.result | 6 + test/box/net.box.test.lua | 2 + test/box/on_replace.result | 8 +- test/box/role.result | 27 +++- test/box/role.test.lua | 13 +- test/box/sequence.result | 20 +-- test/box/sequence.test.lua | 11 +- test/box/transaction.result | 18 ++- test/box/transaction.test.lua | 4 + test/engine/iterator.result | 2 +- test/engine/savepoint.result | 12 +- test/engine/truncate.result | 2 +- 31 files changed, 838 insertions(+), 317 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces 2018-05-16 12:37 [tarantool-patches] [security 0/2] System spaces access control lists Ilya Markov @ 2018-05-16 12:37 ` Ilya Markov 2018-05-16 19:22 ` [tarantool-patches] " Konstantin Osipov 2018-05-16 12:37 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov 1 sibling, 1 reply; 8+ messages in thread From: Ilya Markov @ 2018-05-16 12:37 UTC (permalink / raw) To: georgy; +Cc: kostja, tarantool-patches 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 | 97 +++++++++++++----------- src/box/sysview_index.c | 86 +++++++++++++--------- test/box/access.result | 155 ++++++++++++++++++++++++++++++++------- test/box/access.test.lua | 69 +++++++++++------ test/box/access_bin.result | 16 ++-- test/box/access_bin.test.lua | 4 +- test/box/access_misc.result | 14 ++++ test/box/access_misc.test.lua | 9 ++- test/box/access_sysview.result | 11 +-- test/box/access_sysview.test.lua | 6 +- test/box/ddl.result | 4 + test/box/ddl.test.lua | 2 + test/box/on_replace.result | 8 +- test/box/role.result | 2 +- test/box/transaction.result | 18 ++++- test/box/transaction.test.lua | 4 + test/engine/iterator.result | 2 +- test/engine/savepoint.result | 12 +-- 18 files changed, 368 insertions(+), 151 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 1a616d5..677325c 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -104,12 +104,12 @@ ffi.cdef[[ ]] local function user_or_role_resolve(user) - local _user = box.space[box.schema.VUSER_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 @@ -118,12 +118,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 @@ -133,12 +133,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 @@ -148,12 +148,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 @@ -164,8 +164,9 @@ end -- Revoke all privileges associated with the given object. local function revoke_object_privs(object_type, object_id) + local _vpriv = box.space[box.schema.VPRIV_ID] local _priv = box.space[box.schema.PRIV_ID] - local privs = _priv.index.object:select{object_type, object_id} + local privs = _vpriv.index.object:select{object_type, object_id} for k, tuple in pairs(privs) do local uid = tuple[2] _priv:delete{uid, object_type, object_id} @@ -431,10 +432,15 @@ end -- space format - the metadata about space fields function box.schema.space.format(id, format) local _space = box.space._space + local _vspace = box.space._vspace check_param(id, 'id', 'number') if format == nil then - return _space:get(id)[7] + local tuple = _vspace:get(id) + if tuple == nil then + box.error(box.error.NO_SUCH_SPACE, '#' .. tostring(id)) + end + return tuple[7] else check_param(format, 'format', 'table') format = update_format(format) @@ -450,6 +456,7 @@ box.schema.space.drop = function(space_id, space_name, opts) check_param_table(opts, { if_exists = 'boolean' }) local _space = box.space[box.schema.SPACE_ID] local _index = box.space[box.schema.INDEX_ID] + local _vindex = box.space[box.schema.VINDEX_ID] local _truncate = box.space[box.schema.TRUNCATE_ID] local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID] local sequence_tuple = _space_sequence:delete{space_id} @@ -457,7 +464,7 @@ box.schema.space.drop = function(space_id, space_name, opts) -- Delete automatically generated sequence. box.schema.sequence.drop(sequence_tuple[2]) end - local keys = _index:select(space_id) + local keys = _vindex:select(space_id) for i = #keys, 1, -1 do local v = keys[i] _index:delete{v[1], v[2]} @@ -695,7 +702,8 @@ box.schema.index.create = function(space_id, name, options) options = update_param_table(options, options_defaults) local _index = box.space[box.schema.INDEX_ID] - if _index.index.name:get{space_id, name} then + local _vindex = box.space[box.schema.VINDEX_ID] + if _vindex.index.name:get{space_id, name} then if options.if_not_exists then return space.index[name], "not created" else @@ -708,7 +716,7 @@ box.schema.index.create = function(space_id, name, options) iid = options.id else -- max - local tuple = _index.index[0] + local tuple = _vindex.index[0] :select(space_id, { limit = 1, iterator = 'LE' })[1] if tuple then local id = tuple[1] @@ -1741,12 +1749,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] @@ -1762,12 +1770,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] @@ -1785,13 +1793,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 @@ -1805,7 +1813,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) @@ -1822,12 +1831,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] @@ -1843,12 +1853,12 @@ box.schema.func.drop = function(name, opts) end function box.schema.func.exists(name_or_id) - local _func = box.space[box.schema.VFUNC_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 @@ -2003,8 +2013,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] @@ -2039,7 +2050,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 @@ -2068,32 +2080,32 @@ 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 - local sequences = box.space[box.schema.SEQUENCE_ID].index.owner:select{uid} + local sequences = box.space[box.schema.VSEQUENCE_ID].index.owner:select{uid} for k, tuple in pairs(sequences) do box.schema.sequence.drop(tuple[1]) end -- 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 @@ -2150,8 +2162,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/src/box/sysview_index.c b/src/box/sysview_index.c index 8bfc39b..c8e9a1d 100644 --- a/src/box/sysview_index.c +++ b/src/box/sysview_index.c @@ -197,23 +197,35 @@ static const struct index_vtab sysview_index_vtab = { /* .end_build = */ generic_index_end_build, }; +const uint32_t PRIV_WRDA = PRIV_W | PRIV_D | PRIV_A | PRIV_R; + + +/* + * System view filters. + * Filter must give read access to object, if: + * 1. User has modification or read access to universe. + * 2. User has read access to according system space. + * 3. User has modification or read access to object. + * 4. User is an owner of the object. + * 5. Other specific conditions of the object of object type. + */ static bool vspace_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ - if (PRIV_R & source->access[cr->auth_token].effective) - return true; /* read access to _space space */ - + /* If user has global alter, drop privilege she may access all spaces. */ + if (cr->universal_access & PRIV_WRDA) + return true; + if (source->access[cr->auth_token].effective & PRIV_R) + return true; uint32_t space_id; if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &space_id) != 0) return false; struct space *space = space_cache_find(space_id); if (space == NULL) return false; - user_access_t effective = space->access[cr->auth_token].effective; - return ((PRIV_R | PRIV_W) & (cr->universal_access | effective) || + uint32_t effective = space->access[cr->auth_token].effective; + return (PRIV_WRDA & effective || space->def->uid == cr->uid); } @@ -221,10 +233,11 @@ static bool vuser_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ - if (PRIV_R & source->access[cr->auth_token].effective) - return true; /* read access to _user space */ + /* If user has global alter, drop privilege she may access all users. */ + if (cr->universal_access & PRIV_WRDA) + return true; + if (source->access[cr->auth_token].effective & PRIV_R) + return true; /* read access to _user space. */ uint32_t uid; if (tuple_field_u32(tuple, BOX_USER_FIELD_ID, &uid) != 0) @@ -232,16 +245,19 @@ vuser_filter(struct space *source, struct tuple *tuple) uint32_t owner_id; if (tuple_field_u32(tuple, BOX_USER_FIELD_UID, &owner_id) != 0) return false; - return uid == cr->uid || owner_id == cr->uid; + return uid == cr->uid || owner_id == cr->uid || uid == PUBLIC; } static bool vpriv_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ - if (PRIV_R & source->access[cr->auth_token].effective) + /* If user has global alter, drop privilege + * she may access all privileges + */ + if (cr->universal_access & PRIV_WRDA) + return true; + if (source->access[cr->auth_token].effective & PRIV_R) return true; /* read access to _priv space */ uint32_t grantor_id; @@ -250,37 +266,43 @@ vpriv_filter(struct space *source, struct tuple *tuple) uint32_t grantee_id; if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &grantee_id) != 0) return false; - return grantor_id == cr->uid || grantee_id == cr->uid; + const char *type; + uint32_t obj_id; + if ((type = tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE)) == NULL || + tuple_field_u32(tuple, BOX_PRIV_FIELD_OBJECT_ID, &obj_id) != 0) + return false; + return grantor_id == cr->uid || grantee_id == cr->uid || + (strncmp(type, "role", 4) == 0 && obj_id == PUBLIC); } static bool vfunc_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if ((PRIV_R | PRIV_X) & cr->universal_access) - return true; /* read or execute access to universe */ - if (PRIV_R & source->access[cr->auth_token].effective) + if (cr->universal_access & (PRIV_WRDA | PRIV_X)) + return true; + if (source->access[cr->auth_token].effective & PRIV_R) return true; /* read access to _func space */ - const char *name = tuple_field_cstr(tuple, BOX_FUNC_FIELD_NAME); + uint32_t name_len; + const char *name = tuple_field_str(tuple, BOX_FUNC_FIELD_NAME, + &name_len); if (name == NULL) return false; - uint32_t name_len = strlen(name); struct func *func = func_by_name(name, name_len); assert(func != NULL); - user_access_t effective = func->access[cr->auth_token].effective; - if (func->def->uid == cr->uid || (PRIV_X & effective)) - return true; - return false; + uint32_t effective = func->access[cr->auth_token].effective; + return func->def->uid == cr->uid || + ((PRIV_WRDA | PRIV_X) & effective); } static bool vsequence_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if ((PRIV_R | PRIV_X) & cr->universal_access) - return true; /* read or execute access to universe */ - if (PRIV_R & source->access[cr->auth_token].effective) + if (cr->universal_access & PRIV_WRDA) + return true; + if (source->access[cr->auth_token].effective & PRIV_R) return true; /* read access to _sequence space */ uint32_t id; @@ -289,13 +311,11 @@ vsequence_filter(struct space *source, struct tuple *tuple) struct sequence *sequence = sequence_by_id(id); if (sequence == NULL) return false; - user_access_t effective = sequence->access[cr->auth_token].effective; - if (sequence->def->uid == cr->uid || ((PRIV_W | PRIV_R) & effective)) - return true; - return false; + uint32_t effective = sequence->access[cr->auth_token].effective; + return sequence->def->uid == cr->uid || + (PRIV_WRDA & effective); } - struct sysview_index * sysview_index_new(struct sysview_engine *sysview, struct index_def *def, const char *space_name) diff --git a/test/box/access.result b/test/box/access.result index 5c39bc9..476e594 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') --- @@ -1133,6 +1134,9 @@ s = box.schema.space.create("admin_space") box.schema.user.grant('guest', 'super') --- ... +box.schema.user.grant('guest', 'read', "universe") +--- +... box.session.su('guest') --- ... @@ -1173,15 +1177,18 @@ 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') --- ... +box.schema.user.revoke('guest', 'read', "universe") +--- +... -- -- gh-2911 on_access_denied trigger -- @@ -1357,28 +1364,59 @@ box.schema.user.create("tester") s = box.schema.space.create("test") --- ... +_ = s:create_index("primary") +--- +... +seq = box.schema.sequence.create("test") +--- +... u = box.schema.user.create("test") --- ... f = box.schema.func.create("test") --- ... -box.schema.user.grant("tester", "read,execute", "universe") +-- failed create, auto_increment requires read. +box.session.su("tester") +--- +... +box.schema.space.create("test_space") +--- +- error: Write access to space '_schema' is denied for user 'tester' +... +box.schema.user.create('test_user') +--- +- error: Read access to space '_user' is denied for user 'tester' +... +box.schema.func.create('test_func') +--- +- error: Read access to space '_func' is denied for user 'tester' +... +box.session.su("admin") +--- +... +box.schema.user.grant("tester", "read", "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 +1425,36 @@ 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') +_ = s1:create_index("primary") --- ... -_ = box.session.su("tester", box.schema.func.create, 'test_func') +_ = box.schema.user.create('test_user') +--- +... +_ = box.schema.func.create('test_func') +--- +... +seq1 = box.schema.sequence.create('test_seq') --- ... -- successful drop of owned objects -_ = box.session.su("tester", s1.drop, s1) +s1:drop() --- ... -_ = box.session.su("tester", box.schema.user.drop, 'test_user') +seq1:drop() --- ... -_ = box.session.su("tester", box.schema.func.drop, 'test_func') +box.schema.user.drop('test_user') +--- +... +box.schema.func.drop('test_func') --- ... -- failed alter @@ -1414,22 +1464,24 @@ _ = 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") +s:drop() --- +- error: Drop access to space 'test' is denied for user 'tester' +... +seq:drop() +--- +- error: Drop access to sequence 'test' is denied for user 'tester' ... box.schema.user.drop("test") --- - error: Revoke access to role 'public' is denied for user 'tester' ... -box.session.su("admin") +box.schema.func.drop("test") --- +- error: Drop access to function 'test' is denied for user 'tester' ... -box.session.su("tester", box.schema.func.drop, "test") +box.session.su("admin") --- -- error: Drop access to function 'test' is denied for user 'tester' ... box.schema.user.grant("tester", "drop", "universe") --- @@ -1438,6 +1490,9 @@ box.schema.user.grant("tester", "drop", "universe") box.session.su("tester", s.drop, s) --- ... +box.session.su("tester", seq.drop, seq) +--- +... box.session.su("tester", box.schema.user.drop, "test") --- ... @@ -1510,36 +1565,86 @@ box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- --- create an object, but 'guest' still has no access to it +-- create an objects, but 'guest' still has no access to them -- box.session.su('admin', box.schema.func.create, 'test') --- ... +s = box.session.su('admin', box.schema.space.create, 'test') +--- +... +_ = box.session.su('admin', box.schema.sequence.create, 'test') +--- +... box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- -- grant access, the object should become visible to guest -- box.session.su('admin', box.schema.user.grant, 'guest', 'execute', 'function', 'test') --- ... +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'space', 'test') +--- +... +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'sequence', 'test') +--- +... box.schema.func.exists('test') --- - true ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- true +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- true +... -- --- drop object +-- drop objects -- box.session.su('admin', box.schema.func.drop, 'test') --- ... +box.session.su('admin', s.drop, s) +--- +... +box.session.su('admin', box.schema.sequence.drop, 'test') +--- +... box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- -- restore -- diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 501c766..81f5ed1 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -438,6 +438,7 @@ s:drop() -- s = box.schema.space.create("admin_space") box.schema.user.grant('guest', 'super') +box.schema.user.grant('guest', 'read', "universe") box.session.su('guest') _ = box.schema.space.create('test') box.space.test:drop() @@ -455,6 +456,7 @@ box.schema.space.create('test') box.schema.user.create('test') box.schema.func.create('test') box.session.su('admin') +box.schema.user.revoke('guest', 'read', "universe") -- -- gh-2911 on_access_denied trigger @@ -509,14 +511,24 @@ s:drop() -- box.schema.user.create("tester") s = box.schema.space.create("test") +_ = s:create_index("primary") +seq = box.schema.sequence.create("test") u = box.schema.user.create("test") f = box.schema.func.create("test") -box.schema.user.grant("tester", "read,execute", "universe") +-- failed create, auto_increment requires read. +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") +box.schema.user.grant("tester", "read", "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 +536,19 @@ 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") +_ = s1:create_index("primary") +_ = box.schema.user.create('test_user') +_ = box.schema.func.create('test_func') +seq1 = box.schema.sequence.create('test_seq') -- 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() +seq1: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"}) @@ -543,19 +559,16 @@ _ = 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") +s:drop() +seq:drop() box.schema.user.drop("test") -box.session.su("admin") - -box.session.su("tester", box.schema.func.drop, "test") +box.schema.func.drop("test") +box.session.su("admin") box.schema.user.grant("tester", "drop", "universe") -- successful drop box.session.su("tester", s.drop, s) +box.session.su("tester", seq.drop, seq) box.session.su("tester", box.schema.user.drop, "test") box.session.su("tester", box.schema.func.drop, "test") @@ -589,22 +602,36 @@ box.session.su('guest') -- has no access to the function -- box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- --- create an object, but 'guest' still has no access to it +-- create an objects, but 'guest' still has no access to them -- box.session.su('admin', box.schema.func.create, 'test') +s = box.session.su('admin', box.schema.space.create, 'test') +_ = box.session.su('admin', box.schema.sequence.create, 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- -- grant access, the object should become visible to guest -- box.session.su('admin', box.schema.user.grant, 'guest', 'execute', 'function', 'test') +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'space', 'test') +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'sequence', 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- --- drop object +-- drop objects -- box.session.su('admin', box.schema.func.drop, 'test') +box.session.su('admin', s.drop, s) +box.session.su('admin', box.schema.sequence.drop, 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- -- restore -- -box.session.su('admin') +box.session.su('admin') \ No newline at end of file diff --git a/test/box/access_bin.result b/test/box/access_bin.result index b81279c..ce09299 100644 --- a/test/box/access_bin.result +++ b/test/box/access_bin.result @@ -50,19 +50,22 @@ box.schema.func.create('setuid_func') box.schema.user.grant('guest', 'execute', 'function', 'setuid_func') --- ... +box.schema.user.grant('guest', 'read', 'universe') +--- +... 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 +88,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 +125,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') --- @@ -296,6 +299,9 @@ test:drop() box.schema.user.grant('guest', 'execute', 'universe') --- ... +box.schema.user.revoke('guest', 'read', 'universe') +--- +... function f1() return box.space._func:get(1)[4] end --- ... @@ -344,7 +350,7 @@ box.session.su('admin', box.session.user()) --- - error: 'bad argument #2 to ''?'' (function expected, got string)' ... --- clenaup +-- cleanup box.session.su('admin') --- ... diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua index cb3a50c..b4f5f64 100644 --- a/test/box/access_bin.test.lua +++ b/test/box/access_bin.test.lua @@ -20,6 +20,7 @@ index = setuid_space:create_index('primary') setuid_func = function() return box.space.setuid_space:auto_increment{} end box.schema.func.create('setuid_func') box.schema.user.grant('guest', 'execute', 'function', 'setuid_func') +box.schema.user.grant('guest', 'read', 'universe') c = remote.connect(box.cfg.listen) c:call("setuid_func") session.su('guest') @@ -112,6 +113,7 @@ test:drop() -- -- notice that guest can execute stuff, but can't read space _func box.schema.user.grant('guest', 'execute', 'universe') +box.schema.user.revoke('guest', 'read', 'universe') function f1() return box.space._func:get(1)[4] end function f2() return box.space._func:get(2)[4] end box.schema.func.create('f1') @@ -131,5 +133,5 @@ box.schema.func.drop('f2') -- box.session.su('admin', box.session.user) box.session.su('admin', box.session.user()) --- clenaup +-- cleanup box.session.su('admin') diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 3a56a4c..8bf99f2 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -175,6 +175,17 @@ gs = box.schema.space.create('guest_space') --- - error: Write access to space '_schema' is denied for user 'guest' ... +-- +-- FIXME: object create calls system space auto_increment, which requires +-- read and write privileges. Create privilege must solve this. +-- +box.schema.func.create('guest_func') +--- +- error: Read access to space '_func' is denied for user 'guest' +... +session.su('admin', box.schema.user.grant, "guest", "read", "universe") +--- +... box.schema.func.create('guest_func') --- - error: Read access to space '_func' is denied for user 'guest' @@ -182,6 +193,9 @@ box.schema.func.create('guest_func') session.su('admin') --- ... +box.schema.user.revoke("guest", "read", "universe") +--- +... s:select() --- - - [2] diff --git a/test/box/access_misc.test.lua b/test/box/access_misc.test.lua index cf6447e..27064c4 100644 --- a/test/box/access_misc.test.lua +++ b/test/box/access_misc.test.lua @@ -70,9 +70,16 @@ s:insert({4}) s:delete({3}) s:drop() gs = box.schema.space.create('guest_space') +-- +-- FIXME: object create calls system space auto_increment, which requires +-- read and write privileges. Create privilege must solve this. +-- +box.schema.func.create('guest_func') +session.su('admin', box.schema.user.grant, "guest", "read", "universe") box.schema.func.create('guest_func') - session.su('admin') +box.schema.user.revoke("guest", "read", "universe") + s:select() -- -- Create user with universe read&write grants diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result index 340ed21..20efd2b 100644 --- a/test/box/access_sysview.result +++ b/test/box/access_sysview.result @@ -266,11 +266,11 @@ box.session.su('guest') ... #box.space._vuser:select{} --- -- 1 +- 5 ... #box.space._vpriv:select{} --- -- 2 +- 15 ... #box.space._vfunc:select{} --- @@ -343,7 +343,7 @@ box.session.su('guest') -- _vuser -- -- a guest user can read information about itself -t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest' +t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false --- - true ... @@ -526,8 +526,9 @@ box.schema.user.grant('guest', 'execute', 'function', 'test') box.session.su('guest') --- ... -#box.space._vfunc:select{} = cnt + 1 +#box.space._vfunc:select{} == func_cnt --- +- true ... box.session.su('admin') --- @@ -551,7 +552,7 @@ box.schema.user.grant('guest', 'execute', 'universe') box.session.su('guest') --- ... -#box.space._vfunc:select{} == cnt + 1 +#box.space._vfunc:select{} == func_cnt --- - true ... diff --git a/test/box/access_sysview.test.lua b/test/box/access_sysview.test.lua index 7955ffc..4cc5611 100644 --- a/test/box/access_sysview.test.lua +++ b/test/box/access_sysview.test.lua @@ -134,7 +134,7 @@ box.session.su('guest') -- -- a guest user can read information about itself -t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest' +t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false -- read access to original space also allow to read a view box.session.su('admin') @@ -218,7 +218,7 @@ box.session.su('admin') box.schema.user.grant('guest', 'execute', 'function', 'test') box.session.su('guest') -#box.space._vfunc:select{} = cnt + 1 +#box.space._vfunc:select{} == func_cnt box.session.su('admin') box.schema.user.revoke('guest', 'execute', 'function', 'test') @@ -230,7 +230,7 @@ box.session.su('admin') box.schema.user.grant('guest', 'execute', 'universe') box.session.su('guest') -#box.space._vfunc:select{} == cnt + 1 +#box.space._vfunc:select{} == func_cnt box.session.su('admin') box.schema.user.revoke('guest', 'execute', 'universe') diff --git a/test/box/ddl.result b/test/box/ddl.result index f249f8f..15faa9a 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -565,6 +565,10 @@ test_run:cmd("setopt delimiter ''"); --- - true ... +-- finish transaction +box.rollback() +--- +... _ = c:get() --- ... diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua index 6029c6e..93501bd 100644 --- a/test/box/ddl.test.lua +++ b/test/box/ddl.test.lua @@ -226,6 +226,8 @@ test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); test_run:cmd("setopt delimiter ''"); +-- finish transaction +box.rollback() _ = c:get() test_latch:drop() -- this is where everything stops diff --git a/test/box/on_replace.result b/test/box/on_replace.result index a961df1..8c52e12 100644 --- a/test/box/on_replace.result +++ b/test/box/on_replace.result @@ -471,7 +471,7 @@ t = s:on_replace(function () s:create_index('sec') end, t) ... s:replace({2, 3}) --- -- error: Space _index 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.create('newu') end, t) --- @@ -506,21 +506,21 @@ t = s:on_replace(function () s:drop() end, t) ... s:replace({5, 6}) --- -- error: DDL does not support multi-statement transactions +- error: A multi-statement transaction can not use multiple storage engines ... 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 0580331..2fb1abf 100644 --- a/test/box/transaction.result +++ b/test/box/transaction.result @@ -69,7 +69,7 @@ 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(); --- @@ -83,7 +83,7 @@ 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(); --- @@ -95,6 +95,20 @@ box.begin() box.space._user:delete{box.schema.GUEST_ID}; box.rollback(); --- ... +box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; +--- +- error: DDL does not support multi-statement transactions +... +box.rollback(); +--- +... +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; +--- +- error: Space _sequence does not support multi-statement transactions +... +box.rollback(); +--- +... box.begin() box.space._schema:insert{'test'}; --- - error: Space _schema does not support multi-statement transactions diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua index ca6de44..14d1a69 100644 --- a/test/box/transaction.test.lua +++ b/test/box/transaction.test.lua @@ -38,6 +38,10 @@ box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv'); box.rollback(); box.begin() box.space._user:delete{box.schema.GUEST_ID}; box.rollback(); +box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; +box.rollback(); +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; +box.rollback(); box.begin() box.space._schema:insert{'test'}; box.rollback(); box.begin() box.space._cluster:insert{123456789, 'abc'}; diff --git a/test/engine/iterator.result b/test/engine/iterator.result index 423ed0b..ae14c43 100644 --- a/test/engine/iterator.result +++ b/test/engine/iterator.result @@ -4211,7 +4211,7 @@ s:replace{35} ... state, value = gen(param,state) --- -- error: 'builtin/box/schema.lua:985: usage: next(param, state)' +- error: 'builtin/box/schema.lua:993: usage: next(param, state)' ... value --- diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result index d440efa..dc2ad79 100644 --- a/test/engine/savepoint.result +++ b/test/engine/savepoint.result @@ -14,7 +14,7 @@ s1 = box.savepoint() ... box.rollback_to_savepoint(s1) --- -- error: 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- error: 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... box.begin() s1 = box.savepoint() --- @@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''"); ok1, errmsg1 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok2, errmsg2 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok3, errmsg3 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok4, errmsg4 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok5, errmsg5 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... s:select{} --- -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [security 1/2] security: Refactor reads from systems spaces 2018-05-16 12:37 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov @ 2018-05-16 19:22 ` Konstantin Osipov 0 siblings, 0 replies; 8+ messages in thread From: Konstantin Osipov @ 2018-05-16 19:22 UTC (permalink / raw) To: Ilya Markov; +Cc: georgy, tarantool-patches * Ilya Markov <imarkov@tarantool.org> [18/05/16 15:39]: > 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 > diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c > index 8bfc39b..c8e9a1d 100644 > --- a/src/box/sysview_index.c > +++ b/src/box/sysview_index.c > @@ -197,23 +197,35 @@ static const struct index_vtab sysview_index_vtab = { > /* .end_build = */ generic_index_end_build, > }; > > +const uint32_t PRIV_WRDA = PRIV_W | PRIV_D | PRIV_A | PRIV_R; Please add a comment explaining why you added each of the 4 ACLs to this bag of ACLs. > + > + > +/* > + * System view filters. > + * Filter must give read access to object, if: > + * 1. User has modification or read access to universe. > + * 2. User has read access to according system space. > + * 3. User has modification or read access to object. > + * 4. User is an owner of the object. > + * 5. Other specific conditions of the object of object type. 5. is misleading. Please either give an example or remove from the comment, and describe these specific conditions in place. This comment is related to all filters, so it's better to place it in module header or near PRIV_WRDA, in other words, not next to vspace_filter. > + */ > static bool > vspace_filter(struct space *source, struct tuple *tuple) > { > struct credentials *cr = effective_user(); > - if (PRIV_R & cr->universal_access) > - return true; /* read access to universe */ > - if (PRIV_R & source->access[cr->auth_token].effective) > - return true; /* read access to _space space */ > - > + /* If user has global alter, drop privilege she may access all spaces. */ > + if (cr->universal_access & PRIV_WRDA) > + return true; > + if (source->access[cr->auth_token].effective & PRIV_R) > + return true; Stray change. This patch is full of these order changes. If you don't like the way the filter conditions were coded, feel free to apply style formatting in a separate patch. I'm fine either way. > uint32_t space_id; > if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &space_id) != 0) > return false; > struct space *space = space_cache_find(space_id); > if (space == NULL) > return false; > - user_access_t effective = space->access[cr->auth_token].effective; > - return ((PRIV_R | PRIV_W) & (cr->universal_access | effective) || > + uint32_t effective = space->access[cr->auth_token].effective; This reverts my change for user_access_t data type. > + return (PRIV_WRDA & effective || > space->def->uid == cr->uid); > } > > @@ -221,10 +233,11 @@ static bool > vuser_filter(struct space *source, struct tuple *tuple) > { > struct credentials *cr = effective_user(); > - if (PRIV_R & cr->universal_access) > - return true; /* read access to universe */ > - if (PRIV_R & source->access[cr->auth_token].effective) > - return true; /* read access to _user space */ > + /* If user has global alter, drop privilege she may access all users. */ > + if (cr->universal_access & PRIV_WRDA) > + return true; > + if (source->access[cr->auth_token].effective & PRIV_R) > + return true; /* read access to _user space. */ Stray change. > > uint32_t uid; > if (tuple_field_u32(tuple, BOX_USER_FIELD_ID, &uid) != 0) > @@ -232,16 +245,19 @@ vuser_filter(struct space *source, struct tuple *tuple) > uint32_t owner_id; > if (tuple_field_u32(tuple, BOX_USER_FIELD_UID, &owner_id) != 0) > return false; > - return uid == cr->uid || owner_id == cr->uid; > + return uid == cr->uid || owner_id == cr->uid || uid == PUBLIC; Requires a minimal comment: 'PUBLIC' is a system role which any user can see. > } > > static bool > vpriv_filter(struct space *source, struct tuple *tuple) > { > struct credentials *cr = effective_user(); > - if (PRIV_R & cr->universal_access) > - return true; /* read access to universe */ > - if (PRIV_R & source->access[cr->auth_token].effective) > + /* If user has global alter, drop privilege > + * she may access all privileges they is gender neutral. This comment is misleading, what about WR in WRDA? Why do they provide access to vpriv? write privilege, in particular? I would try to avoid giving owners of write privilege read access to _v* spaces, unless it's really necessary, and each such access requires a special explanation and a test case. > + */ > + if (cr->universal_access & PRIV_WRDA) > + return true; > + if (source->access[cr->auth_token].effective & PRIV_R) > return true; /* read access to _priv space */ Stray change. > > uint32_t grantor_id; > @@ -250,37 +266,43 @@ vpriv_filter(struct space *source, struct tuple *tuple) > uint32_t grantee_id; > if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &grantee_id) != 0) > return false; > - return grantor_id == cr->uid || grantee_id == cr->uid; > + const char *type; > + uint32_t obj_id; > + if ((type = tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE)) == NULL || > + tuple_field_u32(tuple, BOX_PRIV_FIELD_OBJECT_ID, &obj_id) != 0) > + return false; > + return grantor_id == cr->uid || grantee_id == cr->uid || > + (strncmp(type, "role", 4) == 0 && obj_id == PUBLIC); Ugh. What's going on here? Is there a comment? A test case? > index b81279c..ce09299 100644 > --- a/test/box/access_bin.result > +++ b/test/box/access_bin.result > @@ -50,19 +50,22 @@ box.schema.func.create('setuid_func') > box.schema.user.grant('guest', 'execute', 'function', 'setuid_func') > --- > ... > +box.schema.user.grant('guest', 'read', 'universe') Why did you add this? Please explain the changes you made with the tests in the changeset comment. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [security 2/2] security: Refactor system space access checks 2018-05-16 12:37 [tarantool-patches] [security 0/2] System spaces access control lists Ilya Markov 2018-05-16 12:37 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov @ 2018-05-16 12:37 ` Ilya Markov 2018-05-16 19:27 ` [tarantool-patches] " Konstantin Osipov 1 sibling, 1 reply; 8+ messages in thread From: Ilya Markov @ 2018-05-16 12:37 UTC (permalink / raw) To: georgy; +Cc: kostja, tarantool-patches From: imarkov <imarkov@tarantool.org> Processes of creating, dropping objects, granting contain work with system spaces which so far must be available only to admin or user who has write access to universe. This patch removes the need of special accesses to universe. Introduce virtual access_check function in space object and separate its implementation for user spaces and system ones. Move checks on write to system spaces to on_replace triggers. Follow-up #945 Closes #3250 --- src/box/alter.cc | 280 ++++++++++++++++++++++++++---------------- src/box/lua/schema.lua | 12 +- src/box/schema.cc | 105 ++++++++++++++++ src/box/schema.h | 14 +++ src/box/space.c | 3 +- src/box/space.h | 17 ++- src/box/sysview_engine.c | 1 + src/box/user.h | 7 ++ test/box/access.result | 70 ++++++++--- test/box/access.test.lua | 33 +++-- test/box/access_misc.result | 52 +++++--- test/box/access_misc.test.lua | 17 +-- test/box/net.box.result | 6 + test/box/net.box.test.lua | 2 + test/box/role.result | 25 +++- test/box/role.test.lua | 13 +- test/box/sequence.result | 20 +-- test/box/sequence.test.lua | 11 +- test/engine/truncate.result | 2 +- 19 files changed, 497 insertions(+), 193 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 8766c81..fffde0e 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -64,50 +64,37 @@ static void access_check_ddl(const char *name, uint32_t owner_uid, enum schema_object_type type, - enum priv_type priv_type, - bool is_17_compat_mode) + enum priv_type priv_type, uint32_t system_space_id) { struct credentials *cr = effective_user(); user_access_t has_access = cr->universal_access; + + struct space *space = space_by_id(system_space_id); + if (space != NULL) { + has_access |= space->access[cr->auth_token].effective; + } /* - * XXX: pre 1.7.7 there was no specific 'CREATE' or - * 'ALTER' ACL, instead, read and write access on universe - * was used to allow create/alter. - * For backward compatibility, if a user has read and write - * access on the universe, grant it CREATE access - * automatically. - * The legacy fix does not affect sequences since they - * were added in 1.7.7 only. + * If a user has write access on the universe, + * grant it CREATE, DROP, ALTER access automatically. */ - if (is_17_compat_mode && has_access & PRIV_R && has_access & PRIV_W) - has_access |= PRIV_C | PRIV_A; + if (has_access & PRIV_W) + has_access |= PRIV_C | PRIV_A | PRIV_D; - user_access_t access = ((PRIV_U | (user_access_t) priv_type) & - ~has_access); + user_access_t access = ((user_access_t) priv_type & ~has_access); bool is_owner = owner_uid == cr->uid || cr->uid == ADMIN; /* * Only the owner of the object or someone who has - * specific DDL privilege on the object can execute - * DDL. If a user has no USAGE access and is owner, - * deny access as well. + * specific DDL privilege on the object can execute DDL. */ - if (access == 0 || (is_owner && !(access & PRIV_U))) + if (access == 0 || is_owner) return; /* Access granted. */ struct user *user = user_find_xc(cr->uid); - if (is_owner) { - tnt_raise(AccessDeniedError, - priv_name(PRIV_U), - schema_object_name(SC_UNIVERSE), - "", - user->def->name); - } else { - tnt_raise(AccessDeniedError, - priv_name(access), - schema_object_name(type), - name, - user->def->name); - } + tnt_raise(AccessDeniedError, + priv_name(access), + schema_object_name(type), + name, + user->def->name); } /** @@ -1551,7 +1538,9 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) struct space_def *def = space_def_new_from_tuple(new_tuple, ER_CREATE_SPACE, region); - access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(def->name, ADMIN, SC_SPACE, PRIV_C, + BOX_SPACE_ID); auto def_guard = make_scoped_guard([=] { space_def_delete(def); }); RLIST_HEAD(empty_list); @@ -1578,7 +1567,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) txn_on_rollback(txn, on_rollback); } else if (new_tuple == NULL) { /* DELETE */ access_check_ddl(old_space->def->name, old_space->def->uid, - SC_SPACE, PRIV_D, true); + SC_SPACE, PRIV_D, BOX_SPACE_ID); /* Verify that the space is empty (has no indexes) */ if (old_space->index_count) { tnt_raise(ClientError, ER_DROP_SPACE, @@ -1617,7 +1606,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) struct space_def *def = space_def_new_from_tuple(new_tuple, ER_ALTER_SPACE, region); - access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_A, true); + access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_A, + BOX_SPACE_ID); auto def_guard = make_scoped_guard([=] { space_def_delete(def); }); if (def->id != space_id(old_space)) @@ -1717,7 +1707,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) if (old_tuple && new_tuple) priv_type = PRIV_A; access_check_ddl(old_space->def->name, old_space->def->uid, SC_SPACE, - priv_type, true); + priv_type, BOX_SPACE_ID); struct index *old_index = space_index(old_space, iid); /* @@ -1908,14 +1898,20 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) space_name(old_space)); /* - * Check if a write privilege was given, raise an error if not. + * Perform checks specific for space. Then perform usual check_ddl. */ - access_check_space_xc(old_space, PRIV_W); + credentials *cr = effective_user(); + uint32_t has_access = cr->universal_access | + old_space->access[cr->auth_token].effective; + if (!(has_access & (PRIV_W | PRIV_D))) { + access_check_ddl(old_space->def->name, old_space->def->uid, + SC_SPACE, + PRIV_D, BOX_SPACE_ID); + } struct alter_space *alter = alter_space_new(old_space); auto scoped_guard = make_scoped_guard([=] { alter_space_delete(alter); }); - /* * Recreate all indexes of the truncated space. */ @@ -2110,7 +2106,9 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) struct user *old_user = user_by_id(uid); if (new_tuple != NULL && old_user == NULL) { /* INSERT */ struct user_def *user = user_def_new_from_tuple(new_tuple); - access_check_ddl(user->name, user->owner, SC_USER, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(user->name, ADMIN, SC_USER, PRIV_C, + BOX_USER_ID); auto def_guard = make_scoped_guard([=] { free(user); }); (void) user_cache_replace(user); def_guard.is_active = false; @@ -2119,7 +2117,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) txn_on_rollback(txn, on_rollback); } else if (new_tuple == NULL) { /* DELETE */ access_check_ddl(old_user->def->name, old_user->def->owner, - SC_USER, PRIV_D, true); + SC_USER, PRIV_D, BOX_USER_ID); /* Can't drop guest or super user */ if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) { tnt_raise(ClientError, ER_DROP_USER, @@ -2146,7 +2144,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) */ struct user_def *user = user_def_new_from_tuple(new_tuple); access_check_ddl(user->name, user->uid, SC_USER, PRIV_A, - true); + BOX_USER_ID); auto def_guard = make_scoped_guard([=] { free(user); }); struct trigger *on_commit = txn_alter_trigger_new(user_cache_alter_user, NULL); @@ -2248,7 +2246,9 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) struct func *old_func = func_by_id(fid); if (new_tuple != NULL && old_func == NULL) { /* INSERT */ struct func_def *def = func_def_new_from_tuple(new_tuple); - access_check_ddl(def->name, def->uid, SC_FUNCTION, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(def->name, ADMIN, SC_FUNCTION, PRIV_C, + BOX_FUNC_ID); auto def_guard = make_scoped_guard([=] { free(def); }); func_cache_replace(def); def_guard.is_active = false; @@ -2262,8 +2262,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) * Can only delete func if you're the one * who created it or a superuser. */ - access_check_ddl(old_func->def->name, uid, SC_FUNCTION, - PRIV_D, true); + access_check_ddl(old_func->def->name, uid, SC_FUNCTION, PRIV_D, + BOX_FUNC_ID); /* Can only delete func if it has no grants. */ if (schema_find_grants("function", old_func->def->fid)) { tnt_raise(ClientError, ER_DROP_FUNCTION, @@ -2277,7 +2277,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) struct func_def *def = func_def_new_from_tuple(new_tuple); auto def_guard = make_scoped_guard([=] { free(def); }); access_check_ddl(def->name, def->uid, SC_FUNCTION, PRIV_A, - true); + BOX_FUNC_ID); struct trigger *on_commit = txn_alter_trigger_new(func_cache_replace_func, NULL); txn_on_commit(txn, on_commit); @@ -2421,6 +2421,7 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(coll_cache_rollback, NULL); struct trigger *on_commit = txn_alter_trigger_new(coll_cache_commit, NULL); + struct credentials *cr = effective_user(); if (new_tuple == NULL && old_tuple != NULL) { /* DELETE */ /* TODO: Check that no index uses the collation */ @@ -2428,8 +2429,17 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) BOX_COLLATION_FIELD_ID); struct coll *old_coll = coll_by_id(old_id); assert(old_coll != NULL); - access_check_ddl(old_coll->name, old_coll->owner_id, - SC_COLLATION, PRIV_D, false); + /* + * No privilleges may give access to drop a collation. + */ + if (cr->uid != ADMIN && old_coll->owner_id != cr->uid) { + struct user *user = user_find_xc(cr->uid); + tnt_raise(AccessDeniedError, + priv_name(PRIV_D), + schema_object_name(SC_COLLATION), + old_coll->name, + user->def->name); + } /* * Set on_commit/on_rollback triggers after * deletion from the cache to make trigger logic @@ -2444,8 +2454,8 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) /* INSERT */ struct coll_def new_def; coll_def_new_from_tuple(new_tuple, &new_def); - access_check_ddl(new_def.name, new_def.owner_id, SC_COLLATION, - PRIV_C, false); + access_check_ddl(new_def.name, new_def.owner_id, + SC_COLLATION, PRIV_C, BOX_COLLATION_ID); struct coll *new_coll = coll_new(&new_def); if (new_coll == NULL) diag_raise(); @@ -2483,6 +2493,35 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) priv->access = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ACCESS); } +/** + * Handles case, when current user is not an owner of object and not ADMIN + * and wants to add or modify privileges. + * + * The strategy is following: + * When user has write, create, drop privileges in hierarchy over object, + * she is able to pass privileges. + * This case is required for creating, dropping users. + * TODO: we have to diffirentiate granting, revoking options. As so far, + * this is security breach, user who has create privilege may delete privileges + * of other users. Moreover, we have to differentiate promotion/demotion in altering + * privillege. + */ +static inline void +priv_check_not_owner(struct user *grantor, struct priv_def *priv, + const char *name, enum priv_type priv_type) +{ + uint32_t access = schema_find_access(priv->object_type, + priv->object_id, + grantor); + if (priv->grantee_id == ADMIN || !(access & (PRIV_W | PRIV_C | PRIV_D))) { + tnt_raise(AccessDeniedError, + priv_name(priv_type), + schema_object_name(priv->object_type), + name, + grantor->def->name); + } +} + /* * This function checks that: * - a privilege is granted from an existing user to an existing @@ -2496,35 +2535,33 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) static void priv_def_check(struct priv_def *priv, enum priv_type priv_type) { - struct user *grantor = user_find_xc(priv->grantor_id); /* May be a role */ struct user *grantee = user_by_id(priv->grantee_id); + credentials *cr = effective_user(); + struct user *grantor = user_find_xc(cr->uid); + if (grantee == NULL) { tnt_raise(ClientError, ER_NO_SUCH_USER, int2str(priv->grantee_id)); } const char *name = schema_find_name(priv->object_type, priv->object_id); - access_check_ddl(name, grantor->def->uid, priv->object_type, priv_type, - false); + switch (priv->object_type) { case SC_UNIVERSE: + { if (grantor->def->uid != ADMIN) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_UNIVERSE), - name, - grantor->def->name); + priv_check_not_owner(grantor, priv, name, priv_type); } break; + } case SC_SPACE: { - struct space *space = space_cache_find_xc(priv->object_id); + struct space *space = space_cache_find_xc( + priv->object_id); if (space->def->uid != grantor->def->uid && grantor->def->uid != ADMIN) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_SPACE), name, - grantor->def->name); + priv_check_not_owner(grantor, priv, name, + priv_type); } break; } @@ -2533,22 +2570,19 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) struct func *func = func_cache_find(priv->object_id); if (func->def->uid != grantor->def->uid && grantor->def->uid != ADMIN) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_FUNCTION), name, - grantor->def->name); + priv_check_not_owner(grantor, priv, name, + priv_type); } break; } case SC_SEQUENCE: { - struct sequence *seq = sequence_cache_find(priv->object_id); + struct sequence *seq = sequence_cache_find( + priv->object_id); if (seq->def->uid != grantor->def->uid && grantor->def->uid != ADMIN) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_SEQUENCE), name, - grantor->def->name); + priv_check_not_owner(grantor, priv, name, + priv_type); } break; } @@ -2561,16 +2595,23 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) int2str(priv->object_id)); } /* - * Only the creator of the role can grant or revoke it. - * Everyone can grant 'PUBLIC' role. + * Only the creator or owner of grantee of the role + * or user with appropriate privileges can grant or revoke it. + * If role is 'PUBLIC' it is allowed to grant for anyone, + * Revoke 'PUBLIC' is allowed only to users who has drop access. + * The latter case is require for dropping users. */ if (role->def->owner != grantor->def->uid && - grantor->def->uid != ADMIN && - (role->def->uid != PUBLIC || priv->access != PRIV_X)) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_ROLE), name, - grantor->def->name); + grantor->def->uid != grantee->def->owner && + !((role->def->uid == PUBLIC) && + (((cr->universal_access & PRIV_D) + || priv_type == PRIV_GRANT) + && priv->access == PRIV_X))) { + tnt_raise(AccessDeniedError, + priv_name(priv_type), + schema_object_name(SC_ROLE), + name, + grantor->def->name); } /* Not necessary to do during revoke, but who cares. */ role_check(grantee, role); @@ -2687,6 +2728,7 @@ on_replace_dd_priv(struct trigger * /* trigger */, void *event) } else { /* modify */ priv_def_create_from_tuple(&priv, new_tuple); priv_def_check(&priv, PRIV_GRANT); + /*TODO: check that modified by owner of object or ADMIN or grantor of privillege*/ struct trigger *on_commit = txn_alter_trigger_new(modify_priv, NULL); txn_on_commit(txn, on_commit); @@ -2716,31 +2758,54 @@ on_replace_dd_schema(struct trigger * /* trigger */, void *event) struct tuple *new_tuple = stmt->new_tuple; const char *key = tuple_field_cstr_xc(new_tuple ? new_tuple : old_tuple, BOX_SCHEMA_FIELD_KEY); - if (strcmp(key, "cluster") == 0) { - if (new_tuple == NULL) - tnt_raise(ClientError, ER_REPLICASET_UUID_IS_RO); - tt_uuid uu; - tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu); - REPLICASET_UUID = uu; - } else if (strcmp(key, "version") == 0) { - if (new_tuple != NULL) { - uint32_t major, minor, patch; - if (tuple_field_u32(new_tuple, 1, &major) != 0 || - tuple_field_u32(new_tuple, 2, &minor) != 0) - tnt_raise(ClientError, ER_WRONG_DD_VERSION); - /* Version can be major.minor with no patch. */ - if (tuple_field_u32(new_tuple, 3, &patch) != 0) - patch = 0; - dd_version_id = version_id(major, minor, patch); - } else { - assert(old_tuple != NULL); - /* - * _schema:delete({'version'}) for - * example, for box.internal.bootstrap(). - */ - dd_version_id = tarantool_version_id(); + credentials *cr = effective_user(); + if (strncmp(key, "max_id", 6) == 0) { + if (!(cr->universal_access & (PRIV_W | PRIV_C))) + goto error; + } else { + uint32_t access = cr->universal_access; + struct space *schema_space = space_by_id(BOX_SCHEMA_ID); + access |= schema_space->access[cr->auth_token].effective; + if (~access & PRIV_W) + goto error; + else if (strncmp(key, "cluster", 7) == 0) { + if (new_tuple == NULL) + tnt_raise(ClientError, + ER_REPLICASET_UUID_IS_RO); + tt_uuid uu; + tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, + &uu); + REPLICASET_UUID = uu; + } else if (strncmp(key, "version", 7) == 0) { + if (new_tuple != NULL) { + uint32_t major, minor, patch; + if (tuple_field_u32(new_tuple, 1, &major) != + 0 || + tuple_field_u32(new_tuple, 2, &minor) != 0) + tnt_raise(ClientError, + ER_WRONG_DD_VERSION); + /* Version can be major.minor with no patch. */ + if (tuple_field_u32(new_tuple, 3, &patch) != 0) + patch = 0; + dd_version_id = version_id(major, minor, patch); + } else { + assert(old_tuple != NULL); + /* + * _schema:delete({'version'}) for + * example, for box.internal.bootstrap(). + */ + dd_version_id = tarantool_version_id(); + } } } + return; +error: + struct user *user = user_find_xc(cr->uid); + tnt_raise(AccessDeniedError, + priv_name(PRIV_W), + schema_object_name(SC_SPACE), + "_schema", + user->def->name); } /** @@ -2967,6 +3032,9 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) new_def = sequence_def_new_from_tuple(new_tuple, ER_CREATE_SEQUENCE); assert(sequence_by_id(new_def->id) == NULL); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(new_def->name, ADMIN, SC_SEQUENCE, + PRIV_C, BOX_SEQUENCE_ID); sequence_cache_replace(new_def); alter->new_def = new_def; } else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */ @@ -2975,7 +3043,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) struct sequence *seq = sequence_by_id(id); assert(seq != NULL); access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - PRIV_D, false); + PRIV_D, BOX_SEQUENCE_ID); if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id)) tnt_raise(ClientError, ER_DROP_SEQUENCE, seq->def->name, "the sequence has data"); @@ -2992,7 +3060,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) struct sequence *seq = sequence_by_id(new_def->id); assert(seq != NULL); access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - PRIV_A, false); + PRIV_A, BOX_SEQUENCE_ID); alter->old_def = seq->def; alter->new_def = new_def; } @@ -3072,10 +3140,10 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) /* Check we have the correct access type on the sequence. * */ access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, priv_type, - false); + BOX_SEQUENCE_ID); /** Check we have alter access on space. */ access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A, - false); + BOX_SPACE_ID); struct trigger *on_commit = txn_alter_trigger_new(on_commit_dd_space_sequence, space); diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 677325c..6601337 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1977,10 +1977,7 @@ box.schema.user.create = function(name, opts) uid = _user:auto_increment{session.euid(), name, 'user', auth_mech_list}[1] -- grant role 'public' to the user box.schema.user.grant(uid, 'public') - -- we have to grant global privileges from setuid function, since - -- only admin has the ownership over universe and we don't have - -- grant option - box.session.su('admin', box.schema.user.grant, uid, 'session,usage', 'universe', + box.schema.user.grant(uid, 'session,usage', 'universe', nil, {if_not_exists=true}) end @@ -2098,12 +2095,9 @@ local function drop(uid, opts) for k, tuple in pairs(sequences) do box.schema.sequence.drop(tuple[1]) end - -- 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._vuser:get{uid}[4] == 'user' then - box.session.su('admin', box.schema.user.revoke, uid, - 'session,usage', 'universe', nil, {if_exists = true}) + box.schema.user.revoke(uid, 'session,usage', 'universe', + nil, {if_exists = true}) end local privs = _vpriv.index.primary:select{uid} for k, tuple in pairs(privs) do diff --git a/src/box/schema.cc b/src/box/schema.cc index 1b96f97..8ed2549 100644 --- a/src/box/schema.cc +++ b/src/box/schema.cc @@ -37,6 +37,7 @@ #include "scoped_guard.h" #include "version.h" #include "user.h" +#include "session.h" #include <stdio.h> /** * @module Data Dictionary @@ -520,6 +521,70 @@ sequence_cache_delete(uint32_t id) } } + +/** + * Runs simple read, usage checks. + * All other checks are postponed. + */ +static int +sys_read_access_check(struct space *space, user_access_t access) +{ + credentials *cr = effective_user(); + if (~cr->universal_access & PRIV_U) { + struct user *user = user_find(cr->uid); + if (user != NULL) + diag_set(AccessDeniedError, + priv_name(PRIV_U), + schema_object_name(SC_UNIVERSE), + "", + user->def->name); + return -1; + } + if (access == PRIV_R) { + uint32_t access = cr->universal_access | + space->access[cr->auth_token].effective; + if (~access & PRIV_R) { + struct user *user = user_find(cr->uid); + if (user != NULL) + diag_set(AccessDeniedError, + priv_name(PRIV_R), + schema_object_name(SC_SPACE), + space->def->name, + user->def->name); + return -1; + } + } + /* Other checks are postponed to trigger*/ + return 0; +} + +/** + * Runs simple read, usage, write checks. + * All other checks are postponed. + */ +static int +sys_read_write_access_check(struct space *space, user_access_t access) +{ + credentials *cr = effective_user(); + if (access == PRIV_W) { + uint32_t has_access = space->access[cr->auth_token].effective + | cr->universal_access; + if (~has_access & access) { + struct user *user = user_find(cr->uid); + if (user != NULL) { + diag_set(AccessDeniedError, + priv_name(access), + schema_object_name(SC_SPACE), + space->def->name, + user->def->name); + } + return -1; + } + return 0; + } + return sys_read_access_check(space, access); +} + const char * schema_find_name(enum schema_object_type type, uint32_t object_id) { @@ -562,3 +627,43 @@ schema_find_name(enum schema_object_type type, uint32_t object_id) return "(nil)"; } +uint32_t +schema_find_access(enum schema_object_type type, uint32_t object_id, + struct user *grantor) +{ + struct priv_def def; + def.object_type = type; + def.object_id = object_id; + def.grantor_id = grantor->def->uid; + uint32_t access = universe.access[grantor->auth_token].effective; + struct access *obj_access = access_find(&def); + if (obj_access != NULL) + access |= obj_access->effective; + return access; +} + +access_check_func_t +get_access_check_func(uint32_t space_id) +{ + switch (space_id) { + case BOX_CLUSTER_ID: + case BOX_COLLATION_ID: + return sys_read_write_access_check; + case BOX_SCHEMA_ID: + case BOX_SPACE_ID: + case BOX_TRUNCATE_ID: + case BOX_INDEX_ID: + case BOX_FUNC_ID: + case BOX_USER_ID: + case BOX_SEQUENCE_ID: + case BOX_SEQUENCE_DATA_ID: + case BOX_SPACE_SEQUENCE_ID: + case BOX_PRIV_ID: + /* + * Specialized access checks will be performed in trigger. + */ + return sys_read_access_check; + default: + return access_check_user_space; + } +} diff --git a/src/box/schema.h b/src/box/schema.h index 2b87f5f..5b85e39 100644 --- a/src/box/schema.h +++ b/src/box/schema.h @@ -36,6 +36,7 @@ #include "error.h" #include "space.h" #include "latch.h" +#include "user.h" #if defined(__cplusplus) extern "C" { @@ -103,6 +104,13 @@ schema_find_name(enum schema_object_type type, uint32_t object_id); */ struct sequence * sequence_by_id(uint32_t id); + +uint32_t +schema_find_access(enum schema_object_type type, uint32_t object_id, + struct user *grantor); + +int +access_check_user_space(struct space *space, user_access_t access); #if defined(__cplusplus) } /* extern "C" */ @@ -205,6 +213,12 @@ sequence_cache_delete(uint32_t id); #endif /* defined(__cplusplus) */ /** + * Utility function used to specify access_check function for system space. + */ +access_check_func_t +get_access_check_func(uint32_t space_id); + +/** * Triggers fired after committing a change in space definition. * The space is passed to the trigger callback in the event * argument. It is the new space in case of create/update or diff --git a/src/box/space.c b/src/box/space.c index 02a9792..d72ce29 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -44,7 +44,7 @@ #include "iproto_constants.h" int -access_check_space(struct space *space, user_access_t access) +access_check_user_space(struct space *space, user_access_t access) { struct credentials *cr = effective_user(); /* Any space access also requires global USAGE privilege. */ @@ -132,6 +132,7 @@ space_create(struct space *space, struct engine *engine, rlist_create(&space->on_replace); rlist_create(&space->on_stmt_begin); space->run_triggers = true; + space->access_check = get_access_check_func(def->id); space->format = format; if (format != NULL) diff --git a/src/box/space.h b/src/box/space.h index a024fdc..5139a76 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -50,6 +50,8 @@ struct port; struct tuple; struct tuple_format; +typedef int (*access_check_func_t)(struct space *space, user_access_t access); + struct space_vtab { /** Free a space instance. */ void (*destroy)(struct space *); @@ -127,6 +129,7 @@ struct space_vtab { struct space { /** Virtual function table. */ const struct space_vtab *vtab; + access_check_func_t access_check; /** Cached runtime access information. */ struct access access[BOX_USER_MAX]; /** Engine used by this space. */ @@ -273,8 +276,11 @@ index_name_by_id(struct space *space, uint32_t id); * Check whether or not the current user can be granted * the requested access to the space. */ -int -access_check_space(struct space *space, user_access_t access); +static inline int +access_check_space(struct space *space, user_access_t access) +{ + return space->access_check(space, access); +} static inline int space_apply_initial_join_row(struct space *space, struct request *request) @@ -390,6 +396,12 @@ space_dump_def(const struct space *space, struct rlist *key_list); void space_fill_index_map(struct space *space); +/* + * Utility function used to specify access_check function for system space. + */ +access_check_func_t +get_access_check_func(uint32_t space_id); + #if defined(__cplusplus) } /* extern "C" */ @@ -499,4 +511,5 @@ space_prepare_alter_xc(struct space *old_space, struct space *new_space) #endif /* defined(__cplusplus) */ + #endif /* TARANTOOL_BOX_SPACE_H_INCLUDED */ diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c index 5aea87e..f77ce99 100644 --- a/src/box/sysview_engine.c +++ b/src/box/sysview_engine.c @@ -205,6 +205,7 @@ sysview_engine_create_space(struct engine *engine, struct space_def *def, free(space); return NULL; } + space->access_check = access_check_user_space; return space; } diff --git a/src/box/user.h b/src/box/user.h index 07c4dc5..254f0c9 100644 --- a/src/box/user.h +++ b/src/box/user.h @@ -199,6 +199,13 @@ priv_grant(struct user *grantee, struct priv_def *priv); void priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple); +/** + * Find the corresponding access structure + * given object type and object id. + */ +struct access * +access_find(struct priv_def *priv); + /* }}} */ #endif /* defined(__cplusplus) */ diff --git a/test/box/access.result b/test/box/access.result index 476e594..8918cf7 100644 --- a/test/box/access.result +++ b/test/box/access.result @@ -378,7 +378,6 @@ box.schema.user.create('grantee') ... box.schema.user.grant('grantee', 'read, write, execute', 'universe') --- -- error: Grant access to universe '' is denied for user 'grantor' ... session.su('grantee') --- @@ -1177,11 +1176,11 @@ box.schema.space.create('test') ... box.schema.user.create('test') --- -- error: Write access to space '_user' is denied for user 'guest' +- error: Create access to user 'test' is denied for user 'guest' ... box.schema.func.create('test') --- -- error: Write access to space '_func' is denied for user 'guest' +- error: Create access to function 'test' is denied for user 'guest' ... box.session.su('admin') --- @@ -1395,10 +1394,32 @@ box.schema.func.create('test_func') box.session.su("admin") --- ... +-- failed create because of auto_increment +box.session.su("tester") +--- +... +box.schema.space.create("test_space") +--- +- error: Write access to space '_schema' is denied for user 'tester' +... +box.schema.user.create('test_user') +--- +- error: Read access to space '_user' is denied for user 'tester' +... +box.schema.func.create('test_func') +--- +- error: Read access to space '_func' is denied for user 'tester' +... +box.schema.sequence.create('test_seq') +--- +- error: Read access to space '_sequence' is denied for user 'tester' +... +box.session.su("admin") +--- +... box.schema.user.grant("tester", "read", "universe") --- ... --- failed create box.session.su("tester") --- ... @@ -1408,27 +1429,26 @@ box.schema.space.create("test_space") ... box.schema.user.create('test_user') --- -- error: Write access to space '_user' is denied for user 'tester' +- error: Create access to user 'test_user' is denied for user 'tester' ... box.schema.func.create('test_func') --- -- error: Write access to space '_func' is denied for user 'tester' +- error: Create access to function 'test_func' is denied for user 'tester' +... +box.schema.sequence.create('test_seq') +--- +- error: Create access to sequence 'test_seq' 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 --- tables from ddl --- -box.schema.user.grant("tester", "create,write", "universe") +box.schema.user.grant("tester", "create", "universe") --- ... box.session.su("tester") --- ... --- successful create +-- successful create. Note user still needs read privilege because of auto_increment. s1 = box.schema.space.create("test_space") --- ... @@ -1464,29 +1484,45 @@ 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) +box.session.su("admin") +--- +... +-- revoke read +box.schema.user.revoke("tester", "read", "universe") +--- +... +box.session.su("tester") +--- +... s:drop() --- - error: Drop access to space 'test' is denied for user 'tester' ... seq:drop() --- -- error: Drop access to sequence 'test' is denied for user 'tester' +- error: Sequence '1' does not exist ... box.schema.user.drop("test") --- -- error: Revoke access to role 'public' is denied for user 'tester' +- error: User 'test' is not found ... box.schema.func.drop("test") --- -- error: Drop access to function 'test' is denied for user 'tester' +- error: Function 'test' does not exist ... box.session.su("admin") --- ... +box.schema.user.revoke("tester", "create", "universe") +--- +... box.schema.user.grant("tester", "drop", "universe") --- ... --- successful drop +-- successful truncate, drop +box.session.su("tester", s.truncate, s) +--- +... box.session.su("tester", s.drop, s) --- ... diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 81f5ed1..d01e8c6 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -522,22 +522,25 @@ box.schema.space.create("test_space") box.schema.user.create('test_user') box.schema.func.create('test_func') box.session.su("admin") -box.schema.user.grant("tester", "read", "universe") --- failed create +-- failed create because of auto_increment box.session.su("tester") box.schema.space.create("test_space") box.schema.user.create('test_user') box.schema.func.create('test_func') +box.schema.sequence.create('test_seq') + box.session.su("admin") +box.schema.user.grant("tester", "read", "universe") +box.session.su("tester") --- --- FIXME 2.0: we still need to grant 'write' on universe --- explicitly since we still use process_rw to write to system --- tables from ddl --- -box.schema.user.grant("tester", "create,write", "universe") +box.schema.space.create("test_space") +box.schema.user.create('test_user') +box.schema.func.create('test_func') +box.schema.sequence.create('test_seq') +box.session.su("admin") +box.schema.user.grant("tester", "create", "universe") box.session.su("tester") --- successful create +-- successful create. Note user still needs read privilege because of auto_increment. s1 = box.schema.space.create("test_space") _ = s1:create_index("primary") _ = box.schema.user.create('test_user') @@ -559,14 +562,22 @@ box.schema.func.drop('test_func') -- failed drop -- box.session.su("tester", s.drop, s) +box.session.su("admin") +-- revoke read +box.schema.user.revoke("tester", "read", "universe") +box.session.su("tester") + s:drop() seq:drop() box.schema.user.drop("test") box.schema.func.drop("test") - box.session.su("admin") + + +box.schema.user.revoke("tester", "create", "universe") box.schema.user.grant("tester", "drop", "universe") --- successful drop +-- successful truncate, drop +box.session.su("tester", s.truncate, s) box.session.su("tester", s.drop, s) box.session.su("tester", seq.drop, seq) box.session.su("tester", box.schema.user.drop, "test") diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 8bf99f2..f6ba483 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -61,6 +61,9 @@ box.schema.user.grant('testus', 'read', 'space', 'admin_space') --- - error: User 'testus' already has read access on space 'admin_space' ... +box.schema.user.grant('testus', 'read', 'universe') +--- +... session.su('testus') --- ... @@ -78,7 +81,7 @@ s:delete(1) ... s:drop() --- -- error: Write access to space '_space_sequence' is denied for user 'testus' +- error: Drop access to space 'admin_space' is denied for user 'testus' ... -- -- Check double revoke @@ -93,6 +96,9 @@ box.schema.user.revoke('testus', 'read', 'space', 'admin_space') --- - error: User 'testus' does not have read access on space 'admin_space' ... +box.schema.user.revoke('testus', 'read', 'universe') +--- +... session.su('testus') --- ... @@ -124,9 +130,18 @@ s:insert({3}) --- - [3] ... +session.su('admin') +--- +... +box.schema.user.grant('testus', 'read', 'universe') +--- +... +session.su('testus') +--- +... s:drop() --- -- error: Write access to space '_space_sequence' is denied for user 'testus' +- error: Drop access to space 'admin_space' is denied for user 'testus' ... session.su('admin') --- @@ -167,28 +182,26 @@ s:delete({3}) --- - error: Write access to space 'admin_space' is denied for user 'guest' ... -s:drop() +session.su('admin') --- -- error: Write access to space '_space_sequence' is denied for user 'guest' ... -gs = box.schema.space.create('guest_space') +box.schema.user.grant('guest', 'read', 'universe') --- -- error: Write access to space '_schema' is denied for user 'guest' ... --- --- FIXME: object create calls system space auto_increment, which requires --- read and write privileges. Create privilege must solve this. --- -box.schema.func.create('guest_func') +session.su('guest') --- -- error: Read access to space '_func' is denied for user 'guest' ... -session.su('admin', box.schema.user.grant, "guest", "read", "universe") +s:drop() --- +- error: Drop access to space 'admin_space' is denied for user 'guest' +... +gs = box.schema.space.create('guest_space') +--- +- error: Write access to space '_schema' is denied for user 'guest' ... box.schema.func.create('guest_func') --- -- error: Read access to space '_func' is denied for user 'guest' +- error: Create access to function 'guest_func' is denied for user 'guest' ... session.su('admin') --- @@ -291,7 +304,7 @@ session.su('admin') box.schema.user.create('someuser') --- ... -box.schema.user.grant('someuser', 'read, write, execute', 'universe') +box.schema.user.grant('someuser', 'read', 'universe') --- ... session.su('someuser') @@ -358,7 +371,7 @@ testuser_uid = session.uid() ... _ = box.space._user:delete(2) --- -- error: Drop access to user 'public' is denied for user 'testuser' +- error: 'Failed to drop user or role ''public'': the user or the role is a system' ... box.space._user:select(1) --- @@ -395,7 +408,7 @@ session.su('testuser') ... _ = box.space._user:delete(2) --- -- error: Write access to space '_user' is denied for user 'testuser' +- error: Drop access to user 'public' is denied for user 'testuser' ... box.space._user:select(1) --- @@ -403,7 +416,8 @@ box.space._user:select(1) ... box.space._user:insert{uid, session.uid(), 'someone2', 'user'} --- -- error: Write access to space '_user' is denied for user 'testuser' +- error: Tuple field count 4 is less than required by space format or defined indexes + (expected at least 5) ... session.su('admin') --- @@ -423,7 +437,7 @@ box.space._index:select(272) ... box.space._index:insert{512, 1,'owner','tree', 1, 1, 0,'unsigned'} --- -- error: Write access to space '_index' is denied for user 'testuser' +- error: 'Tuple field 5 type does not match one required by operation: expected map' ... session.su('admin') --- diff --git a/test/box/access_misc.test.lua b/test/box/access_misc.test.lua index 27064c4..470f730 100644 --- a/test/box/access_misc.test.lua +++ b/test/box/access_misc.test.lua @@ -27,6 +27,7 @@ s:insert({2}) -- box.schema.user.grant('testus', 'read', 'space', 'admin_space') box.schema.user.grant('testus', 'read', 'space', 'admin_space') +box.schema.user.grant('testus', 'read', 'universe') session.su('testus') s:select(1) @@ -39,6 +40,7 @@ s:drop() session.su('admin') box.schema.user.revoke('testus', 'read', 'space', 'admin_space') box.schema.user.revoke('testus', 'read', 'space', 'admin_space') +box.schema.user.revoke('testus', 'read', 'universe') session.su('testus') s:select(1) @@ -52,6 +54,9 @@ session.su('testus') s:select(1) s:delete(1) s:insert({3}) +session.su('admin') +box.schema.user.grant('testus', 'read', 'universe') +session.su('testus') s:drop() session.su('admin') -- @@ -68,14 +73,12 @@ box.space._user:select(1) s:select(1) s:insert({4}) s:delete({3}) +session.su('admin') +box.schema.user.grant('guest', 'read', 'universe') +session.su('guest') s:drop() gs = box.schema.space.create('guest_space') --- --- FIXME: object create calls system space auto_increment, which requires --- read and write privileges. Create privilege must solve this. --- -box.schema.func.create('guest_func') -session.su('admin', box.schema.user.grant, "guest", "read", "universe") + box.schema.func.create('guest_func') session.su('admin') box.schema.user.revoke("guest", "read", "universe") @@ -123,7 +126,7 @@ box.schema.func.create('uniuser_func') session.su('admin') box.schema.user.create('someuser') -box.schema.user.grant('someuser', 'read, write, execute', 'universe') +box.schema.user.grant('someuser', 'read', 'universe') session.su('someuser') -- -- Check drop objects of another user diff --git a/test/box/net.box.result b/test/box/net.box.result index 1674c27..7861b4d 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -2146,6 +2146,9 @@ box.schema.user.grant('guest', 'write', 'space', '_space') box.schema.user.grant('guest', 'write', 'space', '_schema') --- ... +box.schema.user.grant('guest', 'create', 'universe') +--- +... count = 0 --- ... @@ -2194,6 +2197,9 @@ box.schema.user.revoke('guest', 'write', 'space', '_space') box.schema.user.revoke('guest', 'write', 'space', '_schema') --- ... +box.schema.user.revoke('guest', 'create', 'universe') +--- +... c:close() --- ... diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index c34616a..3e87350 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -877,6 +877,7 @@ box.session.on_disconnect(nil, on_disconnect) -- box.schema.user.grant('guest', 'write', 'space', '_space') box.schema.user.grant('guest', 'write', 'space', '_schema') +box.schema.user.grant('guest', 'create', 'universe') count = 0 function create_space(name) count = count + 1 box.schema.create_space(name) return true end c = net.connect(box.cfg.listen) @@ -891,6 +892,7 @@ box.space.test2:drop() box.space.test3:drop() box.schema.user.revoke('guest', 'write', 'space', '_space') box.schema.user.revoke('guest', 'write', 'space', '_schema') +box.schema.user.revoke('guest', 'create', 'universe') c:close() -- diff --git a/test/box/role.result b/test/box/role.result index 806cea9..38dc43e 100644 --- a/test/box/role.result +++ b/test/box/role.result @@ -671,7 +671,7 @@ box.session.su('admin') _ = box.schema.space.create('test') --- ... -box.schema.user.grant('john', 'read,write,execute', 'universe') +box.schema.user.grant('john', 'read', 'universe') --- ... box.session.su('john') @@ -695,14 +695,33 @@ box.schema.user.grant('grantee', 'public') - error: User 'grantee' already has role 'public' ... -- --- revoking role 'public' is another deal - only the --- superuser can do that, and even that would be useless, +-- revoking role 'public' is another deal: +-- the superuser or creator of user can do that, and even that would be useless, -- since one can still re-grant it back to oneself. -- box.schema.user.revoke('grantee', 'public') --- - error: Revoke access to role 'public' is denied for user 'john' ... +box.session.su("admin") +--- +... +box.schema.user.grant("john", "create", 'universe') +--- +... +box.session.su('john') +--- +... +box.schema.user.create("grantee2") +--- +... +-- must be ok +box.schema.user.revoke('grantee2', 'public') +--- +... +box.schema.user.drop('grantee2') +--- +... box.session.su('admin') --- ... diff --git a/test/box/role.test.lua b/test/box/role.test.lua index e97339f..abdd1b1 100644 --- a/test/box/role.test.lua +++ b/test/box/role.test.lua @@ -261,7 +261,7 @@ box.schema.user.grant('grantee', 'role') -- box.session.su('admin') _ = box.schema.space.create('test') -box.schema.user.grant('john', 'read,write,execute', 'universe') +box.schema.user.grant('john', 'read', 'universe') box.session.su('john') box.schema.user.grant('grantee', 'role') box.schema.user.grant('grantee', 'read', 'space', 'test') @@ -272,11 +272,18 @@ box.schema.user.grant('grantee', 'read', 'space', 'test') -- box.schema.user.grant('grantee', 'public') -- --- revoking role 'public' is another deal - only the --- superuser can do that, and even that would be useless, +-- revoking role 'public' is another deal: +-- the superuser or creator of user can do that, and even that would be useless, -- since one can still re-grant it back to oneself. -- box.schema.user.revoke('grantee', 'public') +box.session.su("admin") +box.schema.user.grant("john", "create", 'universe') +box.session.su('john') +box.schema.user.create("grantee2") +-- must be ok +box.schema.user.revoke('grantee2', 'public') +box.schema.user.drop('grantee2') box.session.su('admin') box.schema.user.drop('john') diff --git a/test/box/sequence.result b/test/box/sequence.result index f0164b6..91cd1a9 100644 --- a/test/box/sequence.result +++ b/test/box/sequence.result @@ -1451,7 +1451,7 @@ box.session.su('admin') --- ... -- A user cannot alter sequences created by other users. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read', 'universe') --- ... box.session.su('user') @@ -1471,6 +1471,9 @@ box.session.su('admin') sq:drop() --- ... +box.schema.user.grant('user', 'create', 'universe') +--- +... -- A user can alter/use sequences that he owns. box.session.su('user') --- @@ -1522,7 +1525,7 @@ s1 = box.schema.space.create('space1') _ = s1:create_index('pk') --- ... -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read, create', 'universe') --- ... box.session.su('user') @@ -1536,15 +1539,14 @@ s2 = box.schema.space.create('space2') ... _ = s2:create_index('pk', {sequence = 'seq1'}) -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' ... s1.index.pk:alter({sequence = 'seq1'}) -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to space 'space1' is denied for user 'user' ... box.space._space_sequence:replace{s1.id, sq1.id, false} -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to space 'space1' is denied for user 'user' ... box.space._space_sequence:replace{s1.id, sq2.id, false} -- error --- @@ -1552,7 +1554,7 @@ box.space._space_sequence:replace{s1.id, sq2.id, false} -- error ... box.space._space_sequence:replace{s2.id, sq1.id, false} -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to sequence 'seq1' is denied for user 'user' ... s2.index.pk:alter({sequence = 'seq2'}) -- ok --- @@ -1563,7 +1565,7 @@ box.session.su('admin') -- If the user owns a sequence attached to a space, -- it can use it for auto increment, otherwise it -- needs privileges. -box.schema.user.revoke('user', 'read,write', 'universe') +box.schema.user.revoke('user', 'read,create', 'universe') --- ... box.session.su('user') @@ -1701,10 +1703,10 @@ box.schema.user.create('user1') box.schema.user.create('user2') --- ... -box.schema.user.grant('user1', 'read,write', 'universe') +box.schema.user.grant('user1', 'read, create', 'universe') --- ... -box.schema.user.grant('user2', 'read,write', 'universe') +box.schema.user.grant('user2', 'read', 'universe') --- ... box.session.su('user1') diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua index af3432f..e0c0fb6 100644 --- a/test/box/sequence.test.lua +++ b/test/box/sequence.test.lua @@ -482,12 +482,13 @@ sq:reset() -- error box.session.su('admin') -- A user cannot alter sequences created by other users. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read', 'universe') box.session.su('user') sq:alter{step = 2} -- error sq:drop() -- error box.session.su('admin') sq:drop() +box.schema.user.grant('user', 'create', 'universe') -- A user can alter/use sequences that he owns. box.session.su('user') @@ -508,7 +509,7 @@ sq:drop() sq1 = box.schema.sequence.create('seq1') s1 = box.schema.space.create('space1') _ = s1:create_index('pk') -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read, create', 'universe') box.session.su('user') sq2 = box.schema.sequence.create('seq2') s2 = box.schema.space.create('space2') @@ -523,7 +524,7 @@ box.session.su('admin') -- If the user owns a sequence attached to a space, -- it can use it for auto increment, otherwise it -- needs privileges. -box.schema.user.revoke('user', 'read,write', 'universe') +box.schema.user.revoke('user', 'read,create', 'universe') box.session.su('user') s2:insert{nil, 1} -- ok: {1, 1} box.session.su('admin') @@ -571,8 +572,8 @@ box.sequence -- to a sequence. box.schema.user.create('user1') box.schema.user.create('user2') -box.schema.user.grant('user1', 'read,write', 'universe') -box.schema.user.grant('user2', 'read,write', 'universe') +box.schema.user.grant('user1', 'read, create', 'universe') +box.schema.user.grant('user2', 'read', 'universe') box.session.su('user1') sq = box.schema.sequence.create('test') box.session.su('user2') diff --git a/test/engine/truncate.result b/test/engine/truncate.result index 3ad400e..33ff70a 100644 --- a/test/engine/truncate.result +++ b/test/engine/truncate.result @@ -506,7 +506,7 @@ con = require('net.box').connect(box.cfg.listen) ... con:eval([[box.space.access_truncate:truncate()]]) --- -- error: Write access to space 'access_truncate' is denied for user 'guest' +- error: Drop access to space 'access_truncate' is denied for user 'guest' ... con.space.access_truncate:select() --- -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [security 2/2] security: Refactor system space access checks 2018-05-16 12:37 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov @ 2018-05-16 19:27 ` Konstantin Osipov 2018-05-17 16:15 ` [tarantool-patches] [security 0/2] Access control lists Ilya Markov 0 siblings, 1 reply; 8+ messages in thread From: Konstantin Osipov @ 2018-05-16 19:27 UTC (permalink / raw) To: Ilya Markov; +Cc: georgy, tarantool-patches * Ilya Markov <imarkov@tarantool.org> [18/05/16 15:39]: > /* > - * XXX: pre 1.7.7 there was no specific 'CREATE' or > - * 'ALTER' ACL, instead, read and write access on universe > - * was used to allow create/alter. > - * For backward compatibility, if a user has read and write > - * access on the universe, grant it CREATE access > - * automatically. > - * The legacy fix does not affect sequences since they > - * were added in 1.7.7 only. > + * If a user has write access on the universe, > + * grant it CREATE, DROP, ALTER access automatically. > */ > - if (is_17_compat_mode && has_access & PRIV_R && has_access & PRIV_W) > - has_access |= PRIV_C | PRIV_A; > + if (has_access & PRIV_W) > + has_access |= PRIV_C | PRIV_A | PRIV_D; > I see no point in such automatic grant other than 1.7.7 compatibility. > - user_access_t access = ((PRIV_U | (user_access_t) priv_type) & > - ~has_access); > + user_access_t access = ((user_access_t) priv_type & ~has_access); > bool is_owner = owner_uid == cr->uid || cr->uid == ADMIN; > /* > * Only the owner of the object or someone who has > - * specific DDL privilege on the object can execute > - * DDL. If a user has no USAGE access and is owner, > - * deny access as well. > + * specific DDL privilege on the object can execute DDL. > */ > - if (access == 0 || (is_owner && !(access & PRIV_U))) > + if (access == 0 || is_owner) I don't understand this change. Why did 'usage' disappear? Even the owner of the object can do nothing if they have no 'usage' access. > access_check_ddl(old_space->def->name, old_space->def->uid, SC_SPACE, > - priv_type, true); > + priv_type, BOX_SPACE_ID); Why did you need these changes? Could you describe in the changeset comment the idea of this patch? > /* > - * Check if a write privilege was given, raise an error if not. > + * Perform checks specific for space. Then perform usual check_ddl. > */ > - access_check_space_xc(old_space, PRIV_W); > + credentials *cr = effective_user(); > + uint32_t has_access = cr->universal_access | > + old_space->access[cr->auth_token].effective; > + if (!(has_access & (PRIV_W | PRIV_D))) { > + access_check_ddl(old_space->def->name, old_space->def->uid, > + SC_SPACE, > + PRIV_D, BOX_SPACE_ID); > + } (stopped the review) This is a huge patch. Any chance of splitting it? -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [security 0/2] Access control lists 2018-05-16 19:27 ` [tarantool-patches] " Konstantin Osipov @ 2018-05-17 16:15 ` Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov 0 siblings, 2 replies; 8+ messages in thread From: Ilya Markov @ 2018-05-17 16:15 UTC (permalink / raw) To: kostja; +Cc: georgy, tarantool-patches Ilya Markov (2): security: Refactor reads from systems spaces security: Refactor system space access checks branch: gh-3250-system-space-access src/box/alter.cc | 156 ++++++++++++++++++----------- src/box/lua/schema.lua | 97 ++++++++++-------- src/box/schema.cc | 90 +++++++++++++++++ src/box/schema.h | 10 ++ src/box/space.c | 3 +- src/box/space.h | 17 +++- src/box/sysview_engine.c | 1 + src/box/sysview_index.c | 78 ++++++++++----- test/box/access.result | 192 +++++++++++++++++++++++++++++------- test/box/access.test.lua | 86 +++++++++++----- test/box/access_bin.result | 2 +- test/box/access_bin.test.lua | 2 +- test/box/access_escalation.result | 41 +++++++- test/box/access_escalation.test.lua | 17 +++- test/box/access_misc.result | 76 ++++++++++++-- test/box/access_misc.test.lua | 24 ++++- test/box/access_sysview.result | 11 ++- test/box/access_sysview.test.lua | 6 +- test/box/ddl.result | 4 + test/box/ddl.test.lua | 2 + test/box/net.box.result | 10 +- test/box/net.box.test.lua | 6 +- test/box/on_replace.result | 8 +- test/box/role.result | 27 ++++- test/box/role.test.lua | 13 ++- test/box/sequence.result | 22 +++-- test/box/sequence.test.lua | 13 +-- test/box/transaction.result | 18 +++- test/box/transaction.test.lua | 4 + test/engine/iterator.result | 2 +- test/engine/savepoint.result | 12 +-- test/engine/truncate.result | 2 +- 32 files changed, 793 insertions(+), 259 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces 2018-05-17 16:15 ` [tarantool-patches] [security 0/2] Access control lists Ilya Markov @ 2018-05-17 16:15 ` Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov 1 sibling, 0 replies; 8+ messages in thread From: Ilya Markov @ 2018-05-17 16:15 UTC (permalink / raw) To: kostja; +Cc: georgy, tarantool-patches 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 | 97 ++++++++++++++------------ src/box/sysview_index.c | 78 ++++++++++++++------- test/box/access.result | 145 ++++++++++++++++++++++++++++++++------- test/box/access.test.lua | 67 ++++++++++++------ test/box/access_bin.result | 2 +- test/box/access_bin.test.lua | 2 +- test/box/access_misc.result | 14 ++++ test/box/access_misc.test.lua | 9 ++- test/box/access_sysview.result | 11 +-- test/box/access_sysview.test.lua | 6 +- test/box/ddl.result | 4 ++ test/box/ddl.test.lua | 2 + test/box/on_replace.result | 8 +-- test/box/role.result | 2 +- test/box/transaction.result | 18 ++++- test/box/transaction.test.lua | 4 ++ test/engine/iterator.result | 2 +- test/engine/savepoint.result | 12 ++-- 18 files changed, 347 insertions(+), 136 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 1a616d5..677325c 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -104,12 +104,12 @@ ffi.cdef[[ ]] local function user_or_role_resolve(user) - local _user = box.space[box.schema.VUSER_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 @@ -118,12 +118,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 @@ -133,12 +133,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 @@ -148,12 +148,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 @@ -164,8 +164,9 @@ end -- Revoke all privileges associated with the given object. local function revoke_object_privs(object_type, object_id) + local _vpriv = box.space[box.schema.VPRIV_ID] local _priv = box.space[box.schema.PRIV_ID] - local privs = _priv.index.object:select{object_type, object_id} + local privs = _vpriv.index.object:select{object_type, object_id} for k, tuple in pairs(privs) do local uid = tuple[2] _priv:delete{uid, object_type, object_id} @@ -431,10 +432,15 @@ end -- space format - the metadata about space fields function box.schema.space.format(id, format) local _space = box.space._space + local _vspace = box.space._vspace check_param(id, 'id', 'number') if format == nil then - return _space:get(id)[7] + local tuple = _vspace:get(id) + if tuple == nil then + box.error(box.error.NO_SUCH_SPACE, '#' .. tostring(id)) + end + return tuple[7] else check_param(format, 'format', 'table') format = update_format(format) @@ -450,6 +456,7 @@ box.schema.space.drop = function(space_id, space_name, opts) check_param_table(opts, { if_exists = 'boolean' }) local _space = box.space[box.schema.SPACE_ID] local _index = box.space[box.schema.INDEX_ID] + local _vindex = box.space[box.schema.VINDEX_ID] local _truncate = box.space[box.schema.TRUNCATE_ID] local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID] local sequence_tuple = _space_sequence:delete{space_id} @@ -457,7 +464,7 @@ box.schema.space.drop = function(space_id, space_name, opts) -- Delete automatically generated sequence. box.schema.sequence.drop(sequence_tuple[2]) end - local keys = _index:select(space_id) + local keys = _vindex:select(space_id) for i = #keys, 1, -1 do local v = keys[i] _index:delete{v[1], v[2]} @@ -695,7 +702,8 @@ box.schema.index.create = function(space_id, name, options) options = update_param_table(options, options_defaults) local _index = box.space[box.schema.INDEX_ID] - if _index.index.name:get{space_id, name} then + local _vindex = box.space[box.schema.VINDEX_ID] + if _vindex.index.name:get{space_id, name} then if options.if_not_exists then return space.index[name], "not created" else @@ -708,7 +716,7 @@ box.schema.index.create = function(space_id, name, options) iid = options.id else -- max - local tuple = _index.index[0] + local tuple = _vindex.index[0] :select(space_id, { limit = 1, iterator = 'LE' })[1] if tuple then local id = tuple[1] @@ -1741,12 +1749,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] @@ -1762,12 +1770,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] @@ -1785,13 +1793,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 @@ -1805,7 +1813,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) @@ -1822,12 +1831,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] @@ -1843,12 +1853,12 @@ box.schema.func.drop = function(name, opts) end function box.schema.func.exists(name_or_id) - local _func = box.space[box.schema.VFUNC_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 @@ -2003,8 +2013,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] @@ -2039,7 +2050,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 @@ -2068,32 +2080,32 @@ 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 - local sequences = box.space[box.schema.SEQUENCE_ID].index.owner:select{uid} + local sequences = box.space[box.schema.VSEQUENCE_ID].index.owner:select{uid} for k, tuple in pairs(sequences) do box.schema.sequence.drop(tuple[1]) end -- 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 @@ -2150,8 +2162,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/src/box/sysview_index.c b/src/box/sysview_index.c index 8bfc39b..2fb1f76 100644 --- a/src/box/sysview_index.c +++ b/src/box/sysview_index.c @@ -197,15 +197,35 @@ static const struct index_vtab sysview_index_vtab = { /* .end_build = */ generic_index_end_build, }; +/* + * System view filters. + * Filter must give read access to object, if one of the following conditions is true: + * 1. User has read, write, drop or alter access to universe. + * 2. User has read access to according system space. + * 3. User has read, write, drop or alter access to object. + * 4. User is an owner of the object. + * 5. Other specific conditions of the object of object type. For example, + * role Public can be seen by anyone. Execute privilege on function must + * allow to see the function in _vfunc space. + * In case of _vpriv, users who have read, write(for 1.7 compatibility), + * drop access to universe may need to delete some privileges of the object + * they are going to drop. + */ + +/* + * Constant-group of privileges for checks on the 3rd condition. + */ +const uint32_t PRIV_WRDA = PRIV_W | PRIV_D | PRIV_A | PRIV_R; + static bool vspace_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ + /* If user has global alter, drop privilege she may access all spaces. */ + if (cr->universal_access & PRIV_WRDA) + return true; if (PRIV_R & source->access[cr->auth_token].effective) - return true; /* read access to _space space */ - + return true; /* read access to _space space. */ uint32_t space_id; if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &space_id) != 0) return false; @@ -213,7 +233,7 @@ vspace_filter(struct space *source, struct tuple *tuple) if (space == NULL) return false; user_access_t effective = space->access[cr->auth_token].effective; - return ((PRIV_R | PRIV_W) & (cr->universal_access | effective) || + return (PRIV_WRDA & effective || space->def->uid == cr->uid); } @@ -221,10 +241,13 @@ static bool vuser_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ + /* If users have global alter, drop privilege + * they may access all users. + */ + if (cr->universal_access & PRIV_WRDA) + return true; if (PRIV_R & source->access[cr->auth_token].effective) - return true; /* read access to _user space */ + return true; /* read access to _user space. */ uint32_t uid; if (tuple_field_u32(tuple, BOX_USER_FIELD_ID, &uid) != 0) @@ -232,15 +255,20 @@ vuser_filter(struct space *source, struct tuple *tuple) uint32_t owner_id; if (tuple_field_u32(tuple, BOX_USER_FIELD_UID, &owner_id) != 0) return false; - return uid == cr->uid || owner_id == cr->uid; + /* Anyone can see PUBLIC sytem role. */ + return uid == cr->uid || owner_id == cr->uid || uid == PUBLIC; } static bool vpriv_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if (PRIV_R & cr->universal_access) - return true; /* read access to universe */ + /* + * Users who have write or drop privilege may alter or drop objects. + * To do that, they have to be able to read privileges of that user. + */ + if (cr->universal_access & (PRIV_W | PRIV_D | PRIV_R)) + return true; if (PRIV_R & source->access[cr->auth_token].effective) return true; /* read access to _priv space */ @@ -250,6 +278,10 @@ vpriv_filter(struct space *source, struct tuple *tuple) uint32_t grantee_id; if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &grantee_id) != 0) return false; + /* Users may read privilege if they are grantor or grantee. + * TODO: With introducing of grant option, users must see also + * privileges on the objects they own. + */ return grantor_id == cr->uid || grantee_id == cr->uid; } @@ -257,29 +289,29 @@ static bool vfunc_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if ((PRIV_R | PRIV_X) & cr->universal_access) - return true; /* read or execute access to universe */ + if (cr->universal_access & (PRIV_WRDA | PRIV_X)) + return true; if (PRIV_R & source->access[cr->auth_token].effective) return true; /* read access to _func space */ - const char *name = tuple_field_cstr(tuple, BOX_FUNC_FIELD_NAME); + uint32_t name_len; + const char *name = tuple_field_str(tuple, BOX_FUNC_FIELD_NAME, + &name_len); if (name == NULL) return false; - uint32_t name_len = strlen(name); struct func *func = func_by_name(name, name_len); assert(func != NULL); user_access_t effective = func->access[cr->auth_token].effective; - if (func->def->uid == cr->uid || (PRIV_X & effective)) - return true; - return false; + return func->def->uid == cr->uid || + ((PRIV_WRDA | PRIV_X) & effective); } static bool vsequence_filter(struct space *source, struct tuple *tuple) { struct credentials *cr = effective_user(); - if ((PRIV_R | PRIV_X) & cr->universal_access) - return true; /* read or execute access to universe */ + if (cr->universal_access & PRIV_WRDA) + return true; if (PRIV_R & source->access[cr->auth_token].effective) return true; /* read access to _sequence space */ @@ -290,12 +322,10 @@ vsequence_filter(struct space *source, struct tuple *tuple) if (sequence == NULL) return false; user_access_t effective = sequence->access[cr->auth_token].effective; - if (sequence->def->uid == cr->uid || ((PRIV_W | PRIV_R) & effective)) - return true; - return false; + return sequence->def->uid == cr->uid || + (PRIV_WRDA & effective); } - struct sysview_index * sysview_index_new(struct sysview_engine *sysview, struct index_def *def, const char *space_name) diff --git a/test/box/access.result b/test/box/access.result index 5c39bc9..131a215 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') --- @@ -1357,28 +1358,59 @@ box.schema.user.create("tester") s = box.schema.space.create("test") --- ... +_ = s:create_index("primary") +--- +... +seq = box.schema.sequence.create("test") +--- +... u = box.schema.user.create("test") --- ... f = box.schema.func.create("test") --- ... -box.schema.user.grant("tester", "read,execute", "universe") +-- failed create, auto_increment requires read. +box.session.su("tester") +--- +... +box.schema.space.create("test_space") +--- +- error: Write access to space '_schema' is denied for user 'tester' +... +box.schema.user.create('test_user') +--- +- error: Read access to space '_user' is denied for user 'tester' +... +box.schema.func.create('test_func') +--- +- error: Read access to space '_func' is denied for user 'tester' +... +box.session.su("admin") +--- +... +box.schema.user.grant("tester", "read", "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 +1419,36 @@ 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') +_ = s1:create_index("primary") --- ... -_ = box.session.su("tester", box.schema.func.create, 'test_func') +_ = box.schema.user.create('test_user') +--- +... +_ = box.schema.func.create('test_func') +--- +... +seq1 = box.schema.sequence.create('test_seq') --- ... -- successful drop of owned objects -_ = box.session.su("tester", s1.drop, s1) +s1:drop() --- ... -_ = box.session.su("tester", box.schema.user.drop, 'test_user') +seq1:drop() --- ... -_ = box.session.su("tester", box.schema.func.drop, 'test_func') +box.schema.user.drop('test_user') +--- +... +box.schema.func.drop('test_func') --- ... -- failed alter @@ -1414,22 +1458,24 @@ _ = 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") +s:drop() --- +- error: Drop access to space 'test' is denied for user 'tester' +... +seq:drop() +--- +- error: Drop access to sequence 'test' is denied for user 'tester' ... box.schema.user.drop("test") --- - error: Revoke access to role 'public' is denied for user 'tester' ... -box.session.su("admin") +box.schema.func.drop("test") --- +- error: Drop access to function 'test' is denied for user 'tester' ... -box.session.su("tester", box.schema.func.drop, "test") +box.session.su("admin") --- -- error: Drop access to function 'test' is denied for user 'tester' ... box.schema.user.grant("tester", "drop", "universe") --- @@ -1438,6 +1484,9 @@ box.schema.user.grant("tester", "drop", "universe") box.session.su("tester", s.drop, s) --- ... +box.session.su("tester", seq.drop, seq) +--- +... box.session.su("tester", box.schema.user.drop, "test") --- ... @@ -1510,36 +1559,86 @@ box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- --- create an object, but 'guest' still has no access to it +-- create an objects, but 'guest' still has no access to them -- box.session.su('admin', box.schema.func.create, 'test') --- ... +s = box.session.su('admin', box.schema.space.create, 'test') +--- +... +_ = box.session.su('admin', box.schema.sequence.create, 'test') +--- +... box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- -- grant access, the object should become visible to guest -- box.session.su('admin', box.schema.user.grant, 'guest', 'execute', 'function', 'test') --- ... +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'space', 'test') +--- +... +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'sequence', 'test') +--- +... box.schema.func.exists('test') --- - true ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- true +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- true +... -- --- drop object +-- drop objects -- box.session.su('admin', box.schema.func.drop, 'test') --- ... +box.session.su('admin', s.drop, s) +--- +... +box.session.su('admin', box.schema.sequence.drop, 'test') +--- +... box.schema.func.exists('test') --- - false ... +box.space._vspace.index.name:get{"test"} ~= nil +--- +- false +... +box.space._vsequence.index.name:get{"test"} ~= nil +--- +- false +... -- -- restore -- diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 501c766..4bd34e4 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -509,14 +509,24 @@ s:drop() -- box.schema.user.create("tester") s = box.schema.space.create("test") +_ = s:create_index("primary") +seq = box.schema.sequence.create("test") u = box.schema.user.create("test") f = box.schema.func.create("test") -box.schema.user.grant("tester", "read,execute", "universe") +-- failed create, auto_increment requires read. +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") +box.schema.user.grant("tester", "read", "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 +534,19 @@ 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") +_ = s1:create_index("primary") +_ = box.schema.user.create('test_user') +_ = box.schema.func.create('test_func') +seq1 = box.schema.sequence.create('test_seq') -- 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() +seq1: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"}) @@ -543,19 +557,16 @@ _ = 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") +s:drop() +seq:drop() box.schema.user.drop("test") -box.session.su("admin") - -box.session.su("tester", box.schema.func.drop, "test") +box.schema.func.drop("test") +box.session.su("admin") box.schema.user.grant("tester", "drop", "universe") -- successful drop box.session.su("tester", s.drop, s) +box.session.su("tester", seq.drop, seq) box.session.su("tester", box.schema.user.drop, "test") box.session.su("tester", box.schema.func.drop, "test") @@ -589,22 +600,36 @@ box.session.su('guest') -- has no access to the function -- box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- --- create an object, but 'guest' still has no access to it +-- create an objects, but 'guest' still has no access to them -- box.session.su('admin', box.schema.func.create, 'test') +s = box.session.su('admin', box.schema.space.create, 'test') +_ = box.session.su('admin', box.schema.sequence.create, 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- -- grant access, the object should become visible to guest -- box.session.su('admin', box.schema.user.grant, 'guest', 'execute', 'function', 'test') +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'space', 'test') +box.session.su('admin', box.schema.user.grant, 'guest', 'read', 'sequence', 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- --- drop object +-- drop objects -- box.session.su('admin', box.schema.func.drop, 'test') +box.session.su('admin', s.drop, s) +box.session.su('admin', box.schema.sequence.drop, 'test') box.schema.func.exists('test') +box.space._vspace.index.name:get{"test"} ~= nil +box.space._vsequence.index.name:get{"test"} ~= nil -- -- restore -- -box.session.su('admin') +box.session.su('admin') \ No newline at end of file diff --git a/test/box/access_bin.result b/test/box/access_bin.result index b81279c..7b30d11 100644 --- a/test/box/access_bin.result +++ b/test/box/access_bin.result @@ -344,7 +344,7 @@ box.session.su('admin', box.session.user()) --- - error: 'bad argument #2 to ''?'' (function expected, got string)' ... --- clenaup +-- cleanup box.session.su('admin') --- ... diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua index cb3a50c..4c7a6d0 100644 --- a/test/box/access_bin.test.lua +++ b/test/box/access_bin.test.lua @@ -131,5 +131,5 @@ box.schema.func.drop('f2') -- box.session.su('admin', box.session.user) box.session.su('admin', box.session.user()) --- clenaup +-- cleanup box.session.su('admin') diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 3a56a4c..8bf99f2 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -175,6 +175,17 @@ gs = box.schema.space.create('guest_space') --- - error: Write access to space '_schema' is denied for user 'guest' ... +-- +-- FIXME: object create calls system space auto_increment, which requires +-- read and write privileges. Create privilege must solve this. +-- +box.schema.func.create('guest_func') +--- +- error: Read access to space '_func' is denied for user 'guest' +... +session.su('admin', box.schema.user.grant, "guest", "read", "universe") +--- +... box.schema.func.create('guest_func') --- - error: Read access to space '_func' is denied for user 'guest' @@ -182,6 +193,9 @@ box.schema.func.create('guest_func') session.su('admin') --- ... +box.schema.user.revoke("guest", "read", "universe") +--- +... s:select() --- - - [2] diff --git a/test/box/access_misc.test.lua b/test/box/access_misc.test.lua index cf6447e..27064c4 100644 --- a/test/box/access_misc.test.lua +++ b/test/box/access_misc.test.lua @@ -70,9 +70,16 @@ s:insert({4}) s:delete({3}) s:drop() gs = box.schema.space.create('guest_space') +-- +-- FIXME: object create calls system space auto_increment, which requires +-- read and write privileges. Create privilege must solve this. +-- +box.schema.func.create('guest_func') +session.su('admin', box.schema.user.grant, "guest", "read", "universe") box.schema.func.create('guest_func') - session.su('admin') +box.schema.user.revoke("guest", "read", "universe") + s:select() -- -- Create user with universe read&write grants diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result index 340ed21..20efd2b 100644 --- a/test/box/access_sysview.result +++ b/test/box/access_sysview.result @@ -266,11 +266,11 @@ box.session.su('guest') ... #box.space._vuser:select{} --- -- 1 +- 5 ... #box.space._vpriv:select{} --- -- 2 +- 15 ... #box.space._vfunc:select{} --- @@ -343,7 +343,7 @@ box.session.su('guest') -- _vuser -- -- a guest user can read information about itself -t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest' +t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false --- - true ... @@ -526,8 +526,9 @@ box.schema.user.grant('guest', 'execute', 'function', 'test') box.session.su('guest') --- ... -#box.space._vfunc:select{} = cnt + 1 +#box.space._vfunc:select{} == func_cnt --- +- true ... box.session.su('admin') --- @@ -551,7 +552,7 @@ box.schema.user.grant('guest', 'execute', 'universe') box.session.su('guest') --- ... -#box.space._vfunc:select{} == cnt + 1 +#box.space._vfunc:select{} == func_cnt --- - true ... diff --git a/test/box/access_sysview.test.lua b/test/box/access_sysview.test.lua index 7955ffc..4cc5611 100644 --- a/test/box/access_sysview.test.lua +++ b/test/box/access_sysview.test.lua @@ -134,7 +134,7 @@ box.session.su('guest') -- -- a guest user can read information about itself -t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest' +t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false -- read access to original space also allow to read a view box.session.su('admin') @@ -218,7 +218,7 @@ box.session.su('admin') box.schema.user.grant('guest', 'execute', 'function', 'test') box.session.su('guest') -#box.space._vfunc:select{} = cnt + 1 +#box.space._vfunc:select{} == func_cnt box.session.su('admin') box.schema.user.revoke('guest', 'execute', 'function', 'test') @@ -230,7 +230,7 @@ box.session.su('admin') box.schema.user.grant('guest', 'execute', 'universe') box.session.su('guest') -#box.space._vfunc:select{} == cnt + 1 +#box.space._vfunc:select{} == func_cnt box.session.su('admin') box.schema.user.revoke('guest', 'execute', 'universe') diff --git a/test/box/ddl.result b/test/box/ddl.result index f249f8f..15faa9a 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -565,6 +565,10 @@ test_run:cmd("setopt delimiter ''"); --- - true ... +-- finish transaction +box.rollback() +--- +... _ = c:get() --- ... diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua index 6029c6e..93501bd 100644 --- a/test/box/ddl.test.lua +++ b/test/box/ddl.test.lua @@ -226,6 +226,8 @@ test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); test_run:cmd("setopt delimiter ''"); +-- finish transaction +box.rollback() _ = c:get() test_latch:drop() -- this is where everything stops diff --git a/test/box/on_replace.result b/test/box/on_replace.result index a961df1..8c52e12 100644 --- a/test/box/on_replace.result +++ b/test/box/on_replace.result @@ -471,7 +471,7 @@ t = s:on_replace(function () s:create_index('sec') end, t) ... s:replace({2, 3}) --- -- error: Space _index 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.create('newu') end, t) --- @@ -506,21 +506,21 @@ t = s:on_replace(function () s:drop() end, t) ... s:replace({5, 6}) --- -- error: DDL does not support multi-statement transactions +- error: A multi-statement transaction can not use multiple storage engines ... 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 0580331..2fb1abf 100644 --- a/test/box/transaction.result +++ b/test/box/transaction.result @@ -69,7 +69,7 @@ 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(); --- @@ -83,7 +83,7 @@ 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(); --- @@ -95,6 +95,20 @@ box.begin() box.space._user:delete{box.schema.GUEST_ID}; box.rollback(); --- ... +box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; +--- +- error: DDL does not support multi-statement transactions +... +box.rollback(); +--- +... +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; +--- +- error: Space _sequence does not support multi-statement transactions +... +box.rollback(); +--- +... box.begin() box.space._schema:insert{'test'}; --- - error: Space _schema does not support multi-statement transactions diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua index ca6de44..14d1a69 100644 --- a/test/box/transaction.test.lua +++ b/test/box/transaction.test.lua @@ -38,6 +38,10 @@ box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv'); box.rollback(); box.begin() box.space._user:delete{box.schema.GUEST_ID}; box.rollback(); +box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; +box.rollback(); +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; +box.rollback(); box.begin() box.space._schema:insert{'test'}; box.rollback(); box.begin() box.space._cluster:insert{123456789, 'abc'}; diff --git a/test/engine/iterator.result b/test/engine/iterator.result index 423ed0b..ae14c43 100644 --- a/test/engine/iterator.result +++ b/test/engine/iterator.result @@ -4211,7 +4211,7 @@ s:replace{35} ... state, value = gen(param,state) --- -- error: 'builtin/box/schema.lua:985: usage: next(param, state)' +- error: 'builtin/box/schema.lua:993: usage: next(param, state)' ... value --- diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result index d440efa..dc2ad79 100644 --- a/test/engine/savepoint.result +++ b/test/engine/savepoint.result @@ -14,7 +14,7 @@ s1 = box.savepoint() ... box.rollback_to_savepoint(s1) --- -- error: 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- error: 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... box.begin() s1 = box.savepoint() --- @@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''"); ok1, errmsg1 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok2, errmsg2 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok3, errmsg3 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok4, errmsg4 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... ok5, errmsg5 --- - false -- 'builtin/box/schema.lua:300: Usage: box.rollback_to_savepoint(savepoint)' +- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)' ... s:select{} --- -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [security 2/2] security: Refactor system space access checks 2018-05-17 16:15 ` [tarantool-patches] [security 0/2] Access control lists Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov @ 2018-05-17 16:15 ` Ilya Markov 1 sibling, 0 replies; 8+ messages in thread From: Ilya Markov @ 2018-05-17 16:15 UTC (permalink / raw) To: kostja; +Cc: georgy, tarantool-patches Processes of creating, dropping objects contain work with system spaces which so far must be available only to admin or user who has write access to universe. This patch removes the need of write accesses to universe. Introduce access_check virtual function for space objects. The patch separates access checks of system and user spaces. The access_check functions of system spaces verifies mostly usage and read access, if needed. All specific checks on write to system spaces(in fact ddl checks) are performed in on_replace triggers of that spaces. For backward compatibility, write and read on universe are equivalent to create, alter, drop privileges. Refactor tests with changing write to universe privilege to create/drop ones. Follow-up #945 In scope of #3250 --- src/box/alter.cc | 156 ++++++++++++++++++++++-------------- src/box/schema.cc | 90 +++++++++++++++++++++ src/box/schema.h | 10 +++ src/box/space.c | 3 +- src/box/space.h | 17 +++- src/box/sysview_engine.c | 1 + test/box/access.result | 49 ++++++++--- test/box/access.test.lua | 29 ++++--- test/box/access_escalation.result | 41 +++++++++- test/box/access_escalation.test.lua | 17 +++- test/box/access_misc.result | 82 ++++++++++++++----- test/box/access_misc.test.lua | 27 +++++-- test/box/net.box.result | 10 +-- test/box/net.box.test.lua | 6 +- test/box/role.result | 25 +++++- test/box/role.test.lua | 13 ++- test/box/sequence.result | 22 ++--- test/box/sequence.test.lua | 13 +-- test/engine/truncate.result | 2 +- 19 files changed, 468 insertions(+), 145 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 8766c81..f26ca1b 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -80,34 +80,22 @@ access_check_ddl(const char *name, uint32_t owner_uid, * were added in 1.7.7 only. */ if (is_17_compat_mode && has_access & PRIV_R && has_access & PRIV_W) - has_access |= PRIV_C | PRIV_A; - - user_access_t access = ((PRIV_U | (user_access_t) priv_type) & - ~has_access); + has_access |= PRIV_C | PRIV_A | PRIV_D; + user_access_t access = ((user_access_t) priv_type & ~has_access); bool is_owner = owner_uid == cr->uid || cr->uid == ADMIN; /* * Only the owner of the object or someone who has - * specific DDL privilege on the object can execute - * DDL. If a user has no USAGE access and is owner, - * deny access as well. + * specific DDL privilege on the object can execute DDL. */ - if (access == 0 || (is_owner && !(access & PRIV_U))) + if (access == 0 || is_owner) return; /* Access granted. */ struct user *user = user_find_xc(cr->uid); - if (is_owner) { - tnt_raise(AccessDeniedError, - priv_name(PRIV_U), - schema_object_name(SC_UNIVERSE), - "", - user->def->name); - } else { - tnt_raise(AccessDeniedError, - priv_name(access), - schema_object_name(type), - name, - user->def->name); - } + tnt_raise(AccessDeniedError, + priv_name(access), + schema_object_name(type), + name, + user->def->name); } /** @@ -1551,7 +1539,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) struct space_def *def = space_def_new_from_tuple(new_tuple, ER_CREATE_SPACE, region); - access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(def->name, ADMIN, SC_SPACE, PRIV_C, true); auto def_guard = make_scoped_guard([=] { space_def_delete(def); }); RLIST_HEAD(empty_list); @@ -1908,9 +1897,17 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) space_name(old_space)); /* - * Check if a write privilege was given, raise an error if not. + * Perform checks on write or drop specific for space being truncated. + * Then check ddl if the user is owner. */ - access_check_space_xc(old_space, PRIV_W); + credentials *cr = effective_user(); + uint32_t has_access = cr->universal_access | + old_space->access[cr->auth_token].effective; + if (!(has_access & (PRIV_W | PRIV_D))) { + access_check_ddl(old_space->def->name, old_space->def->uid, + SC_SPACE, + PRIV_D, true); + } struct alter_space *alter = alter_space_new(old_space); auto scoped_guard = @@ -2110,7 +2107,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) struct user *old_user = user_by_id(uid); if (new_tuple != NULL && old_user == NULL) { /* INSERT */ struct user_def *user = user_def_new_from_tuple(new_tuple); - access_check_ddl(user->name, user->owner, SC_USER, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(user->name, ADMIN, SC_USER, PRIV_C, true); auto def_guard = make_scoped_guard([=] { free(user); }); (void) user_cache_replace(user); def_guard.is_active = false; @@ -2248,7 +2246,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) struct func *old_func = func_by_id(fid); if (new_tuple != NULL && old_func == NULL) { /* INSERT */ struct func_def *def = func_def_new_from_tuple(new_tuple); - access_check_ddl(def->name, def->uid, SC_FUNCTION, PRIV_C, true); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(def->name, ADMIN, SC_FUNCTION, PRIV_C, true); auto def_guard = make_scoped_guard([=] { free(def); }); func_cache_replace(def); def_guard.is_active = false; @@ -2496,9 +2495,11 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple) static void priv_def_check(struct priv_def *priv, enum priv_type priv_type) { - struct user *grantor = user_find_xc(priv->grantor_id); /* May be a role */ struct user *grantee = user_by_id(priv->grantee_id); + credentials *cr = effective_user(); + struct user *grantor = user_find_xc(cr->uid); + if (grantee == NULL) { tnt_raise(ClientError, ER_NO_SUCH_USER, int2str(priv->grantee_id)); @@ -2561,16 +2562,23 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) int2str(priv->object_id)); } /* - * Only the creator of the role can grant or revoke it. - * Everyone can grant 'PUBLIC' role. + * Only the creator or owner of grantee of the role + * or user with appropriate privileges can grant or revoke it. + * If role is 'PUBLIC' it is allowed to grant for anyone, + * Revoke 'PUBLIC' is allowed only to users who has drop access. + * The latter case is required for dropping users. */ if (role->def->owner != grantor->def->uid && - grantor->def->uid != ADMIN && - (role->def->uid != PUBLIC || priv->access != PRIV_X)) { - tnt_raise(AccessDeniedError, - priv_name(priv_type), - schema_object_name(SC_ROLE), name, - grantor->def->name); + grantor->def->uid != grantee->def->owner && + !((role->def->uid == PUBLIC) && + (((cr->universal_access & PRIV_D) + || priv_type == PRIV_GRANT) + && priv->access == PRIV_X))) { + tnt_raise(AccessDeniedError, + priv_name(priv_type), + schema_object_name(SC_ROLE), + name, + grantor->def->name); } /* Not necessary to do during revoke, but who cares. */ role_check(grantee, role); @@ -2716,31 +2724,60 @@ on_replace_dd_schema(struct trigger * /* trigger */, void *event) struct tuple *new_tuple = stmt->new_tuple; const char *key = tuple_field_cstr_xc(new_tuple ? new_tuple : old_tuple, BOX_SCHEMA_FIELD_KEY); - if (strcmp(key, "cluster") == 0) { - if (new_tuple == NULL) - tnt_raise(ClientError, ER_REPLICASET_UUID_IS_RO); - tt_uuid uu; - tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu); - REPLICASET_UUID = uu; - } else if (strcmp(key, "version") == 0) { - if (new_tuple != NULL) { - uint32_t major, minor, patch; - if (tuple_field_u32(new_tuple, 1, &major) != 0 || - tuple_field_u32(new_tuple, 2, &minor) != 0) - tnt_raise(ClientError, ER_WRONG_DD_VERSION); - /* Version can be major.minor with no patch. */ - if (tuple_field_u32(new_tuple, 3, &patch) != 0) - patch = 0; - dd_version_id = version_id(major, minor, patch); - } else { - assert(old_tuple != NULL); - /* - * _schema:delete({'version'}) for - * example, for box.internal.bootstrap(). - */ - dd_version_id = tarantool_version_id(); + credentials *cr = effective_user(); + /* + * Access check to _schema is following. + * If users have create or write privileges on universe or write on _schema, + * they can't update max_id key. + * Otherwise, simple write check is performed. + */ + uint32_t access = cr->universal_access; + struct space *schema_space = space_by_id(BOX_SCHEMA_ID); + access |= schema_space->access[cr->auth_token].effective; + if (strncmp(key, "max_id", 6) == 0) { + if (!(access & (PRIV_W | PRIV_C))) + goto error; + } else { + if (~access & PRIV_W) + goto error; + else if (strncmp(key, "cluster", 7) == 0) { + if (new_tuple == NULL) + tnt_raise(ClientError, + ER_REPLICASET_UUID_IS_RO); + tt_uuid uu; + tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, + &uu); + REPLICASET_UUID = uu; + } else if (strncmp(key, "version", 7) == 0) { + if (new_tuple != NULL) { + uint32_t major, minor, patch; + if (tuple_field_u32(new_tuple, 1, &major) != + 0 || + tuple_field_u32(new_tuple, 2, &minor) != 0) + tnt_raise(ClientError, + ER_WRONG_DD_VERSION); + /* Version can be major.minor with no patch. */ + if (tuple_field_u32(new_tuple, 3, &patch) != 0) + patch = 0; + dd_version_id = version_id(major, minor, patch); + } else { + assert(old_tuple != NULL); + /* + * _schema:delete({'version'}) for + * example, for box.internal.bootstrap(). + */ + dd_version_id = tarantool_version_id(); + } } } + return; +error: + struct user *user = user_find_xc(cr->uid); + tnt_raise(AccessDeniedError, + priv_name(PRIV_W), + schema_object_name(SC_SPACE), + "_schema", + user->def->name); } /** @@ -2967,6 +3004,9 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) new_def = sequence_def_new_from_tuple(new_tuple, ER_CREATE_SEQUENCE); assert(sequence_by_id(new_def->id) == NULL); + /* When object is not created, we assume ADMIN owns it */ + access_check_ddl(new_def->name, ADMIN, SC_SEQUENCE, + PRIV_C, false); sequence_cache_replace(new_def); alter->new_def = new_def; } else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */ diff --git a/src/box/schema.cc b/src/box/schema.cc index 1b96f97..fc7840f 100644 --- a/src/box/schema.cc +++ b/src/box/schema.cc @@ -37,6 +37,7 @@ #include "scoped_guard.h" #include "version.h" #include "user.h" +#include "session.h" #include <stdio.h> /** * @module Data Dictionary @@ -520,6 +521,70 @@ sequence_cache_delete(uint32_t id) } } + +/** + * Runs simple read, usage checks. + * All other checks are postponed. + */ +static int +sys_read_access_check(struct space *space, user_access_t access) +{ + credentials *cr = effective_user(); + if (~cr->universal_access & PRIV_U) { + struct user *user = user_find(cr->uid); + if (user != NULL) + diag_set(AccessDeniedError, + priv_name(PRIV_U), + schema_object_name(SC_UNIVERSE), + "", + user->def->name); + return -1; + } + if (access == PRIV_R) { + uint32_t access = cr->universal_access | + space->access[cr->auth_token].effective; + if (~access & PRIV_R) { + struct user *user = user_find(cr->uid); + if (user != NULL) + diag_set(AccessDeniedError, + priv_name(PRIV_R), + schema_object_name(SC_SPACE), + space->def->name, + user->def->name); + return -1; + } + } + /* Other checks are postponed to trigger*/ + return 0; +} + +/** + * Runs simple read, usage, write checks. + * All other checks are postponed. + */ +static int +sys_read_write_access_check(struct space *space, user_access_t access) +{ + credentials *cr = effective_user(); + if (access == PRIV_W) { + uint32_t has_access = space->access[cr->auth_token].effective + | cr->universal_access; + if (~has_access & access) { + struct user *user = user_find(cr->uid); + if (user != NULL) { + diag_set(AccessDeniedError, + priv_name(access), + schema_object_name(SC_SPACE), + space->def->name, + user->def->name); + } + return -1; + } + return 0; + } + return sys_read_access_check(space, access); +} + const char * schema_find_name(enum schema_object_type type, uint32_t object_id) { @@ -562,3 +627,28 @@ schema_find_name(enum schema_object_type type, uint32_t object_id) return "(nil)"; } +access_check_func_t +get_access_check_func(uint32_t space_id) +{ + switch (space_id) { + case BOX_CLUSTER_ID: + case BOX_COLLATION_ID: + return sys_read_write_access_check; + case BOX_SCHEMA_ID: + case BOX_SPACE_ID: + case BOX_TRUNCATE_ID: + case BOX_INDEX_ID: + case BOX_FUNC_ID: + case BOX_USER_ID: + case BOX_SEQUENCE_ID: + case BOX_SEQUENCE_DATA_ID: + case BOX_SPACE_SEQUENCE_ID: + case BOX_PRIV_ID: + /* + * Specialized access checks will be performed in trigger. + */ + return sys_read_access_check; + default: + return access_check_user_space; + } +} diff --git a/src/box/schema.h b/src/box/schema.h index 2b87f5f..d3fb7ce 100644 --- a/src/box/schema.h +++ b/src/box/schema.h @@ -36,6 +36,7 @@ #include "error.h" #include "space.h" #include "latch.h" +#include "user.h" #if defined(__cplusplus) extern "C" { @@ -103,6 +104,9 @@ schema_find_name(enum schema_object_type type, uint32_t object_id); */ struct sequence * sequence_by_id(uint32_t id); + +int +access_check_user_space(struct space *space, user_access_t access); #if defined(__cplusplus) } /* extern "C" */ @@ -205,6 +209,12 @@ sequence_cache_delete(uint32_t id); #endif /* defined(__cplusplus) */ /** + * Utility function used to specify access_check function for system space. + */ +access_check_func_t +get_access_check_func(uint32_t space_id); + +/** * Triggers fired after committing a change in space definition. * The space is passed to the trigger callback in the event * argument. It is the new space in case of create/update or diff --git a/src/box/space.c b/src/box/space.c index 02a9792..d72ce29 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -44,7 +44,7 @@ #include "iproto_constants.h" int -access_check_space(struct space *space, user_access_t access) +access_check_user_space(struct space *space, user_access_t access) { struct credentials *cr = effective_user(); /* Any space access also requires global USAGE privilege. */ @@ -132,6 +132,7 @@ space_create(struct space *space, struct engine *engine, rlist_create(&space->on_replace); rlist_create(&space->on_stmt_begin); space->run_triggers = true; + space->access_check = get_access_check_func(def->id); space->format = format; if (format != NULL) diff --git a/src/box/space.h b/src/box/space.h index a024fdc..5139a76 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -50,6 +50,8 @@ struct port; struct tuple; struct tuple_format; +typedef int (*access_check_func_t)(struct space *space, user_access_t access); + struct space_vtab { /** Free a space instance. */ void (*destroy)(struct space *); @@ -127,6 +129,7 @@ struct space_vtab { struct space { /** Virtual function table. */ const struct space_vtab *vtab; + access_check_func_t access_check; /** Cached runtime access information. */ struct access access[BOX_USER_MAX]; /** Engine used by this space. */ @@ -273,8 +276,11 @@ index_name_by_id(struct space *space, uint32_t id); * Check whether or not the current user can be granted * the requested access to the space. */ -int -access_check_space(struct space *space, user_access_t access); +static inline int +access_check_space(struct space *space, user_access_t access) +{ + return space->access_check(space, access); +} static inline int space_apply_initial_join_row(struct space *space, struct request *request) @@ -390,6 +396,12 @@ space_dump_def(const struct space *space, struct rlist *key_list); void space_fill_index_map(struct space *space); +/* + * Utility function used to specify access_check function for system space. + */ +access_check_func_t +get_access_check_func(uint32_t space_id); + #if defined(__cplusplus) } /* extern "C" */ @@ -499,4 +511,5 @@ space_prepare_alter_xc(struct space *old_space, struct space *new_space) #endif /* defined(__cplusplus) */ + #endif /* TARANTOOL_BOX_SPACE_H_INCLUDED */ diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c index 5aea87e..f77ce99 100644 --- a/src/box/sysview_engine.c +++ b/src/box/sysview_engine.c @@ -205,6 +205,7 @@ sysview_engine_create_space(struct engine *engine, struct space_def *def, free(space); return NULL; } + space->access_check = access_check_user_space; return space; } diff --git a/test/box/access.result b/test/box/access.result index 131a215..aa18193 100644 --- a/test/box/access.result +++ b/test/box/access.result @@ -1389,10 +1389,32 @@ box.schema.func.create('test_func') box.session.su("admin") --- ... +-- failed create because of auto_increment +box.session.su("tester") +--- +... +box.schema.space.create("test_space") +--- +- error: Write access to space '_schema' is denied for user 'tester' +... +box.schema.user.create('test_user') +--- +- error: Read access to space '_user' is denied for user 'tester' +... +box.schema.func.create('test_func') +--- +- error: Read access to space '_func' is denied for user 'tester' +... +box.schema.sequence.create('test_seq') +--- +- error: Read access to space '_sequence' is denied for user 'tester' +... +box.session.su("admin") +--- +... box.schema.user.grant("tester", "read", "universe") --- ... --- failed create box.session.su("tester") --- ... @@ -1402,27 +1424,26 @@ box.schema.space.create("test_space") ... box.schema.user.create('test_user') --- -- error: Write access to space '_user' is denied for user 'tester' +- error: Create access to user 'test_user' is denied for user 'tester' ... box.schema.func.create('test_func') --- -- error: Write access to space '_func' is denied for user 'tester' +- error: Create access to function 'test_func' is denied for user 'tester' +... +box.schema.sequence.create('test_seq') +--- +- error: Create access to sequence 'test_seq' 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 --- tables from ddl --- -box.schema.user.grant("tester", "create,write", "universe") +box.schema.user.grant("tester", "create", "universe") --- ... box.session.su("tester") --- ... --- successful create +-- successful create. Note user still needs read privilege because of auto_increment. s1 = box.schema.space.create("test_space") --- ... @@ -1477,10 +1498,16 @@ box.schema.func.drop("test") box.session.su("admin") --- ... +box.schema.user.revoke("tester", "create", "universe") +--- +... box.schema.user.grant("tester", "drop", "universe") --- ... --- successful drop +-- successful truncate, drop +box.session.su("tester", s.truncate, s) +--- +... box.session.su("tester", s.drop, s) --- ... diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 4bd34e4..38019db 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -520,22 +520,25 @@ box.schema.space.create("test_space") box.schema.user.create('test_user') box.schema.func.create('test_func') box.session.su("admin") -box.schema.user.grant("tester", "read", "universe") --- failed create +-- failed create because of auto_increment box.session.su("tester") box.schema.space.create("test_space") box.schema.user.create('test_user') box.schema.func.create('test_func') +box.schema.sequence.create('test_seq') + box.session.su("admin") +box.schema.user.grant("tester", "read", "universe") +box.session.su("tester") --- --- FIXME 2.0: we still need to grant 'write' on universe --- explicitly since we still use process_rw to write to system --- tables from ddl --- -box.schema.user.grant("tester", "create,write", "universe") +box.schema.space.create("test_space") +box.schema.user.create('test_user') +box.schema.func.create('test_func') +box.schema.sequence.create('test_seq') +box.session.su("admin") +box.schema.user.grant("tester", "create", "universe") box.session.su("tester") --- successful create +-- successful create. Note user still needs read privilege because of auto_increment. s1 = box.schema.space.create("test_space") _ = s1:create_index("primary") _ = box.schema.user.create('test_user') @@ -557,14 +560,18 @@ box.schema.func.drop('test_func') -- failed drop -- box.session.su("tester", s.drop, s) + s:drop() seq:drop() box.schema.user.drop("test") box.schema.func.drop("test") - box.session.su("admin") + + +box.schema.user.revoke("tester", "create", "universe") box.schema.user.grant("tester", "drop", "universe") --- successful drop +-- successful truncate, drop +box.session.su("tester", s.truncate, s) box.session.su("tester", s.drop, s) box.session.su("tester", seq.drop, seq) box.session.su("tester", box.schema.user.drop, "test") diff --git a/test/box/access_escalation.result b/test/box/access_escalation.result index 9d6cb99..a776e98 100644 --- a/test/box/access_escalation.result +++ b/test/box/access_escalation.result @@ -81,7 +81,7 @@ connection:close() box.schema.user.create('underprivileged') --- ... -box.schema.user.grant('underprivileged', 'read,write', 'space', '_func') +box.schema.user.grant('underprivileged', 'read,write', 'universe') --- ... box.session.su('underprivileged') @@ -90,6 +90,45 @@ box.session.su('underprivileged') box.schema.func.create('setuid', {setuid=true}) --- ... +box.schema.func.drop('setuid') +--- +... +box.session.su('admin') +--- +... +-- user needs read access because of auto_increment in func.create. +box.schema.user.revoke('underprivileged', 'write', 'universe') +--- +... +box.schema.user.grant('underprivileged', 'create', 'universe') +--- +... +-- check drop other user's func +box.schema.func.create('setuid2', {setuid=true}) +--- +... +box.session.su('underprivileged') +--- +... +box.schema.func.create('setuid', {setuid=true}) +--- +... +box.schema.func.drop('setuid2') +--- +- error: Drop access to function 'setuid2' is denied for user 'underprivileged' +... +box.session.su('admin') +--- +... +box.schema.user.grant('underprivileged', 'drop', 'universe') +--- +... +box.session.su('underprivileged') +--- +... +box.schema.func.drop('setuid2') +--- +... box.session.su('admin') --- ... diff --git a/test/box/access_escalation.test.lua b/test/box/access_escalation.test.lua index 8b30870..36cd6fb 100644 --- a/test/box/access_escalation.test.lua +++ b/test/box/access_escalation.test.lua @@ -60,10 +60,25 @@ connection:close() -- create a deprived user box.schema.user.create('underprivileged') -box.schema.user.grant('underprivileged', 'read,write', 'space', '_func') +box.schema.user.grant('underprivileged', 'read,write', 'universe') box.session.su('underprivileged') box.schema.func.create('setuid', {setuid=true}) +box.schema.func.drop('setuid') +box.session.su('admin') +-- user needs read access because of auto_increment in func.create. +box.schema.user.revoke('underprivileged', 'write', 'universe') +box.schema.user.grant('underprivileged', 'create', 'universe') +-- check drop other user's func +box.schema.func.create('setuid2', {setuid=true}) +box.session.su('underprivileged') +box.schema.func.create('setuid', {setuid=true}) +box.schema.func.drop('setuid2') box.session.su('admin') +box.schema.user.grant('underprivileged', 'drop', 'universe') +box.session.su('underprivileged') +box.schema.func.drop('setuid2') +box.session.su('admin') + -- -- create a deprived function -- diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 8bf99f2..1d6ccfb 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -61,6 +61,9 @@ box.schema.user.grant('testus', 'read', 'space', 'admin_space') --- - error: User 'testus' already has read access on space 'admin_space' ... +box.schema.user.grant('testus', 'read', 'universe') +--- +... session.su('testus') --- ... @@ -78,7 +81,7 @@ s:delete(1) ... s:drop() --- -- error: Write access to space '_space_sequence' is denied for user 'testus' +- error: Drop access to space 'admin_space' is denied for user 'testus' ... -- -- Check double revoke @@ -93,6 +96,9 @@ box.schema.user.revoke('testus', 'read', 'space', 'admin_space') --- - error: User 'testus' does not have read access on space 'admin_space' ... +box.schema.user.revoke('testus', 'read', 'universe') +--- +... session.su('testus') --- ... @@ -124,9 +130,18 @@ s:insert({3}) --- - [3] ... +session.su('admin') +--- +... +box.schema.user.grant('testus', 'read', 'universe') +--- +... +session.su('testus') +--- +... s:drop() --- -- error: Write access to space '_space_sequence' is denied for user 'testus' +- error: Drop access to space 'admin_space' is denied for user 'testus' ... session.su('admin') --- @@ -167,28 +182,26 @@ s:delete({3}) --- - error: Write access to space 'admin_space' is denied for user 'guest' ... -s:drop() +session.su('admin') --- -- error: Write access to space '_space_sequence' is denied for user 'guest' ... -gs = box.schema.space.create('guest_space') +box.schema.user.grant('guest', 'read', 'universe') --- -- error: Write access to space '_schema' is denied for user 'guest' ... --- --- FIXME: object create calls system space auto_increment, which requires --- read and write privileges. Create privilege must solve this. --- -box.schema.func.create('guest_func') +session.su('guest') --- -- error: Read access to space '_func' is denied for user 'guest' ... -session.su('admin', box.schema.user.grant, "guest", "read", "universe") +s:drop() +--- +- error: Drop access to space 'admin_space' is denied for user 'guest' +... +gs = box.schema.space.create('guest_space') --- +- error: Write access to space '_schema' is denied for user 'guest' ... box.schema.func.create('guest_func') --- -- error: Read access to space '_func' is denied for user 'guest' +- error: Create access to function 'guest_func' is denied for user 'guest' ... session.su('admin') --- @@ -291,7 +304,7 @@ session.su('admin') box.schema.user.create('someuser') --- ... -box.schema.user.grant('someuser', 'read, write, execute', 'universe') +box.schema.user.grant('someuser', 'read', 'universe') --- ... session.su('someuser') @@ -360,12 +373,38 @@ _ = box.space._user:delete(2) --- - error: Drop access to user 'public' is denied for user 'testuser' ... +session.su('admin') +--- +... +box.schema.user.grant('testuser', 'drop', 'universe') +--- +... +session.su('testuser') +--- +... +_ = box.space._user:delete(2) +--- +- error: 'Failed to drop user or role ''public'': the user or the role is a system' +... box.space._user:select(1) --- - error: Read access to space '_user' is denied for user 'testuser' ... uid = box.space._user:insert{maxuid+1, session.uid(), 'someone', 'user', EMPTY_MAP}[1] --- +- error: Create access to user 'someone' is denied for user 'testuser' +... +session.su('admin') +--- +... +box.schema.user.grant('testuser', 'create', 'universe') +--- +... +session.su('testuser') +--- +... +uid = box.space._user:insert{maxuid+1, session.uid(), 'someone', 'user', EMPTY_MAP}[1] +--- ... _ = box.space._user:delete(uid) --- @@ -384,6 +423,12 @@ _ = box.space._user:delete(testuser_uid) box.schema.user.revoke('testuser', 'write', 'space', '_user') --- ... +box.schema.user.revoke('testuser', 'drop', 'universe') +--- +... +box.schema.user.revoke('testuser', 'create', 'universe') +--- +... -- -- Check read grant on _user -- @@ -395,7 +440,7 @@ session.su('testuser') ... _ = box.space._user:delete(2) --- -- error: Write access to space '_user' is denied for user 'testuser' +- error: Drop access to user 'public' is denied for user 'testuser' ... box.space._user:select(1) --- @@ -403,7 +448,8 @@ box.space._user:select(1) ... box.space._user:insert{uid, session.uid(), 'someone2', 'user'} --- -- error: Write access to space '_user' is denied for user 'testuser' +- error: Tuple field count 4 is less than required by space format or defined indexes + (expected at least 5) ... session.su('admin') --- @@ -423,7 +469,7 @@ box.space._index:select(272) ... box.space._index:insert{512, 1,'owner','tree', 1, 1, 0,'unsigned'} --- -- error: Write access to space '_index' is denied for user 'testuser' +- error: 'Tuple field 5 type does not match one required by operation: expected map' ... session.su('admin') --- diff --git a/test/box/access_misc.test.lua b/test/box/access_misc.test.lua index 27064c4..8c26b98 100644 --- a/test/box/access_misc.test.lua +++ b/test/box/access_misc.test.lua @@ -27,6 +27,7 @@ s:insert({2}) -- box.schema.user.grant('testus', 'read', 'space', 'admin_space') box.schema.user.grant('testus', 'read', 'space', 'admin_space') +box.schema.user.grant('testus', 'read', 'universe') session.su('testus') s:select(1) @@ -39,6 +40,7 @@ s:drop() session.su('admin') box.schema.user.revoke('testus', 'read', 'space', 'admin_space') box.schema.user.revoke('testus', 'read', 'space', 'admin_space') +box.schema.user.revoke('testus', 'read', 'universe') session.su('testus') s:select(1) @@ -52,6 +54,9 @@ session.su('testus') s:select(1) s:delete(1) s:insert({3}) +session.su('admin') +box.schema.user.grant('testus', 'read', 'universe') +session.su('testus') s:drop() session.su('admin') -- @@ -68,14 +73,12 @@ box.space._user:select(1) s:select(1) s:insert({4}) s:delete({3}) +session.su('admin') +box.schema.user.grant('guest', 'read', 'universe') +session.su('guest') s:drop() gs = box.schema.space.create('guest_space') --- --- FIXME: object create calls system space auto_increment, which requires --- read and write privileges. Create privilege must solve this. --- -box.schema.func.create('guest_func') -session.su('admin', box.schema.user.grant, "guest", "read", "universe") + box.schema.func.create('guest_func') session.su('admin') box.schema.user.revoke("guest", "read", "universe") @@ -123,7 +126,7 @@ box.schema.func.create('uniuser_func') session.su('admin') box.schema.user.create('someuser') -box.schema.user.grant('someuser', 'read, write, execute', 'universe') +box.schema.user.grant('someuser', 'read', 'universe') session.su('someuser') -- -- Check drop objects of another user @@ -150,14 +153,24 @@ box.schema.user.grant('testuser', 'write', 'space', '_user') session.su('testuser') testuser_uid = session.uid() _ = box.space._user:delete(2) +session.su('admin') +box.schema.user.grant('testuser', 'drop', 'universe') +session.su('testuser') +_ = box.space._user:delete(2) box.space._user:select(1) uid = box.space._user:insert{maxuid+1, session.uid(), 'someone', 'user', EMPTY_MAP}[1] +session.su('admin') +box.schema.user.grant('testuser', 'create', 'universe') +session.su('testuser') +uid = box.space._user:insert{maxuid+1, session.uid(), 'someone', 'user', EMPTY_MAP}[1] _ = box.space._user:delete(uid) session.su('admin') box.space._user:select(1) _ = box.space._user:delete(testuser_uid) box.schema.user.revoke('testuser', 'write', 'space', '_user') +box.schema.user.revoke('testuser', 'drop', 'universe') +box.schema.user.revoke('testuser', 'create', 'universe') -- -- Check read grant on _user -- diff --git a/test/box/net.box.result b/test/box/net.box.result index 1674c27..b6bb383 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -2140,10 +2140,7 @@ box.session.on_disconnect(nil, on_disconnect) -- gh-2666: check that netbox.call is not repeated on schema -- change. -- -box.schema.user.grant('guest', 'write', 'space', '_space') ---- -... -box.schema.user.grant('guest', 'write', 'space', '_schema') +box.schema.user.grant('guest', 'create', 'universe') --- ... count = 0 @@ -2188,10 +2185,7 @@ box.space.test2:drop() box.space.test3:drop() --- ... -box.schema.user.revoke('guest', 'write', 'space', '_space') ---- -... -box.schema.user.revoke('guest', 'write', 'space', '_schema') +box.schema.user.revoke('guest', 'create', 'universe') --- ... c:close() diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index c34616a..e1636c8 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -875,8 +875,7 @@ box.session.on_disconnect(nil, on_disconnect) -- gh-2666: check that netbox.call is not repeated on schema -- change. -- -box.schema.user.grant('guest', 'write', 'space', '_space') -box.schema.user.grant('guest', 'write', 'space', '_schema') +box.schema.user.grant('guest', 'create', 'universe') count = 0 function create_space(name) count = count + 1 box.schema.create_space(name) return true end c = net.connect(box.cfg.listen) @@ -889,8 +888,7 @@ count box.space.test1:drop() box.space.test2:drop() box.space.test3:drop() -box.schema.user.revoke('guest', 'write', 'space', '_space') -box.schema.user.revoke('guest', 'write', 'space', '_schema') +box.schema.user.revoke('guest', 'create', 'universe') c:close() -- diff --git a/test/box/role.result b/test/box/role.result index 806cea9..38dc43e 100644 --- a/test/box/role.result +++ b/test/box/role.result @@ -671,7 +671,7 @@ box.session.su('admin') _ = box.schema.space.create('test') --- ... -box.schema.user.grant('john', 'read,write,execute', 'universe') +box.schema.user.grant('john', 'read', 'universe') --- ... box.session.su('john') @@ -695,14 +695,33 @@ box.schema.user.grant('grantee', 'public') - error: User 'grantee' already has role 'public' ... -- --- revoking role 'public' is another deal - only the --- superuser can do that, and even that would be useless, +-- revoking role 'public' is another deal: +-- the superuser or creator of user can do that, and even that would be useless, -- since one can still re-grant it back to oneself. -- box.schema.user.revoke('grantee', 'public') --- - error: Revoke access to role 'public' is denied for user 'john' ... +box.session.su("admin") +--- +... +box.schema.user.grant("john", "create", 'universe') +--- +... +box.session.su('john') +--- +... +box.schema.user.create("grantee2") +--- +... +-- must be ok +box.schema.user.revoke('grantee2', 'public') +--- +... +box.schema.user.drop('grantee2') +--- +... box.session.su('admin') --- ... diff --git a/test/box/role.test.lua b/test/box/role.test.lua index e97339f..abdd1b1 100644 --- a/test/box/role.test.lua +++ b/test/box/role.test.lua @@ -261,7 +261,7 @@ box.schema.user.grant('grantee', 'role') -- box.session.su('admin') _ = box.schema.space.create('test') -box.schema.user.grant('john', 'read,write,execute', 'universe') +box.schema.user.grant('john', 'read', 'universe') box.session.su('john') box.schema.user.grant('grantee', 'role') box.schema.user.grant('grantee', 'read', 'space', 'test') @@ -272,11 +272,18 @@ box.schema.user.grant('grantee', 'read', 'space', 'test') -- box.schema.user.grant('grantee', 'public') -- --- revoking role 'public' is another deal - only the --- superuser can do that, and even that would be useless, +-- revoking role 'public' is another deal: +-- the superuser or creator of user can do that, and even that would be useless, -- since one can still re-grant it back to oneself. -- box.schema.user.revoke('grantee', 'public') +box.session.su("admin") +box.schema.user.grant("john", "create", 'universe') +box.session.su('john') +box.schema.user.create("grantee2") +-- must be ok +box.schema.user.revoke('grantee2', 'public') +box.schema.user.drop('grantee2') box.session.su('admin') box.schema.user.drop('john') diff --git a/test/box/sequence.result b/test/box/sequence.result index f0164b6..56dcb71 100644 --- a/test/box/sequence.result +++ b/test/box/sequence.result @@ -1451,7 +1451,7 @@ box.session.su('admin') --- ... -- A user cannot alter sequences created by other users. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read', 'universe') --- ... box.session.su('user') @@ -1471,6 +1471,9 @@ box.session.su('admin') sq:drop() --- ... +box.schema.user.grant('user', 'create', 'universe') +--- +... -- A user can alter/use sequences that he owns. box.session.su('user') --- @@ -1522,7 +1525,7 @@ s1 = box.schema.space.create('space1') _ = s1:create_index('pk') --- ... -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read, create', 'universe') --- ... box.session.su('user') @@ -1536,15 +1539,14 @@ s2 = box.schema.space.create('space2') ... _ = s2:create_index('pk', {sequence = 'seq1'}) -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' ... s1.index.pk:alter({sequence = 'seq1'}) -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to space 'space1' is denied for user 'user' ... box.space._space_sequence:replace{s1.id, sq1.id, false} -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to space 'space1' is denied for user 'user' ... box.space._space_sequence:replace{s1.id, sq2.id, false} -- error --- @@ -1552,7 +1554,7 @@ box.space._space_sequence:replace{s1.id, sq2.id, false} -- error ... box.space._space_sequence:replace{s2.id, sq1.id, false} -- error --- -- error: Create access to sequence 'seq1' is denied for user 'user' +- error: Alter access to sequence 'seq1' is denied for user 'user' ... s2.index.pk:alter({sequence = 'seq2'}) -- ok --- @@ -1563,7 +1565,7 @@ box.session.su('admin') -- If the user owns a sequence attached to a space, -- it can use it for auto increment, otherwise it -- needs privileges. -box.schema.user.revoke('user', 'read,write', 'universe') +box.schema.user.revoke('user', 'read,create', 'universe') --- ... box.session.su('user') @@ -1671,7 +1673,7 @@ s:drop() --- ... -- When a user is dropped, all his sequences are dropped as well. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'create', 'universe') --- ... box.session.su('user') @@ -1701,10 +1703,10 @@ box.schema.user.create('user1') box.schema.user.create('user2') --- ... -box.schema.user.grant('user1', 'read,write', 'universe') +box.schema.user.grant('user1', 'read, create', 'universe') --- ... -box.schema.user.grant('user2', 'read,write', 'universe') +box.schema.user.grant('user2', 'read', 'universe') --- ... box.session.su('user1') diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua index af3432f..577b3ee 100644 --- a/test/box/sequence.test.lua +++ b/test/box/sequence.test.lua @@ -482,12 +482,13 @@ sq:reset() -- error box.session.su('admin') -- A user cannot alter sequences created by other users. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read', 'universe') box.session.su('user') sq:alter{step = 2} -- error sq:drop() -- error box.session.su('admin') sq:drop() +box.schema.user.grant('user', 'create', 'universe') -- A user can alter/use sequences that he owns. box.session.su('user') @@ -508,7 +509,7 @@ sq:drop() sq1 = box.schema.sequence.create('seq1') s1 = box.schema.space.create('space1') _ = s1:create_index('pk') -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'read, create', 'universe') box.session.su('user') sq2 = box.schema.sequence.create('seq2') s2 = box.schema.space.create('space2') @@ -523,7 +524,7 @@ box.session.su('admin') -- If the user owns a sequence attached to a space, -- it can use it for auto increment, otherwise it -- needs privileges. -box.schema.user.revoke('user', 'read,write', 'universe') +box.schema.user.revoke('user', 'read,create', 'universe') box.session.su('user') s2:insert{nil, 1} -- ok: {1, 1} box.session.su('admin') @@ -559,7 +560,7 @@ box.session.su('admin') s:drop() -- When a user is dropped, all his sequences are dropped as well. -box.schema.user.grant('user', 'read,write', 'universe') +box.schema.user.grant('user', 'create', 'universe') box.session.su('user') _ = box.schema.sequence.create('test1') _ = box.schema.sequence.create('test2') @@ -571,8 +572,8 @@ box.sequence -- to a sequence. box.schema.user.create('user1') box.schema.user.create('user2') -box.schema.user.grant('user1', 'read,write', 'universe') -box.schema.user.grant('user2', 'read,write', 'universe') +box.schema.user.grant('user1', 'read, create', 'universe') +box.schema.user.grant('user2', 'read', 'universe') box.session.su('user1') sq = box.schema.sequence.create('test') box.session.su('user2') diff --git a/test/engine/truncate.result b/test/engine/truncate.result index 3ad400e..33ff70a 100644 --- a/test/engine/truncate.result +++ b/test/engine/truncate.result @@ -506,7 +506,7 @@ con = require('net.box').connect(box.cfg.listen) ... con:eval([[box.space.access_truncate:truncate()]]) --- -- error: Write access to space 'access_truncate' is denied for user 'guest' +- error: Drop access to space 'access_truncate' is denied for user 'guest' ... con.space.access_truncate:select() --- -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-17 16:15 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-16 12:37 [tarantool-patches] [security 0/2] System spaces access control lists Ilya Markov 2018-05-16 12:37 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov 2018-05-16 19:22 ` [tarantool-patches] " Konstantin Osipov 2018-05-16 12:37 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov 2018-05-16 19:27 ` [tarantool-patches] " Konstantin Osipov 2018-05-17 16:15 ` [tarantool-patches] [security 0/2] Access control lists Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 1/2] security: Refactor reads from systems spaces Ilya Markov 2018-05-17 16:15 ` [tarantool-patches] [security 2/2] security: Refactor system space access checks Ilya Markov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox