maria-developers team mailing list archive
Mailing list archive
Re: PLEASE REVIEW: (MDEV-7574) Security definer views don't work with CONNECT ODBC tables
On 07/24/2015 10:55 PM, Sergei Golubchik wrote:
On Jul 24, Alexander Barkov wrote:
Sorry for delay, I was busy with 10.1 issues.
That's perfectly fine. And even good - there's little sense to spend
time on 10.0 bugs when the next release in line is 10.1.
Thanks for review. A new patch is attached.
This is a major rewrite since last time.
I think the code now looks much easier to understand.
Not really, I couldn't understand what you're trying to do this time :(
Besides it's 32K of changes in 10.0-GA, and for what? Corner case
CONNECT issue. Could you try to come up with a less intrusive solution,
Let me try to explain. The idea is very simple, really.
Calling check_access() inside external_lock() is a bad idea:
- it's simply not correct in case of VIEW or SP,
because it checks privileges for the current INVOKER
and ignores the "SQL SECURITY DEFINER" clause.
- and by the way, it's slow, and if it's already done in sql_oarse.cc,
why call it for the second time? It's nice to reuse the value
returned in the first call of check_access().
The correct privilege value that takes into account "SQL SECURITY
DEFINER" becomes known in sql_parse.cc, in mysql_execute_command()
and its callees, like insert_precheck() etc.
So I'm trying to make this correct privilege value be consistently
available in external_lock() for all SQLCOM_XXXX commands by
assigning it to table->grant.privilege.
Before the patch, table->grant.privilege is just set to 0 in many cases
and therefore is useless in external_lock().
In order to initialize table->grant.privilege properly in all cases,
I added a new parameter "ulong privilege" to these functions:
The idea is to make the correct privilege value available at the point
where a TABLE instance is initialized and assign this value to
table->grant.privilege instead of 0.
Does this explanation help?
The patch looks quite safe for 10.0 for me:
It does not seem to affect the other code, except ConnectSE.
It should not be harmful to have the real privilege value instead of 0
in table->grant.privilege. Or, can it be harmful somehow?