From ff64d58a4dfbde4e741cd72bc9df5f75e8b9c6e1 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Fri, 21 Jul 2023 13:11:06 +0300
Subject: [PATCH] box: add sql grant object type

Closes #8803

@TarantoolBot document
Title: Document `lua_eval`, `lua_call`, and `sql` grant object types

In Tarantool 3.0 we introduced the new `lua_eval`, `lua_call`, and `sql`
object types for `box.schema.user.grant` to control access to code
execution over the network protocol (IPROTO).

1. Granting the 'execute' privilege on `lua_eval` permits the user to
   execute arbitrary Lua code with the `IPROTO_EVAL` request.

   Example:

   ```Lua
   box.cfg({listen = 3301})
   box.schema.user.create('alice', {password = 'secret'})
   conn = require('net.box').connect(
       box.cfg.listen, {user = 'alice', password = 'secret'})
   conn:eval('return true') -- access denied
   box.schema.user.grant('alice', 'execute', 'lua_eval')
   conn:eval('return true') -- ok
   ```

2. Granting the 'execute' privilege on `lua_call` permits the user to
   call any global (accessible via the `_G` Lua table) user-defined
   Lua function with the `IPROTO_CALL` request. It does **not** permit
   the user to call built-in Lua functions, such as `loadstring` or
   `box.session.su`. It does **not** permit the user to call functions
   registered in the `_func` system space with `box.schema.func.create`
   (access to those functions is still controlled by privileges granted
   on `function`).

   Example:

   ```Lua
   function my_func() end
   box.cfg({listen = 3301})
   box.schema.user.create('alice', {password = 'secret'})
   conn = require('net.box').connect(
       box.cfg.listen, {user = 'alice', password = 'secret'})
   conn:call('my_func') -- access denied
   box.schema.user.grant('alice', 'execute', 'lua_call')
   conn:call('my_func') -- ok
   conn:call('box.session.su', {'admin'}) -- access denied
   ```

3. Granting the 'execute' privilege on `sql` permits the user to
   execute an arbitrary SQL expression with the `IPROTO_PREPARE`
   and `IPROTO_EXECUTE` requests. Without this privilege or the
   'execute' privilege granted on `universe`, the user is **not**
   permitted to execute SQL expressions over IPROTO anymore.
   Note that before Tarantool 3.0 any user (even guest) could execute
   SQL expressions over IPROTO. It is possible to revert to the old
   behavior by toggling the `sql_priv` compat option. Please add
   a description to https://tarantool.io/compat/sql_priv

   Example:

   ```Lua
   box.cfg({listen = 3301})
   box.schema.user.create('alice', {password = 'secret'})
   conn = require('net.box').connect(
       box.cfg.listen, {user = 'alice', password = 'secret'})
   conn:execute('SELECT 1') -- access denied
   box.schema.user.grant('alice', 'execute', 'sql')
   conn:execute('SELECT 1') -- ok
   ```
---
 changelogs/unreleased/gh-8803-exec-priv.md    |  5 ++
 src/box/execute.c                             | 28 +++++++++++
 src/box/lua/schema.lua                        |  2 +
 src/box/schema_def.c                          |  1 +
 src/box/schema_def.h                          |  1 +
 src/box/user.cc                               |  2 +
 src/box/user.h                                |  2 +
 src/lua/compat.lua                            | 15 ++++++
 test/box-luatest/gh_8803_exec_priv_test.lua   | 48 ++++++++++++++++++-
 test/box/tx_man.result                        |  3 ++
 test/box/tx_man.test.lua                      |  1 +
 test/sql-tap/array.test.lua                   |  2 +
 test/sql-tap/map.test.lua                     |  2 +
 test/sql/gh-2362-select-access-rights.result  |  6 +++
 .../sql/gh-2362-select-access-rights.test.lua |  2 +
 test/sql/gh-4104-view-access-check.result     |  6 +++
 test/sql/gh-4104-view-access-check.test.lua   |  2 +
 17 files changed, 127 insertions(+), 1 deletion(-)

