#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

Content reproduced on this site is the property of the respective copyright holders. It is not reviewed in advance by Oracle and does not necessarily represent the opinion of Oracle or any other party.