List:Commits« Previous MessageNext Message »
From:mhansson Date:April 21 2008 1:46pm
Subject:bk commit into 6.0 tree (mhansson:1.2621) 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-21 15:45:48+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. (From SQL:2003 std sect. 4.3: "The result of a 
  query is called a derived table"). Fixed by introducing a 
  predicate to tell the difference.

  mysql-test/r/view_grant.result@stripped, 2008-04-21 15:45:46+02:00, mhansson@riffraff.(none) +60 -0
    Bug36086: Test case

  mysql-test/t/view_grant.test@stripped, 2008-04-21 15:45:46+02:00, mhansson@riffraff.(none) +85 -0
    Bug36086: Test result

  sql/sql_acl.cc@stripped, 2008-04-21 15:45:46+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-21 15:45:46+02:00, mhansson@riffraff.(none) +52 -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-21 15:45:46+02:00, mhansson@riffraff.(none) +52 -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 in the from clause.
      Currently, this is the only case that mysql_derived_prepare
      has to handle.

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:02:23 +01:00
+++ b/mysql-test/r/view_grant.result	2008-04-21 15:45:46 +02:00
@@ -948,3 +948,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-03-04 18:35:39 +01:00
+++ b/mysql-test/t/view_grant.test	2008-04-21 15:45:46 +02:00
@@ -1216,3 +1216,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-14 13:30:04 +02:00
+++ b/sql/sql_acl.cc	2008-04-21 15:45:46 +02:00
@@ -4224,7 +4224,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-21 15:45:46 +02:00
@@ -73,29 +73,58 @@ 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 
+
+   This function is called before any command containing derived tables is
+   executed.  Currently the function is used for
+
+   - derived tables, or 
+   - views with the @c TEMPTABLE algorithm.
+    
+   The table reference, contained in the TABLE_LIST, is updated with the
+   fields of a new temporary table.
+
+   Derived tables are stored in thd->derived_tables and closed by
+   close_thread_tables().
+
+   This function is part of the procedure that starts in
+   open_and_lock_tables(), a procedure that - among many other things -
+   introduces new table and table reference objects (to represent views or
+   derived tables) that don't exist in the privilege database. This means that
+   normal privilege checking cannot handle them. Hence this function does some
+   extra tricks in order to bypass normal privilege checking, by exploiting
+   the fact that the current state of privilege verification is attached on
+   the relevant TABLE and TABLE_REF objects.
+
+   For table references, the current state of accrued access is stored inside
+   TABLE_LIST::grant. Hence this function must update the state of fulfilled
+   privileges for the new TABLE_LIST, an operation which is normally performed
+   exclusively by the table and database access checking functions,
+   check_access() and check_grant(), respectively. This modifification is done
+   differently depending on whether a view or a derived table is modified.
+
+   - 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.
+
+  This function implements a signature called "derived table processor", and
+  is passed as a function pointer to mysql_handle_derived().
+
+  @see mysql_handle_derived(), mysql_derived_filling(), GRANT_INFO
+
+  @return
+    false  OK
+    true   Error
 */
 
 bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *orig_table_list)
@@ -185,7 +214,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_in_from_clause())
         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-13 10:17:01 +02:00
+++ b/sql/table.h	2008-04-21 15:45:46 +02:00
@@ -66,13 +66,54 @@ typedef struct st_order {
   table_map used, depend_map;
 } ORDER;
 
+/**
+   @brief The current state of the privilege checking process for the current
+   user, SQL statement and SQL object.
+
+   @details The privilege checking process is divided into phases depending on
+   the level of the privilege to be checked and the type of object to be
+   accessed. Due to the distributed nature of privilege checking, it is
+   necessary to keep track of the state of the process. This information is
+   stored in privilege, want_privilege, and orig_want_privilege.
+
+   A GRANT_INFO also serves as a local repository of the privilege
+   database. Relevant members are grant_table and version.
+ */
 typedef struct st_grant_info
 {
+  /**
+     @brief A copy of the privilege information regarding the current host,
+     database, object and user.
+
+     @details The version of this copy is found in GRANT_INFO::version.
+   */
   GRANT_TABLE *grant_table;
+  /**
+     @brief Used for cache invalidation when caching privile information.
+
+     @details The privilege information is stored on disk, with dedicated
+     caches residing in memory: table-level and column-level privileges,
+     respectively, have their own dedicated cache.
+   */
   uint version;
+  /**
+     @brief The set of privileges that the current user has fulfilled for a
+     certain host, database, and object.
+     
+     @details This field is continually updated throughout the access checking
+     process. In each step the "wanted privilege" is checked against the
+     fulfilled privileges. When/if 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 the 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.
   */
@@ -1261,6 +1302,16 @@ struct TABLE_LIST
   inline void init_child_def_version()
   {
     child_def_version= ~0UL;
+  }
+
+  /**
+     @brief If true, this @c TABLE_LIST represents a derived table in the @c
+     FROM clause.
+     
+     @note By derived table, we mean the result of a nested query.
+  */
+  bool is_derived_table_in_from_clause() { 
+    return derived != NULL && view == NULL; 
   }
 
 private:
Thread
bk commit into 6.0 tree (mhansson:1.2621) BUG#36086mhansson21 Apr