Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Yukhin <kyukhin@tarantool.org>
To: korablev@tarantool.org
Cc: tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org>
Subject: [tarantool-patches] [PATCH v2 2/2] sql: check read access while executing SQL query
Date: Fri, 12 Oct 2018 14:38:19 +0300	[thread overview]
Message-ID: <9f7c0a46a7576c9a07e4e20b262d486adec4d975.1539344128.git.kyukhin@tarantool.org> (raw)
In-Reply-To: <cover.1539344128.git.kyukhin@tarantool.org>

Since SQL front-end is not using box API,
no checkes for read access are performed by VDBE engine.
Add check to IteratorOpen op-code to make sure that read
privilege exists for given space.
Note, that there's is no need to perform DML/DDL checkes as
they're performed by Tarantool's core.

@TarantoolBot document
Title: Document behaviour of SQL in presence of
read access restrictions. Need to clarify, that
if there's no read access to the space, then not
only SELECT statements will fail, but also those DML
which implies reading from spaces indirectly, e.g.:
  UPDATE t1 SET a=2 WHERE b=3;

Closes #2362
---
 src/box/sql/vdbe.c                            |   5 +
 test/sql/gh-2362-select-access-rights.result  | 110 ++++++++++++++++++
 .../sql/gh-2362-select-access-rights.test.lua |  42 +++++++
 3 files changed, 157 insertions(+)
 create mode 100644 test/sql/gh-2362-select-access-rights.result
 create mode 100644 test/sql/gh-2362-select-access-rights.test.lua

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7c1015c..10b58b4 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3099,6 +3099,11 @@ case OP_IteratorOpen:
 	else
 		space = aMem[pOp->p3].u.p;
 	assert(space != NULL);
+	if (access_check_space(space, PRIV_R) != 0) {
+		rc = SQL_TARANTOOL_ERROR;
+		goto abort_due_to_error;
+	}
+
 	struct index *index = space_index(space, pOp->p2);
 	assert(index != NULL);
 	assert(pOp->p1 >= 0);
