From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 0926D28246 for ; Mon, 30 Jul 2018 04:37:06 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8aJ03p2unj7z for ; Mon, 30 Jul 2018 04:37:04 -0400 (EDT) Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4B07424A28 for ; Mon, 30 Jul 2018 04:37:04 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: [tarantool-patches] Re: [PATCH v2 3/4] Add single object privilege checks to access_check_ddl. From: Sergey Petrenko In-Reply-To: <20180726203731.GA32000@chai> Date: Mon, 30 Jul 2018 11:37:01 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20180726203731.GA32000@chai> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Konstantin Osipov Cc: tarantool-patches@freelists.org 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 =D0=B8=D1=8E=D0=BB=D1=8F 2018 =D0=B3., =D0=B2 23:37, Konstantin = Osipov =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > * Serge Petrenko [18/07/17 21:31]: >=20 > 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. >=20 >> 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. >>=20 >> 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. >=20 > 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=E2=80=99s 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) >=20 >> 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(-) >>=20 >> 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. */ >>=20 >> 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 =3D=3D 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. */ >=20 > 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. >=20 >> + switch (type) { >> + case SC_SPACE: >> + { >> + struct space *space =3D space_by_id(object_id); >> + if (space) >> + access &=3D = ~space->access[cr->auth_token].effective; >> + break; >> + } >> + case SC_FUNCTION: >> + { >> + struct func *func =3D func_by_id(object_id); >> + if (func) >> + access &=3D = ~func->access[cr->auth_token].effective; >> + break; >> + } >> + case SC_USER: >> + case SC_ROLE: >> + { >> + struct user *user_or_role =3D = user_by_id(object_id); >> + if (user_or_role) >> + access &=3D = ~user_or_role->access[cr->auth_token].effective; >=20 >> + break; >> + } >> + case SC_SEQUENCE: >> + { >> + struct sequence *seq =3D = sequence_by_id(object_id); >> + if (seq) >> + access &=3D = ~seq->access[cr->auth_token].effective; >> + break; >> + } >> + default: >> + break; >> + } >=20 > OK. >=20 >> * 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); >>=20 >> struct trigger *on_commit =3D >> 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 =3D { >> ["role"] =3D bit.bor(box.priv.X, box.priv.U, >> box.priv.C, box.priv.D), >> ["user"] =3D 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) >=20 > --=20 > Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 > http://tarantool.io - www.twitter.com/kostja_osipov =20 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. */ =20 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 =3D=3D 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 =3D space_by_id(object_id); + if (space) + access &=3D = ~space->access[cr->auth_token].effective; + break; + } + case SC_FUNCTION: + { + struct func *func =3D func_by_id(object_id); + if (func) + access &=3D = ~func->access[cr->auth_token].effective; + break; + } + case SC_USER: + case SC_ROLE: + { + struct user *user_or_role =3D user_by_id(object_id); + if (user_or_role) + access &=3D = ~user_or_role->access[cr->auth_token].effective; + break; + } + case SC_SEQUENCE: + { + struct sequence *seq =3D sequence_by_id(object_id); + if (seq) + access &=3D = ~seq->access[cr->auth_token].effective; + break; + } + default: + break; + } + if (access =3D=3D 0) + return; /* Access granted. */ =20 /* Create a meaningful error message. */ - struct user *user =3D user_find_xc(cr->uid); +error: struct user *user =3D 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 =3D 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 =3D make_scoped_guard([=3D] { 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 =3D=3D 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 =3D 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 =3D make_scoped_guard([=3D] { space_def_delete(def); = }); if (def->id !=3D space_id(old_space)) @@ -1774,8 +1819,8 @@ on_replace_dd_index(struct trigger * /* trigger = */, void *event) enum priv_type priv_type =3D new_tuple ? PRIV_C : PRIV_D; if (old_tuple && new_tuple) priv_type =3D 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 =3D space_index(old_space, iid); =20 /* @@ -2170,7 +2215,8 @@ on_replace_dd_user(struct trigger * /* trigger */, = void *event) struct user *old_user =3D user_by_id(uid); if (new_tuple !=3D NULL && old_user =3D=3D NULL) { /* INSERT */ struct user_def *user =3D = 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 =3D make_scoped_guard([=3D] { free(user); = }); (void) user_cache_replace(user); def_guard.is_active =3D 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 =3D=3D 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 <=3D (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid =3D=3D= SUPER) { @@ -2205,8 +2252,8 @@ on_replace_dd_user(struct trigger * /* trigger */, = void *event) * correct. */ struct user_def *user =3D = 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 =3D make_scoped_guard([=3D] { free(user); = }); struct trigger *on_commit =3D 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 =3D func_by_id(fid); if (new_tuple !=3D NULL && old_func =3D=3D NULL) { /* INSERT */ struct func_def *def =3D = 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 =3D make_scoped_guard([=3D] { free(def); = }); func_cache_replace(def); def_guard.is_active =3D 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 =3D = func_def_new_from_tuple(new_tuple); auto def_guard =3D make_scoped_guard([=3D] { 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 =3D 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 =3D coll_by_id(old_id); assert(old_coll_id !=3D 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 =3D coll_id_new(&new_def); if (new_coll_id =3D=3D 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 =3D 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 !=3D ADMIN) { @@ -3075,8 +3124,8 @@ on_replace_dd_sequence(struct trigger * /* trigger = */, void *event) new_def =3D sequence_def_new_from_tuple(new_tuple, = ER_CREATE_SEQUENCE); assert(sequence_by_id(new_def->id) =3D=3D 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 =3D new_def; } else if (old_tuple !=3D NULL && new_tuple =3D=3D NULL) { = /* DELETE */ @@ -3084,8 +3133,8 @@ on_replace_dd_sequence(struct trigger * /* trigger = */, void *event) = BOX_SEQUENCE_DATA_FIELD_ID); struct sequence *seq =3D sequence_by_id(id); assert(seq !=3D 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 =3D sequence_by_id(new_def->id); assert(seq !=3D 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 =3D seq->def; alter->new_def =3D new_def; } @@ -3182,21 +3231,21 @@ on_replace_dd_space_sequence(struct trigger * /* = trigger */, void *event) =20 /* 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); =20 struct trigger *on_commit =3D 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 =3D function(name, opts) uid =3D _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 =3D user_by_id(priv->object_id); + if (user) + access =3D user->access; break; } case SC_ROLE: { - /* No grants on a single object role yet. */ + struct user *role =3D user_by_id(priv->object_id); + if (role) + access =3D 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]; }; =20 /** 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 -=20 + - - alter + - user + - test_user ... box.schema.role.info('test_role') --- @@ -995,6 +1007,9 @@ box.schema.user.info('test_user') - - session,usage - universe -=20 + - - 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") +--- +... +_ =3D 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] =3D=3D 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") +--- +... +_ =3D 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] ~=3D 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=3D'}] + - [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=3D'}] + - [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=3D'}] + - [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=3D'}] + - [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=3D'}] + - [33, 1, 'user2', 'user', {'chap-sha1': = 'oVTFJWXp5/lL/Aih/nAmJO2O/9o=3D'}] + - [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 @@ _ =3D box.schema.sequence.create('test_sequence') box.session.su('admin') box.schema.user.drop('tester') =20 + +-- +-- test case for 3530: do not ignore single object privileges in +-- access_check_ddl. +-- +box.schema.user.create("test") +_ =3D 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] =3D=3D 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") +_ =3D box.space.space1:create_index("pk") +box.space.space1:insert{5} +box.session.su("admin") +box.space.space1.index[0] ~=3D 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 -=20 + - - alter + - user + - tester ... box.schema.user.grant('tester', 'execute', 'role', 'iddqd') --- @@ -64,6 +67,9 @@ box.schema.user.info('tester') - - session,usage - universe -=20 + - - 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 -=20 + - - 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 -=20 + - - alter + - user + - user ... sq:set(100) -- ok --- --=20 2.15.2 (Apple Git-101.1)