From 61c2948ce6cfe28aab8a7ed181cd5b5431dccd7e Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Tue, 25 Nov 2014 20:58:21 +0300
Subject: [PATCH] Roles (partial implementation).
Track granted roles.
Represent roles granted to a user with a bitmap.
Disallow loops in role grants.
Add tests for box.schema.user.grant('username', 'rolename')
Add basic tests for loops in grant sequences.
Rewrite largely buggy code in authentication token lookup.
Do not waste space on authentication token bitmap, correctly
check for overflow of the bitmap.
---
src/box/alter.cc | 57 ++++++++--
src/box/lua/schema.lua | 16 +++
src/box/session.h | 2 +-
src/box/user.cc | 245 ++++++++++++++++++++++++++++++++++-------
src/box/user.h | 57 +++++++++-
src/errcode.h | 3 +-
test/box/misc.result | 1 +
test/box/role.result | 63 +++++++++++
test/box/role.test.lua | 20 ++++
test/box/select.result | 2 +-
10 files changed, 414 insertions(+), 52 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index c0d6312c3a..1f55dc9fa7 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -74,8 +74,13 @@ access_check_ddl(uint32_t owner_uid)
{
struct credentials *cr = current_user();
/*
- * Only the creator of the space or superuser can modify
- * the space, since we don't have ALTER privilege.
+ * For privileges, only the current user can claim he's
+ * the grantor/owner of the privilege that is being
+ * granted.
+ * For spaces/funcs/other objects, only the creator
+ * of the object or admin can modify the space, since
+ * there is no such thing in Tarantool as GRANT OPTION or
+ * ALTER privilege.
*/
if (owner_uid != cr->uid && cr->uid != ADMIN) {
struct user *user = user_cache_find(cr->uid);
@@ -1448,9 +1453,9 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
static void
priv_def_check(struct priv_def *priv)
{
- struct user_def *grantor = user_cache_find(priv->grantor_id);
+ struct user *grantor = user_cache_find(priv->grantor_id);
/* May be a role */
- struct user_def *grantee = user_by_id(priv->grantee_id);
+ struct user *grantee = user_by_id(priv->grantee_id);
if (grantee == NULL) {
tnt_raise(ClientError, ER_NO_SUCH_USER,
int2str(priv->grantee_id));
@@ -1481,6 +1486,21 @@ priv_def_check(struct priv_def *priv)
}
break;
}
+ case SC_ROLE:
+ {
+ struct user *role = user_by_id(priv->object_id);
+ if (role == NULL || role->type != SC_ROLE) {
+ tnt_raise(ClientError, ER_NO_SUCH_ROLE,
+ role ? role->name :
+ int2str(priv->object_id));
+ }
+ /* Only the creator of the role can grant it. */
+ if (role->owner != grantor->uid && grantor->uid != ADMIN) {
+ tnt_raise(ClientError, ER_ACCESS_DENIED,
+ role->name, grantor->name);
+ }
+ role_check(role, grantee);
+ }
default:
break;
}
@@ -1500,7 +1520,7 @@ grant_or_revoke(struct priv_def *priv)
switch (priv->object_type) {
case SC_UNIVERSE:
{
- access = &grantee->universal_access;
+ access = universe.access;
/** Update cache at least in the current session. */
struct credentials *cr = current_user();
if (grantee->uid == cr->uid)
@@ -1511,21 +1531,34 @@ grant_or_revoke(struct priv_def *priv)
{
struct space *space = space_by_id(priv->object_id);
if (space)
- access = &space->access[grantee->auth_token];
+ access = space->access;
break;
}
case SC_FUNCTION:
{
struct func_def *func = func_by_id(priv->object_id);
if (func)
- access = &func->access[grantee->auth_token];
+ access = func->access;
+ break;
+ }
+ case SC_ROLE:
+ {
+ struct user *role = user_by_id(priv->object_id);
+ if (role == NULL || role->type != SC_ROLE)
+ break;
+ if (priv->access)
+ role_grant(grantee, role);
+ else
+ role_revoke(grantee, role);
break;
}
default:
break;
}
- if (access)
- access->granted = access->effective = priv->access;
+ if (access) {
+ access[grantee->auth_token].granted = priv->access;
+ access[grantee->auth_token].effective = priv->access;
+ }
}
/** A trigger called on rollback of grant, or on commit of revoke. */
@@ -1538,6 +1571,12 @@ revoke_priv(struct trigger * /* trigger */, void *event)
stmt->new_tuple : stmt->old_tuple);
struct priv_def priv;
priv_def_create_from_tuple(&priv, tuple);
+ /*
+ * Access to the object has been removed altogether so
+ * there should be no grants at all. If only some grants
+ * were removed, modify_priv trigger would have been
+ * invoked.
+ */
priv.access = 0;
grant_or_revoke(&priv);
}
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 27865775ce..a5d0c6ff26 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1021,6 +1021,15 @@ end
box.schema.user.grant = function(user_name, privilege, object_type,
object_name, grantor)
+ -- From user point of view, role is the same thing
+ -- as a privilege. Allow syntax grant(user, role).
+ if object_name == nil and object_type == nil then
+ -- sic: avoid recursion, to not bother with roles
+ -- named 'execute'
+ object_type = 'role'
+ object_name = privilege
+ privilege = 'execute'
+ end
local uid = user_resolve(user_name)
if uid == nil then
box.error(box.error.NO_SUCH_USER, user_name)
@@ -1050,6 +1059,13 @@ box.schema.user.grant = function(user_name, privilege, object_type,
end
box.schema.user.revoke = function(user_name, privilege, object_type, object_name)
+ -- From user point of view, role is the same thing
+ -- as a privilege. Allow syntax revoke(user, role).
+ if object_name == nil and object_type == nil then
+ object_type = 'role'
+ object_name = privilege
+ privilege = 'execute'
+ end
local uid = user_resolve(user_name)
if uid == nil then
box.error(box.error.NO_SUCH_USER, name)
diff --git a/src/box/session.h b/src/box/session.h
index 77d1fb5a12..72107541a9 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -183,7 +183,7 @@ static inline void
credentials_init(struct credentials *cr, struct user *user)
{
cr->auth_token = user->auth_token;
- cr->universal_access = user->universal_access.effective;
+ cr->universal_access = universe.access[cr->auth_token].effective;
cr->uid = user->uid;
}
diff --git a/src/box/user.cc b/src/box/user.cc
index 8334b30c06..61cf2a5f86 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -30,63 +30,161 @@
#include "user_def.h"
#include "assoc.h"
#include "schema.h"
+#include "bit/bit.h"
+struct universe universe;
static struct user users[BOX_USER_MAX];
struct user *guest_user = users;
struct user *admin_user = users + 1;
-/** Bitmap type for used/unused authentication token map. */
-typedef unsigned long user_map_t;
+struct mh_i32ptr_t *user_registry;
-/** A map to quickly look up free slots in users[] array. */
-user_map_t user_map[BOX_USER_MAX/(CHAR_BIT*sizeof(user_map_t)) + 1];
+/* {{{ user_map */
-int user_map_idx = 0;
-struct mh_i32ptr_t *user_registry;
+/* Initialize an empty user map. */
+void
+user_map_init(struct user_map *map)
+{
+ memset(map, 0, sizeof(*map));
+}
-uint8_t
-user_map_get_slot()
+static inline int
+user_map_calc_idx(uint8_t auth_token, uint8_t *bit_no)
{
- uint32_t idx = __builtin_ffsl(user_map[user_map_idx]);
- while (idx == 0) {
- if (user_map_idx == sizeof(user_map)/sizeof(*user_map))
- panic("Out of slots for new users");
+ *bit_no = auth_token & (UMAP_INT_BITS - 1);
+ return auth_token / UMAP_INT_BITS;
+}
- user_map_idx++;
- idx = __builtin_ffsl(user_map[user_map_idx]);
+
+/** Set a bit in the user map - add a user. */
+static inline void
+user_map_set(struct user_map *map, uint8_t auth_token)
+{
+ uint8_t bit_no;
+ int idx = user_map_calc_idx(auth_token, &bit_no);
+ map->m[idx] |= ((umap_int_t) 1) << bit_no;
+}
+
+/** Clear a bit in the user map - remove a user. */
+static inline void
+user_map_clear(struct user_map *map, uint8_t auth_token)
+{
+ uint8_t bit_no;
+ int idx = user_map_calc_idx(auth_token, &bit_no);
+ map->m[idx] &= ~(((umap_int_t) 1) << bit_no);
+}
+
+/* Check if a bit is set in the user map. */
+static inline bool
+user_map_is_set(struct user_map *map, uint8_t auth_token)
+{
+ uint8_t bit_no;
+ int idx = user_map_calc_idx(auth_token, &bit_no);
+ return map->m[idx] & (((umap_int_t) 1) << bit_no);
+}
+
+/**
+ * Merge two sets of users: add all users from right argument
+ * to the left one.
+ */
+void
+user_map_union(struct user_map *lhs, struct user_map *rhs)
+{
+ for (int i = 0; i < USER_MAP_SIZE; i++)
+ lhs->m[i] |= rhs->m[i];
+}
+
+/** Iterate over users in the set of users. */
+struct user_map_iterator
+{
+ struct bit_iterator it;
+};
+
+void
+user_map_iterator_init(struct user_map_iterator *it, struct user_map *map)
+{
+ bit_iterator_init(&it->it, map->m,
+ USER_MAP_SIZE * sizeof(umap_int_t), true);
+}
+
+struct user *
+user_map_iterator_next(struct user_map_iterator *it)
+{
+ size_t auth_token = bit_iterator_next(&it->it);
+ if (auth_token != SIZE_MAX)
+ return users + auth_token;
+ return NULL;
+}
+
+/* }}} */
+
+/* {{{ authentication tokens */
+
+/** A map to quickly look up free slots in users[] array. */
+static umap_int_t tokens[USER_MAP_SIZE];
+/**
+ * Index of the minimal element of the tokens array which
+ * has an unused token.
+ */
+static int min_token_idx = 0;
+
+/**
+ * Find and return a spare authentication token.
+ * Raise an exception when the maximal number of users
+ * is reached (and we're out of tokens).
+ */
+uint8_t
+auth_token_get()
+{
+ uint8_t bit_no = 0;
+ while (min_token_idx < USER_MAP_SIZE) {
+ bit_no = __builtin_ffs(tokens[min_token_idx]);
+ if (bit_no)
+ break;
+ min_token_idx++;
}
+ if (bit_no == 0 || bit_no > BOX_USER_MAX) {
+ /* A cap on the number of users was reached.
+ * Check for BOX_USER_MAX to cover case when
+ * USER_MAP_BITS > BOX_USER_MAX.
+ */
+ tnt_raise(LoggedError, ER_USER_MAX, BOX_USER_MAX);
+ }
/*
* find-first-set returns bit index starting from 1,
* or 0 if no bit is set. Rebase the index to offset 0.
*/
- idx--;
- if (idx == BOX_USER_MAX) {
- /* A cap on the number of users was reached. */
- tnt_raise(LoggedError, ER_USER_MAX, BOX_USER_MAX);
- }
- user_map[user_map_idx] ^= ((user_map_t) 1) << idx;
- idx += user_map_idx * sizeof(*user_map) * CHAR_BIT;
- assert(idx < UINT8_MAX);
- return idx;
+ bit_no--;
+ tokens[min_token_idx] ^= ((umap_int_t) 1) << bit_no;
+ int auth_token = min_token_idx * UMAP_INT_BITS + bit_no;
+ assert(auth_token < UINT8_MAX);
+ return auth_token;
}
+/**
+ * Return an authentication token to the set of unused
+ * tokens.
+ */
void
-user_map_put_slot(uint8_t auth_token)
+auth_token_put(uint8_t auth_token)
{
- memset(users + auth_token, 0, sizeof(struct user_def));
- uint32_t bit_no = auth_token & (sizeof(user_map_t) * CHAR_BIT - 1);
- auth_token /= sizeof(user_map_t) * CHAR_BIT;
- user_map[auth_token] |= ((user_map_t) 1) << bit_no;
- if (auth_token > user_map_idx)
- user_map_idx = auth_token;
+ uint8_t bit_no;
+ int idx = user_map_calc_idx(auth_token, &bit_no);
+ tokens[idx] |= ((umap_int_t) 1) << bit_no;
+ if (idx < min_token_idx)
+ min_token_idx = idx;
}
+/* }}} */
+
+/* {{{ user cache */
+
struct user *
user_cache_replace(struct user_def *def)
{
struct user *user = user_by_id(def->uid);
if (user == NULL) {
- uint8_t auth_token = user_map_get_slot();
+ uint8_t auth_token = auth_token_get();
user = users + auth_token;
assert(user->auth_token == 0);
user->auth_token = auth_token;
@@ -100,11 +198,17 @@ user_cache_replace(struct user_def *def)
void
user_cache_delete(uint32_t uid)
{
- struct user *old = user_by_id(uid);
- if (old) {
- assert(old->auth_token > ADMIN);
- user_map_put_slot(old->auth_token);
- memset(old, 0, sizeof(*old));
+ struct user *user = user_by_id(uid);
+ if (user) {
+ assert(user->auth_token > ADMIN);
+ auth_token_put(user->auth_token);
+ /*
+ * Sic: we don't have to remove a deleted
+ * user from users hash of roles, since
+ * to drop a user, one has to revoke
+ * all privileges from them first.
+ */
+ memset(user, 0, sizeof(*user));
mh_i32ptr_del(user_registry, uid, NULL);
}
}
@@ -146,7 +250,8 @@ user_cache_find_by_name(const char *name, uint32_t len)
void
user_cache_init()
{
- memset(user_map, 0xFF, sizeof(user_map));
+ /** Mark all tokens as unused. */
+ memset(tokens, 0xFF, sizeof(tokens));
user_registry = mh_i32ptr_new();
/*
* Solve a chicken-egg problem:
@@ -180,3 +285,69 @@ user_cache_free()
if (user_registry)
mh_i32ptr_delete(user_registry);
}
+
+/* }}} user cache */
+
+/** {{{ roles */
+
+void
+role_check(struct user *role, struct user *grantee)
+{
+ /*
+ * Check that there is no loop from grantee to role:
+ * if grantee is a role, build up a closure of all
+ * immediate and indirect users of grantee, and ensure
+ * the granted role is not in this set.
+ */
+ struct user_map transitive_closure;
+ user_map_init(&transitive_closure);
+ user_map_set(&transitive_closure, grantee->auth_token);
+ struct user_map current_layer = transitive_closure;
+ while (true) {
+ /*
+ * As long as we're traversing a directed
+ * acyclic graph, we're bound to end at some
+ * point in a layer with no incoming edges.
+ */
+ struct user_map next_layer;
+ user_map_init(&next_layer);
+ bool found = false;
+ struct user_map_iterator it;
+ user_map_iterator_init(&it, ¤t_layer);
+ struct user *user;
+ while ((user = user_map_iterator_next(&it))) {
+ user_map_union(&next_layer, &user->users);
+ found = true;
+ }
+ user_map_union(&transitive_closure, &next_layer);
+ if (! found)
+ break;
+ current_layer = next_layer;
+ }
+
+ if (user_map_is_set(&transitive_closure,
+ role->auth_token)) {
+ tnt_raise(ClientError, ER_ROLE_LOOP,
+ role->name, grantee->name);
+ }
+}
+
+void
+role_grant(struct user *grantee, struct user *role)
+{
+ user_map_set(&role->users, grantee->auth_token);
+}
+
+void
+role_revoke(struct user *grantee, struct user *role)
+{
+ user_map_clear(&role->users, grantee->auth_token);
+}
+
+void
+user_set_effective_access(struct user * /* grantee */,
+ struct user * /* role */)
+{
+}
+
+/** }}} */
diff --git a/src/box/user.h b/src/box/user.h
index 2397639cdb..dfb0d551ab 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -31,18 +31,37 @@
#include <stdint.h>
#include "user_def.h"
+/** Global grants. */
+struct universe {
+ /** Global privileges this user has on the universe. */
+ struct access access[BOX_USER_MAX];
+};
+
+/** A single instance of the universe. */
+extern struct universe universe;
+
+/** Bitmap type for used/unused authentication token map. */
+typedef unsigned int umap_int_t;
+enum {
+ UMAP_INT_BITS = CHAR_BIT * sizeof(umap_int_t),
+ USER_MAP_SIZE = (BOX_USER_MAX + UMAP_INT_BITS - 1)/UMAP_INT_BITS
+};
+
+struct user_map {
+ umap_int_t m[USER_MAP_SIZE];
+};
+
struct user: public user_def
{
- /** Global privileges this user has on the universe. */
- struct access universal_access;
/**
* An id in privileges array to quickly find a
* respective privilege.
*/
uint8_t auth_token;
+ /** List of users or roles this role has been granted to */
+ struct user_map users;
};
-
/**
* For best performance, all users are maintained in this array.
* Position in the array is store in user->auth_token and also
@@ -97,4 +116,36 @@ user_cache_init();
void
user_cache_free();
+/* {{{ Roles */
+
+/**
+ * Check, mainly, that users & roles form an acyclic graph,
+ * and no loop in the graph will occur when grantee gets
+ * a given role.
+ */
+void
+role_check(struct user *role, struct user *grantee);
+
+/**
+ * Grant a role to a user or another role.
+ */
+void
+role_grant(struct user *grantee, struct user *role);
+
+/**
+ * Revoke a role from a user or another role.
+ */
+void
+role_revoke(struct user *grantee, struct user *role);
+
+/**
+ * Re-evaluate effective access of a user based on this role
+ * that is granted to him/her.
+ * The role has its effective access already re-evaluated.
+ */
+void
+user_set_effective_access(struct user *grantee, struct user *role);
+
+/* }}} */
+
#endif /* INCLUDES_TARANTOOL_BOX_USER_H */
diff --git a/src/errcode.h b/src/errcode.h
index b010f0e82d..a89a9c34ef 100644
--- a/src/errcode.h
+++ b/src/errcode.h
@@ -135,7 +135,8 @@ enum { TNT_ERRMSG_MAX = 512 };
/* 83 */_(ER_ROLE_EXISTS, 2, "Role '%s' already exists") \
/* 84 */_(ER_CREATE_ROLE, 2, "Failed to create role '%s': %s") \
/* 85 */_(ER_INDEX_EXISTS, 2, "Index '%s' already exists") \
- /* 86 */_(ER_TUPLE_REF_OVERFLOW, 1, "Tuple reference counter is overflowed") \
+ /* 86 */_(ER_TUPLE_REF_OVERFLOW, 1, "Tuple reference counter overflow") \
+ /* 87 */_(ER_ROLE_LOOP, 2, "Granting role '%s' to role '%s' would create a loop") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/test/box/misc.result b/test/box/misc.result
index 247bff00e6..249ece55c0 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -195,6 +195,7 @@ t;
- 'box.error.UNKNOWN_SERVER : 62'
- 'box.error.FUNCTION_EXISTS : 52'
- 'box.error.NO_SUCH_FUNCTION : 51'
+ - 'box.error.ROLE_LOOP : 87'
- 'box.error.TUPLE_NOT_FOUND : 4'
- 'box.error.DROP_USER : 44'
- 'box.error.CROSS_ENGINE_TRANSACTION : 81'
diff --git a/test/box/role.result b/test/box/role.result
index aae49703b6..37f2f9f9c0 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -69,4 +69,67 @@ box.schema.user.grant('tester', 'read', 'role', 'iddqd')
-- test granting role to a role
box.schema.user.grant('iddqd', 'execute', 'role', 'iddqd')
---
+- error: Granting role 'iddqd' to role 'iddqd' would create a loop
+...
+box.schema.user.grant('iddqd', 'iddqd')
+---
+- error: Granting role 'iddqd' to role 'iddqd' would create a loop
+...
+box.schema.user.revoke('iddqd', 'iddqd')
+---
+...
+box.schema.user.grant('tester', 'iddqd')
+---
+...
+box.schema.user.revoke('tester', 'iddqd')
+---
+...
+box.schema.user.revoke('tester', 'no-such-role')
+---
+- error: Role 'no-such-role' is not found
+...
+box.schema.user.grant('tester', 'no-such-role')
+---
+- error: Role 'no-such-role' is not found
+...
+-- check for loops in role grants
+box.schema.role.create('a')
+---
+...
+box.schema.role.create('b')
+---
+...
+box.schema.role.create('c')
+---
+...
+box.schema.role.create('d')
+---
+...
+box.schema.user.grant('b', 'a')
+---
+...
+box.schema.user.grant('c', 'a')
+---
+...
+box.schema.user.grant('d', 'b')
+---
+...
+box.schema.user.grant('d', 'c')
+---
+...
+box.schema.user.grant('a', 'd')
+---
+- error: Granting role 'd' to role 'a' would create a loop
+...
+box.schema.role.drop('d')
+---
+...
+box.schema.role.drop('b')
+---
+...
+box.schema.role.drop('c')
+---
+...
+box.schema.role.drop('a')
+---
...
diff --git a/test/box/role.test.lua b/test/box/role.test.lua
index 9d63d60331..fd08324200 100644
--- a/test/box/role.test.lua
+++ b/test/box/role.test.lua
@@ -22,3 +22,23 @@ box.schema.user.grant('tester', 'write', 'role', 'iddqd')
box.schema.user.grant('tester', 'read', 'role', 'iddqd')
-- test granting role to a role
box.schema.user.grant('iddqd', 'execute', 'role', 'iddqd')
+box.schema.user.grant('iddqd', 'iddqd')
+box.schema.user.revoke('iddqd', 'iddqd')
+box.schema.user.grant('tester', 'iddqd')
+box.schema.user.revoke('tester', 'iddqd')
+box.schema.user.revoke('tester', 'no-such-role')
+box.schema.user.grant('tester', 'no-such-role')
+-- check for loops in role grants
+box.schema.role.create('a')
+box.schema.role.create('b')
+box.schema.role.create('c')
+box.schema.role.create('d')
+box.schema.user.grant('b', 'a')
+box.schema.user.grant('c', 'a')
+box.schema.user.grant('d', 'b')
+box.schema.user.grant('d', 'c')
+box.schema.user.grant('a', 'd')
+box.schema.role.drop('d')
+box.schema.role.drop('b')
+box.schema.role.drop('c')
+box.schema.role.drop('a')
diff --git a/test/box/select.result b/test/box/select.result
index 82fa43b537..44ce0acc7c 100644
--- a/test/box/select.result
+++ b/test/box/select.result
@@ -555,7 +555,7 @@ ref_count = 0
...
while (true) do table.insert(lots_of_links, s:get{0}) ref_count = ref_count + 1 end
---
-- error: Tuple reference counter is overflowed
+- error: Tuple reference counter overflow
...
ref_count
---
--
GitLab