[tarantool-patches] [PATCH v2 2/2] access: update credentials without reconnect

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Sat Oct 5 01:26:00 MSK 2019


Credentials is a cache of user universal privileges. And that
cache can become outdated in case user privs were changed after
creation of the cache.

The patch makes user notify all its credentials caches about privs
change, via a trigger. Each credentials object now registers
itself in user and gets all updates.

That solves a couple of real life problems:

- If a user managed to connect after box.cfg started listening
port, but before access was granted, then he needed a reconnect;

- Even if access was granted, a user may connect after box.cfg
listen, but before access *is recovered* from _priv space. It
was not possible to fix without a reconnect. And this problem
affected replication.

Closes #2763
Part of #4535
Part of #4536

@TarantoolBot document
Title: User privileges update affects existing sessions and objects
Previously if user privileges were updated (via
`box.schema.user.grant/revoke`), it was not reflected in already
existing sessions and objects like functions. Now it is.

For example:
```
        box.cfg{listen = 3313}
        box.schema.user.create('test_user', {password = '1'})
        function test1() return 'success' end

        c = require('net.box').connect(box.cfg.listen, {
                user = 'test_user', password = '1'
        })
        -- Error, no access for this connection.
        c:call('test1')

        box.schema.user.grant('test_user', 'execute', 'universe')
        -- Now works, even though access was granted after
        -- connection.
        c:call('test1')
```

A similar thing happens now with `box.session.su` and functions
created via `box.schema.func.create` with `setuid` flag.

In other words, now user privileges update is reflected
everywhere immediately.
---
 src/box/user.cc                               |  31 +++-
 src/box/user.h                                |   5 +
 src/box/user_def.h                            |   6 +
 test/box/access_bin.result                    |   9 +-
 test/box/access_bin.test.lua                  |   5 +-
 test/box/access_misc.result                   |   2 +-
 .../gh-2763-session-credentials-update.result | 170 ++++++++++++++++++
 ...h-2763-session-credentials-update.test.lua |  93 ++++++++++
 8 files changed, 304 insertions(+), 17 deletions(-)
 create mode 100644 test/box/gh-2763-session-credentials-update.result
 create mode 100644 test/box/gh-2763-session-credentials-update.test.lua

diff --git a/src/box/user.cc b/src/box/user.cc
index f12d65d27..d3aa22031 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -35,7 +35,8 @@
 #include "func.h"
 #include "index.h"
 #include "bit/bit.h"
-#include "session.h"
+#include "fiber.h"
+#include "trigger.h"
 #include "scoped_guard.h"
 #include "sequence.h"
 #include "tt_static.h"
@@ -160,11 +161,13 @@ user_create(struct user *user, uint8_t auth_token)
 	user->auth_token = auth_token;
 	privset_new(&user->privs);
 	region_create(&user->pool, &cord()->slabc);
+	rlist_create(&user->on_access_update);
 }
 
 static void
 user_destroy(struct user *user)
 {
+	trigger_destroy(&user->on_access_update);
 	/*
 	 * Sic: we don't have to remove a deleted
 	 * user from users set of roles, since
@@ -282,7 +285,6 @@ access_find(enum schema_object_type object_type, uint32_t object_id)
 static void
 user_set_effective_access(struct user *user)
 {
-	struct credentials *cr = effective_user();
 	struct privset_iterator it;
 	privset_ifirst(&user->privs, &it);
 	struct priv_def *priv;
@@ -294,11 +296,6 @@ user_set_effective_access(struct user *user)
 			continue;
 		struct access *access = &object[user->auth_token];
 		access->effective = access->granted | priv->access;
-		/** Update global access in the current session. */
-		if (priv->object_type == SC_UNIVERSE &&
-		    user->def->uid == cr->uid) {
-			cr->universal_access = access->effective;
-		}
 	}
 }
 
@@ -364,6 +361,7 @@ user_reload_privs(struct user *user)
 	}
 	user_set_effective_access(user);
 	user->is_dirty = false;
+	trigger_run(&user->on_access_update, user);
 }
 
 /** }}} */
@@ -715,12 +713,28 @@ priv_grant(struct user *grantee, struct priv_def *priv)
 
 /** }}} */
 
