From: Dmitry Lenev Date: March 28 2011 11:51am Subject: bzr push into mysql-trunk branch (Dmitry.Lenev:3532 to 3533) Bug#11746602 List-Archive: http://lists.mysql.com/commits/134038 X-Bug: 11746602 Message-Id: <20110328115153.A9AA67406A5@bandersnatch> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3533 Dmitry Lenev 2011-03-28 A follow-up for the patch for Bug#11746602 (27480: Extend CREATE TEMPORARY TABLES privilege to allow temp table operations). After main patch for this bug additional check for privileges required for SHOW statements might require opening and closing of temporary tables. Since doing this from within check_table_access() function looks like a dangerous thing, the current patch moves this additional check outside of this function. To support this change it also removes some duplicated code. @ sql/sql_parse.cc - Moved code responsible for the first stage of checking privileges for SELECT statement (global, db and table-level privileges) to a separate function - select_precheck(). Adjusted code for select-like statements to use this function. - Got rid of duplicate code handling SHOW EVENTS and SHOW PROCEDURE/FUNCTION STATUS. As a side-effect of this change now these statements reset last_query_cost status variable like most of other SHOW statements. - Moved code which performs additional check for privileges required to perform SHOW statement from check_table_access() function to newly created select_precheck() function. Doing this privilege check, which might require opening and closing of temporary tables, inside of check_table_access() looks like a dangerous thing. - Got rid of redundant code in check_show_access() which automatically granted SELECT_ACL on I_S table for SHOW statement. This is already done for all I_S tables in IS_internal_schema_access::check() member function. @ sql/sql_parse.h Introduced select_precheck() function which performs first stage of privilege checking for SELECT statements. @ sql/sql_prepare.cc Code responsible for the first stage of checking privileges for SELECT statements (global, db and table-level privileges) has been moved to new function select_precheck(). modified: sql/sql_parse.cc sql/sql_parse.h sql/sql_prepare.cc 3532 Dmitry Lenev 2011-03-26 Patch for Bug#11746602 (27480: Extend CREATE TEMPORARY TABLES privilege to allow temp table operations). The problem was that user with CREATE TEMPORARY TABLES privilege which was able to create temporary table, needed additional privileges which also applied to non-temporary tables to do any useful work on it. As result it was impossible to allow user to work with temporary tables without also granting extra privileges on normal tables. The idea of this patch is to allow any relevant operation on a temporary table which already exists. Creation of temporary table still requires CREATE TEMPORARY TABLES privilege on database in which this table to be created. @ mysql-test/include/handler.inc Adjusted test case to the fact that now HANDLER OPEN pre-opens temporary tables first and only after that checks if it is called under LOCK TABLES. @ mysql-test/r/flush_read_lock.result Adjusted test case to the fact that DROP TABLE no longer acquires metadata locks if it is going to drop temporary table and thus doesn't conflict with global read lock in this case. @ mysql-test/r/grant2.result Added test coverage for privilege handling for temporary tables (i.e. for bug#11746602 "27480: Extend CREATE TEMPORARY TABLES privilege to allow temp table operations"). @ mysql-test/r/handler_innodb.result Adjusted test case to the fact that now HANDLER OPEN pre-opens temporary tables first and only after that checks if it is called under LOCK TABLES. @ mysql-test/r/handler_myisam.result Adjusted test case to the fact that now HANDLER OPEN pre-opens temporary tables first and only after that checks if it is called under LOCK TABLES. @ mysql-test/r/merge.result Since ALTER TABLE now pre-opens temporary table for UNION clause, ALTER TABLE temp_table UNION=(..., temp_table) is no longer legal. Adjusted test case to avoid using this construct. @ mysql-test/t/flush_read_lock.test Adjusted test case to the fact that DROP TABLE no longer acquires metadata locks if it is going to drop temporary table and thus doesn't conflict with global read lock in this case. @ mysql-test/t/grant2.test Added test coverage for privilege handling for temporary tables (i.e. for bug#11746602 "27480: Extend CREATE TEMPORARY TABLES privilege to allow temp table operations"). @ mysql-test/t/merge.test Since ALTER TABLE now pre-opens temporary table for UNION clause, ALTER TABLE temp_table UNION=(..., temp_table) is no longer legal. Adjusted test case to avoid using this construct. @ sql/sql_acl.cc Changed check_grant() to skip its and further privilege checks on pre-opened temporary tables. This is achieved by automatically adding all relevant table-level privileges to the set of granted privileges for every temporary table. @ sql/sql_acl.h Added define for set of table-level privileges which are relevant for temporary tables. @ sql/sql_alter.cc Changed code checking privileges for tables from UNION clause of ALTER TABLE. Now we pre-open temporary tables for this clause in order properly check privileges for them (or rather skip privilege checking). @ sql/sql_base.cc Now drop_temporary_table() assumes that temporary table to be dropped was pre-opened using table list element which it gets as parameter. Adjusted code accordingly. @ sql/sql_base.h Adjusted is_temporary_table() function to ignore views and schema tables so it can properly differentiate real temporary tables from ones used in view and information schema implementation. @ sql/sql_handler.cc Removed comment which is no longer relevant. @ sql/sql_insert.cc Since drop_temporary_table() function now expects that table to be dropped is pre-opened we can no longer use it to drop temporary table which we have created but failed to open. Replaced call to this function with assert since such situation should not be possible anyway. @ sql/sql_parse.cc - Now we pre-open temporary tables for DROP TABLE and HANDLER OPEN statements. This allows correctly handle (i.e. ignore) privilege checks in cases when we are going to drop or open handler for temporary table. As a consequence DROP TABLE now needs to close open handlers for tables to be dropped before this pre-opening happens. - Moved handling of privilege checks for UNION clause of CREATE TABLE statement inside of create_table_precheck() function. Adjusted the latter to correctly handle privilege checking in cases when UNION clause contains temporary tables. Also we no longer require INSERT privilege for CREATE TEMPORARY TABLE ... SELECT statement. It is enough to have CREATE TEMPORARY privilege for it. - Introduced lock_tables_precheck() function which allows to skip temporary tables when checking privileges for LOCK TABLES statement (we can't handle LOCK TABLES privilege in the same way as other privileges for temporary tables as it is a db-level privilege). - Changed code in check_show_access() to take into account temporary tables when checking privileges for SHOW FIELDS and SHOW KEYS statements. - Adjusted code in multi_delete_precheck() to update elements of auxiliary table list from corresponding elements of main table list. This allows to correctly distinguish temporary tables when auxiliary table list is used for privilege checking. @ sql/sql_prepare.cc Checking of privileges on tables from UNION clause of CREATE TABLE statement has been moved into create_table_precheck() function. @ sql/sql_table.cc Changed DROP TABLES implementation to rely on the fact that temporary tables to be dropped are pre-opened. Pre-opening has to be done in any case in order to correctly skip those tables during privilege check for this statement. modified: mysql-test/include/handler.inc mysql-test/r/flush_read_lock.result mysql-test/r/grant2.result mysql-test/r/handler_innodb.result mysql-test/r/handler_myisam.result mysql-test/r/merge.result mysql-test/t/flush_read_lock.test mysql-test/t/grant2.test mysql-test/t/merge.test sql/sql_acl.cc sql/sql_acl.h sql/sql_alter.cc sql/sql_base.cc sql/sql_base.h sql/sql_handler.cc sql/sql_insert.cc sql/sql_parse.cc sql/sql_prepare.cc sql/sql_table.cc === modified file 'sql/sql_parse.cc' --- a/sql/sql_parse.cc 2011-03-26 10:56:27 +0000 +++ b/sql/sql_parse.cc 2011-03-28 11:49:45 +0000 @@ -113,6 +113,7 @@ "FUNCTION" : "PROCEDURE") static bool execute_sqlcom_select(THD *thd, TABLE_LIST *all_tables); +static bool check_show_access(THD *thd, TABLE_LIST *table); static void sql_kill(THD *thd, ulong id, bool only_kill_query); static bool lock_tables_precheck(THD *thd, TABLE_LIST *tables); @@ -2109,25 +2110,14 @@ mysql_execute_command(THD *thd) switch (lex->sql_command) { - case SQLCOM_SHOW_EVENTS: -#ifndef HAVE_EVENT_SCHEDULER - my_error(ER_NOT_SUPPORTED_YET, MYF(0), "embedded server"); - break; -#endif - case SQLCOM_SHOW_STATUS_PROC: - case SQLCOM_SHOW_STATUS_FUNC: - if ((res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, - UINT_MAX, FALSE))) - goto error; - res= execute_sqlcom_select(thd, all_tables); - break; case SQLCOM_SHOW_STATUS: { system_status_var old_status_var= thd->status_var; thd->initial_status_var= &old_status_var; - if (!(res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, - UINT_MAX, FALSE))) + + if (!(res= select_precheck(thd, lex, all_tables, first_table))) res= execute_sqlcom_select(thd, all_tables); + /* Don't log SHOW STATUS commands to slow query log */ thd->server_status&= ~(SERVER_QUERY_NO_INDEX_USED | SERVER_QUERY_NO_GOOD_INDEX_USED); @@ -2142,6 +2132,13 @@ mysql_execute_command(THD *thd) mysql_mutex_unlock(&LOCK_status); break; } + case SQLCOM_SHOW_EVENTS: +#ifndef HAVE_EVENT_SCHEDULER + my_error(ER_NOT_SUPPORTED_YET, MYF(0), "embedded server"); + break; +#endif + case SQLCOM_SHOW_STATUS_PROC: + case SQLCOM_SHOW_STATUS_FUNC: case SQLCOM_SHOW_DATABASES: case SQLCOM_SHOW_TABLES: case SQLCOM_SHOW_TRIGGERS: @@ -2159,21 +2156,7 @@ mysql_execute_command(THD *thd) { thd->status_var.last_query_cost= 0.0; - /* - lex->exchange != NULL implies SELECT .. INTO OUTFILE and this - requires FILE_ACL access. - */ - ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : - SELECT_ACL; - - if (all_tables) - res= check_table_access(thd, - privileges_requested, - all_tables, FALSE, UINT_MAX, FALSE); - else - res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0); - - if (res) + if ((res= select_precheck(thd, lex, all_tables, first_table))) break; res= execute_sqlcom_select(thd, all_tables); @@ -4937,20 +4920,19 @@ check_access(THD *thd, ulong want_access } +/** + Check if user has enough privileges for execution of SHOW statement, + which was converted to query to one of I_S tables. + + @param thd Thread context. + @param table Table list element for I_S table to be queried.. + + @retval FALSE - Success. + @retval TRUE - Failure. +*/ + static bool check_show_access(THD *thd, TABLE_LIST *table) { - /* - This is a SHOW command using an INFORMATION_SCHEMA table. - check_access() has not been called for 'table', - and SELECT is currently always granted on the I_S, so we automatically - grant SELECT on table here, to bypass a call to check_access(). - Note that not calling check_access(table) is an optimization, - which needs to be revisited if the INFORMATION_SCHEMA does - not always automatically grant SELECT but use the grant tables. - See Bug#38837 need a way to disable information_schema for security - */ - table->grant.privilege= SELECT_ACL; - switch (get_schema_table_idx(table->schema_table)) { case SCH_SCHEMATA: return (specialflag & SPECIAL_SKIP_SHOW_DB) && @@ -5088,12 +5070,16 @@ check_table_access(THD *thd, ulong requi */ tables->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); - if (tables->schema_table_reformed) - { - if (check_show_access(thd, tables)) - goto deny; - continue; - } + /* + We should not encounter table list elements for reformed SHOW + statements unless this is first table list element in the main + select. + Such table list elements require additional privilege check + (see check_show_access()). This check is carried out by caller, + but only for the first table list element from the main select. + */ + DBUG_ASSERT(!tables->schema_table_reformed || + tables == thd->lex->select_lex.table_list.first); DBUG_PRINT("info", ("derived: %d view: %d", tables->derived != 0, tables->view != 0)); @@ -5223,6 +5209,13 @@ bool check_some_access(THD *thd, ulong w DBUG_RETURN(1); } +#else + +static bool check_show_access(THD *thd, TABLE_LIST *table) +{ + return false; +} + #endif /*NO_EMBEDDED_ACCESS_CHECKS*/ @@ -6672,6 +6665,45 @@ Item * all_any_subquery_creator(Item *le /** + Perform first stage of privilege checking for SELECT statement. + + @param thd Thread context. + @param lex LEX for SELECT statement. + @param tables List of tables used by statement. + @param first_table First table in the main SELECT of the SELECT + statement. + + @retval FALSE - Success (column-level privilege checks might be required). + @retval TRUE - Failure, privileges are insufficient. +*/ + +bool select_precheck(THD *thd, LEX *lex, TABLE_LIST *tables, + TABLE_LIST *first_table) +{ + bool res; + /* + lex->exchange != NULL implies SELECT .. INTO OUTFILE and this + requires FILE_ACL access. + */ + ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : + SELECT_ACL; + + if (tables) + { + res= check_table_access(thd, + privileges_requested, + tables, FALSE, UINT_MAX, FALSE) || + (first_table && first_table->schema_table_reformed && + check_show_access(thd, first_table)); + } + else + res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0); + + return res; +} + + +/** Multi update query pre-check. @param thd Thread handler === modified file 'sql/sql_parse.h' --- a/sql/sql_parse.h 2011-02-18 09:56:51 +0000 +++ b/sql/sql_parse.h 2011-03-28 11:49:45 +0000 @@ -35,6 +35,8 @@ enum enum_mysql_completiontype { extern "C" int test_if_data_home_dir(const char *dir); +bool select_precheck(THD *thd, LEX *lex, TABLE_LIST *tables, + TABLE_LIST *first_table); bool multi_update_precheck(THD *thd, TABLE_LIST *tables); bool multi_delete_precheck(THD *thd, TABLE_LIST *tables); int mysql_multi_update_prepare(THD *thd); === modified file 'sql/sql_prepare.cc' --- a/sql/sql_prepare.cc 2011-03-26 10:56:27 +0000 +++ b/sql/sql_prepare.cc 2011-03-28 11:49:45 +0000 @@ -1457,13 +1457,7 @@ static int mysql_test_select(Prepared_st lex->select_lex.context.resolve_in_select_list= TRUE; - ulong privilege= lex->exchange ? SELECT_ACL | FILE_ACL : SELECT_ACL; - if (tables) - { - if (check_table_access(thd, privilege, tables, FALSE, UINT_MAX, FALSE)) - goto error; - } - else if (check_access(thd, privilege, any_db, NULL, NULL, 0, 0)) + if (select_precheck(thd, lex, tables, lex->select_lex.table_list.first)) goto error; if (!lex->result && !(lex->result= new (stmt->mem_root) select_send)) No bundle (reason: useless for push emails).