[PATCH v4 3/6] box: rework func object as a function frontend

Kirill Shcherbatov kshcherbatov at tarantool.org
Sun Jun 23 16:57:54 MSK 2019


The function func object used to provide a call method only for
C functions. In scope of this patch it reworked to be a uniform
function call frontend both for C and Lua functions.

Introduced classes func_c and func_lua, that provide own
constructors which produce implementation-specific object with
call and destroy methods.

Needed for #4182, #1260
---
 src/box/alter.cc   |   1 +
 src/box/call.c     | 131 ++--------------------------------
 src/box/func.c     | 170 ++++++++++++++++++++++++++++++++++++++++-----
 src/box/func.h     |  23 +++---
 src/box/func_def.h |   2 +
 src/box/lua/call.c |  45 ++++++++++++
 src/box/lua/call.h |   5 ++
 src/box/session.cc |  11 ++-
 src/box/session.h  |   9 +++
 9 files changed, 240 insertions(+), 157 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index adad957f2..32c4b566a 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2556,6 +2556,7 @@ func_def_new_from_tuple(struct tuple *tuple)
 	}
 	memcpy(def->name, name, len);
 	def->name[len] = 0;
+	def->name_len = len;
 	if (tuple_field_count(tuple) > BOX_FUNC_FIELD_SETUID)
 		def->setuid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_SETUID);
 	else
diff --git a/src/box/call.c b/src/box/call.c
index 573a0d698..bd03e8b96 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -76,90 +76,6 @@ static const struct port_vtab port_msgpack_vtab = {
 	.destroy = NULL,
 };
 
