List:Commits« Previous MessageNext Message »
From:Luís Soares Date:March 5 2009 12:52am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (gni:2827) Bug#42217
View as plain text  
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

Thread
bzr commit into mysql-5.1-bugteam branch (gni:2827) Bug#42217Guangbao Ni4 Mar
  • Re: bzr commit into mysql-5.1-bugteam branch (gni:2827) Bug#42217Luís Soares5 Mar
    • Re: bzr commit into mysql-5.1-bugteam branch (gni:2827) Bug#42217Guangbao Ni5 Mar