diff --git a/changelogs/unreleased/gh-8803-exec-priv.md b/changelogs/unreleased/gh-8803-exec-priv.md
index 30a4a73708..3383441a88 100644
--- a/changelogs/unreleased/gh-8803-exec-priv.md
+++ b/changelogs/unreleased/gh-8803-exec-priv.md
@@ -6,3 +6,8 @@
   `IPROTO_EVAL` request. Granting the `'execute'` privilege on `lua_call`
   allows the user to execute any global user-defined Lua function with
   the `IPROTO_CALL` request (gh-8803).
+* **[Breaking change]** Introduced the new `sql` object type for
+  `box.schema.user.grant`. Now only users with the `'execute'` privilege
+  granted on `sql` or `universe` can execute SQL expressions with the
+  `IPROTO_EXECUTE` or `IPROTO_PREPARE` requests. To revert to the old behavior
+  (no SQL access checks), use the `sql_priv` compat option (gh-8803).
diff --git a/src/box/execute.c b/src/box/execute.c
index 44d465602d..a07cd89d31 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -49,12 +49,38 @@
 #include "session.h"
 #include "rmean.h"
 #include "box/sql/port.h"
+#include "tweaks.h"
 
 const char *sql_info_key_strs[] = {
 	"row_count",
 	"autoincrement_ids",
 };
 
+/** Whether to enable access checks for SQL requests. */
+static bool sql_access_check_is_enabled = true;
+TWEAK_BOOL(sql_access_check_is_enabled);
+
+/** Checks if the current user may execute an SQL request. */
+static int
+access_check_sql(void)
+{
+	if (!sql_access_check_is_enabled)
+		return 0;
+	struct credentials *cr = effective_user();
+	user_access_t access = PRIV_X | PRIV_U;
+	access &= ~cr->universal_access;
+	if (access == 0)
+		return 0;
+	access &= ~universe.access_sql[cr->auth_token].effective;
+	if (access == 0)
+		return 0;
+	struct user *user = user_find(cr->uid);
+	if (user != NULL)
+		diag_set(AccessDeniedError, priv_name(PRIV_X),
+			 schema_object_name(SC_SQL), "", user->def->name);
+	return -1;
+}
+
 /**
  * Convert sql row into a tuple and append to a port.
  * @param stmt Started prepared statement. At least one
@@ -261,6 +287,8 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 int
 box_process_sql(const struct sql_request *request, struct port *port)
 {
+	if (access_check_sql() != 0)
+		return -1;
 	struct region *region = &fiber()->gc;
 	struct sql_bind *bind = NULL;
 	int bind_count = 0;
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 7d99f15f67..ffa377de3e 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -2875,6 +2875,7 @@ local priv_object_combo = {
     ["universe"] = box.priv.ALL,
     ["lua_call"] = bit.bor(box.priv.X, box.priv.U),
     ["lua_eval"] = bit.bor(box.priv.X, box.priv.U),
+    ["sql"]      = bit.bor(box.priv.X, box.priv.U),
 -- sic: we allow to grant 'execute' on space. This is a legacy
 -- bug, please fix it in 2.0
     ["space"]    = bit.bxor(box.priv.ALL, box.priv.S,
@@ -2953,6 +2954,7 @@ local singleton_object_types = {
     ['universe'] = true,
     ['lua_call'] = true,
     ['lua_eval'] = true,
+    ['sql'] = true,
 }
 
 local function is_singleton_object_type(object_type)
diff --git a/src/box/schema_def.c b/src/box/schema_def.c
index b635422076..98a1d207a8 100644
--- a/src/box/schema_def.c
+++ b/src/box/schema_def.c
@@ -40,6 +40,7 @@ static const char *object_type_strs[] = {
 	/* [SC_UNIVERSE]        = */ "universe",
 	/* [SC_LUA_CALL]        = */ "lua_call",
 	/* [SC_LUA_EVAL]        = */ "lua_eval",
