List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:March 28 2011 11:51am
Subject:bzr push into mysql-trunk branch (Dmitry.Lenev:3532 to 3533) Bug#11746602
View as plain text  
 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).
Thread
bzr push into mysql-trunk branch (Dmitry.Lenev:3532 to 3533) Bug#11746602Dmitry Lenev28 Mar