Hi, Alexander! On Feb 17, Alexander Barkov wrote:
From what I understood, FILE_ACL is written (among the other privileges) into thd->security_ctx.privilege in TABLE_LIST::prepare_security(). In case of a DEFINER view, thd->security_ctx.privilege is filled exactly with the definer privileges, and to the invoker privileges otherwise.
So inside ha_connect::check_privileges() the fact that there is FILE_ACL in thd->security_ctx.privilege means that TABLE_LIST::prepare_security() was previously called and FILE_ACL is set to DEFINER or INVOKER, according to the view definition. This is exactly what we need.
Agree, looks good so far :)
I'm not sure about the opposite: if there is no FILE_ACL in thd->security_ctx.privilege, what does it mean? Does it mean that there is no FILE_ACL for the effective user? Or can it also mean that TABLE_LIST::prepare_security() was not called?
As far as I can see, TABLE_LIST::prepare_security() is always called for a view that was successfully opened. That is, no FILE_ACL bit means that there is no FILE privilege.
=== modified file 'storage/connect/ha_connect.cc' --- storage/connect/ha_connect.cc 2015-01-20 00:21:56 +0000 +++ storage/connect/ha_connect.cc 2015-02-17 13:46:34 +0000 @@ -3865,6 +3865,8 @@ bool ha_connect::check_privileges(THD *t case TAB_MAC: case TAB_WMI: case TAB_OEM: + if (table && (table->grant.privilege & FILE_ACL)) + return false; return check_access(thd, FILE_ACL, db, NULL, NULL, 0, 0);
I don't like that you use both approaches to check the privileges. This just looks wrong. It should rather be * if it's called from ::external_lock() - use table->grant.privilege. * otherwise (::create() or ::delete_or_rename_table()) - don't use table->grant.privilege, only use check_access(). Something like if (called_from_external_lock) return table->grant.privilege & FILE_ACL; // respect view's definer else return check_access(thd, FILE_ACL, db, NULL, NULL, 0, 0); Regards, Sergei