[tarantool-patches] [PATCH 1/1] session: update credentials without reconnect
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Oct 3 18:07:59 MSK 2019
Session has real user's credentials. The credentials have auth
token to obtain object access privileges (to a space, to an index,
to a function, etc), and a mask of global universe privileges.
Object privileges are not cached. Each access to an object is a
lookup in a tree of user's privileges.
Universe privileges are cached. And before this patch when
universe privs of a user were changed, it was not reflected in his
sessions. Therefore, the sessions opened before priv change had
outdated universe privs.
Now each session subscribes on privileges update of the real user.
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
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-2763-credentials-cache-update
Issue: https://github.com/tarantool/tarantool/issues/2763
src/box/authentication.cc | 3 +-
src/box/lua/session.c | 3 +-
src/box/session.cc | 37 ++++++++++--
src/box/session.h | 20 ++++---
src/box/user.cc | 12 ++--
src/box/user.h | 13 ++++
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 | 60 +++++++++++++++++++
...h-2763-session-credentials-update.test.lua | 35 +++++++++++
11 files changed, 167 insertions(+), 32 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/authentication.cc b/src/box/authentication.cc
index fdad7395a..33b43b2d4 100644
--- a/src/box/authentication.cc
+++ b/src/box/authentication.cc
@@ -100,6 +100,5 @@ ok:
if (! rlist_empty(&session_on_auth) &&
session_run_on_auth_triggers(&auth_res) != 0)
diag_raise();
- credentials_init(&session->credentials, user->auth_token,
- user->def->uid);
+ session_set_user(session, user);
}
diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index b9495e7a6..f731a424d 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -185,8 +185,7 @@ lbox_session_su(struct lua_State *L)
luaT_error(L);
if (top == 1) {
- credentials_init(&session->credentials, user->auth_token,
- user->def->uid);
+ session_set_user(session, user);
fiber_set_user(fiber(), &session->credentials);
return 0; /* su */
}
diff --git a/src/box/session.cc b/src/box/session.cc
index 59bf226dd..0c9c21bea 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -124,6 +124,24 @@ session_set_type(struct session *session, enum session_type type)
session->vtab = &session_vtab_registry[type];
}
+/**
+ * Callback invoked on each update of real user's privileges. The
+ * callback updates session's credentials cache. Without an update
+ * a session would have outdated credentials in case privileges
+ * are changed from another session. Strictly speaking, even a
+ * change from the same session needs this callback.
+ */
+static void
+session_user_on_credentials_update(struct trigger *trigger, void *event)
+{
+ struct session *session = container_of(trigger, struct session,
+ user_on_access_update);
+ struct user *user = (struct user *) event;
+ assert(user->def->uid == session->credentials.uid);
+ credentials_init(&session->credentials, user->auth_token,
+ user->def->uid);
+}
+
struct session *
session_create(enum session_type type)
{
@@ -140,10 +158,11 @@ session_create(enum session_type type)
session_set_type(session, type);
session->sql_flags = default_flags;
session->sql_default_engine = SQL_STORAGE_ENGINE_MEMTX;
+ trigger_create(&session->user_on_access_update,
+ session_user_on_credentials_update, NULL, NULL);
/* For on_connect triggers. */
- credentials_init(&session->credentials, guest_user->auth_token,
- guest_user->def->uid);
+ session_set_user(session, guest_user);
struct mh_i64ptr_node_t node;
node.key = session->id;
node.val = session;
@@ -151,6 +170,7 @@ session_create(enum session_type type)
mh_int_t k = mh_i64ptr_put(session_registry, &node, NULL, NULL);
if (k == mh_end(session_registry)) {
+ trigger_clear(&session->user_on_access_update);
mempool_free(&session_pool, session);
diag_set(OutOfMemory, 0, "session hash", "new session");
return NULL;
@@ -172,8 +192,7 @@ session_create_on_demand()
};
/* Add a trigger to destroy session on fiber stop */
trigger_add(&fiber()->on_stop, &s->fiber_on_stop);
- credentials_init(&s->credentials, admin_user->auth_token,
- admin_user->def->uid);
+ session_set_user(s, admin_user);
/*
* At bootstrap, admin user access is not loaded yet (is
* 0), force global access. @sa comment in session_init()
@@ -184,6 +203,15 @@ session_create_on_demand()
return s;
}
+void
+session_set_user(struct session *session, struct user *user)
+{
+ trigger_clear(&session->user_on_access_update);
+ credentials_init(&session->credentials, user->auth_token,
+ user->def->uid);
+ trigger_add(&user->on_access_update, &session->user_on_access_update);
+}
+
/**
* To quickly switch to admin user when executing
* on_connect/on_disconnect triggers in iproto.
@@ -232,6 +260,7 @@ session_destroy(struct session *session)
session_storage_cleanup(session->id);
struct mh_i64ptr_node_t node = { session->id, NULL };
mh_i64ptr_remove(session_registry, &node, NULL);
+ trigger_clear(&session->user_on_access_update);
mempool_free(&session_pool, session);
}
diff --git a/src/box/session.h b/src/box/session.h
index 85a2d940b..cd48d7304 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -105,6 +105,11 @@ struct session {
struct credentials credentials;
/** Trigger for fiber on_stop to cleanup created on-demand session */
struct trigger fiber_on_stop;
+ /**
+ * Trigger to update cached credentials when user's
+ * privileges are changed.
+ */
+ struct trigger user_on_access_update;
};
struct session_vtab {
@@ -193,14 +198,6 @@ fiber_set_session(struct fiber *fiber, struct session *session)
fiber->storage.session = session;
}
-static inline void
-credentials_init(struct credentials *cr, uint8_t auth_token, uint32_t uid)
-{
- cr->auth_token = auth_token;
- cr->universal_access = universe.access[cr->auth_token].effective;
- cr->uid = uid;
-}
-
/*
* For use in local hot standby, which runs directly
* from ev watchers (without current fiber), but needs
@@ -275,6 +272,13 @@ session_storage_cleanup(int sid);
struct session *
session_create(enum session_type type);
+/**
+ * Set @a user as a real user. His credentials are cached and the
+ * session subscribes on update of the credentials.
+ */
+void
+session_set_user(struct session *session, struct user *user);
+
/**
* Destroy a session.
* Must be called by the networking layer on disconnect.
diff --git a/src/box/user.cc b/src/box/user.cc
index c46ff67d1..9d5490924 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);
}
/** }}} */
diff --git a/src/box/user.h b/src/box/user.h
index 527fb2e7c..815f12ba3 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -47,6 +47,14 @@ struct universe {
/** A single instance of the universe. */
extern struct universe universe;
+static inline void
+credentials_init(struct credentials *cr, uint8_t auth_token, uint32_t uid)
+{
+ cr->auth_token = auth_token;
+ cr->universal_access = universe.access[cr->auth_token].effective;
+ cr->uid = uid;
+}
+
/** Bitmap type for used/unused authentication token map. */
typedef unsigned int umap_int_t;
enum {
@@ -88,6 +96,11 @@ struct user
bool is_dirty;
/** Memory pool for privs */
struct region pool;
+ /**
+ * Triggers to invoke on privileges 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/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..2c4b10be3
--- /dev/null
+++ b/test/box/gh-2763-session-credentials-update.result
@@ -0,0 +1,60 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+netbox = require('net.box')
+ | ---
+ | ...
+--
+-- gh-2763: when credentials of a user are updated, it should be
+-- reflected in all his sessions.
+--
+
+box.schema.user.create('test_user', {password = '1'})
+ | ---
+ | ...
+function testf() 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, 'testf') \
+ 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('testf') == 'success') \
+ c:close() \
+end
+ | ---
+ | ...
+
+box.schema.user.revoke('guest', 'execute', 'universe')
+ | ---
+ | ...
+box.schema.user.drop('test_user')
+ | ---
+ | ...
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..d237d1869
--- /dev/null
+++ b/test/box/gh-2763-session-credentials-update.test.lua
@@ -0,0 +1,35 @@
+env = require('test_run')
+test_run = env.new()
+netbox = require('net.box')
+--
+-- gh-2763: when credentials of a user are updated, it should be
+-- reflected in all his sessions.
+--
+
+box.schema.user.create('test_user', {password = '1'})
+function testf() 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, 'testf') \
+ 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('testf') == 'success') \
+ c:close() \
+end
+
+box.schema.user.revoke('guest', 'execute', 'universe')
+box.schema.user.drop('test_user')
--
2.21.0 (Apple Git-122)
More information about the Tarantool-patches
mailing list