From: Sergey Petrenko <sergepetrenko@tarantool.org> To: Konstantin Osipov <kostja@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v2 3/4] Add single object privilege checks to access_check_ddl. Date: Mon, 30 Jul 2018 11:37:01 +0300 [thread overview] Message-ID: <D79F007B-84B4-4C04-A72E-440B041A2FFB@tarantool.org> (raw) In-Reply-To: <20180726203731.GA32000@chai> Hi, I rebased this patch on top of the new one regarding #3524, and fixed your comments. Please see 2 my comments and the new diff below. > 26 июля 2018 г., в 23:37, Konstantin Osipov <kostja@tarantool.org> написал(а): > > * Serge Petrenko <sergepetrenko@tarantool.org> [18/07/17 21:31]: > > This patch is generally LGTM, but it's based on entity access, so > I can't push it, since I requested some changes in entity access > patch. A few questions below. > >> access_check_ddl() didn't check for single object privileges, e.g. user >> with alter access on a space couldn't create an index in this space. It >> would only succeed if it had alter on entire entity space. >> Fix this by adding single object privilege checks to access_check_ddl and >> adding access cache to struct user, to hold other users' privileges on it. >> >> Also checking for single object privilege made it possible to grant >> every user alter privilege on itself, so that a user may change its own >> password (previously it was possible because of a hack). Removed the >> hack, and added grant alter to itself upon user creation. >> Modified tests accordingly, and added a couple of test cases. > > What hack did you remove? The problem I have with self-granting > 'alter' is that old deployments will break, since they don't have > a self-grant. Maybe we leave the hack in place for a while, begin > self-granting 'alter', and only remove the hack in 2.0? We > shouldn't forget to grant everyone an 'alter' on self in 2.0 upgrade script. > But to begin with, I can't find where this hack was in the code, > and where you removed it. Here’s the hack I was talking about: in function on_replace_dd_user in alter.cc - access_check_ddl(user->name, user->uid, SC_USER, PRIV_A, - true); We pass user as its own owner, which enables it to change its own password/username. I leave the hack in place for now, and also add grant of alter on itself upon user creation: + access_check_ddl(user->name, user->uid, user->uid, SC_USER, + PRIV_A, true); schema.lua: function box.schema.user.create(): + -- grant user 'alter' on itself, so it can + -- change its password or username. + box.schema.user.grant(uid, 'alter', 'user', uid) > >> Closes #3530 >> --- >> src/box/alter.cc | 123 ++++++++++++++++++++++---------- >> src/box/lua/schema.lua | 5 +- >> src/box/user.cc | 10 ++- >> src/box/user.h | 2 + >> test/box/access.result | 182 +++++++++++++++++++++++++++++++++++++++++++++++ >> test/box/access.test.lua | 53 ++++++++++++++ >> test/box/role.result | 9 +++ >> test/box/sequence.result | 3 + >> 8 files changed, 347 insertions(+), 40 deletions(-) >> >> diff --git a/src/box/alter.cc b/src/box/alter.cc >> index 6293dcc50..54a09664b 100644 >> --- a/src/box/alter.cc >> +++ b/src/box/alter.cc >> @@ -62,7 +62,8 @@ >> /* {{{ Auxiliary functions and methods. */ >> >> static void >> -access_check_ddl(const char *name, uint32_t owner_uid, >> +access_check_ddl(const char *name, uint32_t object_id, >> + uint32_t owner_uid, >> enum schema_object_type type, >> enum priv_type priv_type, >> bool is_17_compat_mode) >> @@ -103,7 +104,48 @@ access_check_ddl(const char *name, uint32_t owner_uid, >> */ >> if (access == 0 || (is_owner && !(access & (PRIV_U | PRIV_C)))) >> return; /* Access granted. */ >> - >> + /* >> + * You can't grant CREATE privilege to a non-existing object. >> + * USAGE can be granted only globally. >> + */ >> + if (!(access & (PRIV_U | PRIV_C))) { >> + /* Check for privileges on a single object. */ > > Please avoid this deep nesting, use early returns from the > function for example, or create a separate function, > access_check_ddl_object()? Fixed. Added a jump to error case. > >> + switch (type) { >> + case SC_SPACE: >> + { >> + struct space *space = space_by_id(object_id); >> + if (space) >> + access &= ~space->access[cr->auth_token].effective; >> + break; >> + } >> + case SC_FUNCTION: >> + { >> + struct func *func = func_by_id(object_id); >> + if (func) >> + access &= ~func->access[cr->auth_token].effective; >> + break; >> + } >> + case SC_USER: >> + case SC_ROLE: >> + { >> + struct user *user_or_role = user_by_id(object_id); >> + if (user_or_role) >> + access &= ~user_or_role->access[cr->auth_token].effective; > >> + break; >> + } >> + case SC_SEQUENCE: >> + { >> + struct sequence *seq = sequence_by_id(object_id); >> + if (seq) >> + access &= ~seq->access[cr->auth_token].effective; >> + break; >> + } >> + default: >> + break; >> + } > > OK. > >> * check that it has read and write access. >> */ >> - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, >> - PRIV_R, false); >> - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, >> - PRIV_W, false); >> + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, >> + SC_SEQUENCE, PRIV_R, false); >> + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, >> + SC_SEQUENCE, PRIV_W, false); >> } >> /** Check we have alter access on space. */ >> - access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A, >> - false); >> + access_check_ddl(space->def->name, space->def->id, space->def->uid, >> + SC_SPACE, PRIV_A, false); >> >> 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 4b7a14411..a098e44fe 100644 >> --- a/src/box/lua/schema.lua >> +++ b/src/box/lua/schema.lua >> @@ -1740,7 +1740,7 @@ local priv_object_combo = { >> ["role"] = bit.bor(box.priv.X, box.priv.U, >> box.priv.C, box.priv.D), >> ["user"] = bit.bor(box.priv.C, box.priv.U, >> - box.priv.D), >> + box.priv.A, box.priv.D), >> } >> -- grant role 'public' to the user >> box.schema.user.grant(uid, 'public') >> + -- grant user 'alter' on itself, so it can >> + -- change its password or username. >> + box.schema.user.grant(uid, 'alter', 'user', uid) > > -- > Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 > http://tarantool.io - www.twitter.com/kostja_osipov src/box/alter.cc | 121 +++++++++++++++++++++--------- src/box/lua/schema.lua | 3 + src/box/user.cc | 8 +- src/box/user.h | 2 + test/box/access.result | 186 +++++++++++++++++++++++++++++++++++++++++++++++ test/box/access.test.lua | 57 +++++++++++++++ test/box/role.result | 9 +++ test/box/sequence.result | 3 + 8 files changed, 351 insertions(+), 38 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index d4545f31c..35e4d0999 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -62,7 +62,8 @@ /* {{{ Auxiliary functions and methods. */ static void -access_check_ddl(const char *name, uint32_t owner_uid, +access_check_ddl(const char *name, uint32_t object_id, + uint32_t owner_uid, enum schema_object_type type, enum priv_type priv_type, bool is_17_compat_mode) @@ -103,9 +104,51 @@ access_check_ddl(const char *name, uint32_t owner_uid, */ if (access == 0 || (is_owner && !(access & (PRIV_U | PRIV_C)))) return; /* Access granted. */ + /* + * You can't grant CREATE privilege to a non-existing object. + * USAGE can be granted only globally. + */ + if (access & (PRIV_U | PRIV_C)) + goto error; + /* Check for privileges on a single object. */ + switch (type) { + case SC_SPACE: + { + struct space *space = space_by_id(object_id); + if (space) + access &= ~space->access[cr->auth_token].effective; + break; + } + case SC_FUNCTION: + { + struct func *func = func_by_id(object_id); + if (func) + access &= ~func->access[cr->auth_token].effective; + break; + } + case SC_USER: + case SC_ROLE: + { + struct user *user_or_role = user_by_id(object_id); + if (user_or_role) + access &= ~user_or_role->access[cr->auth_token].effective; + break; + } + case SC_SEQUENCE: + { + struct sequence *seq = sequence_by_id(object_id); + if (seq) + access &= ~seq->access[cr->auth_token].effective; + break; + } + default: + break; + } + if (access == 0) + return; /* Access granted. */ /* Create a meaningful error message. */ - struct user *user = user_find_xc(cr->uid); +error: struct user *user = user_find_xc(cr->uid); const char *object_name; const char *pname; if (access & PRIV_U) { @@ -1590,7 +1633,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); + access_check_ddl(def->name, def->id, def->uid, SC_SPACE, + PRIV_C, true); auto def_guard = make_scoped_guard([=] { space_def_delete(def); }); RLIST_HEAD(empty_list); @@ -1623,8 +1667,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(on_create_space_rollback, space); 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); + access_check_ddl(old_space->def->name, old_space->def->id, + old_space->def->uid, SC_SPACE, PRIV_D, true); /* Verify that the space is empty (has no indexes) */ if (old_space->index_count) { tnt_raise(ClientError, ER_DROP_SPACE, @@ -1669,7 +1713,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->id, def->uid, SC_SPACE, + PRIV_A, true); auto def_guard = make_scoped_guard([=] { space_def_delete(def); }); if (def->id != space_id(old_space)) @@ -1774,8 +1819,8 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) enum priv_type priv_type = new_tuple ? PRIV_C : PRIV_D; 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); + access_check_ddl(old_space->def->name, old_space->def->id, + old_space->def->uid, SC_SPACE, priv_type, true); struct index *old_index = space_index(old_space, iid); /* @@ -2170,7 +2215,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, user->type, PRIV_C, true); + access_check_ddl(user->name, user->uid, user->owner, user->type, + PRIV_C, true); auto def_guard = make_scoped_guard([=] { free(user); }); (void) user_cache_replace(user); def_guard.is_active = false; @@ -2178,7 +2224,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) txn_alter_trigger_new(user_cache_remove_user, NULL); txn_on_rollback(txn, on_rollback); } else if (new_tuple == NULL) { /* DELETE */ - access_check_ddl(old_user->def->name, old_user->def->owner, + access_check_ddl(old_user->def->name, old_user->def->uid, + old_user->def->owner, old_user->def->type, PRIV_D, true); /* Can't drop guest or super user */ if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) { @@ -2205,8 +2252,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event) * correct. */ struct user_def *user = user_def_new_from_tuple(new_tuple); - access_check_ddl(user->name, user->uid, SC_USER, PRIV_A, - true); + access_check_ddl(user->name, user->uid, user->uid, SC_USER, + PRIV_A, true); auto def_guard = make_scoped_guard([=] { free(user); }); struct trigger *on_commit = txn_alter_trigger_new(user_cache_alter_user, NULL); @@ -2308,7 +2355,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); + access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION, + PRIV_C, true); auto def_guard = make_scoped_guard([=] { free(def); }); func_cache_replace(def); def_guard.is_active = false; @@ -2322,7 +2370,7 @@ 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, + access_check_ddl(old_func->def->name, fid, uid, SC_FUNCTION, PRIV_D, true); /* Can only delete func if it has no grants. */ if (schema_find_grants("function", old_func->def->fid)) { @@ -2336,8 +2384,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) } else { /* UPDATE, REPLACE */ 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); + access_check_ddl(def->name, def->fid, def->uid, + SC_FUNCTION, PRIV_A, true); struct trigger *on_commit = txn_alter_trigger_new(func_cache_replace_func, NULL); txn_on_commit(txn, on_commit); @@ -2493,8 +2541,9 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) BOX_COLLATION_FIELD_ID); struct coll_id *old_coll_id = coll_by_id(old_id); assert(old_coll_id != NULL); - access_check_ddl(old_coll_id->name, old_coll_id->owner_id, - SC_COLLATION, PRIV_D, false); + access_check_ddl(old_coll_id->name, old_id, + old_coll_id->owner_id, SC_COLLATION, PRIV_D, + false); /* * Set on_commit/on_rollback triggers after * deletion from the cache to make trigger logic @@ -2509,8 +2558,8 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event) /* INSERT */ struct coll_id_def new_def; coll_id_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.id, new_def.owner_id, + SC_COLLATION, PRIV_C, false); struct coll_id *new_coll_id = coll_id_new(&new_def); if (new_coll_id == NULL) diag_raise(); @@ -2569,8 +2618,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type) 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); + access_check_ddl(name, priv->object_id, grantor->def->uid, + priv->object_type, priv_type, false); switch (priv->object_type) { case SC_UNIVERSE: if (grantor->def->uid != ADMIN) { @@ -3075,8 +3124,8 @@ 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); - access_check_ddl(new_def->name, new_def->uid, SC_SEQUENCE, - PRIV_C, false); + access_check_ddl(new_def->name, new_def->id, new_def->uid, + SC_SEQUENCE, PRIV_C, false); sequence_cache_replace(new_def); alter->new_def = new_def; } else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */ @@ -3084,8 +3133,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) BOX_SEQUENCE_DATA_FIELD_ID); struct sequence *seq = sequence_by_id(id); assert(seq != NULL); - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - PRIV_D, false); + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, + SC_SEQUENCE, PRIV_D, false); if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id)) tnt_raise(ClientError, ER_DROP_SEQUENCE, seq->def->name, "the sequence has data"); @@ -3101,8 +3150,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event) ER_ALTER_SEQUENCE); 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); + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, + SC_SEQUENCE, PRIV_A, false); alter->old_def = seq->def; alter->new_def = new_def; } @@ -3182,21 +3231,21 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event) /* Check we have the correct access type on the sequence. * */ if (is_generated || !stmt->new_tuple) { - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - priv_type, false); + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, + SC_SEQUENCE, priv_type, false); } else { /* * In case user wants to attach an existing sequence, * check that it has read and write access. */ - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - PRIV_R, false); - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, - PRIV_W, false); + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, + SC_SEQUENCE, PRIV_R, false); + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid, + SC_SEQUENCE, PRIV_W, false); } /** Check we have alter access on space. */ - access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A, - false); + access_check_ddl(space->def->name, space->def->id, space->def->uid, + SC_SPACE, PRIV_A, false); 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 28f68cbda..350da210a 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -2103,6 +2103,9 @@ 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') + -- grant user 'alter' on itself, so it can + -- change its password or username. + box.schema.user.grant(uid, 'alter', 'user', uid) -- we have to grant global privileges from setuid function, since -- only admin has the ownership over universe and we don't have -- grant option diff --git a/src/box/user.cc b/src/box/user.cc index b4fb65a59..83d07f7b3 100644 --- a/src/box/user.cc +++ b/src/box/user.cc @@ -248,12 +248,16 @@ access_find(struct priv_def *priv) } case SC_USER: { - /* No grants on a single object user yet. */ + struct user *user = user_by_id(priv->object_id); + if (user) + access = user->access; break; } case SC_ROLE: { - /* No grants on a single object role yet. */ + struct user *role = user_by_id(priv->object_id); + if (role) + access = role->access; break; } case SC_SEQUENCE: diff --git a/src/box/user.h b/src/box/user.h index 07c4dc504..069d9b77e 100644 --- a/src/box/user.h +++ b/src/box/user.h @@ -88,6 +88,8 @@ struct user bool is_dirty; /** Memory pool for privs */ struct region pool; + /** Cached runtime access imformation. */ + struct access access[BOX_USER_MAX]; }; /** Find user by id. */ diff --git a/test/box/access.result b/test/box/access.result index 7acd6fa43..819d8c471 100644 --- a/test/box/access.result +++ b/test/box/access.result @@ -136,6 +136,9 @@ box.schema.user.revoke('rich', 'read,write', 'universe') box.schema.user.revoke('rich', 'public') --- ... +box.schema.user.revoke('rich', 'alter', 'user', 'rich') +--- +... box.schema.user.disable("rich") --- ... @@ -501,6 +504,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 27] + - [1, 32, 'user', 32, 128] ... box.schema.user.grant('user', 'read', 'universe') --- @@ -510,6 +514,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 27] + - [1, 32, 'user', 32, 128] ... box.schema.user.revoke('user', 'write', 'universe') --- @@ -518,6 +523,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 25] + - [1, 32, 'user', 32, 128] ... box.schema.user.revoke('user', 'read', 'universe') --- @@ -526,6 +532,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 24] + - [1, 32, 'user', 32, 128] ... box.schema.user.grant('user', 'write', 'universe') --- @@ -534,6 +541,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 26] + - [1, 32, 'user', 32, 128] ... box.schema.user.grant('user', 'read', 'universe') --- @@ -542,6 +550,7 @@ box.space._priv:select{id} --- - - [1, 32, 'role', 2, 4] - [1, 32, 'universe', 0, 27] + - [1, 32, 'user', 32, 128] ... box.schema.user.drop('user') --- @@ -965,6 +974,9 @@ box.schema.user.info('test_user') - - session,usage - universe - + - - alter + - user + - test_user ... box.schema.role.info('test_role') --- @@ -995,6 +1007,9 @@ box.schema.user.info('test_user') - - session,usage - universe - + - - alter + - user + - test_user ... box.schema.role.info('test_role') --- @@ -1860,3 +1875,174 @@ box.session.su('admin') box.schema.user.drop('tester') --- ... +-- +-- test case for 3530: do not ignore single object privileges in +-- access_check_ddl. +-- +box.schema.user.create("test") +--- +... +_ = box.schema.space.create("space1") +--- +... +box.schema.user.grant("test", "read", "space", "space1") +--- +... +box.schema.user.grant("test", "write", "space", "_index") +--- +... +box.session.su("test") +--- +... +box.space.space1:create_index("pk") +--- +- error: Create access to space 'space1' is denied for user 'test' +... +box.session.su("admin") +--- +... +box.space.space1.index[0] == nil +--- +- true +... +-- fixme: cannot grant create on a single space +-- this is because when checking for create +-- access_check_ddl ignores space privileges, +-- assuming that there is no space yet. +box.schema.user.grant("test", "create", "space") +--- +... +box.session.su("test") +--- +... +_ = box.space.space1:create_index("pk") +--- +... +box.space.space1:insert{5} +--- +- error: Write access to space 'space1' is denied for user 'test' +... +box.session.su("admin") +--- +... +box.space.space1.index[0] ~= nil +--- +- true +... +box.space.space1:select{} +--- +- [] +... +box.schema.user.grant("test", "write", "space", "space1") +--- +... +box.session.su("test") +--- +... +box.space.space1:insert{5} +--- +- [5] +... +box.session.su("admin") +--- +... +box.space.space1:select{} +--- +- - [5] +... +box.schema.user.drop("test") +--- +... +box.space.space1:drop() +--- +... +-- +-- test that it is possible to grant privileges on a single user. +box.schema.user.create("user1") +--- +... +box.schema.user.create("user2") +--- +... +box.schema.user.create("user3") +--- +... +box.schema.user.grant("user1", "write", "space", "_user") +--- +... +box.schema.user.grant("user1", "read", "space", "_user") +--- +... +box.space._user:select{} +--- +- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}] + - [1, 1, 'admin', 'user', {}] + - [2, 1, 'public', 'role', {}] + - [3, 1, 'replication', 'role', {}] + - [31, 1, 'super', 'role', {}] + - [32, 1, 'user1', 'user', {}] + - [33, 1, 'user2', 'user', {}] + - [34, 1, 'user3', 'user', {}] +... +box.session.su("user1") +--- +... +-- can alter itself, but can't alter others without privileges. +box.schema.user.passwd("user1", "abcd") +--- +... +box.schema.user.passwd("user2", "abcd") +--- +- error: Alter access to user 'user2' is denied for user 'user1' +... +box.session.su("admin") +--- +... +box.space._user:select{} +--- +- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}] + - [1, 1, 'admin', 'user', {}] + - [2, 1, 'public', 'role', {}] + - [3, 1, 'replication', 'role', {}] + - [31, 1, 'super', 'role', {}] + - [32, 1, 'user1', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}] + - [33, 1, 'user2', 'user', {}] + - [34, 1, 'user3', 'user', {}] +... +box.schema.user.grant("user1", "alter", "user", "user2") +--- +... +box.session.su("user1") +--- +... +box.schema.user.passwd("user2", "abcd") +--- +... +-- still fails +box.schema.user.passwd("user3", "qewr") +--- +- error: Alter access to user 'user3' is denied for user 'user1' +... +box.session.su("admin") +--- +... +box.space._user:select{} +--- +- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}] + - [1, 1, 'admin', 'user', {}] + - [2, 1, 'public', 'role', {}] + - [3, 1, 'replication', 'role', {}] + - [31, 1, 'super', 'role', {}] + - [32, 1, 'user1', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}] + - [33, 1, 'user2', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}] + - [34, 1, 'user3', 'user', {}] +... +box.schema.user.drop("user1") +--- +... +box.schema.user.drop("user2") +--- +... +box.schema.user.drop("user3") +--- +... diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 9b7510e64..991ddf6ba 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -60,6 +60,7 @@ box.schema.func.drop('dummy') box.space['_user']:delete{uid} box.schema.user.revoke('rich', 'read,write', 'universe') box.schema.user.revoke('rich', 'public') +box.schema.user.revoke('rich', 'alter', 'user', 'rich') box.schema.user.disable("rich") -- test double disable is a no op box.schema.user.disable("rich") @@ -726,3 +727,59 @@ _ = box.schema.sequence.create('test_sequence') box.session.su('admin') box.schema.user.drop('tester') + +-- +-- test case for 3530: do not ignore single object privileges in +-- access_check_ddl. +-- +box.schema.user.create("test") +_ = box.schema.space.create("space1") +box.schema.user.grant("test", "read", "space", "space1") +box.schema.user.grant("test", "write", "space", "_index") +box.session.su("test") +box.space.space1:create_index("pk") +box.session.su("admin") +box.space.space1.index[0] == nil +-- fixme: cannot grant create on a single space +-- this is because when checking for create +-- access_check_ddl ignores space privileges, +-- assuming that there is no space yet. +box.schema.user.grant("test", "create", "space") +box.session.su("test") +_ = box.space.space1:create_index("pk") +box.space.space1:insert{5} +box.session.su("admin") +box.space.space1.index[0] ~= nil +box.space.space1:select{} +box.schema.user.grant("test", "write", "space", "space1") +box.session.su("test") +box.space.space1:insert{5} +box.session.su("admin") +box.space.space1:select{} +box.schema.user.drop("test") +box.space.space1:drop() + +-- +-- test that it is possible to grant privileges on a single user. +box.schema.user.create("user1") +box.schema.user.create("user2") +box.schema.user.create("user3") +box.schema.user.grant("user1", "write", "space", "_user") +box.schema.user.grant("user1", "read", "space", "_user") +box.space._user:select{} +box.session.su("user1") +-- can alter itself, but can't alter others without privileges. +box.schema.user.passwd("user1", "abcd") +box.schema.user.passwd("user2", "abcd") +box.session.su("admin") +box.space._user:select{} +box.schema.user.grant("user1", "alter", "user", "user2") +box.session.su("user1") +box.schema.user.passwd("user2", "abcd") +-- still fails +box.schema.user.passwd("user3", "qewr") +box.session.su("admin") +box.space._user:select{} +box.schema.user.drop("user1") +box.schema.user.drop("user2") +box.schema.user.drop("user3") diff --git a/test/box/role.result b/test/box/role.result index 243c7bc6c..5666f7ef7 100644 --- a/test/box/role.result +++ b/test/box/role.result @@ -49,6 +49,9 @@ box.schema.user.info('tester') - - session,usage - universe - + - - alter + - user + - tester ... box.schema.user.grant('tester', 'execute', 'role', 'iddqd') --- @@ -64,6 +67,9 @@ box.schema.user.info('tester') - - session,usage - universe - + - - alter + - user + - tester ... -- test granting user to a user box.schema.user.grant('tester', 'execute', 'role', 'tester') @@ -956,6 +962,9 @@ box.schema.user.info('test_user') - - session,usage - universe - + - - alter + - user + - test_user ... box.schema.role.info('test_user') --- diff --git a/test/box/sequence.result b/test/box/sequence.result index a2a1a60ea..b3907659f 100644 --- a/test/box/sequence.result +++ b/test/box/sequence.result @@ -1362,6 +1362,9 @@ box.schema.user.info() - - session,usage - universe - + - - alter + - user + - user ... sq:set(100) -- ok --- -- 2.15.2 (Apple Git-101.1)
next prev parent reply other threads:[~2018-07-30 8:37 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-17 15:47 [tarantool-patches] [PATCH 0/4] Fixes in access control and privileges Serge Petrenko 2018-07-17 15:47 ` [tarantool-patches] [PATCH 1/4] Make access_check_ddl check for entity privileges Serge Petrenko 2018-07-17 15:47 ` [tarantool-patches] [PATCH 2/4] Add entities user, role to access control Serge Petrenko 2018-07-17 15:47 ` [tarantool-patches] [PATCH 3/4] Add single object privilege checks to access_check_ddl Serge Petrenko 2018-07-17 15:47 ` [tarantool-patches] [PATCH 4/4] Add a privilege upgrade script and update tests Serge Petrenko [not found] ` <cover.1531843622.git.sergepetrenko@tarantool.org> 2018-07-17 16:08 ` [tarantool-patches] [PATCH v2 1/4] Make access_check_ddl check for entity privileges Serge Petrenko 2018-07-17 16:08 ` [tarantool-patches] [PATCH v2 2/4] Add entities user, role to access control Serge Petrenko 2018-07-17 16:08 ` [tarantool-patches] [PATCH v2 3/4] Add single object privilege checks to access_check_ddl Serge Petrenko 2018-07-26 20:37 ` [tarantool-patches] " Konstantin Osipov 2018-07-30 8:37 ` Sergey Petrenko [this message] 2018-07-17 16:08 ` [tarantool-patches] [PATCH v2 4/4] Add a privilege upgrade script and update tests Serge Petrenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=D79F007B-84B4-4C04-A72E-440B041A2FFB@tarantool.org \ --to=sergepetrenko@tarantool.org \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v2 3/4] Add single object privilege checks to access_check_ddl.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox