[tarantool-patches] Re: [PATCH v2 2/2] access: update credentials without reconnect
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Oct 5 17:11:59 MSK 2019
Changes in this commit are below.
The full new commit with a new commit message is at the bottom of
the email.
============================================================================
diff --git a/src/box/user.cc b/src/box/user.cc
index d3aa22031..123c46f19 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -36,7 +36,6 @@
#include "index.h"
#include "bit/bit.h"
#include "fiber.h"
-#include "trigger.h"
#include "scoped_guard.h"
#include "sequence.h"
#include "tt_static.h"
@@ -161,13 +160,14 @@ 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);
+ rlist_create(&user->credentials_list);
}
static void
user_destroy(struct user *user)
{
- trigger_destroy(&user->on_access_update);
+ while (!rlist_empty(&user->credentials_list))
+ rlist_shift(&user->credentials_list);
/*
* Sic: we don't have to remove a deleted
* user from users set of roles, since
@@ -361,7 +361,11 @@ user_reload_privs(struct user *user)
}
user_set_effective_access(user);
user->is_dirty = false;
- trigger_run(&user->on_access_update, user);
+
+ struct credentials *creds;
+ user_access_t new_access = universe.access[user->auth_token].effective;
+ rlist_foreach_entry(creds, &user->credentials_list, in_user)
+ creds->universal_access = new_access;
}
/** }}} */
@@ -713,28 +717,13 @@ 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);
+ rlist_add_entry(&user->credentials_list, cr, in_user);
}
void
@@ -743,11 +732,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);
+ rlist_create(&cr->in_user);
}
void
credentials_destroy(struct credentials *cr)
{
- trigger_clear(&cr->user_on_access_update);
+ rlist_del_entry(cr, in_user);
}
diff --git a/src/box/user.h b/src/box/user.h
index 7b96cd131..b9b225b5f 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -89,10 +89,11 @@ struct user
/** Memory pool for privs */
struct region pool;
/**
- * Triggers to invoke on privilege update. The triggers
- * take the user pointer as an argument.
+ * List of all currently existing credentials caches of
+ * the user. Any update of user privileges is applied to
+ * them.
*/
- struct rlist on_access_update;
+ struct rlist credentials_list;
/** 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 6be3a42d1..486a4ae50 100644
--- a/src/box/user_def.h
+++ b/src/box/user_def.h
@@ -34,7 +34,7 @@
#include "scramble.h" /* for SCRAMBLE_SIZE */
#define RB_COMPACT 1
#include "small/rb.h"
-#include "trigger.h"
+#include "small/rlist.h"
#if defined(__cplusplus)
extern "C" {
@@ -63,10 +63,11 @@ struct credentials {
/** User id of the authenticated user. */
uint32_t uid;
/**
- * Trigger to subscribe on user access changes and update
- * the cached universal access.
+ * Member of credentials list of the source user. The list
+ * is used to collect privilege updates to keep the
+ * credentials up to date.
*/
- struct trigger user_on_access_update;
+ struct rlist in_user;
};
enum priv_type {
============================================================================
Full commit:
============================================================================
access: update credentials without reconnect
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 update all its credentials caches with new
privileges, via a list of all creds.
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.
diff --git a/src/box/user.cc b/src/box/user.cc
index f12d65d27..123c46f19 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -35,7 +35,7 @@
#include "func.h"
#include "index.h"
#include "bit/bit.h"
-#include "session.h"
+#include "fiber.h"
#include "scoped_guard.h"
#include "sequence.h"
#include "tt_static.h"
@@ -160,11 +160,14 @@ 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->credentials_list);
}
static void
user_destroy(struct user *user)
{
+ while (!rlist_empty(&user->credentials_list))
+ rlist_shift(&user->credentials_list);
/*
* 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,11 @@ user_reload_privs(struct user *user)
}
user_set_effective_access(user);
user->is_dirty = false;
+
+ struct credentials *creds;
+ user_access_t new_access = universe.access[user->auth_token].effective;
+ rlist_foreach_entry(creds, &user->credentials_list, in_user)
+ creds->universal_access = new_access;
}
/** }}} */
@@ -721,6 +723,7 @@ 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;
+ rlist_add_entry(&user->credentials_list, cr, in_user);
}
void
@@ -729,10 +732,11 @@ credentials_create_empty(struct credentials *cr)
cr->auth_token = BOX_USER_MAX;
cr->universal_access = 0;
cr->uid = BOX_USER_MAX;
+ rlist_create(&cr->in_user);
}
void
credentials_destroy(struct credentials *cr)
{
- (void) cr;
+ rlist_del_entry(cr, in_user);
}
diff --git a/src/box/user.h b/src/box/user.h
index 5f320f739..b9b225b5f 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -88,6 +88,12 @@ struct user
bool is_dirty;
/** Memory pool for privs */
struct region pool;
+ /**
+ * List of all currently existing credentials caches of
+ * the user. Any update of user privileges is applied to
+ * them.
+ */
+ struct rlist credentials_list;
/** 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..486a4ae50 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 "small/rlist.h"
#if defined(__cplusplus)
extern "C" {
@@ -61,6 +62,12 @@ struct credentials {
user_access_t universal_access;
/** User id of the authenticated user. */
uint32_t uid;
+ /**
+ * Member of credentials list of the source user. The list
+ * is used to collect privilege updates to keep the
+ * credentials up to date.
+ */
+ struct rlist in_user;
};
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')
More information about the Tarantool-patches
mailing list