+	/* [SC_SQL]             = */ "sql",
 	/* [SC_SPACE]           = */ "space",
 	/* [SC_FUNCTION]        = */ "function",
 	/* [SC_USER]            = */ "user",
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index 34d8571223..bb35c911e3 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -305,6 +305,7 @@ enum schema_object_type {
 	SC_UNIVERSE,
 	SC_LUA_CALL,
 	SC_LUA_EVAL,
+	SC_SQL,
 	SC_SPACE,
 	SC_FUNCTION,
 	SC_USER,
diff --git a/src/box/user.cc b/src/box/user.cc
index 7231481cd7..3b84156906 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -220,6 +220,8 @@ access_find(const struct priv_def *priv)
 		return universe.access_lua_call;
 	case SC_LUA_EVAL:
 		return universe.access_lua_eval;
+	case SC_SQL:
+		return universe.access_sql;
 	case SC_SPACE:
 	{
 		if (priv->is_entity_access)
diff --git a/src/box/user.h b/src/box/user.h
index 5a24cf79ee..294a265cd0 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -47,6 +47,8 @@ struct universe {
 	struct access access_lua_call[BOX_USER_MAX];
 	/** Privileges to execute any Lua expression with IPROTO_EVAL. */
 	struct access access_lua_eval[BOX_USER_MAX];
+	/** Privileges to execute SQL requests with IPROTO_EXECUTE. */
+	struct access access_sql[BOX_USER_MAX];
 };
 
 /** A single instance of the universe. */
diff --git a/src/lua/compat.lua b/src/lua/compat.lua
index 7719a42171..6642b82a4b 100644
--- a/src/lua/compat.lua
+++ b/src/lua/compat.lua
@@ -44,6 +44,15 @@ channel close.
 https://tarantool.io/compat/fiber_channel_close_mode
 ]]
 
