From c209e5db18815afdb962bb46bef51fabb2ef8971 Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja@tarantool.org>
Date: Mon, 8 Sep 2014 09:55:17 +0400
Subject: [PATCH] gh-488 setuid functions
Implement functions which change their effective
user id ot the one of the definer during execution.
---
src/box/access.h | 2 +
src/box/alter.cc | 7 ++++
src/box/key_def.h | 10 +++++
src/box/lua/call.cc | 73 ++++++++++++++++++++++++++++++------
src/box/lua/schema.lua | 11 ++++--
test/box/access.test.lua | 2 +-
test/box/access_bin.result | 67 +++++++++++++++++++++++++++++++++
test/box/access_bin.test.lua | 24 ++++++++++++
8 files changed, 179 insertions(+), 17 deletions(-)
diff --git a/src/box/access.h b/src/box/access.h
index 94c5b1d485..b0cbf366c4 100644
--- a/src/box/access.h
+++ b/src/box/access.h
@@ -41,6 +41,8 @@ enum {
PRIV_W = 2,
/* CALL */
PRIV_X = 4,
+ /** Everything. */
+ PRIV_ALL = PRIV_R + PRIV_W + PRIV_X
};
/* Privilege name for error messages */
diff --git a/src/box/alter.cc b/src/box/alter.cc
index e0468e056a..858d14806e 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -62,6 +62,9 @@
#define PRIV_OBJECT_ID 3
#define PRIV_ACCESS 4
+/** _func columns */
+#define FUNC_SETUID 3
+
/* {{{ Auxiliary functions and methods. */
void
@@ -1324,6 +1327,8 @@ func_def_create_from_tuple(struct func_def *func, struct tuple *tuple)
{
func->fid = tuple_field_u32(tuple, ID);
func->uid = tuple_field_u32(tuple, UID);
+ func->auth_token = user_cache_find(func->uid)->auth_token;
+ func->setuid = false;
const char *name = tuple_field_cstr(tuple, NAME);
uint32_t len = strlen(name);
if (len >= sizeof(func->name)) {
@@ -1333,6 +1338,8 @@ func_def_create_from_tuple(struct func_def *func, struct tuple *tuple)
snprintf(func->name, sizeof(func->name), "%s", name);
/** Nobody has access to the function but the owner. */
memset(func->access, 0, sizeof(func->access));
+ if (tuple_field_count(tuple) > FUNC_SETUID)
+ func->setuid = tuple_field_u32(tuple, FUNC_SETUID);
}
/** Remove a function from function cache */
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 152e4a08b0..63e7251357 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -301,6 +301,16 @@ struct func_def {
uint32_t fid;
/** Owner of the function. */
uint32_t uid;
+ /**
+ * True if the function requires change of user id before
+ * invocaction.
+ */
+ bool setuid;
+ /**
+ * Authentication id of the owner of the function,
+ * used for set-user-id functions.
+ */
+ uint8_t auth_token;
/** Function name. */
char name[BOX_NAME_MAX + 1];
/**
diff --git a/src/box/lua/call.cc b/src/box/lua/call.cc
index 4a394417a3..8fc9b24539 100644
--- a/src/box/lua/call.cc
+++ b/src/box/lua/call.cc
@@ -459,26 +459,74 @@ lbox_call_loadproc(struct lua_State *L)
return box_lua_find(L, name, name + name_len);
}
-static inline void
-access_check_func(const char *name, uint32_t name_len,
- struct user *user, uint8_t access)
+/**
+ * Check access to a function and change the current
+ * user id if the function is a set-definer-user-id one.
+ * The original user is restored in the destructor.
+ */
+struct SetuidGuard
{
- access &= ~user->universal_access.effective;
- /*
- * No special check for ADMIN user is necessary
- * since ADMIN has universal access.
- */
- if (access == 0)
+ /** True if the function was set-user-id one. */
+ bool setuid;
+ /** Original authentication token, only set if setuid = true. */
+ uint8_t orig_auth_token;
+ /** Original user id, only set if setuid = true. */
+ uint32_t orig_uid;
+
+ inline SetuidGuard(const char *name, uint32_t name_len,
+ struct user *user, uint8_t access);
+ inline ~SetuidGuard();
+};
+
+SetuidGuard::SetuidGuard(const char *name, uint32_t name_len,
+ struct user *user, uint8_t access)
+ :setuid(false)
+{
+ /*
+ * If the user has universal access, don't bother with setuid.
+ * No special check for ADMIN user is necessary
+ * since ADMIN has universal access.
+ */
+ if (user->universal_access.effective & PRIV_ALL)
return;
+ access &= ~user->universal_access.effective;
+ /*
+ * We need to look up the function by name even if
+ * the user has access to it, since it could require
+ * a set of other user id.
+ */
struct func_def *func = func_by_name(name, name_len);
+ if (func == NULL && access == 0) {
+ /**
+ * Well, the function is not explicitly defined,
+ * so it's obviously not a setuid one. Wasted
+ * cycles on look up while the user had universal
+ * access :(
+ */
+ return;
+ }
if (func == NULL || (func->uid != user->uid &&
- access & ~func->access[user->auth_token].effective)) {
+ access & ~func->access[user->auth_token].effective)) {
+ /* Access violation, report error. */
char name_buf[BOX_NAME_MAX + 1];
snprintf(name_buf, sizeof(name_buf), "%.*s", name_len, name);
tnt_raise(ClientError, ER_FUNCTION_ACCESS_DENIED,
priv_name(access), user->name, name_buf);
}
+ if (func->setuid) {
+ /** Remember and change the current user id. */
+ setuid = func->setuid;
+ orig_auth_token = user->auth_token;
+ orig_uid = user->uid;
+ session_set_user(session(), func->auth_token, func->uid);
+ }
+}
+
+SetuidGuard::~SetuidGuard()
+{
+ if (setuid)
+ session_set_user(session(), orig_auth_token, orig_uid);
}
/**
@@ -497,12 +545,13 @@ box_lua_call(struct request *request, struct port *port)
/* Try to find a function by name */
int oc = box_lua_find(L, name, name + name_len);
/**
- * Check access to the function. Sic: the order
+ * Check access to the function and optionally change
+ * execution time user id (set user id). Sic: the order
* is important, as is described in
* https://github.com/tarantool/tarantool/issues/300
* - if a function does not exist, say it first.
*/
- access_check_func(name, name_len, user, PRIV_X);
+ SetuidGuard setuid(name, name_len, user, PRIV_X);
/* Push the rest of args (a tuple). */
const char *args = request->tuple;
uint32_t arg_count = mp_decode_array(&args);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 6160d6019b..bb1e9a46f2 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -911,13 +911,16 @@ local function object_name(object_type, object_id)
end
box.schema.func = {}
-box.schema.func.create = function(name)
+box.schema.func.create = function(name, opts)
local _func = box.space[box.schema.FUNC_ID]
local func = _func.index.name:get{name}
if func then
box.error(box.error.FUNCTION_EXISTS, name)
end
- _func:auto_increment{session.uid(), name}
+ check_param_table(opts, { setuid = 'boolean' })
+ opts = update_param_table(opts, { setuid = false })
+ opts.setuid = opts.setuid and 1 or 0
+ _func:auto_increment{session.uid(), name, opts.setuid}
end
box.schema.func.drop = function(name)
@@ -926,8 +929,8 @@ box.schema.func.drop = function(name)
local fid = object_resolve('function', name)
local privs = _priv:select{}
for k, tuple in pairs(privs) do
- if tuple[2] == 'function' and tuple[3] == function_id then
- box.schema.user.revoke(tuple[1], tuple[4], tuple[2], tuple[3])
+ if tuple[3] == 'function' and tuple[4] == fid then
+ box.schema.user.revoke(tuple[2], tuple[5], tuple[3], tuple[4])
end
end
_func:delete{fid}
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 347d166c0a..5358c7d37b 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -167,7 +167,6 @@ box.schema.user.grant('user', 'read', 'universe')
box.space._priv:select{id}
box.schema.user.drop('user')
box.space._priv:select{id}
-session = nil
-- -----------------------------------------------------------
-- Be a bit more rigorous in what is accepted in space _user
-- -----------------------------------------------------------
@@ -175,3 +174,4 @@ box.space._user:insert{10, 1, 'name'}
box.space._user:insert{10, 1, 'name', 'strange-object-type'}
box.space._user:insert{10, 1, 'name', 'user', 'password'}
box.space._user:insert{10, 1, 'name', 'role', 'password'}
+session = nil
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index 1e08ae289f..23aa5b2d6f 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -28,3 +28,70 @@ c:close()
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
---
...
+-- gh-488 suid functions
+--
+setuid_space = box.schema.space.create('setuid_space')
+---
+...
+setuid_space:create_index('primary')
+---
+...
+setuid_func = function() return box.space.setuid_space:auto_increment{} end
+---
+...
+box.schema.func.create('setuid_func')
+---
+...
+box.schema.user.grant('guest', 'execute', 'function', 'setuid_func')
+---
+...
+c = net.box:new(0, box.cfg.listen)
+---
+...
+c:call("setuid_func")
+---
+- error: Read access denied for user 'guest' to space 'setuid_space'
+...
+session.su('guest')
+---
+...
+setuid_func()
+---
+- error: Read access denied for user 'guest' to space 'setuid_space'
+...
+session.su('admin')
+---
+...
+box.schema.func.drop('setuid_func')
+---
+...
+box.schema.func.create('setuid_func', { setuid = true })
+---
+...
+box.schema.user.grant('guest', 'execute', 'function', 'setuid_func')
+---
+...
+c:call("setuid_func")
+---
+- - [1]
+...
+session.su('guest')
+---
+...
+setuid_func()
+---
+- error: Read access denied for user 'guest' to space 'setuid_space'
+...
+session.su('admin')
+---
+...
+c:close()
+---
+...
+box.schema.func.drop('setuid_func')
+---
+...
+setuid_space:drop()
+---
+...
+--
diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua
index 692f60e332..f9841d7ea5 100644
--- a/test/box/access_bin.test.lua
+++ b/test/box/access_bin.test.lua
@@ -10,3 +10,27 @@ c:call("dostring", "session.su('admin')")
c:call("dostring", "return session.user()")
c:close()
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+
+-- gh-488 suid functions
+--
+setuid_space = box.schema.space.create('setuid_space')
+setuid_space:create_index('primary')
+setuid_func = function() return box.space.setuid_space:auto_increment{} end
+box.schema.func.create('setuid_func')
+box.schema.user.grant('guest', 'execute', 'function', 'setuid_func')
+c = net.box:new(0, box.cfg.listen)
+c:call("setuid_func")
+session.su('guest')
+setuid_func()
+session.su('admin')
+box.schema.func.drop('setuid_func')
+box.schema.func.create('setuid_func', { setuid = true })
+box.schema.user.grant('guest', 'execute', 'function', 'setuid_func')
+c:call("setuid_func")
+session.su('guest')
+setuid_func()
+session.su('admin')
+c:close()
+box.schema.func.drop('setuid_func')
+setuid_space:drop()
+--
--
GitLab