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 Jul 24, Alexander Barkov wrote:
> >> 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,
> > please?
> Let me try to explain. The idea is very simple, really.
> Does this explanation help?
Yes, thanks. But it still affects the rather hot execution path -
opening of a table - for every user and every table. While it is only
needed when a user opens a CONNECT table via a view with SQL SECURITY
DEFINER. See? In the absolute majority of cases, practically all of
them, your patch only adds an overhead.
Instead, I suggest another patch - this is based on your first fix for
diff --git a/storage/connect/ha_connect.cc b/storage/connect/ha_connect.cc
index c2fb648..0a024b0 100644
@@ -4020,7 +4020,24 @@ bool ha_connect::check_privileges(THD *thd, PTOS options,
- return check_access(thd, FILE_ACL, db, NULL, NULL, 0, 0);
+ If table or table->mdl_ticket is NULL - it's a DLL, e.g. CREATE TABLE.
+ if the table has an MDL_EXCLUSIVE lock - it's a DDL too, e.g. the
+ insert step of CREATE ... SELECT.
+ Otherwise it's a DML, the table was normally opened, locked,
+ privilege were already checked, and table->grant.privilege is set.
+ With SQL SECURITY DEFINER, table->grant.privilege has definer's privileges.
+ if (!table || !table->mdl_ticket || table->mdl_ticket->get_type() == MDL_EXCLUSIVE)
+ return check_access(thd, FILE_ACL, db, NULL, NULL, 0, 0);
+ if (table->grant.privilege & FILE_ACL)
+ return false;
+ return true;
// This is temporary until a solution is found
It passes your test case. In fact, your first fix passes it too :)
This one also passes the additional test I've added - where a user can
access the table, but view's definer cannot:
CREATE DEFINER=user@localhost SQL SECURITY DEFINER VIEW v1_baddefiner AS SELECT * FROM t1;
SELECT * FROM v1_baddefiner;
See the commit mail with the complete patch.