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

[Kirill Yukhin]

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