maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

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
this bug:

diff --git a/storage/connect/ha_connect.cc b/storage/connect/ha_connect.cc
index c2fb648..0a024b0 100644
--- a/storage/connect/ha_connect.cc
+++ b/storage/connect/ha_connect.cc
@@ -4020,7 +4020,24 @@ bool ha_connect::check_privileges(THD *thd, PTOS options,
     case TAB_MAC:
     case TAB_WMI:
     case TAB_OEM:
-      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
     case TAB_TBL:

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:

  --connection default
  CREATE DEFINER=user@localhost SQL SECURITY DEFINER VIEW v1_baddefiner AS SELECT * FROM t1;
  --error ER_ACCESS_DENIED_ERROR
  SELECT * FROM v1_baddefiner;

See the commit mail with the complete patch.

Regards,
Sergei