Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] Fix nested calls to box.session.su()
@ 2018-07-03 10:40 Serge Petrenko
  2018-07-03 15:35 ` [tarantool-patches] [PATCH v2] " Serge Petrenko
  0 siblings, 1 reply; 2+ messages in thread
From: Serge Petrenko @ 2018-07-03 10:40 UTC (permalink / raw)
  To: tarantool-patches; +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)

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-03 15:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-03 10:40 [tarantool-patches] [PATCH] Fix nested calls to box.session.su() Serge Petrenko
2018-07-03 15:35 ` [tarantool-patches] [PATCH v2] " Serge Petrenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox