List:Commits« Previous MessageNext Message »
From:mhansson Date:April 17 2008 3:13pm
Subject:bk commit into 6.0 tree (mhansson:1.2636) BUG#36086
View as plain text  
Below is the list of changes that have just been committed into a local
6.0 repository of mhansson.  When mhansson does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-17 17:12:28+02:00, mhansson@riffraff.(none) +5 -0
  Bug#36086: SELECT * from views don't check column grants
  
  A "SELECT *" against an ALGORITHM=TEMPTABLE view was 
  wrongfully treated as derived table (i.e. access checking
  was skipped). Fixed by introducing a predicate to tell
  the difference.

  mysql-test/r/view_grant.result@stripped, 2008-04-17 17:12:26+02:00, mhansson@riffraff.(none) +60 -0
    Bug36086: Test case

  mysql-test/t/view_grant.test@stripped, 2008-04-17 17:12:26+02:00, mhansson@riffraff.(none) +85 -0
    Bug36086: Test result

  sql/sql_acl.cc@stripped, 2008-04-17 17:12:26+02:00, mhansson@riffraff.(none) +1 -1
    Bug#36086: Updated comment. This function was previously
    not called for views.

  sql/sql_derived.cc@stripped, 2008-04-17 17:12:26+02:00, mhansson@riffraff.(none) +37 -23
    Bug#36086: 
    - changed comment to Doxygen standard and added content.
    - The fix. We must not give the appearance that 
    the user has fulfilled the SELECT privilige on a view just
    because the view is implemented as a temporary table. For 
    derived tables, however, we should.

  sql/table.h@stripped, 2008-04-17 17:12:26+02:00, mhansson@riffraff.(none) +64 -1
    Bug#36086: 
    
    - Commented the GRANT_INFO structure and members.
    - Added predicate to be able to explicitly tell when a
      TABLE_LIST represents a derived table. Not knowing this 
      has been the source of numerous bugs.

diff -Nrup a/mysql-test/r/view_grant.result b/mysql-test/r/view_grant.result
--- a/mysql-test/r/view_grant.result	2008-03-22 09:01:30 +01:00
+++ b/mysql-test/r/view_grant.result	2008-04-17 17:12:26 +02:00
@@ -957,3 +957,63 @@ Warning	1356	View 'test.v1' references i
 DROP VIEW v1;
 DROP TABLE t1;
 End of 5.1 tests.
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+GRANT USAGE, SELECT, CREATE VIEW, SHOW VIEW
+ON mysqltest2.* 
+TO mysqluser1@localhost;
+USE mysqltest1;
+CREATE TABLE t1 ( a INT, b INT );
+CREATE TABLE t2 ( a INT, b INT );
+CREATE VIEW v1 AS SELECT 1 AS a, 2 AS b;
+CREATE VIEW v2 AS SELECT 1 AS a, 2 AS b;
+GRANT SELECT( a ), SELECT( b ) ON t1 TO mysqluser1@localhost;
+GRANT INSERT( a ), INSERT( b ) ON t2 TO mysqluser1@localhost;
+GRANT SELECT( a ), SELECT( b ) ON v1 TO mysqluser1@localhost;
+GRANT INSERT( a ), INSERT( b ) ON v2 TO mysqluser1@localhost;
+SELECT a, b FROM mysqltest1.t1;
+a	b
+SELECT * FROM mysqltest1.t1;
+a	b
+SELECT a, b FROM mysqltest1.t2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 't2'
+SELECT * FROM mysqltest1.t2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 't2'
+SELECT a, b FROM mysqltest1.v1;
+a	b
+1	2
+SELECT * FROM mysqltest1.v1;
+a	b
+1	2
+SELECT a, b FROM mysqltest1.v2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2'
+SELECT * FROM mysqltest1.v2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2'
+REVOKE SELECT( a ) ON t1 FROM mysqluser1@localhost;
+REVOKE INSERT( a ) ON t2 FROM mysqluser1@localhost;
+REVOKE SELECT( a ) ON v1 FROM mysqluser1@localhost;
+REVOKE INSERT( a ) ON v2 FROM mysqluser1@localhost;
+SELECT a, b FROM mysqltest1.t1;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 't1'
+SELECT * FROM mysqltest1.t1;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 't1'
+SELECT a, b FROM mysqltest1.t2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 't2'
+SELECT * FROM mysqltest1.t2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 't2'
+SELECT a, b FROM mysqltest1.v2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2'
+SELECT * FROM mysqltest1.v2;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for table 'v2'
+SELECT a, b FROM mysqltest1.v1;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 'v1'
+SELECT * FROM mysqltest1.v1;
+ERROR 42000: SELECT command denied to user 'mysqluser1'@'localhost' for column 'a' in table 'v1'
+SELECT * FROM (SELECT 1 AS a) b JOIN (SELECT 2 AS a) c USING(a);
+a
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+DROP USER mysqluser1@localhost;
diff -Nrup a/mysql-test/t/view_grant.test b/mysql-test/t/view_grant.test
--- a/mysql-test/t/view_grant.test	2008-02-21 10:22:18 +01:00
+++ b/mysql-test/t/view_grant.test	2008-04-17 17:12:26 +02:00
@@ -1219,3 +1219,88 @@ DROP VIEW v1;
 DROP TABLE t1;
 
 --echo End of 5.1 tests.
+
+#
+# SELECT * from views don't check column grants
+#
+CREATE USER mysqluser1@localhost;
+CREATE DATABASE mysqltest1;
+CREATE DATABASE mysqltest2;
+GRANT USAGE, SELECT, CREATE VIEW, SHOW VIEW
+ON mysqltest2.* 
+TO mysqluser1@localhost;
+
+USE mysqltest1;
+
+CREATE TABLE t1 ( a INT, b INT );
+CREATE TABLE t2 ( a INT, b INT );
+
+CREATE VIEW v1 AS SELECT 1 AS a, 2 AS b;
+CREATE VIEW v2 AS SELECT 1 AS a, 2 AS b;
+
+GRANT SELECT( a ), SELECT( b ) ON t1 TO mysqluser1@localhost;
+GRANT INSERT( a ), INSERT( b ) ON t2 TO mysqluser1@localhost;
+
+GRANT SELECT( a ), SELECT( b ) ON v1 TO mysqluser1@localhost;
+GRANT INSERT( a ), INSERT( b ) ON v2 TO mysqluser1@localhost;
+
+--connect (connection1, localhost, mysqluser1, , mysqltest2)
+
+SELECT a, b FROM mysqltest1.t1;
+SELECT * FROM mysqltest1.t1;
+--error 1142
+SELECT a, b FROM mysqltest1.t2;
+--error 1142
+SELECT * FROM mysqltest1.t2;
+SELECT a, b FROM mysqltest1.v1;
+SELECT * FROM mysqltest1.v1;
+--error 1142
+SELECT a, b FROM mysqltest1.v2;
+--error 1142
+SELECT * FROM mysqltest1.v2;
+
+--connection default
+
+REVOKE SELECT( a ) ON t1 FROM mysqluser1@localhost;
+REVOKE INSERT( a ) ON t2 FROM mysqluser1@localhost;
+
+REVOKE SELECT( a ) ON v1 FROM mysqluser1@localhost;
+REVOKE INSERT( a ) ON v2 FROM mysqluser1@localhost;
+
+--connection connection1
+
+# Works prior to fix
+--error 1143
+SELECT a, b FROM mysqltest1.t1;
+# Works prior to fix
+--error 1143
+SELECT * FROM mysqltest1.t1;
+
+# Works prior to fix
+--error 1142
+SELECT a, b FROM mysqltest1.t2;
+# Works prior to fix
+--error 1142
+SELECT * FROM mysqltest1.t2;
+
+# Works prior to fix
+--error 1142
+SELECT a, b FROM mysqltest1.v2;
+# Works prior to fix
+--error 1142
+SELECT * FROM mysqltest1.v2;
+# Works prior to fix
+--error 1143
+SELECT a, b FROM mysqltest1.v1;
+--error 1143
+SELECT * FROM mysqltest1.v1;
+
+--connection default
+
+SELECT * FROM (SELECT 1 AS a) b JOIN (SELECT 2 AS a) c USING(a);
+
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;
+DROP DATABASE mysqltest1;
+DROP DATABASE mysqltest2;
+DROP USER mysqluser1@localhost;
diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
--- a/sql/sql_acl.cc	2008-04-07 15:51:24 +02:00
+++ b/sql/sql_acl.cc	2008-04-17 17:12:26 +02:00
@@ -4114,7 +4114,7 @@ bool check_column_grant_in_table_ref(THD
     @retval 1 Falure
   @details This function walks over the columns of a table reference 
    The columns may originate from different tables, depending on the kind of
-   table reference, e.g. join.
+   table reference, e.g. join, view.
    For each table it will retrieve the grant information and will use it
    to check the required access privileges for the fields requested from it.
 */    
diff -Nrup a/sql/sql_derived.cc b/sql/sql_derived.cc
--- a/sql/sql_derived.cc	2007-12-12 16:40:35 +01:00
+++ b/sql/sql_derived.cc	2008-04-17 17:12:26 +02:00
@@ -73,29 +73,43 @@ out:
 }
 
 
-/*
-  Create temporary table structure (but do not fill it)
+/**
+   @brief Create temporary table structure (but do not fill it).
 
-  SYNOPSIS
-    mysql_derived_prepare()
-    thd			Thread handle
-    lex                 LEX for this thread
-    orig_table_list     TABLE_LIST for the upper SELECT
-
-  IMPLEMENTATION
-    Derived table is resolved with temporary table.
-
-    After table creation, the above TABLE_LIST is updated with a new table.
-
-    This function is called before any command containing derived table
-    is executed.
-
-    Derived tables is stored in thd->derived_tables and freed in
-    close_thread_tables()
-
-  RETURN
-    FALSE  OK
-    TRUE   Error
+   @param thd Thread handle
+   @param lex LEX for this thread
+   @param orig_table_list TABLE_LIST for the upper SELECT
+
+   @details The table reference (currently this function is used for derived
+   tables @e or @e views with the @c TEMPTABLE algorithm) is updated with the
+   fields of a temporary table. After table creation, the TABLE_LIST
+   references the new table. This function is called before any command
+   containing derived tables is executed. Derived tables are stored in
+   thd->derived_tables and freed in close_thread_tables().
+
+   This function is part of a procedure that introduces new table and table
+   reference objects that don't exist in the privilege database, meaning that
+   normal privilege checking cannot handle it. So it does some extra tricks to
+   bypass normal privilege checking by exploiting the fact that the state of
+   privilege verification is 'cached' on the relevant TABLE and TABLE_REF
+   objects. It just so happens that code relying on this
+   (e.g. insert_fields()) does not bother to check if that cache is invalid.
+
+   For ordinary table references, the current state of access checking is
+   cached in TABLE_LIST::grant, so this function updates the state of
+   fulfilled privileges in this case, depending on whether it is modifying a
+   view or a derived table.
+
+   - If a derived table is referenced, the SELECT privilege is set as
+     fulfilled by the user.
+
+   - If a view is referenced and the table reference is queried against
+     directly, the state of privilege checking (GRANT_INFO struct) is copied
+     as-is to the temporary table.
+
+  @return
+    false  OK
+    true   Error
 */
 
 bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *orig_table_list)
@@ -185,7 +199,7 @@ exit:
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
       if (orig_table_list->referencing_view)
         table->grant= orig_table_list->grant;
-      else
+      else if (orig_table_list->is_derived_table())
         table->grant.privilege= SELECT_ACL;
 #endif
       orig_table_list->db= (char *)"";
diff -Nrup a/sql/table.h b/sql/table.h
--- a/sql/table.h	2008-04-01 15:44:55 +02:00
+++ b/sql/table.h	2008-04-17 17:12:26 +02:00
@@ -66,13 +66,65 @@ typedef struct st_order {
   table_map used, depend_map;
 } ORDER;
 
+/**
+   @brief A snapshot of the privilege checking process for the current user
+   and SQL statement.
+
+   @details The privilege checking process is not localized to one phase, it
+   is rather spread out over different areas of the server depending on the
+   privilege to be checked and the type of object to be accessed.
+
+   Database and table level privileges are normally checked by the
+   check_XXX_access() family of functions found in sql_parse.cc. Calls end up
+   in check_access() which checks a database-level "wanted access" (set of
+   privileges) against a required set of privileges. 
+
+   If check_access() does not find the wanted access to be fulfilled,
+   check_grant() is asked to check if the table-level privileges are
+   sufficient to allow the user to execute the SQL statement.
+
+   Column-level privileges are normally checked during the execution of
+   setup_fields().
+
+   Due to the distributed nature of privilege checking, it is necessary to
+   keep track of the state of the process.
+
+   @note There are many exceptions to the above mentioned access checking
+   process.
+ */
 typedef struct st_grant_info
 {
   GRANT_TABLE *grant_table;
+  /**
+     @brief Used for cache invalidation when caching privileges in table
+     objects (TABLE and TABLE_LIST).
+
+     @detatils The privilege information is stored in dedicated databases
+     (hash tables); table-level and column-level privileges respectively have
+     their own dedicated databases. A copy of the GRANT_INFO is also attached
+     to TABLE and TABLE_LIST (table reference) objects as a kind of cache. In
+     order to keep track of when the cache becomes invalid, a global variable
+     grant_version in sql_acl.cc is used.
+   */
   uint version;
+  /**
+     @brief the set of privileges that the current user has fulfilled for the
+     object that this GRANT_INFO belongs to.
+     
+     @details This field is continually updated throughout the access checking
+     process. In each step the "wanted privilege" is checked against the
+     fulfilled privileges. When the intersection of these sets is empty,
+     access is granted.
+
+     The set is implemented as a bitmap, with the bits defined in sql_acl.h.
+   */
   ulong privilege;
+  /**
+     @brief the set of privileges that the current user needs to fulfil in
+     order to carry out some requested operation.
+   */
   ulong want_privilege;
-  /*
+  /**
     Stores the requested access acl of top level tables list. Is used to
     check access rights to the underlying tables of a view.
   */
@@ -1260,6 +1312,17 @@ struct TABLE_LIST
   {
     child_def_version= ~0UL;
   }
+
+  /**
+     @brief If true, this @c TABLE_LIST represents a derived table.
+     
+     @note By derived table, we mean the result of a nested query.
+
+     @bug This function currently identifies only nested queries in the @c
+     FROM clause correctly. Nested queries in @c SELECT or @c WHERE clauses
+     are not recognized.
+  */
+  bool is_derived_table() { return derived != NULL && view == NULL; }
 
 private:
   bool prep_check_option(THD *thd, uint8 check_opt_type);
Thread
bk commit into 6.0 tree (mhansson:1.2636) BUG#36086mhansson17 Apr