box: separate access check and function call in box_process_call

box_process_call() uses func_call(), which not only calls the given
function, but also checks that the current user has the right to execute
it. As a result, we can't add auditing for only those function calls
that passed the access check (apparently, there's no reason to log
function calls that failed with an 'access denied' error - we have a
separate audit event for this).

To fix this, let's introduce func_call_no_access_check() helper, which
calls a function without checking access rights, and use it along with
existing func_access_check() in box_process_call(). func_call() is now
an inline function that calls func_access_check() and then on success
func_call_no_access_check().

It's probably wrong that func_call() checks access rights, because this
means that to use a space with a functional index/constraint, the user
needs not only read/write access to the space itself, but also execute
access to the function. I think we should check the right to execute
such function only once - on functional index/constraint creation, not
on every call, but I'm not going to change this now, because nobody's
complained so far, and a change like this needs a proper discussion
anyway.

NO_TEST=refactoring
NO_DOC=refactoring
NO_CHANGELOG=refactoring

src/box/call.c

@@ -141,20 +141,23 @@ 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);
-
-	int rc;
 	struct port args;
 	port_msgpack_create(&args, request->args,
 			    request->args_end - request->args);
 	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);
+		if (func_access_check(func) != 0)
+			return -1;
+		if (func_call_no_access_check(func, &args, port) != 0)
+			return -1;
+	} else {
+		if (access_check_universe_object(PRIV_X | PRIV_U,
+						 SC_FUNCTION,
+						 tt_cstr(name, name_len)) != 0)
+			return -1;
+		if (box_lua_call(name, name_len, &args, port) != 0)
+			return -1;
 	}
-	if (rc != 0)
-		return -1;
 	return 0;
 }
 

src/box/func.c

@@ -508,8 +508,7 @@ func_delete(struct func *func)
 	free(def);
 }
 
-/** Check "EXECUTE" permissions for a given function. */
-static int
+int
 func_access_check(struct func *func)
 {
 	struct credentials *credentials = effective_user();
@@ -541,10 +540,9 @@ func_access_check(struct func *func)
 }
 
 int
-func_call(struct func *base, struct port *args, struct port *ret)
+func_call_no_access_check(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

src/box/func.h

@@ -91,6 +91,10 @@ func_new(struct func_def *def);
 void
 func_delete(struct func *func);
 
+/** Check "EXECUTE" permissions for a given function. */
+int
+func_access_check(struct func *func);
+
 /**
  * Call function @a func with arguments @a args, put return value to @a ret.
  * Return 0 on success and nonzero on failure.
@@ -100,7 +104,16 @@ func_delete(struct func *func);
  * if and only if func_call returns 0;
  */
 int
-func_call(struct func *func, struct port *args, struct port *ret);
+func_call_no_access_check(struct func *func, struct port *args,
+			  struct port *ret);
+
+static inline int
+func_call(struct func *func, struct port *args, struct port *ret)
+{
+	if (func_access_check(func) != 0)
+		return -1;
+	return func_call_no_access_check(func, args, ret);
+}
 
 /**
  * Reload dynamically loadable schema module.