List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:May 23 2011 9:51am
Subject:bzr commit into mysql-trunk branch (guilhem.bichot:3313)
View as plain text  
#At file:///home/mysql_src/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800/ based on revid:guilhem.bichot@stripped

 3313 Guilhem Bichot	2011-05-23
      review comment: move commens about API to header file. Fixing a couple of bad doxygen formatting.

    modified:
      WL4800_Doxyfile
      sql/opt_trace.h
      sql/opt_trace2server.cc
      sql/opt_trace_context.h
=== modified file 'WL4800_Doxyfile'
--- a/WL4800_Doxyfile	2011-02-16 15:06:06 +0000
+++ b/WL4800_Doxyfile	2011-05-23 09:51:41 +0000
@@ -593,7 +593,7 @@ WARN_LOGFILE           = doxygen.err
 # directories like "/usr/src/myproject". Separate the files or directories
 # with spaces.
 
-INPUT                  =  ./sql/opt_trace.h ./sql/opt_trace.cc ./sql/opt_trace2server.cc ./sql/sys_vars.cc
+INPUT                  =  ./sql/opt_trace_context.h ./sql/opt_trace.h ./sql/opt_trace.cc ./sql/opt_trace2server.cc ./sql/sys_vars.cc
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is
 # also the default input encoding. Doxygen uses libiconv (or the iconv built

=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h	2011-05-20 13:03:28 +0000
+++ b/sql/opt_trace.h	2011-05-23 09:51:41 +0000
@@ -289,18 +289,6 @@ class sp_instr;
   immediately, a key/value pair and its outer object may be 100 lines
   apart in the DBUG log.
 
-  @section NEXT What a developer should read next
-
-  The documentation of those classes, in order
-@code
-  Opt_trace_context
-  Opt_trace_stmt
-  Opt_trace_struct
-  Opt_trace_object
-  Opt_trace_array
-@endcode
-  and then @ref opt_trace.h as a whole.
-
   @section ADDING_TRACING Guidelines for adding tracing
 
   @li Try to limit the number of distinct "words". For example, when
@@ -340,6 +328,52 @@ class sp_instr;
   So we have two calls to "new" (one to create Opt_trace_context_impl, one to
   create Opt_trace_stmt), which may crash. When we have nothrow we should
   change them to new(nothrow).
+
+  @section TRACE_SECURITY Description of trace-induced additional security checks.
+
+  A trace exposes information. For example if one does SELECT on a view, the
+  trace contains the view's body. So, the user should be allowed to see the
+  trace only if she/he has privilege to see the body, i.e. privilege to do
+  SHOW CREATE VIEW.
+  There are similar issues with stored procedures, functions, triggers.
+
+  We implement this by doing additional checks on SQL objects when tracing is
+  on:
+  @li stored procedures, functions, triggers: checks are done when executing
+  those objects
+  @li base tables and views.
+
+  Base tables or views are listed in some @c LEX::query_tables.
+  The LEX may be of the executing statement (statement executed by
+  @c mysql_execute_command(), or by
+  @c sp_lex_keeper::reset_lex_and_exec_core()), we check this LEX in the
+  constructor of Opt_trace_start.
+  Or it may be a LEX describing a view, we check this LEX when
+  opening the view (@c mysql_make_view()).
+
+  Those checks are greatly simplified by disabling traces in case of security
+  context changes. @see opt_trace_disable_if_no_security_context_access().
+
+  Those checks must be done with the security context of the connected
+  user. Checks with the SUID context would be useless: assume the design is
+  that the basic user does not have DML privileges on tables, but only
+  EXECUTE on SUID-highly-privileged routines (which implement _controlled_
+  _approved_ DMLs): then the SUID context would successfully pass all
+  additional privilege checks, routine would generate tracing, and the
+  connected user would view the trace after the routine's execution, seeing
+  secret information.
+
+  @section NEXT What a developer should read next
+
+  The documentation of those classes, in order
+@code
+  Opt_trace_context
+  Opt_trace_stmt
+  Opt_trace_struct
+  Opt_trace_object
+  Opt_trace_array
+@endcode
+  and then @ref opt_trace.h as a whole.
 */
 
 class Opt_trace_struct;
@@ -882,7 +916,6 @@ private:
 
 /**
    @name Helpers connecting the optimizer trace to THD or Information Schema.
-   Implemented in opt_trace2server.cc.
 */
 
 //@{
@@ -925,21 +958,87 @@ private:
 
 class st_select_lex;
 /**
-   Prints SELECT query to optimizer trace. It is not the original query (@see
-   Opt_trace_context::set_query()) but a printout of the parse tree (Item-s).
+   Prints SELECT query to optimizer trace. It is not the original query (as in
+   @c Opt_trace_context::set_query()) but a printout of the parse tree (Item-s).
    @param  thd         the THD
    @param  select_lex  query's parse tree
 */
 void opt_trace_print_expanded_query(THD *thd,
                                     st_select_lex *select_lex);
 
+/**
+   If the security context is not that of the connected user, inform the trace
+   system that a privilege is missing. With one exception: see below.
+
+   @param thd
+
+   This serves to eliminate the following issue.
+   Any information readable by a SELECT may theoretically end up in
+   the trace. And a SELECT may read information from other places than tables:
+   - from views (reading their bodies)
+   - from stored routines (reading their bodies)
+   - from files (reading their content), with LOAD_FILE()
+   - from the list of connections (reading their queries...), with
+   I_S.PROCESSLIST.
+   If the connected user has EXECUTE privilege on a routine which does a
+   security context change, the routine can retrieve information internally
+   (if allowed by the SUID context's privileges), and present only a portion
+   of it to the connected user. But with tracing on, all information is
+   possibly in the trace. So the connected user receives more information than
+   the routine's definer intended to provide.  Fixing this issue would require
+   adding, near many privilege checks in the server, a new
+   optimizer-trace-specific check done against the connected user's context,
+   to verify that the connected user has the right to see the retrieved
+   information.
+
+   Instead, our chosen simpler solution is that if we see a security context
+   change where SUID user is not the connected user, we disable tracing. With
+   only one safe exception: if the connected user has all global privileges
+   (because then she/he can find any information anyway). By "all global
+   privileges" we mean everything but WITH GRANT OPTION (that latter one isn't
+   related to information gathering).
+
+   Read access to I_S.OPTIMIZER_TRACE by another user than the connected user
+   is restricted: @see fill_optimizer_trace_info().
+*/
+void opt_trace_disable_if_no_security_context_access(THD *thd);
+
+/**
+   If tracing is on, checks additional privileges for a view, to make sure that
+   the user has the right to do SHOW CREATE VIEW. For that:
+   - this function checks SHOW VIEW
+   - SELECT is tested in opt_trace_disable_if_no_tables_access()
+   - SELECT + SHOW VIEW is sufficient for SHOW CREATE VIEW.
+   We also check underlying tables.
+   If a privilege is missing, notifies the trace system.
+   This function should be called when the view's underlying tables have not
+   yet been merged.
+
+   @param thd               THD context
+   @param view              view to check
+   @param underlying_tables underlying tables/views of 'view'
+ */
 void opt_trace_disable_if_no_view_access(THD *thd, TABLE_LIST *view,
                                  TABLE_LIST *underlying_tables);
 
+/**
+  If tracing is on, checks additional privileges on a stored routine, to make
+  sure that the user has the right to do SHOW CREATE PROCEDURE/FUNCTION. For
+  that, we use the same checks as in those SHOW commands.
+  If a privilege is missing, notifies the trace system.
+
+  This function is not redundant with
+  opt_trace_disable_if_no_security_context_access().
+  Indeed, for a SQL SECURITY INVOKER routine, there is no context change, but
+  we must still verify that the invoker can do SHOW CREATE.
+
+  For triggers, see note in sp_head::execute_trigger().
+
+  @param thd
+  @param sp  routine to check
+ */
 void opt_trace_disable_if_no_stored_proc_func_access(THD *thd, sp_head *sp);
 
-void opt_trace_disable_if_no_security_context_access(THD *thd);
-
 /**
    Fills information_schema.OPTIMIZER_TRACE with rows (one per trace)
    @retval 0 ok

=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc	2011-05-20 12:38:41 +0000
+++ b/sql/opt_trace2server.cc	2011-05-23 09:51:41 +0000
@@ -235,78 +235,6 @@ void opt_trace_print_expanded_query(THD 
 }
 
 
-/**
-   @name Description of trace-induced additional security checks.
-   A trace exposes information. For example if one does SELECT on a view, the
-   trace contains the view's body. So, the user should be allowed to see the
-   trace only if she/he has privilege to see the body, i.e. privilege to do
-   SHOW CREATE VIEW.
-   There are similar issues with stored procedures, functions, triggers.
-
-   We implement this by doing additional checks on SQL objects when tracing is
-   on:
-   @li stored procedures, functions, triggers: checks are done when executing
-   those objects
-   @li base tables and views.
-
-   Base tables or views are listed in some LEX::query_tables.
-   The LEX may be of the executing statement (statement executed by
-   mysql_execute_command(), or by
-   sp_lex_keeper::reset_lex_and_exec_core()), we check this LEX in the
-   constructor of Opt_trace_start.
-   Or it may be a LEX describing a view, we check this LEX when
-   opening the view (mysql_make_view()).
-
-   Those checks are greatly simplified by disabling traces in case of security
-   context changes (@see opt_trace_disable_if_no_security_context_access()).
-
-   Those checks must be done with the security context of the connected
-   user. Checks with the SUID context would be useless: assume the design is
-   that the basic user does not have DML privileges on tables, but only
-   EXECUTE on SUID-highly-privileged routines (which implement _controlled_
-   _approved_ DMLs): then the SUID context would successfully pass all
-   additional privilege checks, routine would generate tracing, and the
-   connected user would view the trace after the routine's execution, seeing
-   secret information.
-*/
-
-//@{
-
-/**
-   If the security context is not that of the connected user, inform the trace
-   system that a privilege is missing. With one exception: see below.
-
-   @param thd
-
-   This serves to eliminate the following issue.
-   Any information readable by a SELECT may theoretically end up in
-   the trace. And a SELECT may read information from other places than tables:
-   - from views (reading their bodies)
-   - from stored routines (reading their bodies)
-   - from files (reading their content), with LOAD_FILE()
-   - from the list of connections (reading their queries...), with
-   I_S.PROCESSLIST.
-   If the connected user has EXECUTE privilege on a routine which does a
-   security context change, the routine can retrieve information internally
-   (if allowed by the SUID context's privileges), and present only a portion
-   of it to the connected user. But with tracing on, all information is
-   possibly in the trace. So the connected user receives more information than
-   the routine's definer intended to provide.  Fixing this issue would require
-   adding, near many privilege checks in the server, a new
-   optimizer-trace-specific check done against the connected user's context,
-   to verify that the connected user has the right to see the retrieved
-   information.
-
-   Instead, our chosen simpler solution is that if we see a security context
-   change where SUID user is not the connected user, we disable tracing. With
-   only one safe exception: if the connected user has all global privileges
-   (because then she/he can find any information anyway). By "all global
-   privileges" we mean everything but WITH GRANT OPTION (that latter one isn't
-   related to information gathering).
-
-   Read access to I_S.OPTIMIZER_TRACE by another user than the connected user
-   is restricted: @see fill_optimizer_trace_info().
-*/
 void opt_trace_disable_if_no_security_context_access(THD *thd)
 {
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
@@ -371,22 +299,6 @@ void opt_trace_disable_if_no_security_co
 }
 
 
