STATUS
------
Approved pending changes.
REQUIRED
--------
1. Remove unused variable.
COMMENTARY
----------
I still think that the CREATE privilege check for individual tables/views
makes no sense. But I approve the patch as we agreed during team meeting.
DETAILS
-------
Chuck Bell wrote:
> #At file:///Users/cbell/source/bzr/mysql-6.0-bug-44787-precheck/ based on
> revid:charles.bell@stripped
>
> 2888 Chuck Bell 2009-11-06
> BUG#44787 : Backup: Check privileges before executing BACKUP/RESTORE
>
> Restore can fail in the middle if the user does not have
> sufficient privileges to create and populate all of the
> objects.
>
> This patch implements a privilege precheck step to check
> all objects for proper access. If any object fails the
> privilege check, restore halts with an error.
>
> The object-level privileges checked include the following.
>
> RESTORE,CREATE,DROP on db.*
> CREATE on db.x (if table or view x)
> CREATE_TABLESPACE on *.* (if tablespace)
> SUPER on *.* (if view, stored routine, event or trigger)
> CREATE_PROC on db.* (if stored routine)
> EVENT on db.* (if event)
> GRANT on db.* (if privilege)
> TRIGGER on db.* (if trigger but table not found)
> TRIGGER on db.t (if trigger on t)
>
> Note: This is patch 1 of 3. Patch 2 implements elevation,
> patch 3 implements the options to skip precheck and turn
> elevation off.
> @ mysql-test/suite/backup/include/error_name_to_number.inc
> Added new error code for debug insertion testing.
> @ mysql-test/suite/backup/r/backup_errors_debug_3.result
> Corrected resut file.
> @ mysql-test/suite/backup/r/backup_restore_security.result
> New result file.
> @ mysql-test/suite/backup/r/backup_security.result
> Corrected resut file.
> @ mysql-test/suite/backup/t/backup_errors_debug_3.test
> Added test case for when db is not found in catalog.
> @ mysql-test/suite/backup/t/backup_restore_security.test
> New test for testing restore security prechecks.
> @ mysql-test/suite/backup/t/backup_security.test
> Corrected errors now that restore prechecking is complete.
> @ sql/backup/kernel.cc
> Added call to do object-level privilege checking.
> @ sql/backup/restore_info.h
> Added code to conduct privilege checking for all objects
> prior to executing restore. For tables, views, and triggers
> the check is object-level, for tablespace it is global-level,
> and for all other objects it is database-level.
>
> Moved RESTORE_ACL check to new method in kernel.cc so that
> it occurs prior to prechecking object-level privileges.
> @ sql/share/errmsg-utf8.txt
> New error messages.
> @ sql/share/errmsg.txt
> New error messages.
>
> added:
> mysql-test/suite/backup/r/backup_restore_security.result
> mysql-test/suite/backup/t/backup_restore_security.test
> modified:
> mysql-test/suite/backup/include/error_name_to_number.inc
> mysql-test/suite/backup/r/backup_errors_debug_3.result
> mysql-test/suite/backup/r/backup_security.result
> mysql-test/suite/backup/t/backup_errors_debug_3.test
> mysql-test/suite/backup/t/backup_security.test
> sql/backup/kernel.cc
> sql/backup/restore_info.h
> sql/share/errmsg-utf8.txt
> sql/share/errmsg.txt
...
> === modified file 'sql/backup/restore_info.h'
...
> @@ -153,4 +145,196 @@ Restore_info::add_table(Image_info::Db &
> return t;
> }
>
> +/**
> + Perform privilege checking for restore.
> +
> + This method checks the known minimal privileges needed to restore each
> + object in the backup image. This method is called once for every object in
> + the catalog. It ensures there are no privilege-based restrictions that would
> + prohibit a successful restore.
> +
> + It begins by checking that the user has RESTORE for the database. If the user
> + does not have RESTORE on any database in the backup image, an error is
> + generated. Next, specific object-level privilege checking is performed for
> + all of the objects in the database. The object-level privileges checked
> + include the following.
> +
> + RESTORE,CREATE,DROP on db.*
> + CREATE on db.x (if table or view x)
> + CREATE_TABLESPACE on *.* (if tablespace)
> + SUPER on *.* (if view, stored routine, event or trigger)
> + CREATE_PROC on db.* (if stored routine)
> + EVENT on db.* (if event)
> + GRANT on db.* (if privilege)
> + TRIGGER on db.* (if trigger but table not found)
> + TRIGGER on db.t (if trigger on t)
> +
> + @param[in] item The item to check.
> +
> + @returns
> + @retval FALSE User has privileges for the object.
> + @retval TRUE User does not have privileges for object.
> +*/
> +inline
> +bool
> +Restore_info::check_restore_privileges(struct st_bstream_item_info *item)
> +{
> + using namespace backup;
> + int result= 0;
> + THD *thd= ::current_thd;
> + Image_info::Db *db= NULL; // Database object
> + st_bstream_table_info *tbl_info; // Table information
1. I think this variable is not used (which generates compiler warning). If
this is the case please remove it.
Rafal
