List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:November 4 2009 9:51pm
Subject:bzr commit into mysql-6.0-backup branch (charles.bell:2889) Bug#44787
View as plain text  
#At file:///Users/cbell/source/bzr/mysql-6.0-bug-44787-elevaion/ based on revid:charles.bell@stripped

 2889 Chuck Bell	2009-11-04
      BUG#44787 : Backup: Check privileges before executing BACKUP/RESTORE
      
      The backup system must be changed to meet a decision to allow
      elevation of privileges for backup if the user has only the 
      BACKUP_ACL privilege and elevation of privileges for restore 
      if the user has the RESTORE_ACL and SUPER_ACL privileges.
      
      This patch implements the privilege elevation change for backup
      as well as privilege elevation for restore iff the user has both
      RESTORE and SUPER on all databases in the image. The restore
      will fall back to object-level privilege checking if this condition
      is not met.
      
      Note: This is patch 2 of 3. Patch 1 implements privilege checking,
      patch 3 implements the options to skip precheck and turn
      elevation off.
     @ mysql-test/suite/backup/r/backup_restore_security.result
        Corrected result file.
     @ mysql-test/suite/backup/t/backup_restore_security.test
        Added new test cases.
     @ mysql-test/suite/backup/t/backup_security.test
        Corrected error return codes.
     @ sql/backup/backup_info.cc
        Added privilege elevation for backup using security context
        method (similar to replication).
     @ sql/backup/kernel.cc
        Added methods to save and restore privileges prior to and
        after elevation.
     @ sql/backup/restore_info.h
        Added attributes for privilege checking.
        Added code to do restore elevation in the check_restore_privileges()
        method.
     @ sql/si_objects.cc
        Removed privilege elevation from si_objects code.
     @ sql/sql_class.cc
        Added methods to set, save, restore the access levels for turning off 
        elevation and preserving user context.
     @ sql/sql_class.h
        Added methods to set, save, and restore privilege access.

    modified:
      mysql-test/suite/backup/r/backup_restore_security.result
      mysql-test/suite/backup/r/backup_security.result
      mysql-test/suite/backup/t/backup_restore_security.test
      mysql-test/suite/backup/t/backup_security.test
      sql/backup/backup_info.cc
      sql/backup/kernel.cc
      sql/backup/restore_info.h
      sql/si_objects.cc
      sql/sql_class.cc
      sql/sql_class.h
=== modified file 'mysql-test/suite/backup/r/backup_restore_security.result'
--- a/mysql-test/suite/backup/r/backup_restore_security.result	2009-11-04 19:34:44 +0000
+++ b/mysql-test/suite/backup/r/backup_restore_security.result	2009-11-04 21:51:42 +0000
@@ -40,6 +40,13 @@ DELETE FROM backup_test.t1 WHERE a = "no
 #
 GRANT ALL ON backup_test.* TO 'joe'@'user';
 #
+# Create a second database for testing multiple database privilege 
+# checking.
+#
+CREATE DATABASE backup_test_alt;
+CREATE TABLE backup_test_alt.t1 (a int);
+INSERT INTO backup_test_alt.t1 VALUES (10), (11), (12);
+#
 # Revoke grants for bup_some_priv
 #
 REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
@@ -56,6 +63,12 @@ GRANT USAGE ON *.* TO 'bup_some_priv'@'l
 BACKUP DATABASE backup_test to 'backup_test_orig.bak';
 backup_id
 #
+BACKUP DATABASE backup_test, backup_test_alt to 'backup_test_orig_alt1.bak';
+backup_id
+#
+BACKUP DATABASE backup_test_alt, backup_test to 'backup_test_orig_alt2.bak';
+backup_id
+#
 #
 # Show list of all objects in the database.
 #
@@ -88,6 +101,7 @@ backup_id
 #               the database.
 #
 GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT CREATE, DROP ON backup_test.* TO 'bup_some_priv'@'localhost';
 FLUSH PRIVILEGES;
 # 
 # Show grants for user.
@@ -95,7 +109,7 @@ FLUSH PRIVILEGES;
 SHOW GRANTS FOR 'bup_some_priv'@'localhost';
 Grants for bup_some_priv@localhost
 GRANT USAGE ON *.* TO 'bup_some_priv'@'localhost'
-GRANT RESTORE ON `backup_test`.* TO 'bup_some_priv'@'localhost'
+GRANT CREATE, DROP, RESTORE ON `backup_test`.* TO 'bup_some_priv'@'localhost'
 #
 # Connect as user with only some privileges.
 #
@@ -104,10 +118,10 @@ GRANT RESTORE ON `backup_test`.* TO 'bup
 # error ER_RESTORE_ACCESS_OBJS_INCOMPLETE
 #
 RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
-ERROR HY000: Insufficient privileges. You do not have privileges to restore the object 'backup_test' from this backup image.
+ERROR HY000: Insufficient privileges. You do not have privileges to restore the object 'p1' from this backup image.
 SHOW ERRORS;
 Level	Code	Message
-Error	#	Insufficient privileges. You do not have privileges to restore the object 'backup_test' from this backup image.
+Error	#	Insufficient privileges. You do not have privileges to restore the object 'p1' from this backup image.
 #
 # Test Case 2 : Ensure a user with only SELECT on db1.* cannot restore 
 #               the database.
@@ -124,7 +138,7 @@ FLUSH PRIVILEGES;
 SHOW GRANTS FOR 'bup_some_priv'@'localhost';
 Grants for bup_some_priv@localhost
 GRANT USAGE ON *.* TO 'bup_some_priv'@'localhost'
-GRANT SELECT, RESTORE ON `backup_test`.* TO 'bup_some_priv'@'localhost'
+GRANT SELECT, CREATE, DROP, RESTORE ON `backup_test`.* TO 'bup_some_priv'@'localhost'
 #
 # Connect as user with only some privileges.
 #
@@ -133,10 +147,10 @@ GRANT SELECT, RESTORE ON `backup_test`.*
 # error ER_RESTORE_ACCESS_OBJS_INCOMPLETE
 #
 RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
-ERROR HY000: Insufficient privileges. You do not have privileges to restore the object 'backup_test' from this backup image.
+ERROR HY000: Insufficient privileges. You do not have privileges to restore the object 'p1' from this backup image.
 SHOW ERRORS;
 Level	Code	Message
-Error	#	Insufficient privileges. You do not have privileges to restore the object 'backup_test' from this backup image.
+Error	#	Insufficient privileges. You do not have privileges to restore the object 'p1' from this backup image.
 #
 # Test Case 5 : Show that SUPER is needed to complete restore 
 #               when there are objects with definer clauses.
@@ -224,14 +238,13 @@ REVOKE ALL ON *.* FROM 'bup_some_priv'@'
 GRANT INSERT, UPDATE, DELETE, BACKUP, RESTORE, SELECT, CREATE, DROP 
 ON backup_test_trig.* TO 'bup_some_priv'@'localhost' WITH GRANT OPTION;
 GRANT SELECT ON mysql.* TO 'bup_some_priv'@'localhost';
-GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
 FLUSH PRIVILEGES;
 # 
 # Show grants for user.
 #
 SHOW GRANTS FOR 'bup_some_priv'@'localhost';
 Grants for bup_some_priv@localhost
-GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost'
+GRANT USAGE ON *.* TO 'bup_some_priv'@'localhost'
 GRANT SELECT ON `mysql`.* TO 'bup_some_priv'@'localhost'
 GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, BACKUP, RESTORE ON `backup_test_trig`.* TO 'bup_some_priv'@'localhost' WITH GRANT OPTION
 #
@@ -255,16 +268,107 @@ FLUSH PRIVILEGES;
 # Connect as user with only some privileges.
 #
 #
+# conn_some_priv: Attempting restore. Should fail with
+# ER_RESTORE_ACCESS_DEFINER
+#
+RESTORE FROM 'backup_test_trig.bak' OVERWRITE;
+ERROR HY000: Insufficient privileges. You must have the SUPER privilege to restore the object 'backup_test_trig'.'trg'.
+SHOW ERRORS;
+Level	Code	Message
+Error	1799	Insufficient privileges. You must have the SUPER privilege to restore the object 'backup_test_trig'.'trg'.
+#
+# Connect as root and add privilege for trigger.
+# In this case, we must add SUPER to overcome limitation with
+# the definer clause.
+#
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+#
+# Connect as user with only some privileges.
+#
+#
 # conn_some_priv: Attempting restore. Should succeed.
 #
 RESTORE FROM 'backup_test_trig.bak' OVERWRITE;
 backup_id
 #
 #
-# Connect as root and cleanup.
+# Change privileges for elevated restore test case.
 #
 DROP DATABASE backup_test_trig;
 #
+# Test Case 8 : Show that user can have only RESTORE + SUPER and 
+#               perform an elevated restore.
+#
+DROP USER 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+CREATE USER 'bup_some_priv'@'localhost';
+REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
+GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+#
+# Connect as user with only some privileges.
+#
+#
+# conn_some_priv: Attempting restore. Should succeed.
+#
+RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
+backup_id
+#
+#
+# Change privileges for elevated restore test case.
+#
+#
+# Test Case 9 : Show that users who have RESTORE + SUPER for db1 but not
+#               for db2 cannot restore the image.
+#
+DROP USER 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+CREATE USER 'bup_some_priv'@'localhost';
+REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
+GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+#
+# Connect as user with only some privileges.
+#
+#
+# conn_some_priv: Attempting restore. Should fail with
+# error ER_RESTORE_ACCESS_DENIED_ERROR
+#
+RESTORE FROM 'backup_test_orig_alt1.bak' OVERWRITE;
+ERROR HY000: Insufficient privileges. You must have the RESTORE privilege to restore database 'backup_test_alt'.
+SHOW ERRORS;
+Level	Code	Message
+Error	1794	Insufficient privileges. You must have the RESTORE privilege to restore database 'backup_test_alt'.
+#
+# conn_some_priv: Attempting restore. Should fail with
+# error ER_RESTORE_ACCESS_DENIED_ERROR
+#
+RESTORE FROM 'backup_test_orig_alt2.bak' OVERWRITE;
+ERROR HY000: Insufficient privileges. You must have the RESTORE privilege to restore database 'backup_test_alt'.
+SHOW ERRORS;
+Level	Code	Message
+Error	1794	Insufficient privileges. You must have the RESTORE privilege to restore database 'backup_test_alt'.
+#
+# Change privileges for elevated restore test case.
+#
+GRANT RESTORE ON backup_test_alt.* TO 'bup_some_priv'@'localhost';
+FLUSH PRIVILEGES;
+#
+# Connect as user with only some privileges.
+#
+#
+# conn_some_priv: Attempting restore. Should succeed.
+#
+RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
+backup_id
+#
+#
+# Connect as root and cleanup.
+#
+#
 # Compare to original backup image file.
 #
 RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
@@ -294,4 +398,5 @@ trg
 DROP USER 'bup_some_priv'@'localhost';
 DROP USER 'joe'@'user';
 DROP DATABASE backup_test;
+DROP DATABASE backup_test_alt;
 FLUSH PRIVILEGES;

=== modified file 'mysql-test/suite/backup/r/backup_security.result'
--- a/mysql-test/suite/backup/r/backup_security.result	2009-11-04 19:34:44 +0000
+++ b/mysql-test/suite/backup/r/backup_security.result	2009-11-04 21:51:42 +0000
@@ -401,13 +401,13 @@ ERROR HY000: Insufficient privileges. Yo
 #
 #
 # conn_user1: Attempting backup. Should fail with 
-# error ER_BACKUP_ACCESS_DENIED_ERROR
+# error ER_BAD_DB_ERROR
 #
 BACKUP DATABASE backup_test_alt to 'bup_no_priv.bak';
-ERROR HY000: Insufficient privileges. You must have the BACKUP privilege to backup database 'backup_test_alt'.
+ERROR 42000: Unknown database 'backup_test_alt'
 SHOW ERRORS;
 Level	Code	Message
-Error	#	Insufficient privileges. You must have the BACKUP privilege to backup database 'backup_test_alt'.
+Error	#	Unknown database 'backup_test_alt'
 #
 # conn_user1: Attempting backup. Should fail with 
 # error ER_BACKUP_ACCESS_DENIED_ERROR

=== modified file 'mysql-test/suite/backup/t/backup_restore_security.test'
--- a/mysql-test/suite/backup/t/backup_restore_security.test	2009-11-04 19:34:44 +0000
+++ b/mysql-test/suite/backup/t/backup_restore_security.test	2009-11-04 21:51:42 +0000
@@ -13,6 +13,10 @@
 #     all of the required permissions to create and populate all objects.
 #  7. Show that having TRIGGER on a specific table is required to restore
 #     a table with a trigger. 
+#  8. Show that user can have only RESTORE + SUPER and perform an
+#     elevated restore.
+#  9. Show that users who have RESTORE + SUPER for db1 but not for db2
+#     cannot restore the image.
 #
 
 --source include/not_embedded.inc
@@ -40,6 +44,15 @@ CREATE USER 'joe'@'user';
 --source suite/backup/include/basic_data.inc
 
 --echo #
+--echo # Create a second database for testing multiple database privilege 
+--echo # checking.
+--echo #
+
+CREATE DATABASE backup_test_alt;
+CREATE TABLE backup_test_alt.t1 (a int);
+INSERT INTO backup_test_alt.t1 VALUES (10), (11), (12);
+
+--echo #
 --echo # Revoke grants for bup_some_priv
 --echo #
 REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
@@ -58,6 +71,12 @@ SHOW GRANTS FOR 'bup_some_priv'@'localho
 --replace_column 1 #
 BACKUP DATABASE backup_test to 'backup_test_orig.bak';
 
+--replace_column 1 #
+BACKUP DATABASE backup_test, backup_test_alt to 'backup_test_orig_alt1.bak';
+
+--replace_column 1 #
+BACKUP DATABASE backup_test_alt, backup_test to 'backup_test_orig_alt2.bak';
+
 --echo #
 --echo # Show list of all objects in the database.
 --echo #
@@ -83,6 +102,7 @@ BACKUP DATABASE backup_test to 'backup_t
 --echo #
 
 GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT CREATE, DROP ON backup_test.* TO 'bup_some_priv'@'localhost'; 
 
 FLUSH PRIVILEGES;
 
@@ -254,7 +274,6 @@ REVOKE ALL ON *.* FROM 'bup_some_priv'@'
 GRANT INSERT, UPDATE, DELETE, BACKUP, RESTORE, SELECT, CREATE, DROP 
  ON backup_test_trig.* TO 'bup_some_priv'@'localhost' WITH GRANT OPTION;
 GRANT SELECT ON mysql.* TO 'bup_some_priv'@'localhost';
-GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
 
 FLUSH PRIVILEGES;
 
@@ -296,6 +315,33 @@ disconnect conn_root;
 connect (conn_some_priv,localhost,bup_some_priv,,);
 
 --echo #
+--echo # conn_some_priv: Attempting restore. Should fail with
+--echo # ER_RESTORE_ACCESS_DEFINER
+--echo #
+--replace_column 1 #
+--error ER_RESTORE_ACCESS_DEFINER
+RESTORE FROM 'backup_test_trig.bak' OVERWRITE;
+SHOW ERRORS;
+
+disconnect conn_some_priv;
+--echo #
+--echo # Connect as root and add privilege for trigger.
+--echo # In this case, we must add SUPER to overcome limitation with
+--echo # the definer clause.
+--echo #
+connect (conn_root,localhost,root,,);
+
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+disconnect conn_root;
+--echo #
+--echo # Connect as user with only some privileges.
+--echo #
+connect (conn_some_priv,localhost,bup_some_priv,,);
+
+--echo #
 --echo # conn_some_priv: Attempting restore. Should succeed.
 --echo #
 --replace_column 1 #
@@ -303,13 +349,118 @@ RESTORE FROM 'backup_test_trig.bak' OVER
 
 disconnect conn_some_priv;
 --echo #
---echo # Connect as root and cleanup.
+--echo # Change privileges for elevated restore test case.
 --echo #
 connect (conn_root,localhost,root,,);
 
 DROP DATABASE backup_test_trig;
 
 --echo #
+--echo # Test Case 8 : Show that user can have only RESTORE + SUPER and 
+--echo #               perform an elevated restore.
+--echo #
+
+DROP USER 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+CREATE USER 'bup_some_priv'@'localhost';
+
+REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
+GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+disconnect conn_root;
+--echo #
+--echo # Connect as user with only some privileges.
+--echo #
+connect (conn_some_priv,localhost,bup_some_priv,,);
+
+--echo #
+--echo # conn_some_priv: Attempting restore. Should succeed.
+--echo #
+--replace_column 1 #
+RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
+
+disconnect conn_some_priv;
+--echo #
+--echo # Change privileges for elevated restore test case.
+--echo #
+connect (conn_root,localhost,root,,);
+
+--echo #
+--echo # Test Case 9 : Show that users who have RESTORE + SUPER for db1 but not
+--echo #               for db2 cannot restore the image.
+--echo #
+
+DROP USER 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+CREATE USER 'bup_some_priv'@'localhost';
+
+REVOKE ALL ON *.* FROM 'bup_some_priv'@'localhost';
+GRANT RESTORE ON backup_test.* TO 'bup_some_priv'@'localhost';
+GRANT SUPER ON *.* TO 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+disconnect conn_root;
+--echo #
+--echo # Connect as user with only some privileges.
+--echo #
+connect (conn_some_priv,localhost,bup_some_priv,,);
+
+--echo #
+--echo # conn_some_priv: Attempting restore. Should fail with
+--echo # error ER_RESTORE_ACCESS_DENIED_ERROR
+--echo #
+--replace_column 1 #
+--error ER_RESTORE_ACCESS_DENIED_ERROR
+RESTORE FROM 'backup_test_orig_alt1.bak' OVERWRITE;
+SHOW ERRORS;
+
+--echo #
+--echo # conn_some_priv: Attempting restore. Should fail with
+--echo # error ER_RESTORE_ACCESS_DENIED_ERROR
+--echo #
+--replace_column 1 #
+--error ER_RESTORE_ACCESS_DENIED_ERROR
+RESTORE FROM 'backup_test_orig_alt2.bak' OVERWRITE;
+SHOW ERRORS;
+
+disconnect conn_some_priv;
+--echo #
+--echo # Change privileges for elevated restore test case.
+--echo #
+connect (conn_root,localhost,root,,);
+
+GRANT RESTORE ON backup_test_alt.* TO 'bup_some_priv'@'localhost';
+
+FLUSH PRIVILEGES;
+
+disconnect conn_root;
+--echo #
+--echo # Connect as user with only some privileges.
+--echo #
+connect (conn_some_priv,localhost,bup_some_priv,,);
+
+--echo #
+--echo # conn_some_priv: Attempting restore. Should succeed.
+--echo #
+--replace_column 1 #
+RESTORE FROM 'backup_test_orig.bak' OVERWRITE;
+
+
+disconnect conn_some_priv;
+--echo #
+--echo # Connect as root and cleanup.
+--echo #
+connect (conn_root,localhost,root,,);
+
+--echo #
 --echo # Compare to original backup image file.
 --echo #
 
@@ -332,9 +483,12 @@ DROP USER 'bup_some_priv'@'localhost';
 DROP USER 'joe'@'user';
 
 DROP DATABASE backup_test;
+DROP DATABASE backup_test_alt;
 FLUSH PRIVILEGES;
 
 let $MYSQLD_BACKUPDIR= `select @@backupdir`;
 remove_file $MYSQLD_BACKUPDIR/backup_test_orig.bak;
+remove_file $MYSQLD_BACKUPDIR/backup_test_orig_alt1.bak;
+remove_file $MYSQLD_BACKUPDIR/backup_test_orig_alt2.bak;
 remove_file $MYSQLD_BACKUPDIR/backup_test_trig.bak;
 remove_file $MYSQLD_BACKUPDIR/backup_test_no_proc.bak;

=== modified file 'mysql-test/suite/backup/t/backup_security.test'
--- a/mysql-test/suite/backup/t/backup_security.test	2009-11-04 19:34:44 +0000
+++ b/mysql-test/suite/backup/t/backup_security.test	2009-11-04 21:51:42 +0000
@@ -198,10 +198,10 @@ eval RESTORE FROM 'bup_user1.bak' OVERWR
 
 --echo #
 --echo # conn_user1: Attempting backup. Should fail with 
---echo # error ER_BACKUP_ACCESS_DENIED_ERROR
+--echo # error ER_BAD_DB_ERROR
 --echo #
 --replace_column 1 #
---error ER_BACKUP_ACCESS_DENIED_ERROR
+--error ER_BAD_DB_ERROR
 BACKUP DATABASE backup_test_alt to 'bup_no_priv.bak';
 --replace_column 2 #
 SHOW ERRORS;

=== modified file 'sql/backup/backup_info.cc'
--- a/sql/backup/backup_info.cc	2009-10-23 15:41:56 +0000
+++ b/sql/backup/backup_info.cc	2009-11-04 21:51:42 +0000
@@ -665,7 +665,11 @@ backup::Image_info::Db* Backup_info::add
   /*
     Check privileges for this database. User must have BACKUP
     privilege in order to execute a backup.
+   
+    We must turn privilege checking back on first.
   */
+  m_thd->security_ctx->restore_global_privileges();
+  
   DEBUG_SYNC(m_thd, "before_backup_privileges");
   if (check_access(m_thd, BACKUP_ACL, name->ptr(), 0, 1, 1, 0))
   {
@@ -673,6 +677,12 @@ backup::Image_info::Db* Backup_info::add
     return NULL;
   }
 
+  /*
+   The base check for BACKUP_ACL for this database is satisfied,
+   ok to elevate by turning off privilege checking.
+   */
+  m_thd->security_ctx->set_all_global_privileges();
+  
   // Check to see if the user can see all of the objects in the database.
   if (obs::check_user_access(m_thd, name))
   {

=== modified file 'sql/backup/kernel.cc'
--- a/sql/backup/kernel.cc	2009-11-04 19:34:44 +0000
+++ b/sql/backup/kernel.cc	2009-11-04 21:51:42 +0000
@@ -639,6 +639,11 @@ int Backup_restore_ctx::prepare_path(::S
 
 int Backup_restore_ctx::prepare(::String *backupdir, LEX_STRING location)
 {
+  /*
+    Save privilege information for restoring later.
+  */
+  m_thd->security_ctx->save_global_privileges();
+  
   if (m_error)
     return m_error;
 
@@ -1180,6 +1185,11 @@ int Backup_restore_ctx::close()
 {
   int res= 0;
 
+  /*
+    Restore privilege information.
+  */
+  m_thd->security_ctx->restore_global_privileges();
+  
   if (m_state == CLOSED)
     return 0;
 

=== modified file 'sql/backup/restore_info.h'
--- a/sql/backup/restore_info.h	2009-11-04 19:34:44 +0000
+++ b/sql/backup/restore_info.h	2009-11-04 21:51:42 +0000
@@ -48,6 +48,9 @@ public:
                                
   bool check_restore_privileges(struct st_bstream_item_info *item);
   
+  bool m_skip_precheck;    // skip prechecking if privilege check ok
+  bool m_first_priv_check; // determines if this is the first db to be checked  
+
 private:
 
   /*
@@ -79,7 +82,8 @@ private:
 
 inline
 Restore_info::Restore_info(backup::Logger &log, THD *thd)
-  :m_log(log), m_thd(thd), m_data_changed(FALSE)
+  :m_log(log), m_thd(thd), m_data_changed(FALSE), 
+   m_skip_precheck(FALSE), m_first_priv_check(TRUE)
 {}
 
 
@@ -114,7 +118,7 @@ Restore_info::add_ts(const ::String &nam
 }
 
 
-/// Wrapper around Image_info method which reports errors and checks privileges.
+/// Wrapper around Image_info method which reports errors.
 
 inline
 backup::Image_info::Db*
@@ -145,6 +149,7 @@ Restore_info::add_table(Image_info::Db &
   return t;
 }
 
+
 /**
   Perform privilege checking for restore.
  
@@ -193,11 +198,46 @@ Restore_info::check_restore_privileges(s
   /*
     Check privileges for this database. User must have RESTORE
     privilege in order to execute a restore.
+    Check privileges for this database.
+
+    If user has RESTORE + SUPER, do not do object-level privilege checking.
+
+    If the user does not have RESTORE, fail with an error.
+
+    @note Databases are listed after tablesspaces and charsets in the 
+    catalog. Thus, the list of databases are read before the database 
+    objects.
   */
   DEBUG_SYNC(thd, "before_restore_privileges");
   
   if (item->type == BSTREAM_IT_DB)
   {
+    // We must turn privilege checking back on first.
+    m_thd->security_ctx->restore_global_privileges();
+    
+    /*
+      If this is the first check, set m_skip_precheck.
+    */
+    if (m_first_priv_check)
+    {
+      m_skip_precheck= TRUE;
+      m_first_priv_check= FALSE;
+    }
+    if (!check_access(m_thd, RESTORE_ACL, name_str, 0, 1, 1, 0) &&
+        (m_thd->security_ctx->master_access & SUPER_ACL) &&
+         m_skip_precheck)
+    {
+      /*
+        The base check for RESTORE_ACL + SUPER_ACL for this database is 
+        satisfied. It is ok to elevate by turning off privilege checking.
+      */
+      m_thd->security_ctx->set_all_global_privileges();
+      m_skip_precheck= TRUE;
+    }
+    else 
+    {
+      m_skip_precheck= FALSE;
+    }
     if (check_access(thd, RESTORE_ACL, name_str, 0, 1, 1, 0))
     {
       m_log.report_error(ER_RESTORE_ACCESS_DENIED_ERROR, name_str);
@@ -206,136 +246,149 @@ Restore_info::check_restore_privileges(s
   }
 
   /*
-    Check privileges for each object based on type of object.
+    If the user does not have RESTORE + SUPER, as indicated by m_skip_precheck
+    == FALSE, we must perform object-level privilege checking for all objects 
+    being restored. The user must have, at a minimum, the ability to create 
+    each object plus any specific privileges for the object type (e.g. 
+    functions, triggers, events, etc.).
+
+    An error is returned if the user does not have the correct privileges
+    to create the object.
   */
-  switch (item->type) 
+  if (!m_skip_precheck)
   {
-  case BSTREAM_IT_TABLESPACE:
-    {
-      result= check_global_access(thd, CREATE_TABLESPACE_ACL);
-      break;        
-    }
-  case BSTREAM_IT_DB:
+    /*
+      Check privileges for each object based on type of object.
+    */
+    switch (item->type) 
     {
-      result= check_access(thd, CREATE_ACL + DROP_ACL,
-                           name_str, 0, 1, 1, 0);
-      break;
-    }
-  case BSTREAM_IT_TABLE:
-  case BSTREAM_IT_VIEW:
-    {
-      tbl_info= (st_bstream_table_info*) item;
-      DBUG_ASSERT (tbl_info);
+    case BSTREAM_IT_TABLESPACE:
+      {
+        result= check_global_access(thd, CREATE_TABLESPACE_ACL);
+        break;        
+      }
+    case BSTREAM_IT_DB:
+      {
+        result= check_access(thd, CREATE_ACL + DROP_ACL,
+                             name_str, 0, 1, 1, 0);
+        break;
+      }
+    case BSTREAM_IT_TABLE:
+    case BSTREAM_IT_VIEW:
+      {
+        tbl_info= (st_bstream_table_info*) item;
+        DBUG_ASSERT (tbl_info);
 
-      db= get_db(tbl_info->base.db->base.pos); // reports errors
-      DBUG_ASSERT(db);
-      
-      tbl_list.init_one_table(db->name().ptr(), db->name().length(),
-                              name_str, strlen(name_str),
-                              name_str, TL_READ);
-      
-      result= check_table_access(thd, CREATE_ACL, &tbl_list, TRUE, TRUE, 1);
-      
-      break;
-    }
-  case BSTREAM_IT_SPROC:
-  case BSTREAM_IT_SFUNC:
-    {
-      st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
-      DBUG_ASSERT(db_item);
+        db= get_db(tbl_info->base.db->base.pos); // reports errors
+        DBUG_ASSERT(db);
+        
+        tbl_list.init_one_table(db->name().ptr(), db->name().length(),
+                                name_str, strlen(name_str),
+                                name_str, TL_READ);
+        
+        result= check_table_access(thd, CREATE_ACL, &tbl_list, TRUE, TRUE, 1);
+        
+        break;
+      }
+    case BSTREAM_IT_SPROC:
+    case BSTREAM_IT_SFUNC:
+      {
+        st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
+        DBUG_ASSERT(db_item);
 
-      db= (Image_info::Db*) get_db(db_item->db->base.pos);
-      DBUG_ASSERT(db);
+        db= (Image_info::Db*) get_db(db_item->db->base.pos);
+        DBUG_ASSERT(db);
 
-      result= check_access(thd, CREATE_PROC_ACL, db->name().ptr(), 0, 1, 1, 0); 
-      break;
-    }
-  case BSTREAM_IT_EVENT:
-    {
-      st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
-      DBUG_ASSERT(db_item);
+        result= check_access(thd, CREATE_PROC_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        break;
+      }
+    case BSTREAM_IT_EVENT:
+      {
+        st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
+        DBUG_ASSERT(db_item);
 
-      db= (Image_info::Db*) get_db(db_item->db->base.pos);
-      DBUG_ASSERT(db);
+        db= (Image_info::Db*) get_db(db_item->db->base.pos);
+        DBUG_ASSERT(db);
 
-      result= check_access(thd, EVENT_ACL, db->name().ptr(), 0, 1, 1, 0); 
-      break;
-    }
-  case BSTREAM_IT_TRIGGER:
-    {
-      st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
-      DBUG_ASSERT(db_item);
+        result= check_access(thd, EVENT_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        break;
+      }
+    case BSTREAM_IT_TRIGGER:
+      {
+        st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
+        DBUG_ASSERT(db_item);
 
-      db= (Image_info::Db*) get_db(db_item->db->base.pos);
-      DBUG_ASSERT(db);
+        db= (Image_info::Db*) get_db(db_item->db->base.pos);
+        DBUG_ASSERT(db);
 
-      /*
-        Get the table reference for this trigger and check access.
-      */
-      Image_info::Table *tbl= get_table(db_item->snap_num, db_item->pos);
-      
-      /*
-        If the table is not found, revert to checking at the database
-        level.
-      */
-      if (!tbl)
-      {
-       result= check_access(thd, TRIGGER_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        /*
+          Get the table reference for this trigger and check access.
+        */
+        Image_info::Table *tbl= get_table(db_item->snap_num, db_item->pos);
+        
+        /*
+          If the table is not found, revert to checking at the database
+          level.
+        */
+        if (!tbl)
+        {
+         result= check_access(thd, TRIGGER_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        }
+        else 
+        {
+          tbl_list.init_one_table(db->name().ptr(), db->name().length(),
+                                  tbl->name().ptr(), tbl->name().length(),
+                                  tbl->name().ptr(), TL_READ);
+          result= check_table_access(thd, TRIGGER_ACL, &tbl_list, TRUE, TRUE, 1);
+        }
+        break;
       }
-      else 
+    case BSTREAM_IT_PRIVILEGE:
       {
-        tbl_list.init_one_table(db->name().ptr(), db->name().length(),
-                                tbl->name().ptr(), tbl->name().length(),
-                                tbl->name().ptr(), TL_READ);
-        result= check_table_access(thd, TRIGGER_ACL, &tbl_list, TRUE, TRUE, 1);
-      }
-      break;
-    }
-  case BSTREAM_IT_PRIVILEGE:
-    {
-      st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
-      DBUG_ASSERT(db_item);
+        st_bstream_dbitem_info *db_item= (st_bstream_dbitem_info*) item;
+        DBUG_ASSERT(db_item);
 
-      db= (Image_info::Db*) get_db(db_item->db->base.pos);
-      DBUG_ASSERT(db);
+        db= (Image_info::Db*) get_db(db_item->db->base.pos);
+        DBUG_ASSERT(db);
 
-      result= check_access(thd, GRANT_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        result= check_access(thd, GRANT_ACL, db->name().ptr(), 0, 1, 1, 0); 
+        break;
+      }
+    default:
       break;
+    } // switch (item->type)
+      
+    // Return error if privilege check fails.
+    if (result)
+    {
+      m_log.report_error(ER_RESTORE_ACCESS_OBJS_INCOMPLETE, name_str);
+      return TRUE;
     }
-  default:
-    break;
-  } // switch (item->type)
-    
-  // Return error if privilege check fails.
-  if (result)
-  {
-    m_log.report_error(ER_RESTORE_ACCESS_OBJS_INCOMPLETE, name_str);
-    return TRUE;
-  }
 
-  /*
-    Check for SUPER_ACL if any objects exist that have a definer clause.
-  */
-  switch (item->type) 
-  {
-  case BSTREAM_IT_VIEW:
-  case BSTREAM_IT_SPROC:
-  case BSTREAM_IT_SFUNC:
-  case BSTREAM_IT_EVENT:
-  case BSTREAM_IT_TRIGGER:
+    /*
+      Check for SUPER_ACL if any objects exist that have a definer clause.
+    */
+    switch (item->type) 
     {
-      result= !(thd->security_ctx->master_access & SUPER_ACL);
+    case BSTREAM_IT_VIEW:
+    case BSTREAM_IT_SPROC:
+    case BSTREAM_IT_SFUNC:
+    case BSTREAM_IT_EVENT:
+    case BSTREAM_IT_TRIGGER:
+      {
+        result= !(thd->security_ctx->master_access & SUPER_ACL);
+        break;
+      }
+    default:
       break;
+    } // switch (item->type)
+    
+    // Return error if privilege check fails.
+    if (result)
+    {
+      m_log.report_error(ER_RESTORE_ACCESS_DEFINER, db->name().ptr(), name_str);
+      return TRUE;
     }
-  default:
-    break;
-  } // switch (item->type)
-  
-  // Return error if privilege check fails.
-  if (result)
-  {
-    m_log.report_error(ER_RESTORE_ACCESS_DEFINER, db->name().ptr(), name_str);
-    return TRUE;
   }
   return FALSE;
 }

=== modified file 'sql/si_objects.cc'
--- a/sql/si_objects.cc	2009-11-04 14:18:21 +0000
+++ b/sql/si_objects.cc	2009-11-04 21:51:42 +0000
@@ -202,10 +202,7 @@ bool
 run_service_interface_sql(THD *thd, Ed_connection *ed_connection,
                           const LEX_STRING *query, bool get_warnings)
 {
-  Si_session_context session_context;
-  ulong saved_master_access; // Saved master access ACLs
-  ulong saved_db_access;     // Saved db access ACLs
-  
+  Si_session_context session_context;  
   
   DBUG_ENTER("run_service_interface_sql");
   DBUG_PRINT("run_service_interface_sql",
@@ -215,25 +212,6 @@ run_service_interface_sql(THD *thd, Ed_c
   session_context.save_si_ctx(thd);
   session_context.reset_si_ctx(thd);
 
-  /*
-    We only elevate for SELECT or SHOW CREATE queries.
-  */
-  my_bool elevate= strncmp(query->str, "SELECT", 6) == 0 ||
-                   strncmp(query->str, "(SELECT", 7) == 0 ||
-                   strncmp(query->str, "SHOW CREATE", 11) == 0 ? TRUE : FALSE; 
-  
-  /*
-    Temporarily give user SELECT privilege so operations on
-    mysql and information_schema can succeed.
-  */
-  if (elevate)
-  {
-    saved_master_access= thd->security_ctx->master_access; 
-    saved_db_access= thd->security_ctx->db_access; 
-    thd->security_ctx->master_access |= (SELECT_ACL | SHOW_VIEW_ACL | 
-                                         TRIGGER_ACL | PROC_ACLS | EVENT_ACL);
-  }
-  
   bool rc= ed_connection->execute_direct(*query);
 
   if (get_warnings) 
@@ -242,15 +220,6 @@ run_service_interface_sql(THD *thd, Ed_c
     thd->warning_info->append_warnings(thd, ed_connection->get_warn_list());
   }
 
-  /*
-    Remove elevated privilege.
-  */
-  if (elevate)
-  {
-    thd->security_ctx->master_access= saved_master_access; 
-    thd->security_ctx->db_access= saved_db_access; 
-  }
-
   session_context.restore_si_ctx(thd);
 
   DBUG_RETURN(rc);

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2009-10-26 14:02:26 +0000
+++ b/sql/sql_class.cc	2009-11-04 21:51:42 +0000
@@ -2991,6 +2991,24 @@ void Security_context::skip_grants()
   *priv_host= '\0';
 }
 
+// Turns off grants but keeps user context.
+void Security_context::set_all_global_privileges()
+{
+  master_access= ~NO_ACCESS;
+}
+
+// Saves current privilege settings.
+void Security_context::save_global_privileges()
+{
+  /* save the values for restoring later with reset_grant_info() */
+  m_saved_master_access= master_access;
+}
+
+// Turns grants back on.
+void Security_context::restore_global_privileges()
+{
+  master_access= m_saved_master_access;
+}
 
 bool Security_context::set_user(char *user_arg)
 {

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2009-10-28 15:45:46 +0000
+++ b/sql/sql_class.h	2009-11-04 21:51:42 +0000
@@ -852,6 +852,9 @@ public:
   void init();
   void destroy();
   void skip_grants();
+  void set_all_global_privileges();
+  void save_global_privileges();
+  void restore_global_privileges();
   inline char *priv_host_name()
   {
     return (*priv_host ? priv_host : (char *)"%");
@@ -871,6 +874,8 @@ public:
   restore_security_context(THD *thd, Security_context *backup);
 #endif
   bool user_matches(Security_context *);
+private:
+  ulong m_saved_master_access; 
 };
 
 


Attachment: [text/bzr-bundle] bzr/charles.bell@sun.com-20091104215142-1951c6c4g7o1fdcm.bundle
Thread
bzr commit into mysql-6.0-backup branch (charles.bell:2889) Bug#44787Chuck Bell4 Nov