-/**
-  If tracing is on, checks additional privileges on a stored routine, to make
-  sure that the user has the right to do SHOW CREATE PROCEDURE/FUNCTION. For
-  that, we use the same checks as in those SHOW commands.
-  If a privilege is missing, notifies the trace system.
-
-  This function is not redundant with
-  opt_trace_disable_if_no_security_context_access().
-  Indeed, for a SQL SECURITY INVOKER routine, there is no context change, but
-  we must still verify that the invoker can do SHOW CREATE.
-
-  For triggers, see note in sp_head::execute_trigger().
-
-  @param thd
-  @param sp  routine to check
- */
 void opt_trace_disable_if_no_stored_proc_func_access(THD *thd, sp_head *sp)
 {
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
@@ -415,21 +327,6 @@ void opt_trace_disable_if_no_stored_proc
 }
 
 
-/**
-   If tracing is on, checks additional privileges for a view, to make sure that
-   the user has the right to do SHOW CREATE VIEW. For that:
-   - this function checks SHOW VIEW
-   - SELECT is tested in opt_trace_disable_if_no_tables_access()
-   - SELECT + SHOW VIEW is sufficient for SHOW CREATE VIEW.
-   We also check underlying tables.
-   If a privilege is missing, notifies the trace system.
-   This function should be called when the view's underlying tables have not
-   yet been merged.
-
-   @param thd               THD context
-   @param view              view to check
-   @param underlying_tables underlying tables/views of 'view'
- */
 void opt_trace_disable_if_no_view_access(THD *thd, TABLE_LIST *view,
                                          TABLE_LIST *underlying_tables)
 {
@@ -558,8 +455,6 @@ void opt_trace_disable_if_no_tables_acce
 
 } // namespace
 
-//@}
-
 
 int fill_optimizer_trace_info(THD *thd, TABLE_LIST *tables, Item *cond)
 {

=== modified file 'sql/opt_trace_context.h'
--- a/sql/opt_trace_context.h	2011-05-20 13:03:28 +0000
+++ b/sql/opt_trace_context.h	2011-05-23 09:51:41 +0000
@@ -233,6 +233,7 @@ public:
   */
   void missing_privilege();
 
+  /// Optimizer features which are traced by default.
   static const feature_value default_features;
 
   /**
@@ -370,7 +371,7 @@ private:
     long since_offset_0;
   };
 
-  Opt_trace_context_impl *pimpl;
+  Opt_trace_context_impl *pimpl;     /// Dynamically allocated implementation.
 
   /**
     <>0 <=> any to-be-created statement's trace should not be in
@@ -384,7 +385,7 @@ private:
      For example if OFFSET=-1,LIMIT=1, only the last trace is needed. When a
      new trace is started, the previous traces becomes unneeded and this
      function deletes them which frees memory.
-     @param  all  if true, ignore OFFSET and thus delete everything
+     @param  purge_all  if true, ignore OFFSET and thus delete everything
   */
   void purge_stmts(bool purge_all);
 


Attachment: [text/bzr-bundle] bzr/guilhem.bichot@oracle.com-20110523095141-1kp7249rtwpaes7b.bundle
Thread
bzr commit into mysql-trunk branch (guilhem.bichot:3313) Guilhem Bichot23 May