Hi Guangbao,
Good work.
I have some comments, please check below.
On Wed, 2009-03-04 at 14:16 +0000, Guangbao Ni wrote:
> #At file:///home/ngb/mysql/bzr/bug42217-5.1-bugteam/
>
> 2827 Guangbao Ni 2009-03-04
> Bug #42217 mysql.procs_priv does not get replicated
>
> mysql.procs_priv table itself does not get replicated.
> Inserting routine privilege record into mysql.procs_priv table
> is triggered by creating function/procedure statements
> according to current user's privileges.
> Because the current user of SQL thread has GLOBAL_ACL,
> which doesn't need any check mysql.procs_priv privilege
> when create/alter/execute routines.
> Corresponding GLOBAL_ACL privilege user
> doesn't insert routine privilege record into
> mysql.procs_priv when creating a routine.
>
> Fixed by switching the current user of SQL thread to definer user if
> the definer user exists on slave.
> That populates procs_priv, otherwise to keep the SQL thread
> user and procs_priv remains unchanged.
> modified:
> mysql-test/suite/rpl/r/rpl_do_grant.result
> mysql-test/suite/rpl/t/rpl_do_grant.test
> sql/sql_parse.cc
>
> per-file messages:
> mysql-test/suite/rpl/r/rpl_do_grant.result
> Test case result for routine privilege when definer user exist or not on slave
> mysql-test/suite/rpl/t/rpl_do_grant.test
> Test case result for routine privilege when definer user exist or not on slave
> sql/sql_parse.cc
> Switch current user of SQL thread to definer user if the definer user
> existes on slave when checking whether the routine privilege is
> needed to insert mysql.procs_priv table or not.
> === modified file 'mysql-test/suite/rpl/r/rpl_do_grant.result'
> --- a/mysql-test/suite/rpl/r/rpl_do_grant.result 2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_do_grant.result 2009-03-04 14:15:22 +0000
> @@ -89,3 +89,73 @@ show grants for rpl_do_grant2@localhost;
> ERROR 42000: There is no such grant defined for user 'rpl_do_grant2' on host
> 'localhost'
> show grants for rpl_do_grant2@localhost;
> ERROR 42000: There is no such grant defined for user 'rpl_do_grant2' on host
> 'localhost'
> +DROP DATABASE IF EXISTS privdb;
SUGGESTION:
probably protect this with disable_warnings;
> +Warnings:
> +Note 1008 Can't drop database 'privdb'; database doesn't exist
> +CREATE DATABASE privdb;
> +GRANT CREATE ROUTINE ON privdb.* TO 'create_rout_db'@'localhost'
> + IDENTIFIED BY 'create_rout_db' WITH GRANT OPTION;
> +USE privdb;
> +CREATE FUNCTION upgrade_del_func() RETURNS CHAR(30)
> +BEGIN
> +RETURN "INSIDE upgrade_del_func()";
> +END//
> +USE privdb;
> +SELECT * FROM mysql.procs_priv;
> +Host Db User Routine_name Routine_type Grantor Proc_priv Timestamp
>
> +localhost privdb create_rout_db upgrade_del_func FUNCTION create_rout_db@localhost Execute,Alter
> Routine #
> +SELECT upgrade_del_func();
> +upgrade_del_func()
> +INSIDE upgrade_del_func()
> +SELECT * FROM mysql.procs_priv;
> +Host Db User Routine_name Routine_type Grantor Proc_priv Timestamp
>
> +localhost privdb create_rout_db upgrade_del_func FUNCTION create_rout_db@localhost Execute,Alter
> Routine #
> +SHOW GRANTS FOR 'create_rout_db'@'localhost';
> +Grants for create_rout_db@localhost
> +GRANT USAGE ON *.* TO 'create_rout_db'@'localhost' IDENTIFIED BY PASSWORD
> '*08792480350CBA057BDE781B9DF183B263934601'
> +GRANT CREATE ROUTINE ON `privdb`.* TO 'create_rout_db'@'localhost' WITH GRANT
> OPTION
> +GRANT EXECUTE, ALTER ROUTINE ON FUNCTION `privdb`.`upgrade_del_func` TO
> 'create_rout_db'@'localhost'
> +USE privdb;
> +SHOW CREATE FUNCTION upgrade_del_func;
> +Function sql_mode Create Function character_set_client collation_connection Database
> Collation
> +upgrade_del_func CREATE DEFINER=`create_rout_db`@`localhost` FUNCTION
> `upgrade_del_func`() RETURNS char(30) CHARSET latin1
> +BEGIN
> +RETURN "INSIDE upgrade_del_func()";
> +END latin1 latin1_swedish_ci latin1_swedish_ci
> +SELECT upgrade_del_func();
> +upgrade_del_func()
> +INSIDE upgrade_del_func()
> +"Check whether the definer user will be able to execute the replicated routine on
> slave"
> +USE privdb;
> +SHOW CREATE FUNCTION upgrade_del_func;
> +Function sql_mode Create Function character_set_client collation_connection Database
> Collation
> +upgrade_del_func CREATE DEFINER=`create_rout_db`@`localhost` FUNCTION
> `upgrade_del_func`() RETURNS char(30) CHARSET latin1
> +BEGIN
> +RETURN "INSIDE upgrade_del_func()";
> +END latin1 latin1_swedish_ci latin1_swedish_ci
> +SELECT upgrade_del_func();
> +upgrade_del_func()
> +INSIDE upgrade_del_func()
> +"Test the user who creates a function on master doesn't exist on slave."
> +DROP USER 'create_rout_db'@'localhost';
> +CREATE FUNCTION upgrade_alter_func() RETURNS CHAR(30)
> +BEGIN
> +RETURN "INSIDE upgrade_alter_func()";
> +END//
> +SELECT upgrade_alter_func();
> +upgrade_alter_func()
> +INSIDE upgrade_alter_func()
> +SHOW CREATE FUNCTION upgrade_alter_func;
> +Function sql_mode Create Function character_set_client collation_connection Database
> Collation
> +upgrade_alter_func CREATE DEFINER=`create_rout_db`@`localhost` FUNCTION
> `upgrade_alter_func`() RETURNS char(30) CHARSET latin1
> +BEGIN
> +RETURN "INSIDE upgrade_alter_func()";
> +END latin1 latin1_swedish_ci latin1_swedish_ci
> +SELECT upgrade_alter_func();
> +ERROR HY000: The user specified as a definer ('create_rout_db'@'localhost') does not
> exist
> +USE privdb;
> +DROP FUNCTION upgrade_del_func;
> +DROP FUNCTION upgrade_alter_func;
> +DROP DATABASE privdb;
> +DROP USER 'create_rout_db'@'localhost';
> +"End of test"
>
> === modified file 'mysql-test/suite/rpl/t/rpl_do_grant.test'
> --- a/mysql-test/suite/rpl/t/rpl_do_grant.test 2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_do_grant.test 2009-03-04 14:15:22 +0000
> @@ -112,3 +112,86 @@ show grants for rpl_do_grant2@localhost;
> sync_slave_with_master;
> --error 1141
> show grants for rpl_do_grant2@localhost;
> +
> +#####################################################
> +# Purpose
> +# Test whether mysql.procs_priv get replicated
> +# Related bugs:
> +# BUG42217 mysql.procs_priv does not get replicated
> +#####################################################
> +connection master;
> +
> +DROP DATABASE IF EXISTS privdb;
> +CREATE DATABASE privdb;
> +
SUGGESTION:
I usually create databases with name like: bug_42217 . It becomes
easier to understand what is happening if these are left lying around
(by any reason: test is not cleaning properly, somebody mistakenly
erases part of the clean up, etc...).
> +GRANT CREATE ROUTINE ON privdb.* TO 'create_rout_db'@'localhost'
> + IDENTIFIED BY 'create_rout_db' WITH GRANT OPTION;
> +
> +connect (create_rout_db_master, localhost, create_rout_db, create_rout_db,
> privdb,$MASTER_MYPORT,);
> +connect (create_rout_db_slave, localhost, create_rout_db, create_rout_db, privdb,
> $SLAVE_MYPORT,);
> +connection create_rout_db_master;
> +
> +
> +USE privdb;
> +
> +DELIMITER //;
> +CREATE FUNCTION upgrade_del_func() RETURNS CHAR(30)
> +BEGIN
> + RETURN "INSIDE upgrade_del_func()";
> +END//
> +
> +DELIMITER ;//
> +
> +connection master;
> +
> +USE privdb;
> +--replace_column 8 #
> +SELECT * FROM mysql.procs_priv;
> +SELECT upgrade_del_func();
> +
> +sync_slave_with_master;
> +--replace_column 8 #
> +SELECT * FROM mysql.procs_priv;
> +SHOW GRANTS FOR 'create_rout_db'@'localhost';
> +
> +USE privdb;
> +SHOW CREATE FUNCTION upgrade_del_func;
> +
> +SELECT upgrade_del_func();
> +
> +--echo "Check whether the definer user will be able to execute the replicated
> routine on slave"
> +connection create_rout_db_slave;
> +USE privdb;
> +SHOW CREATE FUNCTION upgrade_del_func;
> +
> +SELECT upgrade_del_func();
> +
> +connection slave;
Can you expand your comments on the test below? Maybe it's me, but I
find it a bit confusing and am not 100% sure what is being tested.
I think that the case you are testing is: checking that there is no
definer user on slave, thence SQL thread ACL_GLOBAL privilege jumps in
and no mysql.procs_priv is inserted. Right?
> +--echo "Test the user who creates a function on master doesn't exist on slave."
> +DROP USER 'create_rout_db'@'localhost';
> +
> +connection create_rout_db_master;
> +DELIMITER //;
> +CREATE FUNCTION upgrade_alter_func() RETURNS CHAR(30)
> +BEGIN
> + RETURN "INSIDE upgrade_alter_func()";
> +END//
> +DELIMITER ;//
> +
> +connection master;
> +SELECT upgrade_alter_func();
> +
> +sync_slave_with_master;
> +SHOW CREATE FUNCTION upgrade_alter_func;
> +--error 1449
> +SELECT upgrade_alter_func();
> +
> +###### CLEAN UP SECTION ##############
> +connection master;
> +USE privdb;
> +DROP FUNCTION upgrade_del_func;
> +DROP FUNCTION upgrade_alter_func;
> +DROP DATABASE privdb;
> +DROP USER 'create_rout_db'@'localhost';
You should probably close (disconnect) the opened connections as well
(create_rout_db_master, create_rout_db_slave).
> +
> +--echo "End of test"
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2009-02-16 11:38:15 +0000
> +++ b/sql/sql_parse.cc 2009-03-04 14:15:22 +0000
> @@ -4079,6 +4079,8 @@ end_with_restore_list:
> uint namelen;
> char *name;
> int sp_result= SP_INTERNAL_ERROR;
> + Security_context security_context;
> + bool restore_backup_context= false;
>
> DBUG_ASSERT(lex->sphead != 0);
> DBUG_ASSERT(lex->sphead->m_db.str); /* Must be initialized in the parser
> */
> @@ -4132,6 +4134,27 @@ end_with_restore_list:
> case SP_OK:
> #ifndef NO_EMBEDDED_ACCESS_CHECKS
> /* only add privileges if really neccessary */
This did not build on my machine (BUILD/compile-pentium-debug-max).
The error I get is:
(...)
sql_parse.cc: In function ‘int mysql_execute_command(THD*)’:
sql_parse.cc:4181: error: jump to case label
sql_parse.cc:4139: error: crosses initialization of ‘LEX_USER*
definer’
(...)
I think you may be hitting a problem with the scope of LEX_USER which
prevents the "switch" to skip over its initialization. You can probably
solve this by delimiting its scope (inside the "case SP_OK:") using
curly braces.
> +
> + Security_context *backup;
> + LEX_USER *definer = thd->lex->definer;
Coding guidelines mandates that there should be no space between
variable name and "=".
fix:
LEX_USER *definer= thd->lex->definer;
> + /*
> + Check if the definer exists on slave,
> + then use definer privilege to insert routine privileges to
> mysql.procs_priv.
> +
> + For current user of SQL thread has GLOBAL_ACL privilege,
> + which doesn't any check routine privileges,
> + so no routine privilege record will insert into mysql.procs_priv.
> + */
> + if (thd->slave_thread && is_acl_user(definer->host.str,
> definer->user.str))
> + {
> + security_context.change_security_context(thd,
> +
> &thd->lex->definer->user,
> +
> &thd->lex->definer->host,
> +
> &thd->lex->sphead->m_db,
> + &backup);
> + restore_backup_context= true;
> + }
> +
> if (sp_automatic_privileges && !opt_noacl &&
> check_routine_access(thd, DEFAULT_CREATE_PROC_ACLS,
> lex->sphead->m_db.str, name,
> @@ -4143,6 +4166,16 @@ end_with_restore_list:
> ER_PROC_AUTO_GRANT_FAIL,
> ER(ER_PROC_AUTO_GRANT_FAIL));
> }
> +
> + /*
> + Restore current user with GLOBAL_ACL privilege of SQL thread
> + */
> + if (restore_backup_context)
> + {
> + DBUG_ASSERT(thd->slave_thread == 1);
> + thd->security_ctx->restore_security_context(thd, backup);
> + }
> +
> #endif
> break;
> case SP_WRITE_ROW_FAILED:
>
Best Regards,
Luís
--
Luís