+local SQL_PRIV_BRIEF = [[
+Whether to enable access checks for SQL requests. The old behavior is to let
+any user execute an arbitrary SQL request over IPROTO. With the new behavior,
+only users that were granted the 'execute' privilege on 'sql' or 'universe'
+have access to SQL.
+
+https://tarantool.io/compat/sql_priv
+]]
+
 local SQL_SEQ_SCAN_DEFAULT_BRIEF = [[
 Whether seq_seq_scan session setting should be set to true or false during
 initialization or new session creation.
@@ -119,6 +128,12 @@ local options = {
         action = tweak_action('fiber_channel_close_mode',
                               'forceful', 'graceful'),
     },
+    sql_priv = {
+        default = 'new',
+        obsolete = nil,
+        brief = SQL_PRIV_BRIEF,
+        action = tweak_action('sql_access_check_is_enabled', false, true)
+    },
     sql_seq_scan_default = {
         default = 'new',
         obsolete = nil,
diff --git a/test/box-luatest/gh_8803_exec_priv_test.lua b/test/box-luatest/gh_8803_exec_priv_test.lua
index 3103a6c677..02e1399d1c 100644
--- a/test/box-luatest/gh_8803_exec_priv_test.lua
+++ b/test/box-luatest/gh_8803_exec_priv_test.lua
@@ -3,7 +3,7 @@ local server = require('luatest.server')
 local t = require('luatest')
 
 local g_common = t.group('gh_8803_exec_priv.common', t.helpers.matrix({
-    obj_type = {'lua_call', 'lua_eval'},
+    obj_type = {'lua_call', 'lua_eval', 'sql'},
 }))
 
 g_common.before_all(function(cg)
@@ -170,3 +170,49 @@ g.test_lua_call_builtin = function(cg)
         t.assert_error_msg_equals(errfmt:format(func), c.call, c, func)
     end
 end
+
+-- Checks that execute privilege granted on sql grants access to
+-- executing SQL expressions.
+g.test_sql = function(cg)
+    local c = cg.conn
+    local expr = 'SELECT 1'
+    local errmsg = "Execute access to sql '' is denied for user 'test'"
+    t.assert_error_msg_equals(errmsg, c.execute, c, expr)
+    t.assert_error_msg_equals(errmsg, c.prepare, c, expr)
+    t.assert_error_msg_equals(errmsg, c.unprepare, c, 0)
+    cg.grant('test', 'execute', 'sql')
+    t.assert(pcall(c.execute, c, expr))
+    cg.revoke('test', 'usage', 'universe')
+    t.assert_error_msg_equals(errmsg, c.execute, c, expr)
+    t.assert_error_msg_equals(errmsg, c.prepare, c, expr)
+    t.assert_error_msg_equals(errmsg, c.unprepare, c, 0)
+    cg.grant('test', 'usage', 'sql')
+    t.assert(pcall(c.execute, c, expr))
+end
+
+g.after_test('test_sql_compat', function(cg)
+    cg.server:exec(function()
+        local compat = require('compat')
+        compat.sql_priv = 'default'
+    end)
+end)
+
+-- Checks the sql_priv compat option.
+g.test_sql_compat = function(cg)
+    local c = cg.conn
+    local expr = 'SELECT 1'
+    local errmsg = "Execute access to sql '' is denied for user 'test'"
+    t.assert_error_msg_equals(errmsg, c.execute, c, expr)
+    cg.server:exec(function()
+        local compat = require('compat')
+        t.assert_equals(compat.sql_priv.default, 'new')
+        t.assert_equals(compat.sql_priv.current, 'default')
+        compat.sql_priv = 'old'
+    end)
+    t.assert(pcall(c.execute, c, expr))
+    cg.server:exec(function()
+        local compat = require('compat')
+        compat.sql_priv = 'new'
+    end)
+    t.assert_error_msg_equals(errmsg, c.execute, c, expr)
+end
diff --git a/test/box/tx_man.result b/test/box/tx_man.result
index 8eea6fd2a7..2c58aebc8c 100644
--- a/test/box/tx_man.result
+++ b/test/box/tx_man.result
@@ -2458,6 +2458,9 @@ box.execute([[CREATE TABLE u (column1 INT PRIMARY KEY, column2 INT);]])
  | ---
  | - row_count: 1
  | ...
+box.schema.user.grant('guest', 'execute', 'sql')
+ | ---
+ | ...
 box.schema.user.grant('guest', 'read,write', 'space', 'U')
  | ---
  | ...
diff --git a/test/box/tx_man.test.lua b/test/box/tx_man.test.lua
index 10440c568f..eecf9e9a4a 100644
--- a/test/box/tx_man.test.lua
+++ b/test/box/tx_man.test.lua
@@ -780,6 +780,7 @@ box.space.t:drop()
 
 -- https://github.com/tarantool/tarantool/issues/5892
 box.execute([[CREATE TABLE u (column1 INT PRIMARY KEY, column2 INT);]])
+box.schema.user.grant('guest', 'execute', 'sql')
 box.schema.user.grant('guest', 'read,write', 'space', 'U')
 conn = require('net.box').connect(box.cfg.listen)
 
diff --git a/test/sql-tap/array.test.lua b/test/sql-tap/array.test.lua
index 5fdb74257d..102dde0030 100755
--- a/test/sql-tap/array.test.lua
+++ b/test/sql-tap/array.test.lua
@@ -1037,6 +1037,7 @@ test:do_test(
 
 local remote = require('net.box')
 box.cfg{listen = os.getenv('LISTEN')}
+box.schema.user.grant('guest', 'execute', 'sql')
 local cn = remote.connect(box.cfg.listen)
 test:do_test(
     "builtins-14.2",
@@ -1047,6 +1048,7 @@ test:do_test(
         {1, 2, 3}
     })
 cn:close()
+box.schema.user.revoke('guest', 'execute', 'sql')
 
 box.execute([[DROP TABLE t1;]])
 box.execute([[DROP TABLE t;]])
diff --git a/test/sql-tap/map.test.lua b/test/sql-tap/map.test.lua
index 3606ea3ddd..b5e20c3d4b 100755
--- a/test/sql-tap/map.test.lua
+++ b/test/sql-tap/map.test.lua
@@ -993,6 +993,7 @@ test:do_test(
 
 local remote = require('net.box')
 box.cfg{listen = os.getenv('LISTEN')}
+box.schema.user.grant('guest', 'execute', 'sql')
 local cn = remote.connect(box.cfg.listen)
 test:do_test(
     "builtins-13.2",
@@ -1003,6 +1004,7 @@ test:do_test(
         {abc = 2, [1] = 3}
     })
 cn:close()
+box.schema.user.revoke('guest', 'execute', 'sql')
 
 box.execute([[DROP TABLE t1;]])
 box.execute([[DROP TABLE t;]])
diff --git a/test/sql/gh-2362-select-access-rights.result b/test/sql/gh-2362-select-access-rights.result
index 09105b12b2..0d78427510 100644
--- a/test/sql/gh-2362-select-access-rights.result
+++ b/test/sql/gh-2362-select-access-rights.result
@@ -26,6 +26,9 @@ box.execute("INSERT INTO t2 VALUES (1);")
 ---
 - row_count: 1
 ...
+box.schema.user.grant('guest', 'execute', 'sql')
+---
+...
 box.schema.user.grant('guest','read', 'space', 'T1')
 ---
 ...
@@ -83,6 +86,9 @@ box.schema.user.revoke('guest','read','space', 'V')
 box.schema.user.revoke('guest','read','space', 'T2')
 ---
 ...
+box.schema.user.revoke('guest', 'execute', 'sql')
+---
+...
 box.execute('DROP VIEW v')
 ---
 - row_count: 1
diff --git a/test/sql/gh-2362-select-access-rights.test.lua b/test/sql/gh-2362-select-access-rights.test.lua
index 2d9ff9895d..95f47fda34 100644
--- a/test/sql/gh-2362-select-access-rights.test.lua
+++ b/test/sql/gh-2362-select-access-rights.test.lua
@@ -8,6 +8,7 @@ box.execute("CREATE TABLE t2 (s1 INT PRIMARY KEY);")
 box.execute("INSERT INTO t1 VALUES (1, 1);")
 box.execute("INSERT INTO t2 VALUES (1);")
 
+box.schema.user.grant('guest', 'execute', 'sql')
 box.schema.user.grant('guest','read', 'space', 'T1')
 c = nb.connect(box.cfg.listen)
 c:execute("SELECT * FROM SEQSCAN t1;")
@@ -29,6 +30,7 @@ c:execute('SELECT * FROM v')
 -- Cleanup
 box.schema.user.revoke('guest','read','space', 'V')
 box.schema.user.revoke('guest','read','space', 'T2')
+box.schema.user.revoke('guest', 'execute', 'sql')
 
 box.execute('DROP VIEW v')
 box.execute('DROP TABLE t2')
diff --git a/test/sql/gh-4104-view-access-check.result b/test/sql/gh-4104-view-access-check.result
index 8fb3454cc0..3dcc35f308 100644
--- a/test/sql/gh-4104-view-access-check.result
+++ b/test/sql/gh-4104-view-access-check.result
@@ -18,6 +18,9 @@ box.execute("CREATE VIEW supersecret_leak AS  SELECT * FROM supersecret, superse
 ---
 - row_count: 1
 ...
+box.schema.user.grant('guest', 'execute', 'sql')
+---
+...
 remote = require 'net.box'
 ---
 ...
@@ -48,6 +51,9 @@ box.schema.user.revoke('guest','read', 'space', 'SUPERSECRET')
 box.schema.user.revoke('guest','read', 'space', 'SUPERSECRET_LEAK')
 ---
 ...
+box.schema.user.revoke('guest', 'execute', 'sql')
+---
+...
 box.execute("DROP VIEW supersecret_leak")
 ---
 - row_count: 1
diff --git a/test/sql/gh-4104-view-access-check.test.lua b/test/sql/gh-4104-view-access-check.test.lua
index 139cfcfa14..4a31db1a5e 100644
--- a/test/sql/gh-4104-view-access-check.test.lua
+++ b/test/sql/gh-4104-view-access-check.test.lua
@@ -3,6 +3,7 @@ box.execute("CREATE TABLE supersecret2(id INT PRIMARY KEY, data TEXT);")
 box.execute("INSERT INTO supersecret VALUES(1, 'very very big secret');")
 box.execute("INSERT INTO supersecret2 VALUES(1, 'very big secret 2');")
 box.execute("CREATE VIEW supersecret_leak AS  SELECT * FROM supersecret, supersecret2;")
+box.schema.user.grant('guest', 'execute', 'sql')
 remote = require 'net.box'
 cn = remote.connect(box.cfg.listen)
 cn:execute([[SET SESSION "sql_seq_scan" = true;]])
@@ -14,6 +15,7 @@ cn:execute('SELECT * FROM SUPERSECRET_LEAK')
 
 box.schema.user.revoke('guest','read', 'space', 'SUPERSECRET')
 box.schema.user.revoke('guest','read', 'space', 'SUPERSECRET_LEAK')
+box.schema.user.revoke('guest', 'execute', 'sql')
 box.execute("DROP VIEW supersecret_leak")
 box.execute("DROP TABLE supersecret")
 box.execute("DROP TABLE supersecret2")
-- 
GitLab