diff --git a/test/sql/gh-2362-select-access-rights.result b/test/sql/gh-2362-select-access-rights.result
new file mode 100644
index 0000000..b42ee36
--- /dev/null
+++ b/test/sql/gh-2362-select-access-rights.result
@@ -0,0 +1,110 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+nb = require('net.box')
+---
+...
+box.sql.execute("PRAGMA sql_default_engine='"..engine.."'")
+---
+...
+box.sql.execute("CREATE TABLE t1 (s1 INT PRIMARY KEY, s2 INT UNIQUE);")
+---
+...
+box.sql.execute("CREATE TABLE t2 (s1 INT PRIMARY KEY);")
+---
+...
+box.sql.execute("INSERT INTO t1 VALUES (1, 1);")
+---
+...
+box.sql.execute("INSERT INTO t2 VALUES (1);")
+---
+...
+box.schema.user.grant('guest','read', 'space', 'T1')
+---
+...
+c = nb.connect(box.cfg.listen)
+---
+...
+c:execute("SELECT * FROM t1;")
+---
+- metadata:
+  - name: S1
+  - name: S2
+  rows:
+  - [1, 1]
+...
+box.schema.user.revoke('guest','read', 'space', 'T1')
+---
+...
+c = nb.connect(box.cfg.listen)
+---
+...
+c:execute("SELECT * FROM t1;")
+---
+- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
+    user ''guest'''
+...
+box.schema.user.grant('guest','read', 'space', 'T2')
+---
+...
+c = nb.connect(box.cfg.listen)
+---
+...
+c:execute('SELECT * FROM t1, t2 WHERE t1.s1 = t2.s1')
+---
+- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
+    user ''guest'''
+...
+box.sql.execute("CREATE VIEW v AS SELECT * FROM t1")
+---
+...
+box.schema.user.grant('guest','read', 'space', 'V')
+---
+...
+v = nb.connect(box.cfg.listen)
+---
+...
+c:execute('SELECT * FROM v')
+---
+- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
+    user ''guest'''
+...
+box.sql.execute('CREATE TABLE t3 (s1 INT PRIMARY KEY, fk INT, FOREIGN KEY (fk) REFERENCES t1(s2))')
+---
+...
+box.schema.user.grant('guest','read','space', 'T3')
+---
+...
+v = nb.connect(box.cfg.listen)
+---
+...
+c:execute('INSERT INTO t3 VALUES (1, 1)')
+---
+- error: 'Failed to execute SQL statement: Read access to space ''T1'' is denied for
+    user ''guest'''
+...
+-- Cleanup
+box.schema.user.revoke('guest','read','space', 'V')
+---
+...
+box.schema.user.revoke('guest','read','space', 'T2')
+---
+...
+box.schema.user.revoke('guest','read','space', 'T3')
+---
+...
+box.sql.execute('DROP VIEW v')
+---
+...
+box.sql.execute('DROP TABLE t3')
+---
+...
+box.sql.execute('DROP TABLE t2')
+---
+...
+box.sql.execute("DROP TABLE t1")
+---
+...
diff --git a/test/sql/gh-2362-select-access-rights.test.lua b/test/sql/gh-2362-select-access-rights.test.lua
new file mode 100644
index 0000000..9c50e19
--- /dev/null
+++ b/test/sql/gh-2362-select-access-rights.test.lua
@@ -0,0 +1,42 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+nb = require('net.box')
+
+box.sql.execute("PRAGMA sql_default_engine='"..engine.."'")
+box.sql.execute("CREATE TABLE t1 (s1 INT PRIMARY KEY, s2 INT UNIQUE);")
+box.sql.execute("CREATE TABLE t2 (s1 INT PRIMARY KEY);")
+box.sql.execute("INSERT INTO t1 VALUES (1, 1);")
+box.sql.execute("INSERT INTO t2 VALUES (1);")
+
+box.schema.user.grant('guest','read', 'space', 'T1')
+c = nb.connect(box.cfg.listen)
+c:execute("SELECT * FROM t1;")
+
+box.schema.user.revoke('guest','read', 'space', 'T1')
+c = nb.connect(box.cfg.listen)
+c:execute("SELECT * FROM t1;")
+
+box.schema.user.grant('guest','read', 'space', 'T2')
+c = nb.connect(box.cfg.listen)
+c:execute('SELECT * FROM t1, t2 WHERE t1.s1 = t2.s1')
+
+box.sql.execute("CREATE VIEW v AS SELECT * FROM t1")
+
+box.schema.user.grant('guest','read', 'space', 'V')
+v = nb.connect(box.cfg.listen)
+c:execute('SELECT * FROM v')
+
+box.sql.execute('CREATE TABLE t3 (s1 INT PRIMARY KEY, fk INT, FOREIGN KEY (fk) REFERENCES t1(s2))')
+box.schema.user.grant('guest','read','space', 'T3')
+v = nb.connect(box.cfg.listen)
+c:execute('INSERT INTO t3 VALUES (1, 1)')
+
+-- Cleanup
+box.schema.user.revoke('guest','read','space', 'V')
+box.schema.user.revoke('guest','read','space', 'T2')
+box.schema.user.revoke('guest','read','space', 'T3')
+
+box.sql.execute('DROP VIEW v')
+box.sql.execute('DROP TABLE t3')
+box.sql.execute('DROP TABLE t2')
+box.sql.execute("DROP TABLE t1")
-- 
2.19.1

      parent reply	other threads:[~2018-10-12 11:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 11:38 [tarantool-patches] [PATCH v2 0/2] " Kirill Yukhin
2018-10-12 11:38 ` [tarantool-patches] [PATCH v2 1/2] Add space names cache Kirill Yukhin
2018-10-15 10:08   ` Vladimir Davydov
2018-10-24 13:34     ` Kirill Yukhin
2018-10-12 11:38 ` Kirill Yukhin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9f7c0a46a7576c9a07e4e20b262d486adec4d975.1539344128.git.kyukhin@tarantool.org \
    --to=kyukhin@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v2 2/2] sql: check read access while executing SQL query' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox