bk commit into 6.0 tree (thek:1.2619) BUG#31501

[kpettersson]

Below is the list of changes that have just been committed into a local
6.0 repository of thek.  When thek does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-29 13:36:29+02:00, thek@adventure.(none) +7 -0
  Bug#31501 Stored Routines: droping stored procedure revokes all assotiated privileges
  
  When a routine was dropped all associated privileges were removed even
  though they weren't associated with the definer of the routine.
  
  This patch changes the behavior so that only the automatic privileges set
  by the definer of the routine are dropped when the routine is dropped.

  mysql-test/r/sp-destruct.result@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none)
+42 -0
    Added test case

  mysql-test/t/sp-destruct.test@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +45
-0
    Added test case

  sql/sp.cc@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +15 -1
    Remember the definer field of the target SP. This information is later
    used when revoking privileges set by automatic_sp_privileges.

  sql/sp.h@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +1 -1
    Changed signature of sp_drop_routine

  sql/sql_acl.cc@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +51 -20
    Revoke only those privileges which correspond to the definer of the 
    stored program.

  sql/sql_acl.h@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +1 -1
    Changed signature of sp_revoke_privileges

  sql/sql_parse.cc@stripped, 2008-04-29 13:36:25+02:00, thek@adventure.(none) +21 -20
    In order to avoid revoking privileges from other users than 
    the sp definer the order of execution has changed.
    1. Drop SP (remember definer) 2. Revoke SP privileges

diff -Nrup a/mysql-test/r/sp-destruct.result b/mysql-test/r/sp-destruct.result
--- a/mysql-test/r/sp-destruct.result	2007-06-28 19:34:47 +02:00
+++ b/mysql-test/r/sp-destruct.result	2008-04-29 13:36:25 +02:00
@@ -88,3 +88,45 @@ show procedure status;
 Db	Name	Type	Definer	Modified	Created	Security_type	Comment	character_set_client	collation_connection	Database
Collation
 show function status;
 Db	Name	Type	Definer	Modified	Created	Security_type	Comment	character_set_client	collation_connection	Database
Collation
+# 
+# Bug#31501 Stored Routines: droping stored procedure revokes all assotiated privileges
+#
+SET GLOBAL automatic_sp_privileges=ON;
+DROP PROCEDURE IF EXISTS test.bug31501_proc1;
+Warnings:
+Note	1305	PROCEDURE bug31501_proc1 does not exist
+create procedure bug31501_proc1 (OUT param1 INT) BEGIN
+SELECT COUNT(*) INTO param1 FROM t;
+END; //
+** Grant CREATE ROUTINE to user bug31501_user.
+GRANT CREATE ROUTINE ON test.* TO 'bug31501_user'@'localhost';
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+Grants for bug31501_user@localhost
+GRANT USAGE ON *.* TO 'bug31501_user'@'localhost'
+GRANT CREATE ROUTINE ON `test`.* TO 'bug31501_user'@'localhost'
+** Connecting as bug31501_user
+create procedure bug31501_proc2 (OUT param1 INT) BEGIN
+SELECT COUNT(*) INTO param1 FROM t;
+END; //
+** Creating SP and get automatic privileges (gaining EXECUTE,ALTER ROUTINE)
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+Grants for bug31501_user@localhost
+GRANT USAGE ON *.* TO 'bug31501_user'@'localhost'
+GRANT CREATE ROUTINE ON `test`.* TO 'bug31501_user'@'localhost'
+GRANT EXECUTE, ALTER ROUTINE ON PROCEDURE `test`.`bug31501_proc2` TO
'bug31501_user'@'localhost'
+** Dropping procedure with definer=root
+** This won't result in any grant changes because we don't remove grants from ther users
automatically.
+DROP PROCEDURE test.bug31501_proc1;
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+Grants for bug31501_user@localhost
+GRANT USAGE ON *.* TO 'bug31501_user'@'localhost'
+GRANT CREATE ROUTINE ON `test`.* TO 'bug31501_user'@'localhost'
+GRANT EXECUTE, ALTER ROUTINE ON PROCEDURE `test`.`bug31501_proc2` TO
'bug31501_user'@'localhost'
+** Not dropping procedure as definer=bug31501_user
+DROP PROCEDURE test.bug31501_proc2;
+** Execution grants for bug31501_user should now be revoked.
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+Grants for bug31501_user@localhost
+GRANT USAGE ON *.* TO 'bug31501_user'@'localhost'
+GRANT CREATE ROUTINE ON `test`.* TO 'bug31501_user'@'localhost'
+SET GLOBAL automatic_sp_privileges=DEFAULT;
diff -Nrup a/mysql-test/t/sp-destruct.test b/mysql-test/t/sp-destruct.test
--- a/mysql-test/t/sp-destruct.test	2007-08-07 11:40:51 +02:00
+++ b/mysql-test/t/sp-destruct.test	2008-04-29 13:36:25 +02:00
@@ -154,3 +154,48 @@ drop procedure bug14233_3;
 # Assert: These should show nothing.
 show procedure status;
 show function status;
+
+
+--echo # 
+--echo # Bug#31501 Stored Routines: droping stored procedure revokes all assotiated
privileges
+--echo #
+
+SET GLOBAL automatic_sp_privileges=ON;
+DROP PROCEDURE IF EXISTS test.bug31501_proc1;
+
+delimiter //;
+create procedure bug31501_proc1 (OUT param1 INT) BEGIN
+  SELECT COUNT(*) INTO param1 FROM t;
+END; //
+delimiter ;//
+
+--echo ** Grant CREATE ROUTINE to user bug31501_user.
+GRANT CREATE ROUTINE ON test.* TO 'bug31501_user'@'localhost';
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+
+--echo ** Connecting as bug31501_user
+--connect (bug31501_con,localhost,bug31501_user,,test,,)
+--connection bug31501_con
+delimiter //;
+create procedure bug31501_proc2 (OUT param1 INT) BEGIN
+  SELECT COUNT(*) INTO param1 FROM t;
+END; //
+delimiter ;//
+--echo ** Creating SP and get automatic privileges (gaining EXECUTE,ALTER ROUTINE)
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+
+--connection default
+
+--echo ** Dropping procedure with definer=root
+--echo ** This won't result in any grant changes because we don't remove grants from ther
users automatically.
+DROP PROCEDURE test.bug31501_proc1;
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+
+--echo ** Not dropping procedure as definer=bug31501_user
+DROP PROCEDURE test.bug31501_proc2;
+--echo ** Execution grants for bug31501_user should now be revoked.
+SHOW GRANTS FOR 'bug31501_user'@'localhost';
+--connection default
+--disconnect bug31501_con
+SET GLOBAL automatic_sp_privileges=DEFAULT;
+
diff -Nrup a/sql/sp.cc b/sql/sp.cc
--- a/sql/sp.cc	2008-04-08 18:35:58 +02:00
+++ b/sql/sp.cc	2008-04-29 13:36:25 +02:00
@@ -956,13 +956,15 @@ done:
   @param type Stored routine type
               (TYPE_ENUM_PROCEDURE or TYPE_ENUM_FUNCTION)
   @param name Stored routine name.
+  @param[out] definer The definer of the stored proceedure if it was found or
+                      null.
 
   @return Error code. SP_OK is returned on success. Other SP_ constants are
   used to indicate about errors.
 */
 
 int
-sp_drop_routine(THD *thd, int type, sp_name *name)
+sp_drop_routine(THD *thd, int type, sp_name *name, String *sp_definer)
 {
   TABLE *table;
   int ret;
@@ -984,6 +986,14 @@ sp_drop_routine(THD *thd, int type, sp_n
     DBUG_RETURN(SP_OPEN_TABLE_FAILED);
   if ((ret= db_find_routine_aux(thd, type, name, table)) == SP_OK)
   {
+    bool no_such_field= get_field(thd->mem_root,
+                                  table->field[MYSQL_PROC_FIELD_DEFINER],
+                                  sp_definer);
+    if (no_such_field)
+    {
+      ret= SP_DELETE_ROW_FAILED;
+      goto error;
+    }
     if (table->file->ha_delete_row(table->record[0]))
       ret= SP_DELETE_ROW_FAILED;
   }
@@ -994,6 +1004,7 @@ sp_drop_routine(THD *thd, int type, sp_n
     sp_cache_invalidate();
   }
 
+error:
   close_thread_tables(thd);
   DBUG_RETURN(ret);
 }
@@ -1349,7 +1360,9 @@ sp_exist_routines(THD *thd, TABLE_LIST *
   parsing the definition. (Used for dropping).
 
   @param thd          thread context
+  @param type         TYPE_ENUM_PROCEDURE or TYPE_ENUM_FUNCTION
   @param name         name of procedure
+  @param sp_definer[out] The user name of the 
 
   @retval
     0       Success
@@ -1372,6 +1385,7 @@ sp_routine_exists_in_table(THD *thd, int
       ret= SP_KEY_NOT_FOUND;
     close_system_tables(thd, &open_tables_state_backup);
   }
+
   return ret;
 }
 
diff -Nrup a/sql/sp.h b/sql/sp.h
--- a/sql/sp.h	2008-04-08 18:35:58 +02:00
+++ b/sql/sp.h	2008-04-29 13:36:25 +02:00
@@ -83,7 +83,7 @@ int
 sp_update_routine(THD *thd, int type, sp_name *name, st_sp_chistics *chistics);
 
 int
-sp_drop_routine(THD *thd, int type, sp_name *name);
+sp_drop_routine(THD *thd, int type, sp_name *name, String *sp_definer);
 
 /*
   Procedures for pre-caching of stored routines and building table list
diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
--- a/sql/sql_acl.cc	2008-04-14 13:30:04 +02:00
+++ b/sql/sql_acl.cc	2008-04-29 13:36:25 +02:00
@@ -2879,7 +2879,7 @@ static int replace_routine_table(THD *th
       if ((error=table->file->ha_update_row(table->record[1],
                                             table->record[0])) &&
           error != HA_ERR_RECORD_IS_THE_SAME)
-	goto table_error;
+        goto table_error;
     }
     else if ((error= table->file->ha_delete_row(table->record[1])))
       goto table_error;
@@ -6166,21 +6166,25 @@ Silence_routine_definer_errors::handle_e
   Revoke privileges for all users on a stored procedure.  Use an error handler
   that converts errors about missing grants into warnings.
 
-  @param
-    thd                         The current thread.
-  @param
-    db				DB of the stored procedure
-  @param
-    name			Name of the stored procedure
+  This method works in conjuction with the automatic_sp_privileges system
+  variable and it demands that this variable is set to ON.
 
-  @retval
-    0           OK.
-  @retval
-    < 0         Error. Error message not yet sent.
+  The method only removes privilages related to the user which created the sp
+  and got automatic privileges.
+
+  @param thd      The current thread.
+  @param sp_db    DB of the stored procedure
+  @param sp_name  Name of the stored procedure
+  @param is_proc  TRUE if the named sp is a PROCEDURE
+  @param definer_user Contains username@host of the sp definer.
+
+  @return Did an error occur?
+    @retval FALSE OK.
+    @retval TRUE  Error. Error message not yet sent.
 */
 
 bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name,
-                          bool is_proc)
+                          bool is_proc, String *definer_user)
 {
   uint counter, revoked;
   int result;
@@ -6189,15 +6193,17 @@ bool sp_revoke_privileges(THD *thd, cons
   Silence_routine_definer_errors error_handler;
   DBUG_ENTER("sp_revoke_privileges");
 
+  if (definer_user->length() <1)
+    DBUG_RETURN(0);
   if ((result= open_grant_tables(thd, tables)))
     DBUG_RETURN(result != 1);
 
-  /* Be sure to pop this before exiting this scope! */
-  thd->push_internal_handler(&error_handler);
-
   rw_wrlock(&LOCK_grant);
   VOID(pthread_mutex_lock(&acl_cache->lock));
-
+  /*
+    Be sure to pop this before exiting this scope!
+  */
+  thd->push_internal_handler(&error_handler);
   /*
     This statement will be replicated as a statement, even when using
     row-based replication.  The flag will be reset at the end of the
@@ -6205,14 +6211,30 @@ bool sp_revoke_privileges(THD *thd, cons
   */
   thd->clear_current_stmt_binlog_row_based();
 
-  /* Remove procedure access */
   do
   {
+    String at_sign;
+    at_sign.append("@");
+    String grant_proc_user;
+    String grant_proc_host;
+    int offset= definer_user->strstr(at_sign,0);
+    if (offset > 0)
+    {
+      grant_proc_user.set(*definer_user,0,offset);
+      grant_proc_host.set(*definer_user,offset+1,
+                              definer_user->length()-offset);
+    }
+    else
+    {
+      grant_proc_user.set(*definer_user,0,definer_user->length());
+    }
     for (counter= 0, revoked= 0 ; counter < hash->records ; )
     {
       GRANT_NAME *grant_proc= (GRANT_NAME*) hash_element(hash, counter);
       if (!my_strcasecmp(system_charset_info, grant_proc->db, sp_db) &&
-	  !my_strcasecmp(system_charset_info, grant_proc->tname, sp_name))
+          !my_strcasecmp(system_charset_info, grant_proc->tname, sp_name) &&
+          !my_strcasecmp(system_charset_info, grant_proc->user,
+                         grant_proc_user.c_ptr()))
       {
         LEX_USER lex_user;
 	lex_user.user.str= grant_proc->user;
@@ -6222,6 +6244,16 @@ bool sp_revoke_privileges(THD *thd, cons
 	lex_user.host.length= grant_proc->host.hostname ?
 	  strlen(grant_proc->host.hostname) : 0;
 
+        /* Make sure host name matches */
+        if (my_strcasecmp(system_charset_info, grant_proc_host.c_ptr(),
+                          lex_user.host.str))
+          continue;
+
+        DBUG_PRINT("sp_revoke_privileges",("Finding privileges assigned to sp"
+                                           " definer: '%s'@'%s'",
+                                           grant_proc_user.c_ptr(),
+                                           grant_proc_host.c_ptr()));
+
 	if (replace_routine_table(thd,grant_proc,tables[4].table,lex_user,
 				  grant_proc->db, grant_proc->tname,
                                   is_proc, ~(ulong)0, 1) == 0)
@@ -6234,11 +6266,10 @@ bool sp_revoke_privileges(THD *thd, cons
     }
   } while (revoked);
 
+  thd->pop_internal_handler();
   VOID(pthread_mutex_unlock(&acl_cache->lock));
   rw_unlock(&LOCK_grant);
   close_thread_tables(thd);
-
-  thd->pop_internal_handler();
 
   DBUG_RETURN(error_handler.has_errors());
 }
diff -Nrup a/sql/sql_acl.h b/sql/sql_acl.h
--- a/sql/sql_acl.h	2008-03-28 13:00:42 +01:00
+++ b/sql/sql_acl.h	2008-04-29 13:36:25 +02:00
@@ -263,7 +263,7 @@ bool mysql_revoke_all(THD *thd, List <LE
 void fill_effective_table_privileges(THD *thd, GRANT_INFO *grant,
                                      const char *db, const char *table);
 bool sp_revoke_privileges(THD *thd, const char *sp_db, const char *sp_name,
-                          bool is_proc);
+                          bool is_proc, String *definer_user);
 int sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
                          bool is_proc);
 bool check_routine_level_acl(THD *thd, const char *db, const char *name,
diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
--- a/sql/sql_parse.cc	2008-04-18 09:17:46 +02:00
+++ b/sql/sql_parse.cc	2008-04-29 13:36:25 +02:00
@@ -4255,15 +4255,16 @@ create_sp_error:
   case SQLCOM_DROP_FUNCTION:
     {
       int sp_result;
-      int type= (lex->sql_command == SQLCOM_DROP_PROCEDURE ?
-                 TYPE_ENUM_PROCEDURE : TYPE_ENUM_FUNCTION);
-
+      String sp_definer;
+      char *db= lex->spname->m_db.str;
+      char *name= lex->spname->m_name.str;
+      int type= lex->sql_command == SQLCOM_DROP_PROCEDURE ?
+                TYPE_ENUM_PROCEDURE :
+                TYPE_ENUM_FUNCTION;
       sp_result= sp_routine_exists_in_table(thd, type, lex->spname);
       mysql_reset_errors(thd, 0);
       if (sp_result == SP_OK)
       {
-        char *db= lex->spname->m_db.str;
-	char *name= lex->spname->m_name.str;
 
 	if (check_routine_access(thd, ALTER_PROC_ACL, db, name,
                                  lex->sql_command == SQLCOM_DROP_PROCEDURE, 0))
@@ -4271,23 +4272,10 @@ create_sp_error:
 
         if (end_active_trans(thd)) 
           goto error;
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-	if (sp_automatic_privileges && !opt_noacl &&
-	    sp_revoke_privileges(thd, db, name, 
-                                 lex->sql_command == SQLCOM_DROP_PROCEDURE))
-	{
-	  push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, 
-		       ER_PROC_AUTO_REVOKE_FAIL,
-		       ER(ER_PROC_AUTO_REVOKE_FAIL));
-	}
-#endif
-        /* Conditionally writes to binlog */
 
-        int type= lex->sql_command == SQLCOM_DROP_PROCEDURE ?
-                  TYPE_ENUM_PROCEDURE :
-                  TYPE_ENUM_FUNCTION;
+        /* Conditionally writes to binlog */
 
-        sp_result= sp_drop_routine(thd, type, lex->spname);
+        sp_result= sp_drop_routine(thd, type, lex->spname, &sp_definer);
       }
       else
       {
@@ -4318,6 +4306,19 @@ create_sp_error:
 	}
       }
       res= sp_result;
+
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+      if (sp_automatic_privileges && !opt_noacl &&
+          sp_revoke_privileges(thd, db, name,
+                               lex->sql_command == SQLCOM_DROP_PROCEDURE,
+                               &sp_definer))
+      {
+        push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, 
+                     ER_PROC_AUTO_REVOKE_FAIL,
+                     ER(ER_PROC_AUTO_REVOKE_FAIL));
+      }
+#endif
+
       switch (sp_result) {
       case SP_OK:
 	my_ok(thd);