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 {}
>
>
>
>
>