+static void
+credentials_user_on_access_update(struct trigger *trigger, void *event)
+{
+	struct credentials *creds = container_of(
+		trigger, struct credentials, user_on_access_update);
+	struct user *user = (struct user *) event;
+	(void) event;
+	(void) user;
+	assert(user->def->uid == creds->uid);
+	assert(user->auth_token == creds->auth_token);
+	creds->universal_access = universe.access[creds->auth_token].effective;
+}
+
 void
 credentials_create(struct credentials *cr, struct user *user)
 {
 	cr->auth_token = user->auth_token;
 	cr->universal_access = universe.access[user->auth_token].effective;
 	cr->uid = user->def->uid;
+	trigger_create(&cr->user_on_access_update,
+		       credentials_user_on_access_update, NULL, NULL);
+	trigger_add(&user->on_access_update, &cr->user_on_access_update);
 }
 
 void
@@ -729,10 +743,11 @@ credentials_create_empty(struct credentials *cr)
 	cr->auth_token = BOX_USER_MAX;
 	cr->universal_access = 0;
 	cr->uid = BOX_USER_MAX;
+	trigger_create(&cr->user_on_access_update, NULL, NULL, NULL);
 }
 
 void
 credentials_destroy(struct credentials *cr)
 {
-	(void) cr;
+	trigger_clear(&cr->user_on_access_update);
 }
diff --git a/src/box/user.h b/src/box/user.h
index 5f320f739..7b96cd131 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -88,6 +88,11 @@ struct user
 	bool is_dirty;
 	/** Memory pool for privs */
 	struct region pool;
+	/**
+	 * Triggers to invoke on privilege update. The triggers
+	 * take the user pointer as an argument.
+	 */
+	struct rlist on_access_update;
 	/** Cached runtime access imformation. */
 	struct access access[BOX_USER_MAX];
 };
diff --git a/src/box/user_def.h b/src/box/user_def.h
index 98097c396..6be3a42d1 100644
--- a/src/box/user_def.h
+++ b/src/box/user_def.h
@@ -34,6 +34,7 @@
 #include "scramble.h" /* for SCRAMBLE_SIZE */
 #define RB_COMPACT 1
 #include "small/rb.h"
+#include "trigger.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -61,6 +62,11 @@ struct credentials {
 	user_access_t universal_access;
 	/** User id of the authenticated user. */
 	uint32_t uid;
