List:Commits« Previous MessageNext Message »
From:Luís Soares Date:June 30 2011 5:29pm
Subject:Re: bzr commit into mysql-trunk branch (alfranio.correia:3167) WL#4143
View as plain text  
Hi Alfranio,

   Nice Work. There are some issues to attend. Nothing major.  Please
   find my review comments below.

STATUS
------

   Not approved.

REQUIRED CHANGES
----------------

   RC1. In the BACKGROUND section, item #4.

        I think that it is good to have an explanation on the overall,
        however the text and the diagram are not very clear. Can you
        please make it more simple ? Maybe just enumerate the steps or
        use some kind of pseudo-code...

   RC2. In the PROPOSED section:

        "We will recommend that SSL/TSL should be used and a warning
         message will be (...)"

        Can you please make it clear what it means "We will recommend"
        ? Does it mean that docs people need to document it?

   RC3. In the worklog, DEFAULT_AUTH and PLUGIN_DIR are not clearly
        defined. I think, for documentation purposes, state what they
        refer to (e.g., what's DEFAULT_AUTH for example? refers to
        plugins refers to general auth infrastructure? ... )

        As we have discussed these options are named after similar ones
        in the client tools. Anyway, I think you should document:

        - DEFAULT_AUTH means
        - PLUGIN_DIR means
        - why we still need PASSWORD='yyyy' even if using the plugin


   RC4. In @@ -211,6 +211,12 @@ INSERT INTO global_suppressions VALUES

        + ("Due to security issues one must use SSL/TSL while specifying START SLAVE
USER=xxx PASSWORD=yyy."),


        Is this really a *MUST* or just a *SHOULD* scenario? From
        the worklog specs:

        . We will recommend that SSL/TSL should be used and a
          warning message will be printed out when either SSL or
          TSL is not used.

        Thus if only a warning, this must be a should case, not a
        must (otherwise an error must be raised instead of
        warning). Maybe:

        "Due to security issues, SSL/TLS should be used while
         specifying START SLAVE USER=xxx PASSWORD=yyy. Please,
         check the user manual for additional details."

+ ("It is not recommend to store user and password in the master info repository due to
security issues.*"),

       Why not start like the other sentence above (also recommended):

       "Due to security issues, the server should not store the
        user and password in the master.info repository. Please,
        check the user manual for additional details."

   RC5. @@ -21,7 +21,6 @@

        Please, avoid messing around with empty lines ;).

   RC6. @@ -95,6 +94,7 @@ Master_info::Master_info(

        Please use 'false' values instead of '0' for:

        start_user_configured(0), start_plugin_configured(0),

   RC7. @@ -465,4 +466,55 @@ bool Master_info::write_info(Rpl_info_ha

        The set_password member functions signature is:

        bool Master_info::set_password(const char* password_arg,

        but you assign FALSE and TRUE to ret value. Please use 'false'
        and 'true' instead.


        Same with:
        +bool Master_info::get_password(char *password_arg, int
        *password_arg_size)

        and

        +void Master_info::clean_start_information()


   RC8. @@ -63,13 +63,195 @@ class Rpl_info_factory;

        +  char start_command[FN_REFLEN + 1];

        Please rename to fake_start_command

        Also, I would probably make it less ambiguous:

        +  char password[MAX_PASSWORD_LENGTH + 1];
        +  char user[USERNAME_LENGTH + 1];

        I would prefix these with "master_info_" (or just "mi_").

   RC9. @@ -5832,14 +5902,30 @@ bool change_master(THD* thd, Master_info

       The warning states:

       "It is not recommend to store user and password in (...)"

       But we only issue the warning if the we store the password:

       +  if (lex_mi->password)
+  {
+    sql_print_warning("It is not recommend to store user and password
   in "

       So, my question is, when should we issue the warning:

       1. when storing both user and password ?
       2. when storing any (user, password, user and password)?
       3. when storing password only ?

       I think we should go for #2.

       Also, in this part, shouldn't the warning be pushed to the
       diagnostics area so that the user gets notified in the session ?
       The same question for the hunk in start_slave. The worklog just
       states that a warning message is generated, it does not state
       where the warning message ends up... :) Please, update the
       worklog with that info as well.

   RC10. @@ -2708,7 +2708,40 @@ end_with_restore_list:

      Again, use true instead of TRUE.

REQUESTS
--------

   R1. @@ -5498,6 +5558,16 @@ int start_slave(THD* thd , Master_info*

       Should we issue this warning regardless if the server was built
       with SSL or not ?


SUGGESTIONS
-----------

   S1. @@ -102,10 +102,14 @@ Master_info::Master_info(

       I would probably use strmov instead of strcpy.

   S2. @@ -465,4 +466,55 @@ bool Master_info::write_info(Rpl_info_ha

       I might name Master_info::clean_start_information() as
       Master_info::reset_start_information() instead. No strong
       opinion though.

DETAILS
-------
  n/a

On 06/08/2011 01:48 PM, Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.oracle/repository.mysql/bzrwork/wl-4143/mysql-trunk/ based
> on revid:olav.sandstaa@stripped
>
>   3167 Alfranio Correia	2011-06-08
>        WL#4143
>       @ mysql-test/include/mtr_warnings.sql
>          Suppressed warning messages.
>       @ mysql-test/suite/rpl/r/rpl_master_connection.result
>          Created a new test case.
>       @ mysql-test/suite/rpl/t/rpl_master_connection-master.opt
>          Created a new test case.
>       @ mysql-test/suite/rpl/t/rpl_master_connection-slave.opt
>          Created a new test case.
>       @ mysql-test/suite/rpl/t/rpl_master_connection.test
>          Created a new test case.
>       @ sql/lex.h
>          Changed the parser to allow:
>
>          . START SLAVE USER=<xxxx>  PASSWORD=<yyyy> 
> DEFAULT_AUTH=<zzzz>  PLUGIN_DIR=<wwww>
>       @ sql/rpl_handler.cc
>          User is now private and there is a function to retrieve it.
>       @ sql/rpl_mi.cc
>          There are two main changes:
>            . added information to process the new START SLAVE syntax.
>            . improved the code around the set_info and get_info in order to ease
>              maintenance.
>       @ sql/rpl_mi.h
>          Added information to process the new START SLAVE syntax.
>       @ sql/rpl_rli.cc
>          Improved the code around the set_info and get_info in order to ease
>          maintenance.
>       @ sql/rpl_slave.cc
>          To hide the fact that a user/password may be provided through START SLAVE,
>          and in the future, it may be even encrypted, we have create methods to
>          set/get the user and its password.
>       @ sql/sql_lex.h
>          Added a structure to carry on the new information:
>
>          . START SLAVE USER=<xxxx>  PASSWORD=<yyyy> 
> DEFAULT_AUTH=<zzzz>  PLUGIN_DIR=<wwww>
>       @ sql/sql_parse.cc
>          Set in-memory information in master.info after executing:
>
>          . START SLAVE USER=<xxxx>  PASSWORD=<yyyy> 
> DEFAULT_AUTH=<zzzz>  PLUGIN_DIR=<wwww>
>
>          This is necessary in order to automatically reconnect to master due to a
>          transient failure.
>       @ sql/sql_yacc.yy
>          Changed the parser to allow:
>
>          . START SLAVE USER=<xxxx>  PASSWORD=<yyyy> 
> DEFAULT_AUTH=<zzzz>  PLUGIN_DIR=<wwww>
>
>      added:
>        mysql-test/suite/rpl/r/rpl_master_connection.result
>        mysql-test/suite/rpl/t/rpl_master_connection-master.opt
>        mysql-test/suite/rpl/t/rpl_master_connection-slave.opt
>        mysql-test/suite/rpl/t/rpl_master_connection.test
>      modified:
>        mysql-test/include/mtr_warnings.sql
>        sql/lex.h
>        sql/rpl_handler.cc
>        sql/rpl_mi.cc
>        sql/rpl_mi.h
>        sql/rpl_rli.cc
>        sql/rpl_slave.cc
>        sql/sql_lex.h
>        sql/sql_parse.cc
>        sql/sql_yacc.yy
> === modified file 'mysql-test/include/mtr_warnings.sql'
> --- a/mysql-test/include/mtr_warnings.sql	2011-04-06 10:36:10 +0000
> +++ b/mysql-test/include/mtr_warnings.sql	2011-06-08 12:48:53 +0000
> @@ -211,6 +211,12 @@ INSERT INTO global_suppressions VALUES
>    */
>    (".*If a crash happens this configuration does not guarantee.*"),
>
> + /*
> +   Warning messages introduced in the context of the WL#4143.
> + */
> + ("Due to security issues one must use SSL/TSL while specifying START SLAVE USER=xxx
> PASSWORD=yyy."),
> + ("It is not recommend to store user and password in the master info repository due
> to security issues.*"),
> +
>    ("THE_LAST_SUPPRESSION")||
>
>
>
> === added file 'mysql-test/suite/rpl/r/rpl_master_connection.result'
> --- a/mysql-test/suite/rpl/r/rpl_master_connection.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_master_connection.result	2011-06-08 12:48:53 +0000
> @@ -0,0 +1,124 @@
> +include/master-slave.inc
> +[connection master]
> +SET SQL_LOG_BIN=0;
> +SELECT user, plugin, authentication_string, password FROM mysql.user WHERE user !=
> 'root';
> +user	plugin	authentication_string	password
> +CREATE USER 'plug_user_p' IDENTIFIED WITH 'test_plugin_server' AS 'password';
> +CREATE USER 'plug_user_wp' IDENTIFIED WITH 'test_plugin_server' AS '';
> +CREATE USER 'regular_user_p' IDENTIFIED BY 'password';
> +CREATE USER 'regular_user_wp' IDENTIFIED BY '';
> +SELECT user, plugin, authentication_string, password FROM mysql.user WHERE user !=
> 'root';
> +user	plugin	authentication_string	password
> +plug_user_p	test_plugin_server	password	
> +plug_user_wp	test_plugin_server		
> +regular_user_p		NULL	*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19
> +regular_user_wp		NULL	
> +GRANT REPLICATION SLAVE ON *.* TO plug_user_p;
> +GRANT REPLICATION SLAVE ON *.* TO plug_user_wp;
> +GRANT REPLICATION SLAVE ON *.* TO regular_user_p;
> +GRANT REPLICATION SLAVE ON *.* TO regular_user_wp;
> +SET SQL_LOG_BIN=1;
> +include/stop_slave.inc
> +SET @@GLOBAL.master_info_repository= "TABLE";
> +include/stop_slave.inc
> +Warnings:
> +Note	1255	Slave already has been stopped
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_p' PASSWORD= 'password';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_wp' PASSWORD= '';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_wp';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'root' PASSWORD= '' DEFAULT_AUTH= 'auth_test_plugin' PLUGIN_DIR=
> '/home/acorreia/workspace.oracle/repository.mysql/bzrwork/wl-4143/mysql-trunk/plugin/auth';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'regular_user_p', MASTER_PASSWORD= 'password';
> +START SLAVE;
> +include/wait_for_slave_io_to_start.inc
> +include/check_slave_is_running.inc
> +include/rpl_restart_server.inc [server_number=2]
> +include/start_slave.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'regular_user_wp', MASTER_PASSWORD= '';
> +START SLAVE;
> +include/wait_for_slave_io_to_start.inc
> +include/check_slave_is_running.inc
> +include/rpl_restart_server.inc [server_number=2]
> +include/start_slave.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'plug_user_p', MASTER_PASSWORD= 'password';
> +START SLAVE;
> +include/wait_for_slave_io_error.inc [errno=1251]
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'plug_user_wp', MASTER_PASSWORD= '';
> +START SLAVE;
> +include/wait_for_slave_io_error.inc [errno=1251]
> +include/stop_slave.inc
> +SET @@GLOBAL.master_info_repository= "FILE";
> +include/stop_slave.inc
> +Warnings:
> +Note	1255	Slave already has been stopped
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_p' PASSWORD= 'password';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_wp' PASSWORD= '';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'regular_user_wp';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT EXIST';
> +START SLAVE USER= 'root' PASSWORD= '' DEFAULT_AUTH= 'auth_test_plugin' PLUGIN_DIR=
> '/home/acorreia/workspace.oracle/repository.mysql/bzrwork/wl-4143/mysql-trunk/plugin/auth';
> +include/wait_for_slave_to_start.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'regular_user_p', MASTER_PASSWORD= 'password';
> +START SLAVE;
> +include/wait_for_slave_io_to_start.inc
> +include/check_slave_is_running.inc
> +include/rpl_restart_server.inc [server_number=2]
> +include/start_slave.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'regular_user_wp', MASTER_PASSWORD= '';
> +START SLAVE;
> +include/wait_for_slave_io_to_start.inc
> +include/check_slave_is_running.inc
> +include/rpl_restart_server.inc [server_number=2]
> +include/start_slave.inc
> +include/check_slave_is_running.inc
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'plug_user_p', MASTER_PASSWORD= 'password';
> +START SLAVE;
> +include/wait_for_slave_io_error.inc [errno=1251]
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'plug_user_wp', MASTER_PASSWORD= '';
> +START SLAVE;
> +include/wait_for_slave_io_error.inc [errno=1251]
> +SET SQL_LOG_BIN=0;
> +DROP USER plug_user_p, plug_user_wp, regular_user_p, regular_user_wp;
> +SET SQL_LOG_BIN=1;
> +include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'root', MASTER_PASSWORD = '';
> +SET @@GLOBAL.master_info_repository="FILE";
> +include/start_slave.inc
> +include/rpl_end.inc
>
> === added file 'mysql-test/suite/rpl/t/rpl_master_connection-master.opt'
> --- a/mysql-test/suite/rpl/t/rpl_master_connection-master.opt	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_master_connection-master.opt	2011-06-08 12:48:53
> +0000
> @@ -0,0 +1,2 @@
> +$PLUGIN_AUTH_OPT
> +$PLUGIN_AUTH_LOAD
>
> === added file 'mysql-test/suite/rpl/t/rpl_master_connection-slave.opt'
> --- a/mysql-test/suite/rpl/t/rpl_master_connection-slave.opt	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_master_connection-slave.opt	2011-06-08 12:48:53
> +0000
> @@ -0,0 +1 @@
> +--master-retry-count=1
>
> === added file 'mysql-test/suite/rpl/t/rpl_master_connection.test'
> --- a/mysql-test/suite/rpl/t/rpl_master_connection.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_master_connection.test	2011-06-08 12:48:53 +0000
> @@ -0,0 +1,126 @@
> +--source include/have_plugin_auth.inc
> +--source include/not_embedded.inc
> +--source include/master-slave.inc
> +
> +################################################################################
> +# 1. Prepare the environment
> +################################################################################
> +SET SQL_LOG_BIN=0;
> +
> +--sorted_result
> +SELECT user, plugin, authentication_string, password FROM mysql.user WHERE user !=
> 'root';
> +
> +CREATE USER 'plug_user_p' IDENTIFIED WITH 'test_plugin_server' AS 'password';
> +CREATE USER 'plug_user_wp' IDENTIFIED WITH 'test_plugin_server' AS '';
> +CREATE USER 'regular_user_p' IDENTIFIED BY 'password';
> +CREATE USER 'regular_user_wp' IDENTIFIED BY '';
> +
> +--sorted_result
> +SELECT user, plugin, authentication_string, password FROM mysql.user WHERE user !=
> 'root';
> +
> +GRANT REPLICATION SLAVE ON *.* TO plug_user_p;
> +GRANT REPLICATION SLAVE ON *.* TO plug_user_wp;
> +GRANT REPLICATION SLAVE ON *.* TO regular_user_p;
> +GRANT REPLICATION SLAVE ON *.* TO regular_user_wp;
> +
> +SET SQL_LOG_BIN=1;
> +
> +
> +###############################################################################
> +# 2. Test if different type of users can connect when CHANGE MASTER
> +#    START SLAVE are specified
> +###############################################################################
> +--connection slave
> +--let $slave_io_errno= 1251
> +--let $show_slave_io_error= 0
> +--let $master_info_repository_old= `SELECT @@GLOBAL.master_info_repository`
> +--let $count= 2
> +
> +while ($count != 0)
> +{
> +  --source include/stop_slave.inc
> +  if ($count == 1)
> +  {
> +    SET @@GLOBAL.master_info_repository= "FILE";
> +  }
> +
> +  if ($count == 2)
> +  {
> +    SET @@GLOBAL.master_info_repository= "TABLE";
> +  }
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT
> EXIST';
> +  --eval START SLAVE USER= 'regular_user_p' PASSWORD= 'password'
> +  --source include/wait_for_slave_to_start.inc
> +  --source include/check_slave_is_running.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT
> EXIST';
> +  --eval START SLAVE USER= 'regular_user_wp' PASSWORD= ''
> +  --source include/wait_for_slave_to_start.inc
> +  --source include/check_slave_is_running.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT
> EXIST';
> +  --eval START SLAVE USER= 'regular_user_wp'
> +  --source include/wait_for_slave_to_start.inc
> +  --source include/check_slave_is_running.inc
> +
> +  ############### How can I know if this is working ? ##################
> +  --source include/stop_slave.inc
> +  --replace_result $MASTER_MYSOCK MASTER_MYSOCK $PLUGIN_AUTH_OPT PLUGIN_AUTH_OPT
> +  CHANGE MASTER TO MASTER_USER= 'DOES NOT EXIST', MASTER_PASSWORD = 'DOES NOT
> EXIST';
> +  --eval START SLAVE USER= 'root' PASSWORD= '' DEFAULT_AUTH= 'auth_test_plugin'
> PLUGIN_DIR= '$PLUGIN_AUTH_DIR'
> +  --source include/wait_for_slave_to_start.inc
> +  --source include/check_slave_is_running.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'regular_user_p', MASTER_PASSWORD= 'password';
> +  START SLAVE;
> +  --source include/wait_for_slave_io_to_start.inc
> +  --source include/check_slave_is_running.inc
> +  --let $rpl_server_number= 2
> +  --source include/rpl_restart_server.inc
> +  --connection slave
> +  --source include/start_slave.inc
> +  --source include/check_slave_is_running.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'regular_user_wp', MASTER_PASSWORD= '';
> +  START SLAVE;
> +  --source include/wait_for_slave_io_to_start.inc
> +  --source include/check_slave_is_running.inc
> +  --let $rpl_server_number= 2
> +  --source include/rpl_restart_server.inc
> +  --connection slave
> +  --source include/start_slave.inc
> +  --source include/check_slave_is_running.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'plug_user_p', MASTER_PASSWORD= 'password';
> +  START SLAVE;
> +  --source include/wait_for_slave_io_error.inc
> +
> +  --source include/stop_slave.inc
> +  CHANGE MASTER TO MASTER_USER= 'plug_user_wp', MASTER_PASSWORD= '';
> +  START SLAVE;
> +  --source include/wait_for_slave_io_error.inc
> +
> +  --dec $count
> +}
> +
> +################################################################################
> +# 3. Clean the environment
> +################################################################################
> +--connection master
> +SET SQL_LOG_BIN=0;
> +DROP USER plug_user_p, plug_user_wp, regular_user_p, regular_user_wp;
> +SET SQL_LOG_BIN=1;
> +
> +--connection slave
> +--source include/stop_slave.inc
> +CHANGE MASTER TO MASTER_USER= 'root', MASTER_PASSWORD = '';
> +--eval SET @@GLOBAL.master_info_repository="$master_info_repository_old"
> +--source include/start_slave.inc
> +--source include/rpl_end.inc
>
> === modified file 'sql/lex.h'
> --- a/sql/lex.h	2010-11-25 11:20:16 +0000
> +++ b/sql/lex.h	2011-06-08 12:48:53 +0000
> @@ -164,6 +164,7 @@ static SYMBOL symbols[] = {
>     { "DECIMAL",		SYM(DECIMAL_SYM)},
>     { "DECLARE",          SYM(DECLARE_SYM)},
>     { "DEFAULT",		SYM(DEFAULT)},
> +  { "DEFAULT_AUTH",	SYM(DEFAULT_AUTH_SYM)},
>     { "DEFINER",          SYM(DEFINER_SYM)},
>     { "DELAYED",		SYM(DELAYED_SYM)},
>     { "DELAY_KEY_WRITE",	SYM(DELAY_KEY_WRITE_SYM)},
> @@ -412,6 +413,7 @@ static SYMBOL symbols[] = {
>     { "PHASE",            SYM(PHASE_SYM)},
>     { "PLUGIN",           SYM(PLUGIN_SYM)},
>     { "PLUGINS",          SYM(PLUGINS_SYM)},
> +  { "PLUGIN_DIR",       SYM(PLUGIN_DIR_SYM)},
>     { "POINT",		SYM(POINT_SYM)},
>     { "POLYGON",		SYM(POLYGON)},
>     { "PORT",		SYM(PORT_SYM)},
>
> === modified file 'sql/rpl_handler.cc'
> --- a/sql/rpl_handler.cc	2010-11-05 22:14:29 +0000
> +++ b/sql/rpl_handler.cc	2011-06-08 12:48:53 +0000
> @@ -429,7 +429,7 @@ void Binlog_relay_IO_delegate::init_para
>                                             Master_info *mi)
>   {
>     param->mysql= mi->mysql;
> -  param->user= mi->user;
> +  param->user= const_cast<char *>(mi->get_user());
>     param->host= mi->host;
>     param->port= mi->port;
>     param->master_log_name= const_cast<char
> *>(mi->get_master_log_name());
>
> === modified file 'sql/rpl_mi.cc'
> --- a/sql/rpl_mi.cc	2011-04-28 16:50:10 +0000
> +++ b/sql/rpl_mi.cc	2011-06-08 12:48:53 +0000
> @@ -21,7 +21,6 @@
>   #include "rpl_slave.h"                          // SLAVE_MAX_HEARTBEAT_PERIOD
>
>   #ifdef HAVE_REPLICATION
> -
>   enum {
>     LINES_IN_MASTER_INFO_WITH_SSL= 14,
>
> @@ -95,6 +94,7 @@ Master_info::Master_info(
>                param_key_info_stop_cond, param_key_info_sleep_cond
>   #endif
>               ),
> +   start_user_configured(0), start_plugin_configured(0),
>      ssl(0), ssl_verify_server_cert(0),
>      port(MYSQL_PORT), connect_retry(DEFAULT_CONNECT_RETRY),
>      clock_diff_with_master(0), heartbeat_period(0),
> @@ -102,10 +102,14 @@ Master_info::Master_info(
>      checksum_alg_before_fd(BINLOG_CHECKSUM_ALG_UNDEF),
>      retry_count(master_retry_count)
>   {
> -  host[0] = 0; user[0] = 0; password[0] = 0; bind_addr[0] = 0;
> +  host[0] = 0; user[0] = 0; bind_addr[0] = 0;
> +  password[0]= 0; start_password[0]= 0;
>     ssl_ca[0]= 0; ssl_capath[0]= 0; ssl_cert[0]= 0;
>     ssl_cipher[0]= 0; ssl_key[0]= 0;
>     master_uuid[0]= 0;
> +  start_plugin_auth[0]= 0; start_plugin_dir[0]= 0;
> +  start_user[0]= 0;
> +  strcpy(start_command, "START SLAVE");
>     ignore_server_ids= new Server_ids();
>   }
>
> @@ -316,7 +320,7 @@ bool Master_info::read_info(Rpl_info_han
>     */
>
>     if (from->prepare_info_for_read() ||
> -      from->get_info(master_log_name, sizeof(master_log_name), ""))
> +      from->get_info(master_log_name, sizeof(master_log_name), (char *) ""))
>       DBUG_RETURN(TRUE);
>
>     lines= strtoul(master_log_name,&first_non_digit, 10);
> @@ -325,20 +329,18 @@ bool Master_info::read_info(Rpl_info_han
>         *first_non_digit=='\0'&&  lines>= LINES_IN_MASTER_INFO_WITH_SSL)
>     {
>       /* Seems to be new format =>  read master log name */
> -    if (from->get_info(master_log_name,  sizeof(master_log_name), ""))
> +    if (from->get_info(master_log_name,  sizeof(master_log_name), (char *) ""))
>         DBUG_RETURN(TRUE);
>     }
>     else
>       lines= 7;
>
> -  if (from->get_info(&temp_master_log_pos,
> -                        (ulong) BIN_LOG_HEADER_SIZE) ||
> -      from->get_info(host, sizeof(host), 0) ||
> -      from->get_info(user, sizeof(user), "test") ||
> -      from->get_info(password, sizeof(password), 0) ||
> +  if (from->get_info(&temp_master_log_pos, (ulong) BIN_LOG_HEADER_SIZE) ||
> +      from->get_info(host, sizeof(host), (char *) 0) ||
> +      from->get_info(user, sizeof(user), (char *) "test") ||
> +      from->get_info(password, sizeof(password), (char *) 0) ||
>         from->get_info((int *)&port, (int) MYSQL_PORT) ||
> -      from->get_info((int *)&connect_retry,
> -                        (int) DEFAULT_CONNECT_RETRY))
> +      from->get_info((int *)&connect_retry, (int) DEFAULT_CONNECT_RETRY))
>         DBUG_RETURN(TRUE);
>
>     /*
> @@ -349,12 +351,12 @@ bool Master_info::read_info(Rpl_info_han
>     */
>     if (lines>= LINES_IN_MASTER_INFO_WITH_SSL)
>     {
> -    if (from->get_info(&temp_ssl, 0) ||
> -        from->get_info(ssl_ca, sizeof(ssl_ca), 0) ||
> -        from->get_info(ssl_capath, sizeof(ssl_capath), 0) ||
> -        from->get_info(ssl_cert, sizeof(ssl_cert), 0) ||
> -        from->get_info(ssl_cipher, sizeof(ssl_cipher), 0) ||
> -        from->get_info(ssl_key, sizeof(ssl_key), 0))
> +    if (from->get_info(&temp_ssl, (int) 0) ||
> +        from->get_info(ssl_ca, sizeof(ssl_ca), (char *) 0) ||
> +        from->get_info(ssl_capath, sizeof(ssl_capath), (char *) 0) ||
> +        from->get_info(ssl_cert, sizeof(ssl_cert), (char *) 0) ||
> +        from->get_info(ssl_cipher, sizeof(ssl_cipher), (char *) 0) ||
> +        from->get_info(ssl_key, sizeof(ssl_key), (char *) 0))
>         DBUG_RETURN(TRUE);
>     }
>
> @@ -364,7 +366,7 @@ bool Master_info::read_info(Rpl_info_han
>     */
>     if (lines>= LINE_FOR_MASTER_SSL_VERIFY_SERVER_CERT)
>     {
> -    if (from->get_info(&temp_ssl_verify_server_cert, 0))
> +    if (from->get_info(&temp_ssl_verify_server_cert, (int) 0))
>         DBUG_RETURN(TRUE);
>     }
>
> @@ -383,7 +385,7 @@ bool Master_info::read_info(Rpl_info_han
>     */
>     if (lines>= LINE_FOR_MASTER_BIND)
>     {
> -    if (from->get_info(bind_addr, sizeof(bind_addr), ""))
> +    if (from->get_info(bind_addr, sizeof(bind_addr), (char *) 0))
>         DBUG_RETURN(TRUE);
>     }
>
> @@ -400,7 +402,7 @@ bool Master_info::read_info(Rpl_info_han
>     /* Starting from 5.5 the master_uuid may be in the repository. */
>     if (lines>= LINE_FOR_MASTER_UUID)
>     {
> -    if (from->get_info(master_uuid, sizeof(master_uuid), 0))
> +    if (from->get_info(master_uuid, sizeof(master_uuid), (char *) 0))
>         DBUG_RETURN(TRUE);
>     }
>
> @@ -415,7 +417,7 @@ bool Master_info::read_info(Rpl_info_han
>     ssl= (my_bool) test(temp_ssl);
>     ssl_verify_server_cert= (my_bool) test(temp_ssl_verify_server_cert);
>     master_log_pos= (my_off_t) temp_master_log_pos;
> -#ifndef HAVE_OPENSSL
> +#ifdef HAVE_OPENSSL
>     if (ssl)
>       sql_print_warning("SSL information in the master info file "
>                         "are ignored because this MySQL slave was "
> @@ -436,11 +438,10 @@ bool Master_info::write_info(Rpl_info_ha
>        contents of file). But because of number of lines in the first line
>        of file we don't care about this garbage.
>     */
> -
>     if (to->prepare_info_for_write() ||
>         to->set_info((int) LINES_IN_MASTER_INFO) ||
>         to->set_info(master_log_name) ||
> -      to->set_info((ulong)master_log_pos) ||
> +      to->set_info((ulong) master_log_pos) ||
>         to->set_info(host) ||
>         to->set_info(user) ||
>         to->set_info(password) ||
> @@ -452,7 +453,7 @@ bool Master_info::write_info(Rpl_info_ha
>         to->set_info(ssl_cert) ||
>         to->set_info(ssl_cipher) ||
>         to->set_info(ssl_key) ||
> -      to->set_info(ssl_verify_server_cert) ||
> +      to->set_info((int) ssl_verify_server_cert) ||
>         to->set_info(heartbeat_period) ||
>         to->set_info(bind_addr) ||
>         to->set_info(ignore_server_ids) ||
> @@ -465,4 +466,55 @@ bool Master_info::write_info(Rpl_info_ha
>
>     DBUG_RETURN(FALSE);
>   }
> +
> +bool Master_info::set_password(const char* password_arg,
> +                               int password_arg_size __attribute__((unused)))
> +{
> +  bool ret= TRUE;
> +  DBUG_ENTER("Master_info::set_password");
> +
> +  if (password_arg&&  start_user_configured)
> +  {
> +    strmake(start_password, password_arg, sizeof(start_password) - 1);
> +    ret= FALSE;
> +  }
> +  else if (password_arg)
> +  {
> +    strmake(password, password_arg, sizeof(password) - 1);
> +    ret= FALSE;
> +  }
> +  DBUG_RETURN(ret);
> +}
> +
> +bool Master_info::get_password(char *password_arg, int *password_arg_size)
> +{
> +  bool ret= TRUE;
> +  DBUG_ENTER("Master_info::get_password");
> +
> +  if (password_arg&&  start_user_configured)
> +  {
> +    *password_arg_size= strlen(start_password);
> +    strmake(password_arg, start_password, sizeof(start_password) - 1);
> +    ret= FALSE;
> +  }
> +  else if (password_arg)
> +  {
> +    *password_arg_size= strlen(password);
> +    strmake(password_arg, password, sizeof(password) - 1);
> +    ret= FALSE;
> +  }
> +  DBUG_RETURN(ret);
> +}
> +
> +void Master_info::clean_start_information()
> +{
> +  DBUG_ENTER("Master_info::clean_start_information");
> +  start_plugin_configured= FALSE;
> +  start_plugin_auth[0]= 0;
> +  start_plugin_dir[0]= 0;
> +  start_user_configured= FALSE;
> +  start_user[0]= 0;
> +  start_password[0]= 0;
> +  DBUG_VOID_RETURN;
> +}
>   #endif /* HAVE_REPLICATION */
>
> === modified file 'sql/rpl_mi.h'
> --- a/sql/rpl_mi.h	2011-04-28 16:50:10 +0000
> +++ b/sql/rpl_mi.h	2011-06-08 12:48:53 +0000
> @@ -63,13 +63,195 @@ class Rpl_info_factory;
>
>   class Master_info : public Rpl_info
>   {
> +public:
>     friend class Rpl_info_factory;
>
> +  /**
> +    Stores a fake command that replaces thd->query() if START SLAVE was
> +    used with USER/PASSWORD.
> +  */
> +  char start_command[FN_REFLEN + 1];
> +  /**
> +    Host name or ip address stored in the master.info.
> +  */
> +  char host[HOSTNAME_LENGTH + 1];
> +
> +private:
> +  /**
> +    If true, USER/PASSWORD was specified while running START SLAVE.
> +  */
> +  bool start_user_configured;
> +  /**
> +    If true, DEFAULT_AUTH/PLUGIN_DIR was specified while running
> +    START SLAVE.
> +  */
> +  bool start_plugin_configured;
> +  /**
> +    User's name stored in the master.info.
> +  */
> +  char user[USERNAME_LENGTH + 1];
> +  /**
> +    User's password stored in the master.info.
> +  */
> +  char password[MAX_PASSWORD_LENGTH + 1];
> +  /**
> +    User specified while running START SLAVE.
> +  */
> +  char start_user[USERNAME_LENGTH + 1];
> +  /**
> +    Password specified while running START SLAVE.
> +  */
> +  char start_password[MAX_PASSWORD_LENGTH + 1];
> +  /**
> +    Stores the autentication plugin specified while running START SLAVE.
> +  */
> +  char start_plugin_auth[FN_REFLEN + 1];
> +  /**
> +    Stores the autentication plugin directory specified while running
> +    START SLAVE.
> +  */
> +  char start_plugin_dir[FN_REFLEN + 1];
> +
>   public:
> -  /* the variables below are needed because we can change masters on the fly */
> -  char host[HOSTNAME_LENGTH+1];
> -  char user[USERNAME_LENGTH+1];
> -  char password[MAX_PASSWORD_LENGTH+1];
> +  /**
> +    Returns if USER/PASSWORD was specified while running
> +    START SLAVE.
> +
> +    @return true or false.
> +  */
> +  bool is_start_user_configured() const
> +  {
> +    return start_user_configured;
> +  }
> +  /**
> +    Returns if DEFAULT_AUTH/PLUGIN_DIR was specified while running
> +    START SLAVE.
> +
> +    @return true or false.
> +  */
> +  bool is_start_plugin_configured() const
> +  {
> +    return start_plugin_configured;
> +  }
> +  /**
> +    Defines that USER/PASSWORD was specified or not while running
> +    START SLAVE.
> +
> +    @param config is true or false.
> +  */
> +  void set_start_user_configured(bool config)
> +  {
> +    start_user_configured= config;
> +  }
> +  /**
> +    Defines that DEFAULT_AUTH/PLUGIN_DIR was specified or not while
> +    running START SLAVE.
> +
> +    @param config is true or false.
> +  */
> +  void set_start_plugin_configured(bool config)
> +  {
> +    start_plugin_configured= config;
> +  }
> +  /**
> +    Sets either user's name in the master.info repository when CHANGE
> +    MASTER is executed or user's name used in START SLAVE if USER is
> +    specified.
> +
> +    @param user_arg is user's name.
> +  */
> +  void set_user(const char* user_arg)
> +  {
> +    if (user_arg&&  start_user_configured)
> +    {
> +      strmake(start_user, user_arg, sizeof(start_user) - 1);
> +    }
> +    else if (user_arg)
> +    {
> +      strmake(user, user_arg, sizeof(user) - 1);
> +    }
> +  }
> +  /*
> +    Returns user's size name. See @code get_user().
> +
> +    @return user's size name.
> +  */
> +  size_t get_user_size() const
> +  {
> +    return (start_user_configured ? sizeof(start_user) : sizeof(user));
> +  }
> +  /**
> +    If an user was specified while running START SLAVE, this function returns
> +    such user. Otherwise, it returns the user stored in master.info.
> +
> +    @return user's name.
> +  */
> +  const char *get_user() const
> +  {
> +    return start_user_configured ? start_user : user;
> +  }
> +  /**
> +    Stores either user's password in the master.info repository when CHANGE
> +    MASTER is executed or user's password used in START SLAVE if PASSWORD
> +    is specified.
> +
> +    @param password_arg is user's password.
> +    @param password_arg_size is password's size.
> +
> +    @return false if there is no error, otherwise true is returned.
> +  */
> +  bool set_password(const char* password_arg, int password_arg_size);
> +  /**
> +    Returns either user's password in the master.info repository or
> +    user's password used in START SLAVE.
> +
> +    @param password_arg is user's password.
> +
> +    @return false if there is no error, otherwise true is returned.
> +  */
> +  bool get_password(char *password_arg, int *password_arg_size);
> +  /**
> +    Clean in-memory password defined by START SLAVE.
> +  */
> +  void clean_start_information();
> +  /**
> +    Returns the DEFAULT_AUTH defined by START SLAVE.
> +
> +    @return DEFAULT_AUTH.
> +  */
> +  const char *get_start_plugin_auth()
> +  {
> +    return start_plugin_auth;
> +  }
> +  /**
> +    Returns the PLUGIN_DIR defined by START SLAVE.
> +
> +    @return PLUGIN_DIR.
> +  */
> +  const char *get_start_plugin_dir()
> +  { return start_plugin_dir;
> +  }
> +  /**
> +    Stores the DEFAULT_AUTH defined by START SLAVE.
> +
> +    @param DEFAULT_AUTH.
> +  */
> +  void set_plugin_auth(const char* src)
> +  {
> +    if (src)
> +      strmake(start_plugin_auth, src, sizeof(start_plugin_auth) - 1);
> +  }
> +  /**
> +    Stores the DEFAULT_AUTH defined by START SLAVE.
> +
> +    @param DEFAULT_AUTH.
> +  */
> +  void set_plugin_dir(const char* src)
> +  {
> +    if (src)
> +      strmake(start_plugin_dir, src, sizeof(start_plugin_dir) - 1);
> +  }
> +
>     my_bool ssl; // enables use of SSL connection if true
>     char ssl_ca[FN_REFLEN], ssl_capath[FN_REFLEN], ssl_cert[FN_REFLEN];
>     char ssl_cipher[FN_REFLEN], ssl_key[FN_REFLEN];
>
> === modified file 'sql/rpl_rli.cc'
> --- a/sql/rpl_rli.cc	2011-06-07 08:55:52 +0000
> +++ b/sql/rpl_rli.cc	2011-06-08 12:48:53 +0000
> @@ -1464,7 +1464,7 @@ bool Relay_log_info::read_info(Rpl_info_
>       overwritten by the second row later.
>     */
>     if (from->prepare_info_for_read() ||
> -      from->get_info(group_relay_log_name, sizeof(group_relay_log_name), ""))
> +      from->get_info(group_relay_log_name, sizeof(group_relay_log_name), (char *)
> ""))
>       DBUG_RETURN(TRUE);
>
>     lines= strtoul(group_relay_log_name,&first_non_digit, 10);
> @@ -1473,23 +1473,23 @@ bool Relay_log_info::read_info(Rpl_info_
>         *first_non_digit=='\0'&&  lines>=
> LINES_IN_RELAY_LOG_INFO_WITH_DELAY)
>     {
>       /* Seems to be new format =>  read group relay log name */
> -    if (from->get_info(group_relay_log_name,  sizeof(group_relay_log_name), ""))
> +    if (from->get_info(group_relay_log_name, sizeof(group_relay_log_name), (char
> *) ""))
>         DBUG_RETURN(TRUE);
>     }
>     else
>        DBUG_PRINT("info", ("relay_log_info file is in old format."));
>
>     if (from->get_info((ulong *)&temp_group_relay_log_pos,
> -                        (ulong) BIN_LOG_HEADER_SIZE) ||
> +                     (ulong) BIN_LOG_HEADER_SIZE) ||
>         from->get_info(group_master_log_name,
> -                        sizeof(group_relay_log_name), "") ||
> +                     sizeof(group_relay_log_name), (char *) "") ||
>         from->get_info((ulong *)&temp_group_master_log_pos,
> -                        (ulong) 0))
> +                     (ulong) 0))
>       DBUG_RETURN(TRUE);
>
>     if (lines>= LINES_IN_RELAY_LOG_INFO_WITH_DELAY)
>     {
> -    if (from->get_info((int *)&temp_sql_delay,(int) 0))
> +    if (from->get_info((int *)&temp_sql_delay, (int) 0))
>         DBUG_RETURN(TRUE);
>     }
>
>
> === modified file 'sql/rpl_slave.cc'
> --- a/sql/rpl_slave.cc	2011-05-21 08:25:33 +0000
> +++ b/sql/rpl_slave.cc	2011-06-08 12:48:53 +0000
> @@ -1979,7 +1979,7 @@ bool show_master_info(THD* thd, Master_i
>     field_list.push_back(new Item_empty_string("Master_Host",
>                                                        sizeof(mi->host)));
>     field_list.push_back(new Item_empty_string("Master_User",
> -                                                     sizeof(mi->user)));
> +                                                     mi->get_user_size()));
>     field_list.push_back(new Item_return_int("Master_Port", 7,
>                                              MYSQL_TYPE_LONG));
>     field_list.push_back(new Item_return_int("Connect_Retry", 10,
> @@ -2080,7 +2080,7 @@ bool show_master_info(THD* thd, Master_i
>       mysql_mutex_lock(&mi->rli->err_lock);
>
>       protocol->store(mi->host,&my_charset_bin);
> -    protocol->store(mi->user,&my_charset_bin);
> +    protocol->store(mi->get_user(),&my_charset_bin);
>       protocol->store((uint32) mi->port);
>       protocol->store((uint32) mi->connect_retry);
>       protocol->store(mi->get_master_log_name(),&my_charset_bin);
> @@ -3137,7 +3137,7 @@ pthread_handler_t handle_slave_io(void *
>     {
>       sql_print_information("Slave I/O thread: connected to master '%s@%s:%d',"
>                             "replication started in log '%s' at position %s",
> -                          mi->user, mi->host, mi->port,
> +                          mi->get_user(), mi->host, mi->port,
>   			  mi->get_io_rpl_log_name(),
>   			  llstr(mi->get_master_log_pos(), llbuff));
>     /*
> @@ -3413,7 +3413,11 @@ err:
>     write_ignored_events_info_to_relay_log(thd, mi);
>     THD_STAGE_INFO(thd, stage_waiting_for_slave_mutex_on_exit);
>     mysql_mutex_lock(&mi->run_lock);
> -
> +  /*
> +    Clean information used to start slave in order to avoid
> +    security issues.
> +  */
> +  mi->clean_start_information();
>     /* Forget the relay log's format */
>     delete mi->rli->relay_log.description_event_for_queue;
>     mi->rli->relay_log.description_event_for_queue= 0;
> @@ -4672,6 +4676,8 @@ static int connect_to_master(THD* thd, M
>     int last_errno= -2;                           // impossible error
>     ulong err_count=0;
>     char llbuff[22];
> +  char password[MAX_PASSWORD_LENGTH + 1];
> +  int password_size= sizeof(password);
>     DBUG_ENTER("connect_to_master");
>
>   #ifndef DBUG_OFF
> @@ -4708,10 +4714,30 @@ static int connect_to_master(THD* thd, M
>     /* This one is not strictly needed but we have it here for completeness */
>     mysql_options(mysql, MYSQL_SET_CHARSET_DIR, (char *) charsets_dir);
>
> -  while (!(slave_was_killed = io_slave_killed(thd,mi))&&
> -         (reconnect ? mysql_reconnect(mysql) != 0 :
> -          mysql_real_connect(mysql, mi->host, mi->user, mi->password, 0,
> -                             mi->port, 0, client_flag) == 0))
> +  if (mi->is_start_plugin_configured())
> +  {
> +    DBUG_PRINT("info", ("Slaving is using MYSQL_DEFAULT_AUTH %s",
> +                        mi->get_start_plugin_auth()));
> +    mysql_options(mysql, MYSQL_DEFAULT_AUTH, mi->get_start_plugin_auth());
> +  }
> +
> +  if (mi->is_start_plugin_configured())
> +  {
> +    DBUG_PRINT("info", ("Slaving is using MYSQL_PLUGIN_DIR %s",
> +                        mi->get_start_plugin_dir()));
> +    mysql_options(mysql, MYSQL_PLUGIN_DIR, mi->get_start_plugin_dir());
> +  }
> +
> +  if (!mi->is_start_user_configured())
> +    sql_print_warning("It is not recommend to store user and password in "
> +                      "the master info repository due to security issues. "
> +                      "Please, use START SLAVE USER=xxxx PASSWORD=yyyyy, "
> +                      "instead.");
> +  bool error_pwd= mi->get_password(password,&password_size);
> +  while (!error_pwd&&  !(slave_was_killed = io_slave_killed(thd,mi))
> +&&  (reconnect ? mysql_reconnect(mysql) != 0 :
> +             mysql_real_connect(mysql, mi->host, mi->get_user(),
> +                                password, 0, mi->port, 0, client_flag) == 0))
>     {
>       /*
>          SHOW SLAVE STATUS will display the number of retries which
> @@ -4724,7 +4750,7 @@ static int connect_to_master(THD* thd, M
>                  "error %s to master '%s@%s:%d'"
>                  " - retry-time: %d  retries: %lu",
>                  (reconnect ? "reconnecting" : "connecting"),
> -               mi->user, mi->host, mi->port,
> +               mi->get_user(), mi->host, mi->port,
>                  mi->connect_retry, err_count + 1);
>       /*
>         By default we try forever. The reason is that failure will trigger
> @@ -4747,7 +4773,7 @@ static int connect_to_master(THD* thd, M
>       {
>         if (!suppress_warnings&&  global_system_variables.log_warnings)
>           sql_print_information("Slave: connected to master '%s@%s:%d',\
> -replication resumed in log '%s' at position %s", mi->user,
> +replication resumed in log '%s' at position %s", mi->get_user(),
>                           mi->host, mi->port,
>                           mi->get_io_rpl_log_name(),
>                           llstr(mi->get_master_log_pos(),llbuff));
> @@ -4755,7 +4781,7 @@ replication resumed in log '%s' at posit
>       else
>       {
>         general_log_print(thd, COM_CONNECT_OUT, "%s@%s:%d",
> -                        mi->user, mi->host, mi->port);
> +                        mi->get_user(), mi->host, mi->port);
>       }
>   #ifdef SIGNAL_WITH_VIO_CLOSE
>       thd->set_active_vio(mysql->net.vio);
> @@ -4783,9 +4809,22 @@ static int safe_reconnect(THD* thd, MYSQ
>   }
>
>
> +/*
> +  We need to refactor this function and get rid of this duplicated code.
> +  Howerver, I need to check this with Jasonh in order to understand what
> +  was the intentation behind the code because the comments are not clear
> +  and the function is not used.
> +
> +  This is a callback function that is part of the replication interface/
> +  library and currently there is no plugin calling it.
> +
> +  \Alfranio
> +*/
>   MYSQL *rpl_connect_master(MYSQL *mysql)
>   {
>     THD *thd= current_thd;
> +  char password[MAX_PASSWORD_LENGTH + 1];
> +  int password_size= sizeof(password);
>     Master_info *mi= my_pthread_getspecific_ptr(Master_info*, RPL_MASTER_INFO);
>     if (!mi)
>     {
> @@ -4839,9 +4878,30 @@ MYSQL *rpl_connect_master(MYSQL *mysql)
>     /* This one is not strictly needed but we have it here for completeness */
>     mysql_options(mysql, MYSQL_SET_CHARSET_DIR, (char *) charsets_dir);
>
> -  if (io_slave_killed(thd, mi)
> -      || !mysql_real_connect(mysql, mi->host, mi->user, mi->password, 0,
> -                             mi->port, 0, 0))
> +  if (mi->is_start_plugin_configured())
> +  {
> +    DBUG_PRINT("info", ("Slaving is using MYSQL_DEFAULT_AUTH %s",
> +                        mi->get_start_plugin_auth()));
> +    mysql_options(mysql, MYSQL_DEFAULT_AUTH, mi->get_start_plugin_auth());
> +  }
> +
> +  if (mi->is_start_plugin_configured())
> +  {
> +    DBUG_PRINT("info", ("Slaving is using MYSQL_PLUGIN_DIR %s",
> +                        mi->get_start_plugin_dir()));
> +    mysql_options(mysql, MYSQL_PLUGIN_DIR, mi->get_start_plugin_dir());
> +  }
> +
> +  if (!mi->is_start_user_configured())
> +    sql_print_warning("It is not recommend to store user and password in "
> +                      "the master info repository due to security issues. "
> +                      "Please, use START SLAVE USER=xxxx PASSWORD=yyyyy, "
> +                      "instead.");
> +
> +  if (mi->get_password(password,&password_size)
> +      || io_slave_killed(thd, mi)
> +      || !mysql_real_connect(mysql, mi->host, mi->get_user(),
> +                             password, 0, mi->port, 0, 0))
>     {
>       if (!io_slave_killed(thd, mi))
>         sql_print_error("rpl_connect_master: error connecting to master: %s
> (server_error: %d)",
> @@ -5498,6 +5558,16 @@ int start_slave(THD* thd , Master_info*
>
>     if (check_access(thd, SUPER_ACL, any_db, NULL, NULL, 0, 0))
>       DBUG_RETURN(1);
> +
> +#ifdef HAVE_OPENSSL
> +  if (mi->is_start_user_configured()&&  thd->vio_ok()&&
> +      !thd->net.vio->ssl_arg)
> +  {
> +    sql_print_warning("Due to security issues one must use SSL/TSL while "
> +                      "specifying START SLAVE USER=xxx PASSWORD=yyy.");
> +  }
> +#endif
> +
>     lock_slave_threads(mi);  // this allows us to cleanly read slave_running
>     // Get a mask of _stopped_ threads
>     init_thread_mask(&thread_mask,mi,1 /* inverse */);
> @@ -5832,14 +5902,30 @@ bool change_master(THD* thd, Master_info
>     }
>     DBUG_PRINT("info", ("master_log_pos: %lu", (ulong)
> mi->get_master_log_pos()));
>
> +  if (lex_mi->user)
> +    mi->set_user(lex_mi->user);
> +  if (lex_mi->password)
> +  {
> +    sql_print_warning("It is not recommend to store user and password in "
> +                      "the master info repository due to security issues. "
> +                      "Please, use START SLAVE USER=xxx PASSWORD=yyy, "
> +                      "instead.");
> +    if (mi->set_password(lex_mi->password, strlen(lex_mi->password)))
> +    {
> +      /*
> +        After implementing WL#5769, we should create a better error message
> +        to denote that the call may have failed due to an error while trying
> +        to encrypt/store the password in a secure key store.
> +      */
> +      my_message(ER_MASTER_INFO, ER(ER_MASTER_INFO), MYF(0));
> +      ret= TRUE;
> +      goto err;
> +    }
> +  }
>     if (lex_mi->host)
>       strmake(mi->host, lex_mi->host, sizeof(mi->host)-1);
>     if (lex_mi->bind_addr)
>       strmake(mi->bind_addr, lex_mi->bind_addr, sizeof(mi->bind_addr)-1);
> -  if (lex_mi->user)
> -    strmake(mi->user, lex_mi->user, sizeof(mi->user)-1);
> -  if (lex_mi->password)
> -    strmake(mi->password, lex_mi->password, sizeof(mi->password)-1);
>     if (lex_mi->port)
>       mi->port = lex_mi->port;
>     if (lex_mi->connect_retry)
>
> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h	2011-05-12 17:29:19 +0000
> +++ b/sql/sql_lex.h	2011-06-08 12:48:53 +0000
> @@ -984,6 +984,14 @@ private:
>     Alter_info(const Alter_info&rhs);            // not implemented
>   };
>
> +typedef struct struct_slave_connection
> +{
> +  char *user;
> +  char *password;
> +  char *plugin_auth;
> +  char *plugin_dir;
> +} LEX_SLAVE_CONNECTION;
> +
>   struct st_sp_chistics
>   {
>     LEX_STRING comment;
> @@ -2103,6 +2111,7 @@ struct LEX: public Query_tables_list
>     HA_CREATE_INFO create_info;
>     KEY_CREATE_INFO key_create_info;
>     LEX_MASTER_INFO mi;				// used by CHANGE MASTER
> +  LEX_SLAVE_CONNECTION slave_connection;
>     LEX_SERVER_OPTIONS server_options;
>     USER_RESOURCES mqh;
>     ulong type;
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc	2011-05-30 06:25:47 +0000
> +++ b/sql/sql_parse.cc	2011-06-08 12:48:53 +0000
> @@ -2708,7 +2708,40 @@ end_with_restore_list:
>     case SQLCOM_SLAVE_START:
>     {
>       mysql_mutex_lock(&LOCK_active_mi);
> -    start_slave(thd,active_mi,1 /* net report*/);
> +
> +    if (lex->slave_connection.user)
> +    {
> +      active_mi->set_start_user_configured(TRUE);
> +      active_mi->set_user(lex->slave_connection.user);
> +    }
> +    if (lex->slave_connection.password)
> +    {
> +      thd->set_query(active_mi->start_command,
> +                     strlen(active_mi->start_command));
> +      active_mi->set_start_user_configured(TRUE);
> +      active_mi->set_password(lex->slave_connection.password,
> +                              strlen(lex->slave_connection.password));
> +    }
> +    if (lex->slave_connection.plugin_auth)
> +    {
> +      active_mi->set_start_plugin_configured(TRUE);
> +      active_mi->set_plugin_auth(lex->slave_connection.plugin_auth);
> +    }
> +    if (lex->slave_connection.plugin_dir)
> +    {
> +      active_mi->set_start_plugin_configured(TRUE);
> +      active_mi->set_plugin_dir(lex->slave_connection.plugin_dir);
> +    }
> +
> +    if (start_slave(thd, active_mi, 1 /* net report*/))
> +    {
> +      /*
> +        Clean information used to start slave in order to avoid
> +        security issues.
> +      */
> +      active_mi->clean_start_information();
> +    }
> +
>       mysql_mutex_unlock(&LOCK_active_mi);
>       break;
>     }
>
> === modified file 'sql/sql_yacc.yy'
> --- a/sql/sql_yacc.yy	2011-05-26 15:20:09 +0000
> +++ b/sql/sql_yacc.yy	2011-06-08 12:48:53 +0000
> @@ -921,6 +921,7 @@ bool my_yyoverflow(short **a, YYSTYPE **
>   %token  DECIMAL_SYM                   /* SQL-2003-R */
>   %token  DECLARE_SYM                   /* SQL-2003-R */
>   %token  DEFAULT                       /* SQL-2003-R */
> +%token  DEFAULT_AUTH_SYM              /* INTERNAL */
>   %token  DEFINER_SYM
>   %token  DELAYED_SYM
>   %token  DELAY_KEY_WRITE_SYM
> @@ -1178,8 +1179,9 @@ bool my_yyoverflow(short **a, YYSTYPE **
>   %token  PARTITIONING_SYM
>   %token  PASSWORD
>   %token  PHASE_SYM
> -%token  PLUGINS_SYM
> +%token  PLUGIN_DIR_SYM                /* INTERNAL */
>   %token  PLUGIN_SYM
> +%token  PLUGINS_SYM
>   %token  POINT_SYM
>   %token  POLYGON
>   %token  PORT_SYM
> @@ -6914,6 +6916,8 @@ slave:
>             }
>             slave_until
>             {}
> +          slave_connection_opts
> +          {}
>           | STOP_SYM SLAVE slave_thread_opts
>             {
>               LEX *lex=Lex;
> @@ -6940,6 +6944,50 @@ start_transaction_opts:
>             }
>           ;
>
> +slave_connection_opts:
> +          slave_user_name_opt slave_user_pass_opt
> +          slave_plugin_auth_opt slave_plugin_dir_opt
> +        ;
> +
> +slave_user_name_opt:
> +          {
> +            Lex->slave_connection.user= 0;
> +          }
> +        | USER EQ TEXT_STRING_sys
> +          {
> +            Lex->slave_connection.user= $3.str;
> +          }
> +        ;
> +
> +slave_user_pass_opt:
> +          {
> +            Lex->slave_connection.password= 0;
> +          }
> +        | PASSWORD EQ TEXT_STRING_sys
> +          {
> +            Lex->slave_connection.password= $3.str;
> +          }
> +
> +slave_plugin_auth_opt:
> +          {
> +            Lex->slave_connection.plugin_auth= 0;
> +          }
> +        | DEFAULT_AUTH_SYM EQ TEXT_STRING_sys
> +          {
> +            Lex->slave_connection.plugin_auth= $3.str;
> +          }
> +        ;
> +
> +slave_plugin_dir_opt:
> +          {
> +            Lex->slave_connection.plugin_dir= 0;
> +          }
> +        | PLUGIN_DIR_SYM EQ TEXT_STRING_sys
> +          {
> +            Lex->slave_connection.plugin_dir= $3.str;
> +          }
> +        ;
> +
>   slave_thread_opts:
>             { Lex->slave_thd_opt= 0; }
>             slave_thread_opt_list
> @@ -12643,6 +12691,7 @@ keyword_sp:
>           | DATETIME                 {}
>           | DATE_SYM                 {}
>           | DAY_SYM                  {}
> +        | DEFAULT_AUTH_SYM         {}
>           | DEFINER_SYM              {}
>           | DELAY_KEY_WRITE_SYM      {}
>           | DES_KEY_FILE             {}
> @@ -12767,6 +12816,7 @@ keyword_sp:
>           | PARTITIONS_SYM           {}
>           | PASSWORD                 {}
>           | PHASE_SYM                {}
> +        | PLUGIN_DIR_SYM           {}
>           | PLUGIN_SYM               {}
>           | PLUGINS_SYM              {}
>           | POINT_SYM                {}
>
>
>
>
>

Thread
bzr commit into mysql-trunk branch (alfranio.correia:3167) WL#4143Alfranio Correia8 Jun
  • Re: bzr commit into mysql-trunk branch (alfranio.correia:3167) WL#4143Luís Soares4 Jul