diff --git a/changelogs/unreleased/gh-8803-exec-priv.md b/changelogs/unreleased/gh-8803-exec-priv.md index 30a4a7370844542c94de89b92ad8528b2cd34191..3383441a880707a5f31ea734c1623e0ac92b952f 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 44d465602d5c221c1106d20bec840672ed641231..a07cd89d312d98eb78eedffd339fecf4bec095ed 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 7d99f15f678bc0a88ebead9282672e9c6d28d3e2..ffa377de3e6b0d3b99a2a0b132b55fab5298ef49 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 b635422076dd2d6d40d14bd9e4c60d4454849940..98a1d207a803fad794727c04c8cbb6c08fea937f 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 34d8571223b25c071ad262aa46c0ec479741fe31..bb35c911e383ed131cfee2297ed3401e8d9997de 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 7231481cd7e71a9263070d67d6d9dfa22e6b451d..3b84156906a5d042d5c90176f8dd3d04dca6fe28 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 5a24cf79ee47b21d7863e32a20a70fe684463da9..294a265cd0a8dce29e0a4d7366444250cc652d79 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 7719a42171c7c78c0c7bf8e7b3b21003bfa1f03d..6642b82a4bcc1974b2e8dbf14cdc9b6becde9b57 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 3103a6c677aab281d4b89a49edbb42ad7a10b680..02e1399d1c587e0fa0c0866e8c85a5b5928ce1c9 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 8eea6fd2a7015b51580f112940548c91f8886a18..2c58aebc8ce71070fef22cce64e02938650c78b5 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 10440c568fe0f1a653835dfcea31e00b727bb5f8..eecf9e9a4ab2b01dec98b31d08ae4d8196ae2259 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 5fdb74257d2a3d6ffe29bde30cab08befc79f8d3..102dde0030d3ee6927ccfb0a121ef132e3ffaa90 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 3606ea3ddd23951adc85a9c33d23c4d4057c5cfd..b5e20c3d4bb7bbdd27663b4297b209f93d59ddf4 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 09105b12b24195bc7baaa12e8cdb60225efe220d..0d784275106c47ecb16c14442b61cec9dd78805e 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 2d9ff9895df4cc92e69d51915da40e6f1884dd27..95f47fda34e8692fc8e563010b839aeb4bb02a02 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 8fb3454cc04aba30db94553bca43ff609a8fcd34..3dcc35f308b5816e1937138e0834b1580498bdbd 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 139cfcfa14cf304294dd7eadb762701986aabd77..4a31db1a5e2b23b248a56d779ff985f6514789c7 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")