Re: [PATCH v2 7/9] box: sandbox option for persistent functions

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Thu, Jun 06, 2019 at 03:04:03PM +0300, Kirill Shcherbatov wrote:
> Introduced a new option 'is_sandboxed' to initialize a new
> persistent function inside of isolated sandbox where only limited
> number of functions and modules is available:
> -assert -error -pairs -ipairs -next -pcall -xpcall -type
> -print -select -string -tonumber -tostring -unpack -math -utf8
> 
> Global variables are forbidden in persistent Lua functions.
> 
> To initialize a new persistent function inside of sandbox,
> specify is_sandboxed = true:
> box.schema.func.create('myfunc', {body = body,
>                                   opts = {is_sandboxed = true}})
> 
> Part of #4182
> Needed for #1260
> ---
>  src/box/alter.cc                  | 12 ++++++
>  src/box/errcode.h                 |  1 +
>  src/box/func.c                    |  8 ++++
>  src/box/func_def.c                |  9 +++++
>  src/box/func_def.h                | 23 +++++++++++
>  src/lua/utils.c                   | 67 +++++++++++++++++++++++++++++++
>  src/lua/utils.h                   |  8 ++++
>  test/box/misc.result              |  1 +
>  test/box/persistent_func.result   | 36 +++++++++++++++++
>  test/box/persistent_func.test.lua | 15 +++++++
>  10 files changed, 180 insertions(+)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 11cad77c3..c769e4f3d 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -2449,6 +2449,7 @@ func_def_new_from_tuple(struct tuple *tuple)
>  	if (def == NULL)
>  		tnt_raise(OutOfMemory, def_sz, "malloc", "def");
>  	auto def_guard = make_scoped_guard([=] { free(def); });
> +	func_opts_create(&def->opts);
>  	func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid);
>  	memcpy(def->name, name, name_len);
>  	def->name[name_len] = 0;
> @@ -2492,6 +2493,17 @@ func_def_new_from_tuple(struct tuple *tuple)
>  		}
>  		def->is_deterministic =
>  			tuple_field_bool_xc(tuple, BOX_FUNC_FIELD_IS_DETERMINISTIC);
> +		const char *opts = tuple_field(tuple, BOX_FUNC_FIELD_OPTS);
> +		if (opts_decode(&def->opts, func_opts_reg, &opts,
> +				ER_WRONG_FUNCTION_OPTIONS, BOX_FUNC_FIELD_OPTS,
> +				NULL) != 0)
> +			diag_raise();
> +		if (def->opts.is_sandboxed &&
> +		    (def->language != FUNC_LANGUAGE_LUA || body_len == 0)) {
> +			tnt_raise(ClientError, ER_CREATE_FUNCTION, name,
> +				  "is_sandboxed option is applieble only for "
> +				  "persistent Lua function");
> +		}

Again, the check should be carried out in function constructors so that
we don't change this code when we introduce SQL functions.

>  	} else {
>  		def->returns = FIELD_TYPE_ANY;
>  		def->is_deterministic = false;
> diff --git a/src/box/func_def.c b/src/box/func_def.c
> index 76ed77b24..df74a6d9a 100644
> --- a/src/box/func_def.c
> +++ b/src/box/func_def.c
> @@ -1,3 +1,12 @@
>  #include "func_def.h"
> +#include "opt_def.h"
>  
>  const char *func_language_strs[] = {"LUA", "C"};
> +
> +const struct func_opts func_opts_default = {
> +	/* .is_sandboxed = */ false,
> +};
> +
> +const struct opt_def func_opts_reg[] = {
> +	OPT_DEF("is_sandboxed", OPT_BOOL, struct func_opts, is_sandboxed),
> +};

AFAIK we've decided to make all function options as tuple fields while
the 'option' field is here just in case we need to add something later.

> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 27ff6b396..0d1cca423 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -46,6 +46,14 @@ static uint32_t CTID_STRUCT_IBUF_PTR;
>  uint32_t CTID_CHAR_PTR;
>  uint32_t CTID_CONST_CHAR_PTR;
>  
> +static const char *default_sandbox_exports[] =
> +	{"assert", "error", "ipairs", "math", "next", "pairs", "pcall",
> +	"print", "select", "string", "table", "tonumber", "tostring",
> +	"type", "unpack", "xpcall", "utf8"};
> +
> +static int luaL_deepcopy_func_ref = LUA_REFNIL;
> +static int luaL_default_sandbox_ref = LUA_REFNIL;
> +
>  void *
>  luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
>  {
> @@ -1248,6 +1256,65 @@ luaT_func_find(struct lua_State *L, const char *name, const char *name_end,
>  	return 0;
>  }
>  
> +/**
> + * Assemble a new sandbox with given exports table on top of the
> + * Lua stack. All modules in exports list are copying deeply
> + * to ensure the immutablility of this system object.
> + */
> +static int
> +luaT_prepare_sandbox(struct lua_State *L, const char *exports[],
> +		     uint32_t exports_count)
> +{
> +	assert(luaL_deepcopy_func_ref != LUA_REFNIL);
> +	lua_createtable(L, exports_count, 0);
> +	for (unsigned i = 0; i < exports_count; i++) {
> +		int count;
> +		uint32_t name_len = strlen(exports[i]);
> +		if (luaT_func_find(L, exports[i], exports[i] + name_len,
> +				   &count) != 0)
> +			return -1;
> +		switch (lua_type(L, -1)) {
> +		case LUA_TTABLE:
> +			lua_rawgeti(L, LUA_REGISTRYINDEX,
> +				    luaL_deepcopy_func_ref);
> +			lua_insert(L, -2);
> +			lua_call(L, 1, LUA_MULTRET);
> +			FALLTHROUGH;
> +		case LUA_TFUNCTION:
> +			break;
> +		default:
> +			unreachable();
> +		}
> +		lua_setfield(L, -2, exports[i]);
> +	}
> +	return 0;
> +}
> +
> +int
> +luaT_get_sandbox(struct lua_State *L)

As I mentioned earlier, I don't think we need to have these highly
specialized functions in src/lua/util.[hc]. I'd define them as static
in src/box/lua/call.[hc] as we only need them to invoke persistent Lua
functions.

> +{
> +	if (luaL_deepcopy_func_ref == LUA_REFNIL) {
> +		int count;
> +		const char *deepcopy = "table.deepcopy";
> +		if (luaT_func_find(L, deepcopy, deepcopy + strlen(deepcopy),
> +				&count) != 0)
> +			return -1;

Wouldn't it be more effecient to copy the table in C, i.e. without the
aid of 'table.deepcopy' method?

> +		luaL_deepcopy_func_ref = luaL_ref(L, LUA_REGISTRYINDEX);
> +		assert(luaL_deepcopy_func_ref != LUA_REFNIL);
> +	}
> +	if (luaL_default_sandbox_ref == LUA_REFNIL) {
> +		if (luaT_prepare_sandbox(L, default_sandbox_exports,
> +					 nelem(default_sandbox_exports)) != 0)
> +			return -1;
> +		luaL_default_sandbox_ref = luaL_ref(L, LUA_REGISTRYINDEX);
> +		assert(luaL_default_sandbox_ref != LUA_REFNIL);
> +	}
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_deepcopy_func_ref);
> +	lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_default_sandbox_ref);
> +	lua_call(L, 1, LUA_MULTRET);
> +	return 0;
> +}