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 5568421BCB for ; Tue, 3 Jul 2018 06:40:55 -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 lQ--76mTicdl for ; Tue, 3 Jul 2018 06:40:55 -0400 (EDT) Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (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 1228E21BC1 for ; Tue, 3 Jul 2018 06:40:54 -0400 (EDT) From: Serge Petrenko Subject: [tarantool-patches] [PATCH] Fix nested calls to box.session.su() Date: Tue, 3 Jul 2018 13:40:20 +0300 Message-Id: <20180703104020.43833-1-sergepetrenko@tarantool.org> 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: tarantool-patches@freelists.org Cc: Serge Petrenko box.session.su() set effective user to user after its execution, which made nested calls to it not work. Fixed this by saving current effective user and recovering from the save after sudo execution. This opened up a bug in box.schema.user.drop(): it has unnecessary check for privelege PRIV_REVOKE, which never gets granted to anyone but admin. Also fixed this by adding one extra box.session.su() call. Closes #3090, #3492 --- https://github.com/tarantool/tarantool/tree/sergepetrenko/gh-3090-nested-su-fix https://github.com/tarantool/tarantool/issues/3090 https://github.com/tarantool/tarantool/issues/3492 src/box/lua/schema.lua | 5 +++- src/box/lua/session.c | 7 ++--- test/box/access.result | 69 +++++++++++++++++++++++++++++++++++++++++++----- test/box/access.test.lua | 37 ++++++++++++++++++++++---- 4 files changed, 103 insertions(+), 15 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 30c6bc69c..788b0be89 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -2038,9 +2038,12 @@ local function drop(uid, opts) box.session.su('admin', box.schema.user.revoke, uid, 'session,usage', 'universe', nil, {if_exists = true}) end + local privs = _priv.index.primary:select{uid} for k, tuple in pairs(privs) do - revoke(uid, uid, tuple[5], tuple[3], tuple[4]) + -- we need an additional box.session.su() here, because of + -- unnecessary check for privilege PRIV_REVOKE in priv_def_check() + box.session.su("admin", revoke, uid, uid, tuple[5], tuple[3], tuple[4]) end box.space[box.schema.USER_ID]:delete{uid} end diff --git a/src/box/lua/session.c b/src/box/lua/session.c index 51caf199f..cb8fa411a 100644 --- a/src/box/lua/session.c +++ b/src/box/lua/session.c @@ -188,16 +188,17 @@ lbox_session_su(struct lua_State *L) fiber_set_user(fiber(), &session->credentials); return 0; /* su */ } - + struct credentials su_credentials; + struct credentials *old_credentials = + (struct credentials *)fiber_get_key(fiber(), FIBER_KEY_USER); credentials_init(&su_credentials, user->auth_token, user->def->uid); fiber_set_user(fiber(), &su_credentials); - /* sudo */ luaL_checktype(L, 2, LUA_TFUNCTION); int error = lua_pcall(L, top - 2, LUA_MULTRET, 0); /* Restore the original credentials. */ - fiber_set_user(fiber(), &session->credentials); + fiber_set_user(fiber(), old_credentials); if (error) luaT_error(L); diff --git a/test/box/access.result b/test/box/access.result index 191857f49..039790d91 100644 --- a/test/box/access.result +++ b/test/box/access.result @@ -1414,19 +1414,76 @@ _ = 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") +-- gh-3090: nested su unexpected behaviour. +-- gh-3492: su doesn't grant effective privileges +-- should print guest +test_run:cmd("setopt delimiter ';'") +--- +- true +... +function f() + box.session.su("admin", function() end) + return box.session.effective_user() +end; +--- +... +box.session.su("guest", f); +--- +- guest +... +-- the call of test_admin below shouldn't fail +function test_admin() + box.schema.user.create('storage', {password = 'storage', if_not_exists=true}) + + box.schema.user.grant('storage', 'replication', nil, nil, {if_not_exists=true}) +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +box.session.su("guest") +--- +... +box.session.user() --- +- guest ... -box.schema.user.drop("test") +box.session.effective_user() --- -- error: Revoke access to role 'public' is denied for user 'tester' +- guest +... +box.session.su("admin", test_admin) +--- +... +box.session.user() +--- +- guest +... +box.session.effective_user() +--- +- guest ... box.session.su("admin") --- ... +box.session.user() +--- +- admin +... +box.session.effective_user() +--- +- admin +... +box.schema.user.drop("storage") +--- +... +-- now sudo can be used here since nested sudo is fixed +box.session.su("tester", box.schema.user.drop, "test") +--- +- error: Drop access to user 'test' is denied for user 'tester' +... box.session.su("tester", box.schema.func.drop, "test") --- - error: Drop access to function 'test' is denied for user 'tester' diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 7e880a0cf..c5fa1f138 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -544,13 +544,40 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func') -- failed drop -- box.session.su("tester", s.drop, s) --- can't use here sudo --- because drop use sudo inside --- and currently sudo can't be performed nested -box.session.su("tester") -box.schema.user.drop("test") + +-- gh-3090: nested su unexpected behaviour. +-- gh-3492: su doesn't grant effective privileges + +-- should print guest +test_run:cmd("setopt delimiter ';'") +function f() + box.session.su("admin", function() end) + return box.session.effective_user() +end; +box.session.su("guest", f); + +-- the call of test_admin below shouldn't fail +function test_admin() + box.schema.user.create('storage', {password = 'storage', if_not_exists=true}) + + box.schema.user.grant('storage', 'replication', nil, nil, {if_not_exists=true}) +end; +test_run:cmd("setopt delimiter ''"); + +box.session.su("guest") +box.session.user() +box.session.effective_user() +box.session.su("admin", test_admin) +box.session.user() +box.session.effective_user() box.session.su("admin") +box.session.user() +box.session.effective_user() +box.schema.user.drop("storage") + +-- now sudo can be used here since nested sudo is fixed +box.session.su("tester", box.schema.user.drop, "test") box.session.su("tester", box.schema.func.drop, "test") box.schema.user.grant("tester", "drop", "universe") -- 2.15.2 (Apple Git-101.1)