+	/**
+	 * Trigger to subscribe on user access changes and update
+	 * the cached universal access.
+	 */
+	struct trigger user_on_access_update;
 };
 
 enum priv_type {
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index 9f3ec8ada..c58f331d3 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -213,9 +213,8 @@ box.space._user:replace(u)
 - [1, 1, 'admin', 'user', {}]
 ...
 --
--- Roles: test that universal access of an authenticated
--- session is not updated if grant is made from another
--- session
+-- gh-2763: test that universal access of an authenticated session
+-- is updated if grant is made from another session.
 --
 test = box.schema.space.create('test')
 ---
@@ -251,7 +250,7 @@ box.schema.role.grant('public', 'read', 'universe')
 ...
 c.space.test:select{}
 ---
-- error: Read access to space 'test' is denied for user 'test'
+- - [1]
 ...
 c:close()
 ---
@@ -268,7 +267,7 @@ box.schema.role.revoke('public', 'read', 'universe')
 ...
 c.space.test:select{}
 ---
-- - [1]
+- error: Read access to space 'test' is denied for user 'test'
 ...
 box.session.su('test')
 ---
diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua
index 6ca752dd4..41d5f4245 100644
--- a/test/box/access_bin.test.lua
+++ b/test/box/access_bin.test.lua
@@ -81,9 +81,8 @@ c:call('dostring', { 'return 2 + 2' })
 c:close()
 box.space._user:replace(u)
 --
--- Roles: test that universal access of an authenticated
--- session is not updated if grant is made from another
--- session
+-- gh-2763: test that universal access of an authenticated session
+-- is updated if grant is made from another session.
 --
 test = box.schema.space.create('test')
 _ = test:create_index('primary')
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index a1b6435bc..1eb5f7b94 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -188,7 +188,7 @@ 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'
+- error: Write access to space '_func' is denied for user 'guest'
 ...
 session.su('admin')
 ---
diff --git a/test/box/gh-2763-session-credentials-update.result b/test/box/gh-2763-session-credentials-update.result
new file mode 100644
index 000000000..225aa6d8f
--- /dev/null
+++ b/test/box/gh-2763-session-credentials-update.result
@@ -0,0 +1,170 @@
+-- test-run result file version 2
+netbox = require('net.box')
+ | ---
+ | ...
+fiber = require('fiber')
+ | ---
+ | ...
+--
+-- gh-2763: when credentials of a user are updated, it should be
+-- reflected in all his sessions and objects.
+--
+
+box.schema.user.create('test_user', {password = '1'})
+ | ---
+ | ...
+function test1() return 'success' end
+ | ---
+ | ...
+
+conns = {}
+ | ---
+ | ...
+for i = 1, 10 do                                                    \
+    local c                                                         \
+    if i % 2 == 0 then                                              \
+        c = netbox.connect(                                         \
+            box.cfg.listen, {user = 'test_user', password = '1'})   \
+    else                                                            \
+        c = netbox.connect(box.cfg.listen)                          \
+    end                                                             \
+    local ok, err = pcall(c.call, c, 'test1')                       \
+    assert(not ok and err.code == box.error.ACCESS_DENIED)          \
+    table.insert(conns, c)                                          \
+end
+ | ---
+ | ...
+
+box.schema.user.grant('test_user', 'execute', 'universe')
+ | ---
+ | ...
+box.schema.user.grant('guest', 'execute', 'universe')
+ | ---
+ | ...
+-- Succeeds without a reconnect.
+for _, c in pairs(conns) do                                         \
+    assert(c:call('test1') == 'success')                            \
+    c:close()                                                       \
+end
+ | ---
+ | ...
+
+box.schema.user.revoke('guest', 'execute', 'universe')
+ | ---
+ | ...
+box.schema.user.drop('test_user')
+ | ---
+ | ...
+
+--
+-- Box.session.su() credentials are updated even when su-ed
+-- function is still in progress.
+--
+
+-- Create a persistent function, because normal Lua functions
+-- does not check 'execute' locally.
+box.schema.func.create("test2", {                                   \
+    language = 'LUA', returns = 'string',                           \
+    body = 'function () return "success" end',                      \
+    is_deterministic = true, param_list = {}                        \
+})
+ | ---
+ | ...
+
+do_wait = true
+ | ---
+ | ...
+ok, err = nil
+ | ---
+ | ...
+function call_wait_call()                                           \
+    ok, err = pcall(box.func.test2.call, box.func.test2)            \
+    while do_wait do fiber.yield() end                              \
+    ok, err = pcall(box.func.test2.call, box.func.test2)            \
+end
+ | ---
+ | ...
+f = fiber.create(box.session.su, 'guest', call_wait_call)
+ | ---
+ | ...
+
+while ok == nil do fiber.yield() end
+ | ---
+ | ...
+-- Error, 'guest' does not have access to 'test2'.
+ok, err
+ | ---
+ | - false
+ | - Execute access to function 'test2' is denied for user 'guest'
+ | ...
+
+box.schema.user.grant('guest', 'execute', 'universe')
+ | ---
+ | ...
+do_wait = false
+ | ---
+ | ...
+while f:status() ~= 'dead' do fiber.yield() end
+ | ---
+ | ...
+-- Should be ok even though su() was still in progress.
+ok, err
+ | ---
+ | - true
+ | - success
+ | ...
+box.schema.user.revoke('guest', 'execute', 'universe')
+ | ---
+ | ...
+
+--
+-- Setuid functions initialize their credentials on demand. And
+-- these credentials should be up to date.
+--
+box.schema.user.grant('guest', 'read, write', 'space', '_func')
+ | ---
+ | ...
+box.schema.user.grant('guest', 'create', 'function')
+ | ---
+ | ...
+box.session.su('guest')
+ | ---
+ | ...
+box.schema.func.create("test3", {                                   \
+    language = 'LUA', returns = 'string',                           \
+    body = 'function () return box.func.test2:call() end',          \
+    is_deterministic = true, param_list = {}, setuid = true         \
+})
+ | ---
+ | ...
+box.session.su('admin')
+ | ---
+ | ...
+-- Error, guest does not have access to 'test2' called from
+-- 'test3'.
+box.func.test3:call()
+ | ---
+ | - error: Execute access to function 'test2' is denied for user 'guest'
+ | ...
+box.schema.user.grant('guest', 'execute', 'universe')
+ | ---
+ | ...
+-- Now the function owner's credentials should be updated, and
+-- anyone called test3 should have updated rights.
+box.func.test3:call()
+ | ---
+ | - success
+ | ...
+
+box.func.test3:drop()
+ | ---
+ | ...
+box.func.test2:drop()
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'create', 'function')
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'read, write', 'space', '_func')
+ | ---
+ | ...
diff --git a/test/box/gh-2763-session-credentials-update.test.lua b/test/box/gh-2763-session-credentials-update.test.lua
new file mode 100644
index 000000000..cd4f62e64
--- /dev/null
+++ b/test/box/gh-2763-session-credentials-update.test.lua
@@ -0,0 +1,93 @@
+netbox = require('net.box')
+fiber = require('fiber')
+--
+-- gh-2763: when credentials of a user are updated, it should be
+-- reflected in all his sessions and objects.
+--
+
+box.schema.user.create('test_user', {password = '1'})
+function test1() return 'success' end
+
+conns = {}
+for i = 1, 10 do                                                    \
+    local c                                                         \
+    if i % 2 == 0 then                                              \
+        c = netbox.connect(                                         \
+            box.cfg.listen, {user = 'test_user', password = '1'})   \
+    else                                                            \
+        c = netbox.connect(box.cfg.listen)                          \
+    end                                                             \
+    local ok, err = pcall(c.call, c, 'test1')                       \
+    assert(not ok and err.code == box.error.ACCESS_DENIED)          \
+    table.insert(conns, c)                                          \
+end
+
+box.schema.user.grant('test_user', 'execute', 'universe')
+box.schema.user.grant('guest', 'execute', 'universe')
+-- Succeeds without a reconnect.
+for _, c in pairs(conns) do                                         \
+    assert(c:call('test1') == 'success')                            \
+    c:close()                                                       \
+end
+
+box.schema.user.revoke('guest', 'execute', 'universe')
+box.schema.user.drop('test_user')
+
+--
+-- Box.session.su() credentials are updated even when su-ed
+-- function is still in progress.
+--
+
+-- Create a persistent function, because normal Lua functions
+-- does not check 'execute' locally.
+box.schema.func.create("test2", {                                   \
+    language = 'LUA', returns = 'string',                           \
+    body = 'function () return "success" end',                      \
+    is_deterministic = true, param_list = {}                        \
+})
+
+do_wait = true
+ok, err = nil
+function call_wait_call()                                           \
+    ok, err = pcall(box.func.test2.call, box.func.test2)            \
+    while do_wait do fiber.yield() end                              \
+    ok, err = pcall(box.func.test2.call, box.func.test2)            \
+end
+f = fiber.create(box.session.su, 'guest', call_wait_call)
+
+while ok == nil do fiber.yield() end
+-- Error, 'guest' does not have access to 'test2'.
+ok, err
+
+box.schema.user.grant('guest', 'execute', 'universe')
+do_wait = false
+while f:status() ~= 'dead' do fiber.yield() end
+-- Should be ok even though su() was still in progress.
+ok, err
+box.schema.user.revoke('guest', 'execute', 'universe')
+
+--
+-- Setuid functions initialize their credentials on demand. And
+-- these credentials should be up to date.
+--
+box.schema.user.grant('guest', 'read, write', 'space', '_func')
+box.schema.user.grant('guest', 'create', 'function')
+box.session.su('guest')
+box.schema.func.create("test3", {                                   \
+    language = 'LUA', returns = 'string',                           \
+    body = 'function () return box.func.test2:call() end',          \
+    is_deterministic = true, param_list = {}, setuid = true         \
+})
+box.session.su('admin')
+-- Error, guest does not have access to 'test2' called from
+-- 'test3'.
+box.func.test3:call()
+box.schema.user.grant('guest', 'execute', 'universe')
+-- Now the function owner's credentials should be updated, and
+-- anyone called test3 should have updated rights.
+box.func.test3:call()
+
+box.func.test3:drop()
+box.func.test2:drop()
+box.schema.user.revoke('guest', 'create', 'function')
+box.schema.user.revoke('guest', 'read, write', 'space', '_func')
-- 
2.21.0 (Apple Git-122)





More information about the Tarantool-patches mailing list