[tarantool-patches] Re: [PATCH] sql: check read access while executing SQL query

  • From: Kirill Yukhin <kyukhin@xxxxxxxxxxxxx>
  • To: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • Date: Fri, 12 Oct 2018 14:41:09 +0300

Hello Nikita,
Thanks a lot for review! My answers inlined.
Re-factored patchset send as new thread. Branch force-pushed.

On 11 Oct 14:32, n.pettik wrote:


Since SQL front-end is not using box API,
no checkes for read access are performed by VDBE engine.
Fix this by introducing dedicated opcode which is executed

But in your patch I don’t see any new opcodes.
Fixed comment.

to perform read access check.
Note, that there's is no need to perform DML/DDL checkes as
they're performed by Tarantool's core.

Closes #2362

Please, provide doc-bot request (especially taking into consideration
the fact that ‘read’ privilege turns out to be not real ’select’ privilege).
Done.

+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', '_space’)

Hmm, why do you need to grand read privilege to _space?
Without it test fails:

error: 'Failed to execute SQL statement: no such table: T1’

I guess somewhere in SQL internals invocation to _space appears to fetch 
space,
but , it is not obvious. Can you come up with any workaround to avoid it?

Another problem is:

box.schema.user.revoke('guest’,'read', 'space', 'T1')
box.schema.user.grant('guest’,’write', 'space', 'T1')
c:execute("delete from t1 where s1 = 1;”) 
---
- error: 'Failed to execute SQL statement: Read access to space ''T1'' is 
denied for
    user ''guest'''
...

In terms of Tarantool, it is OK (since we need to iterate through the space 
to find
tuple to be deleted). But for SQL I guess it is not acceptable: this query 
doesn’t
give user any data, so read (i.e. SELECT) privilege is not needed.
The same for example involving FK constraints which you provided below.

I've refactored space pointer retrival mechanism. It is now used space name ->
pointer hash table w/o any access checks. Actual checks are only performed when
iterator op-code is executed.

--
Regards, Kirill Yukhin

Other related posts: