From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v3 3/6] box: rework func object as a function frontend References: <536bcc9ce5085679877fa91cea36f021168ffc2b.1560433806.git.kshcherbatov@tarantool.org> <20190618132320.r2vtyk2hprogwd6s@esperanza> From: Kirill Shcherbatov Message-ID: Date: Wed, 19 Jun 2019 18:51:13 +0300 MIME-Version: 1.0 In-Reply-To: <20190618132320.r2vtyk2hprogwd6s@esperanza> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Vladimir Davydov List-ID: Done. ======================================================== 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 /** @@ -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