-/**
- * Find a function by name and check "EXECUTE" permissions.
- *
- * @param name function name
- * @param name_len length of @a name
- * @param[out] funcp function object
- * Sic: *pfunc == NULL means that perhaps the user has a global
- * "EXECUTE" privilege, so no specific grant to a function.
- *
- * @retval -1 on access denied
- * @retval  0 on success
- */
-static inline int
-access_check_func(const char *name, uint32_t name_len, struct func **funcp)
-{
-	struct func *func = func_by_name(name, name_len);
-	struct credentials *credentials = effective_user();
-	/*
-	 * If the user has universal access, don't bother with checks.
-	 * No special check for ADMIN user is necessary
-	 * since ADMIN has universal access.
-	 */
-	if ((credentials->universal_access & (PRIV_X | PRIV_U)) ==
-	    (PRIV_X | PRIV_U)) {
-
-		*funcp = func;
-		return 0;
-	}
-	user_access_t access = PRIV_X | PRIV_U;
-	/* Check access for all functions. */
-	access &= ~entity_access_get(SC_FUNCTION)[credentials->auth_token].effective;
-	user_access_t func_access = access & ~credentials->universal_access;
-	if (func == NULL ||
-	    /* Check for missing Usage access, ignore owner rights. */
-	    func_access & PRIV_U ||
-	    /* Check for missing specific access, respect owner rights. */
-	    (func->def->uid != credentials->uid &&
-	    func_access & ~func->access[credentials->auth_token].effective)) {
-
-		/* Access violation, report error. */
-		struct user *user = user_find(credentials->uid);
-		if (user != NULL) {
-			if (!(access & credentials->universal_access)) {
-				diag_set(AccessDeniedError,
-					 priv_name(PRIV_U),
-					 schema_object_name(SC_UNIVERSE), "",
-					 user->def->name);
-			} else {
-				diag_set(AccessDeniedError,
-					 priv_name(PRIV_X),
-					 schema_object_name(SC_FUNCTION),
-					 tt_cstr(name, name_len),
-					 user->def->name);
-			}
-		}
-		return -1;
-	}
-
-	*funcp = func;
-	return 0;
-}
-
-static int
-box_c_call(struct func *func, struct port *args, struct port *ret)
-{
-	assert(func != NULL && func->def->language == FUNC_LANGUAGE_C);
-
-	/* Clear all previous errors */
-	diag_clear(&fiber()->diag);
-	assert(!in_txn()); /* transaction is not started */
-
-	/* Call function from the shared library */
-	int rc = func_call(func, args, ret);
-	func = NULL; /* May be deleted by DDL */
-	if (rc != 0) {
-		if (diag_last_error(&fiber()->diag) == NULL) {
-			/* Stored procedure forget to set diag  */
-			diag_set(ClientError, ER_PROC_C, "unknown error");
-		}
-		return -1;
-	}
-	return 0;
-}
-
 int
 box_module_reload(const char *name)
 {
@@ -191,53 +107,20 @@ box_process_call(struct call_request *request, struct port *port)
 	const char *name = request->name;
 	assert(name != NULL);
 	uint32_t name_len = mp_decode_strl(&name);
-
-	struct func *func = NULL;
-	/**
-	 * Sic: func == NULL means that perhaps the user has a global
-	 * "EXECUTE" privilege, so no specific grant to a function.
-	 */
-	if (access_check_func(name, name_len, &func) != 0)
-		return -1; /* permission denied */
-
-	/**
-	 * Change the current user id if the function is
-	 * a set-definer-uid one. If the function is not
-	 * defined, it's obviously not a setuid one.
-	 */
-	struct credentials *orig_credentials = NULL;
-	if (func && func->def->setuid) {
-		orig_credentials = effective_user();
-		/* Remember and change the current user id. */
-		if (func->owner_credentials.auth_token >= BOX_USER_MAX) {
-			/*
-			 * Fill the cache upon first access, since
-			 * when func is created, no user may
-			 * be around to fill it (recovery of
-			 * system spaces from a snapshot).
-			 */
-			struct user *owner = user_find(func->def->uid);
-			if (owner == NULL)
-				return -1;
-			credentials_init(&func->owner_credentials,
-					 owner->auth_token,
-					 owner->def->uid);
-		}
-		fiber_set_user(fiber(), &func->owner_credentials);
-	}
+	/* Transaction is not started. */
+	assert(!in_txn());
 
 	int rc;
 	struct port args;
 	port_msgpack_create(&args, request->args,
 			    request->args_end - request->args);
-	if (func && func->def->language == FUNC_LANGUAGE_C) {
-		rc = box_c_call(func, &args, port);
-	} else {
+	struct func *func = func_by_name(name, name_len);
+	if (func != NULL) {
+		rc = func_call(func, &args, port);
+	} else if ((rc = access_check_universe_object(PRIV_X | PRIV_U,
+				SC_FUNCTION, tt_cstr(name, name_len))) == 0) {
 		rc = box_lua_call(name, name_len, &args, port);
 	}
-	/* Restore the original user */
-	if (orig_credentials)
-		fiber_set_user(fiber(), orig_credentials);
 
 	if (rc != 0) {
 		txn_rollback();
diff --git a/src/box/func.c b/src/box/func.c
index 1c37f6523..c57027809 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -33,9 +33,12 @@
 #include "trivia/config.h"
 #include "assoc.h"
 #include "lua/utils.h"
+#include "lua/call.h"
 #include "error.h"
 #include "diag.h"
 #include "port.h"
+#include "schema.h"
+#include "session.h"
 #include <dlfcn.h>
 
 /**
@@ -50,6 +53,24 @@ struct func_name {
 	const char *package_end;
 };
 
+struct func_c {
+	/** Function object base class. */
+	struct func base;
+	/**
+	 * Anchor for module membership.
+	 */
+	struct rlist item;
+	/**
+	 * For C functions, the body of the function.
+	 */
+	box_function_f func;
+	/**
+	 * Each stored function keeps a handle to the
+	 * dynamic library for the C callback.
+	 */
+	struct module *module;
+};
+
 /***
  * Split function name to symbol and package names.
  * For example, str = foo.bar.baz => sym = baz, package = foo.bar
@@ -314,10 +335,10 @@ module_reload(const char *package, const char *package_end, struct module **modu
 	if (new_module == NULL)
 		return -1;
 
-	struct func *func, *tmp_func;
+	struct func_c *func, *tmp_func;
 	rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) {
 		struct func_name name;
-		func_split_name(func->def->name, &name);
+		func_split_name(func->base.def->name, &name);
 		func->func = module_sym(new_module, name.sym);
 		if (func->func == NULL)
 			goto restore;
@@ -338,7 +359,7 @@ restore:
 	 */
 	do {
 		struct func_name name;
-		func_split_name(func->def->name, &name);
+		func_split_name(func->base.def->name, &name);
 		func->func = module_sym(old_module, name.sym);
 		if (func->func == NULL) {
 			/*
@@ -351,20 +372,27 @@ restore:
 		func->module = old_module;
 		rlist_move(&old_module->funcs, &func->item);
 	} while (func != rlist_first_entry(&old_module->funcs,
-					   struct func, item));
+					   struct func_c, item));
 	assert(rlist_empty(&new_module->funcs));
 	module_delete(new_module);
 	return -1;
 }
 
+static struct func *
+func_c_new(struct func_def *def);
+
 struct func *
 func_new(struct func_def *def)
 {
-	struct func *func = (struct func *) malloc(sizeof(struct func));
-	if (func == NULL) {
-		diag_set(OutOfMemory, sizeof(*func), "malloc", "func");
-		return NULL;
+	struct func *func;
+	if (def->language == FUNC_LANGUAGE_C) {
+		func = func_c_new(def);
+	} else {
+		assert(def->language == FUNC_LANGUAGE_LUA);
+		func = func_lua_new(def);
 	}
+	if (func == NULL)
+		return NULL;
 	func->def = def;
 	/** Nobody has access to the function but the owner. */
 	memset(func->access, 0, sizeof(func->access));
@@ -380,19 +408,35 @@ func_new(struct func_def *def)
 	 * checks (see user_has_data()).
 	 */
 	func->owner_credentials.auth_token = BOX_USER_MAX; /* invalid value */
+	return func;
+}
+
+static struct func_vtab func_c_vtab;
+
+static struct func *
+func_c_new(struct func_def *def)
+{
+	(void) def;
+	assert(def->language == FUNC_LANGUAGE_C);
+	struct func_c *func = (struct func_c *) malloc(sizeof(struct func_c));
+	if (func == NULL) {
+		diag_set(OutOfMemory, sizeof(*func), "malloc", "func");
+		return NULL;
+	}
+	func->base.vtab = &func_c_vtab;
 	func->func = NULL;
 	func->module = NULL;
-	return func;
+	return &func->base;
 }
 
 static void
-func_unload(struct func *func)
+func_c_unload(struct func_c *func)
 {
 	if (func->module) {
 		rlist_del(&func->item);
 		if (rlist_empty(&func->module->funcs)) {
 			struct func_name name;
-			func_split_name(func->def->name, &name);
+			func_split_name(func->base.def->name, &name);
 			module_cache_del(name.package, name.package_end);
 		}
 		module_gc(func->module);
@@ -401,17 +445,27 @@ func_unload(struct func *func)
 	func->func = NULL;
 }
 
+static void
+func_c_destroy(struct func *base)
+{
+	assert(base->vtab == &func_c_vtab);
+	assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
+	struct func_c *func = (struct func_c *) base;
+	func_c_unload(func);
+	free(func);
+}
+
 /**
  * Resolve func->func (find the respective DLL and fetch the
  * symbol from it).
  */
 static int
-func_load(struct func *func)
+func_c_load(struct func_c *func)
 {
 	assert(func->func == NULL);
 
 	struct func_name name;
-	func_split_name(func->def->name, &name);
+	func_split_name(func->base.def->name, &name);
 
 	struct module *module = module_cache_find(name.package,
 						  name.package_end);
@@ -435,11 +489,13 @@ func_load(struct func *func)
 }
 
 int
-func_call(struct func *func, struct port *args, struct port *ret)
+func_c_call(struct func *base, struct port *args, struct port *ret)
 {
-	assert(func != NULL && func->def->language == FUNC_LANGUAGE_C);
+	assert(base->vtab == &func_c_vtab);
+	assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
+	struct func_c *func = (struct func_c *) base;
 	if (func->func == NULL) {
-		if (func_load(func) != 0)
+		if (func_c_load(func) != 0)
 			return -1;
 	}
 
@@ -462,16 +518,94 @@ func_call(struct func *func, struct port *args, struct port *ret)
 	module_gc(module);
 	region_truncate(region, region_svp);
 	if (rc != 0) {
+		if (diag_last_error(&fiber()->diag) == NULL) {
+			/* Stored procedure forget to set diag  */
+			diag_set(ClientError, ER_PROC_C, "unknown error");
+		}
 		port_destroy(ret);
 		return -1;
 	}
 	return rc;
 }
 
+static struct func_vtab func_c_vtab = {
+	.call = func_c_call,
+	.destroy = func_c_destroy,
+};
+
 void
 func_delete(struct func *func)
 {
-	func_unload(func);
 	free(func->def);
-	free(func);
+	func->vtab->destroy(func);
+}
+
+/** Check "EXECUTE" permissions for a given function. */
+static int
+func_access_check(struct func *func)
+{
+	struct credentials *credentials = effective_user();
+	/*
+	 * If the user has universal access, don't bother with
+	 * checks. No special check for ADMIN user is necessary
+	 * since ADMIN has universal access.
+	 */
+	if ((credentials->universal_access & (PRIV_X | PRIV_U)) ==
+	    (PRIV_X | PRIV_U))
+		return 0;
+	user_access_t access = PRIV_X | PRIV_U;
+	/* Check access for all functions. */
+	access &= ~entity_access_get(SC_FUNCTION)[
+					credentials->auth_token].effective;
+	user_access_t func_access = access & ~credentials->universal_access;
+	if ((func_access & PRIV_U) != 0 ||
+	    (func->def->uid != credentials->uid &&
+	     func_access & ~func->access[credentials->auth_token].effective)) {
+		/* Access violation, report error. */
+		struct user *user = user_find(credentials->uid);
+		if (user != NULL) {
+			diag_set(AccessDeniedError, priv_name(PRIV_X),
+				 schema_object_name(SC_FUNCTION),
+				 func->def->name, user->def->name);
+		}
+		return -1;
+	}
+	return 0;
+}
+
+int
+func_call(struct func *base, struct port *args, struct port *ret)
+{
+	if (func_access_check(base) != 0)
+		return -1;
+	/**
+	 * Change the current user id if the function is
+	 * a set-definer-uid one. If the function is not
+	 * defined, it's obviously not a setuid one.
+	 */
+	struct credentials *orig_credentials = NULL;
+	if (base->def->setuid) {
+		orig_credentials = effective_user();
+		/* Remember and change the current user id. */
+		if (base->owner_credentials.auth_token >= BOX_USER_MAX) {
+			/*
+			 * Fill the cache upon first access, since
+			 * when func is created, no user may
+			 * be around to fill it (recovery of
+			 * system spaces from a snapshot).
+			 */
+			struct user *owner = user_find(base->def->uid);
+			if (owner == NULL)
+				return -1;
+			credentials_init(&base->owner_credentials,
+					 owner->auth_token,
+					 owner->def->uid);
+		}
+		fiber_set_user(fiber(), &base->owner_credentials);
+	}
+	int rc = base->vtab->call(base, args, ret);
+	/* Restore the original user */
+	if (orig_credentials)
+		fiber_set_user(fiber(), orig_credentials);
+	return rc;
 }
diff --git a/src/box/func.h b/src/box/func.h
index f6494d064..e121e6bb9 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -59,24 +59,21 @@ struct module {
 	bool is_unloading;
 };
 
+/** Virtual method table for func object. */
+struct func_vtab {
+	/** Call function with given arguments. */
+	int (*call)(struct func *func, struct port *args, struct port *ret);
+	/** Release implementation-specific function context. */
+	void (*destroy)(struct func *func);
+};
+
 /**
  * Stored function.
  */
 struct func {
 	struct func_def *def;
-	/**
-	 * Anchor for module membership.
-	 */
-	struct rlist item;
-	/**
-	 * For C functions, the body of the function.
-	 */
-	box_function_f func;
-	/**
-	 * Each stored function keeps a handle to the
-	 * dynamic library for the C callback.
-	 */
-	struct module *module;
+	/** Virtual method table. */
+	const struct func_vtab *vtab;
 	/**
 	 * Authentication id of the owner of the function,
 	 * used for set-user-id functions.
diff --git a/src/box/func_def.h b/src/box/func_def.h
index ef33cbbaa..866d425a1 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -67,6 +67,8 @@ struct func_def {
 	 * The language of the stored function.
 	 */
 	enum func_language language;
+	/** The length of the function name. */
+	uint32_t name_len;
 	/** Function name. */
 	char name[0];
 };
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 7cac90f3b..8b7223a7c 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -31,6 +31,8 @@
 #include "box/lua/call.h"
 #include "box/call.h"
 #include "box/error.h"
+#include "box/func.h"
+#include "box/func_def.h"
 #include "fiber.h"
 
 #include "lua/utils.h"
@@ -473,6 +475,49 @@ box_lua_eval(const char *expr, uint32_t expr_len,
 	return box_process_lua(execute_lua_eval, &ctx, ret);
 }
 
+struct func_lua {
+	/** Function object base class. */
+	struct func base;
+};
+
+static struct func_vtab func_lua_vtab;
+
+struct func *
+func_lua_new(struct func_def *def)
+{
+	(void) def;
+	assert(def->language == FUNC_LANGUAGE_LUA);
+	struct func_lua *func =
+		(struct func_lua *) malloc(sizeof(struct func_lua));
+	if (func == NULL) {
+		diag_set(OutOfMemory, sizeof(*func), "malloc", "func");
+		return NULL;
+	}
+	func->base.vtab = &func_lua_vtab;
+	return &func->base;
+}
+
+static void
+func_lua_destroy(struct func *func)
+{
+	assert(func != NULL && func->def->language == FUNC_LANGUAGE_LUA);
+	assert(func->vtab == &func_lua_vtab);
+	free(func);
+}
+
+static inline int
+func_lua_call(struct func *func, struct port *args, struct port *ret)
+{
+	assert(func != NULL && func->def->language == FUNC_LANGUAGE_LUA);
+	assert(func->vtab == &func_lua_vtab);
+	return box_lua_call(func->def->name, func->def->name_len, args, ret);
+}
+
+static struct func_vtab func_lua_vtab = {
+	.call = func_lua_call,
+	.destroy = func_lua_destroy,
+};
+
 static int
 lbox_module_reload(lua_State *L)
 {
diff --git a/src/box/lua/call.h b/src/box/lua/call.h
index d03bcd0f8..83aa43949 100644
--- a/src/box/lua/call.h
+++ b/src/box/lua/call.h
@@ -44,6 +44,7 @@ box_lua_call_init(struct lua_State *L);
 
 struct port;
 struct call_request;
+struct func_def;
 
 /**
  * Invoke a Lua stored procedure from the binary protocol
@@ -57,6 +58,10 @@ int
 box_lua_eval(const char *expr, uint32_t expr_len,
 	     struct port *args, struct port *ret);
 
+/** Construct a Lua function object. */
+struct func *
+func_lua_new(struct func_def *def);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/box/session.cc b/src/box/session.cc
index 4bb13d031..3a7c02401 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -297,7 +297,8 @@ access_check_session(struct user *user)
 }
 
 int
-access_check_universe(user_access_t access)
+access_check_universe_object(user_access_t access,
+		enum schema_object_type object_type, const char *object_name)
 {
 	struct credentials *credentials = effective_user();
 	access |= PRIV_U;
@@ -313,7 +314,7 @@ access_check_universe(user_access_t access)
 		if (user != NULL) {
 			diag_set(AccessDeniedError,
 				 priv_name(denied_access),
-				 schema_object_name(SC_UNIVERSE), "",
+				 schema_object_name(object_type), object_name,
 				 user->def->name);
 		} else {
 			/*
@@ -328,6 +329,12 @@ access_check_universe(user_access_t access)
 	return 0;
 }
 
+int
+access_check_universe(user_access_t access)
+{
+	return access_check_universe_object(access, SC_UNIVERSE, "");
+}
+
 int
 generic_session_push(struct session *session, uint64_t sync, struct port *port)
 {
diff --git a/src/box/session.h b/src/box/session.h
index 3a7397146..54a02536e 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -312,6 +312,15 @@ access_check_session(struct user *user);
 int
 access_check_universe(user_access_t access);
 
+/**
+ * Same as access_check_universe(), but in case the current user
+ * doesn't have universal access, set AccessDeniedError for the
+ * given object type and name.
+ */
+int
+access_check_universe_object(user_access_t access,
+		enum schema_object_type object_type, const char *object_name);
+
 static inline int
 session_push(struct session *session, uint64_t sync, struct port *port)
 {
-- 
2.21.0




More information about the Tarantool-patches mailing list