Bug #45889 BACKUP DATABASE command should include all objects
Submitted: 1 Jul 2009 21:38 Modified: 6 Aug 2009 19:44
Reporter: Chuck Bell Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Backup Severity:S3 (Non-critical)
Version:5.4.4, 6.0 OS:Any
Assigned to: Chuck Bell CPU Architecture:Any

[1 Jul 2009 21:38] Chuck Bell
Description:
The BACKUP DATABASE command must attempt to backup all objects in the databases listed or fail with an error.

Note: This was decided on 30 June by the backup team during a weekly team meeting discussing BUG#39580.

How to repeat:
With the BUG#39580 patch, it is possible to assign BACKUP privilege to a user who does not have rights to all of the objects in the database. When the backup is run, no error or warning message is generated stating some objects were skipped (indeed, the user can't 'see' them so she wouldn't know they were skipped). 

Should this backup image be restored by someone with the required privileges to drop and create objects, the restore succeeds and through the course of the inherit DROP DATABASE, all of the missing objects are destroyed. This could result in data loss.

I will attach a test that demonstrates this behavior.

Suggested fix:
Issue a documentation warning and implement a mechanism that permits backup to determine if the user has access to all objects in the database(s). Note: this could be tricky and may require a complex design to overcome the possibility that the user may be granted BACKUP but not SELECT on all of the objects. Thus, the solution must be capable of detecting when a user cannot access a table (e.g. REVOKE ALL ON db1.t1 FROM 'joe'@'user').

Also, BUG#44787 will be needed to check for correct privileges on all objects prior to backup and restore.
[1 Jul 2009 21:39] Chuck Bell
Test file that demonstrates loss of data.

Attachment: backup_security_bad.test (application/octet-stream, text), 4.74 KiB.

[2 Jul 2009 5:58] Rafal Somla
A related issue: currently "BACKUP DATABASE * TO 'all.bkp'"  will backup only these databases which the user can see. Perhaps we would like to implement a different semantics, where above command is supposed to backup all existing databases and will error/warn if some of them are skipped.

Note that we must be careful with error messages. Even disclosing to the user the information that there are some databases he is not able to see can be a security hole.
[2 Jul 2009 21:16] Chuck Bell
Design
------
The problem exists only for the BACKUP DATABASE * form of the BACKUP command. If the user issues a BACKUP command listing databases that she cannot access or do not exist, the code is designed to produce an error. Thus, the solution need only solve the problem for the wildcard form of the command.

The limitation is inherent in how the si_objects code issues selects for the list of databases. The method get_databases() lists all databases that the user can access, not all databases on the server.

The design shall create a new method as follows:

/**
  Create an iterator over all databases in the server using privilege
  elevation.

  Includes system databases, such as "mysql" and "information_schema".

  The client is responsible for destruction of the returned iterator.

  @return a pointer to an iterator object.
*/
Obj_iterator *get_all_databases(THD *thd);

This method shall use elevation to issue the SELECT against the INFORMATION_SCHEMA database to return a complete list of databases. This can then be used to compare the number of elements in the returned list with those of the list returned by get_databases();

If the count is different, it indicates one or more databases are inaccessible by the user. In this case, an error message such as the following shall be issued and the command aborted.

mysql> backup database * to '1.bak';
ERROR 1792 (HY000): Insufficient privileges. One or more databases are not acces
sible by the current user.

Note: There is no 'elements' or counter available for the Obj_iterator class. Thus, either one will have to be created or the list traversed and items counted on-the-fly.

The call to get_all_databases() will occur in the Backup_info.cc file for method Backup_info::add_all_dbs(). The error shall be generated from this method.
[3 Jul 2009 6:45] Rafal Somla
An alternative implementation idea:

- Get_databases() always returns a list of all databases existing in the server, regardless of user privileges.

- Backup kernel checks access of the user to each database returned by get_databses() and this way detects if there is a problem.
[3 Jul 2009 6:45] Ingo Strüwing
I am fine with this approach. However, it has been pointed out that the user shall not be informed that there are more databases than he is able to see. Though this is not my personal opinion, I know that architects have that opinion and thus we are bound to it. Please cut your error message after "insufficient privileges".
[3 Jul 2009 7:49] Jørgen Løland
Cutting the error message to only contain "Insufficient privileges" still reveals that there are databases the user can't see. 

The only way to give an error message like this without revealing any information about whether or not other databases exist is to require a predefined global privilege like "BACKUP on *.*". Backup will then error with "Insufficient privileges - privilege BACKUP on *.* is required to perform operation BACKUP DATABASE *"

Note that "GRANT BACKUP ON *.* to 'limited_priv'@'%'" will reveal all databases to that user:

(root)
mysql> create database db;
mysql> create user limited_priv;

(limited_priv)
mysql> select schema_name from information_schema.schemata;
+--------------------+
| schema_name        |
+--------------------+
| information_schema | 
| test               | 
+--------------------+

(root)
mysql> grant backup on *.* to 'limited_priv'@'%';

(reconnect as limited_priv)
mysql> select schema_name from information_schema.schemata;
+--------------------+
| schema_name        |
+--------------------+
| information_schema | 
| db                 | 
| mtr                | 
| mysql              | 
| test               | 
+--------------------+
[8 Jul 2009 22:17] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78244

2839 Chuck Bell	2009-07-08
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      The code currently silently succeeds if a user is not able to read
      (SELECT) all of the objects in the database or, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects or databases.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of teh objects in a database when
        issuing a BACKUP DATABASE <list> command.
      
      Note: The code currently checks for invalid databases so a BACKUP DATABASE
      a, b, c command could fail elsewhere in the code if a, b, or c is either
      not a database or the user does not have SELECT privilege on at least one
      other database.
     @ mysql-test/suite/backup/r/backup_security.result
        New result file.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test and added new test cases.
     @ sql/backup/backup_info.cc
        Added calls to check_user_access to check access for each database
        if a BACKUP DATABASE <list> command is issued and for all
        databases if a BACKUP DATABASE * command is issued.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods for check_user_access() method to check to see
        if user can read (SELECT) all objects for a given database or
        all databases except mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new method.
        Clarification in comments.
[9 Jul 2009 16:26] Ingo Strüwing
Requested more tests by email. Back to "in-progress".
[9 Jul 2009 20:04] Chuck Bell
After considerable testing, the minimal set of privileges needed to backup a database using a command like 'BACKUP DATABASE backup_test TO <file>' are:

 1. GRANT SELECT ON backup_test.* TO 'bup_select_priv'@'localhost';
 2. GRANT BACKUP ON backup_test.* TO 'bup_select_priv'@'localhost';
 3. GRANT EVENT ON backup_test.* TO 'bup_select_priv'@'localhost';
 4. GRANT TRIGGER ON backup_test.* TO 'bup_select_priv'@'localhost';
 5. GRANT SELECT ON mysql.proc TO 'bup_select_priv'@'localhost';
 6. GRANT SELECT ON mysql.func TO 'bup_select_priv'@'localhost';
 7. GRANT SELECT ON mysql.columns_priv TO 'bup_select_priv'@'localhost';
 8. GRANT SELECT ON mysql.procs_priv TO 'bup_select_priv'@'localhost';
 9. GRANT SELECT ON mysql.tables_priv TO 'bup_select_priv'@'localhost';
10. GRANT SHOW VIEW ON backup_test.* TO 'bup_select_priv'@'localhost';

Notes
--------------------------------------------------------------------
     1. This is covered earlier in the code. If a user does not have
        SELECT on a database, and lists it in the <list>, an error
        will be generated.
     2. This is covered in the code already.
     3. Not covered but could be done using a call to check_access() 
        on the database specifying EVENT_ACL.
     4. Same as (3) but with TRIGGER_ACL.
5. - 9. Could be tricky. We need method(s) that will handle this.
    10. Save as (3) but with SHOW_VIEW_ACL.
[9 Jul 2009 20:55] Chuck Bell
CLARIFICATION:

The BACKUP DATABASE command should fail if the user does not have the proper privileges as described above. 

For BACKUP DATABASE *, this means the user must be able to read all databases and all objects in each database to include the ability to read the metadata for all objects.

For BACKUP DATABASE <list>, this means the user must be able to read all all objects in each database to include the ability to read the metadata for all objects.

The command shall fail with error messages that do not reveal hidden objects, disclose missing privileges, or similar security sensitive information.
[9 Jul 2009 21:07] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78321

2839 Chuck Bell	2009-07-09
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        New result file.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test and added new test cases.
     @ sql/backup/backup_info.cc
        Added calls to check user access access for each database to ensure the
        user has the minimal set of permissions to read all objects and their
        metadata. It also ensures the user can read all databases listed either
        as the wildcard '*' or as a list.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user can read (SELECT, TRIGGER, 
        EVENT) all objects and their metadata (SHOW VIEW, SELECT on specific
        mysql tables) for a given database and read (SELECT) on all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[10 Jul 2009 6:44] Ingo Strüwing
I disagree with the newly introduced requirement, not to disclose required privileges, in the comment of 9 Jul 22:55 from Chuck.

In fact, the patch proposed earlier contained error messages that explicitly tell the user about missing BACKUP or RESTORE privileges.

IMHO it is not a problem to tell the user, which privilege is missing on an object that he knows to exist.

The only requirement so far was that the user must not be told, which objects he cannot see.
[10 Jul 2009 14:19] Chuck Bell
Disregard last requirement stated. The following requirement states the behavior of the system regarding the original problem.

If a user can run backup (she has the BACKUP privilege on the database) and can read all objects (she has the SELECT privilege on all objects plus TRIGGER and EVENT for triggers and events) then the backup image shall contain all of the objects in the database. 

Note: The user may also require privileges on certain mysql database tables and the SHOW VIEW privilege in order to successfully complete a backup. The above requirement concerns visibility of the objects only. The patch shall not check for these additional privileges and if missing may result in an error.
[10 Jul 2009 20:16] Chuck Bell
Further clarification...the above requirement is not possible to implement and work for all test cases.

If a user can run backup (she has the BACKUP privilege on the database) and can see all objects (she has at least the SELECT privilege on all objects and databases) then the backup image shall contain all of the objects in the database(s). If a user cannot see all objects and databases, the backup command shall fail with an error.
[10 Jul 2009 21:56] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78437

2839 Chuck Bell	2009-07-10
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      PROBLEM: The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        New result file with additional test cases.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test file and added new test cases.
     @ sql/backup/backup_info.cc
        Added call to check user access for each database to ensure the
        user has visibility to all objects. 
        Added call to check user access for all databases in the event a
        BACKUP DATABASE * command is issued.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user has visibility on all objects
        for a given database and has visibility for all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[13 Jul 2009 18:43] Chuck Bell
Found an issue with patch. Resubmitting.
[14 Jul 2009 1:07] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78607

2844 Chuck Bell	2009-07-13 [merge]
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      PROBLEM: The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        Updated result file with additional test cases.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test file and added new test cases.
     @ sql/backup/backup_info.cc
        Added call to check user access for each database to ensure the
        user has visibility to all objects. 
        Added call to check user access for all databases in the event a
        BACKUP DATABASE * command is issued.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user has visibility on all objects
        for a given database and has visibility for all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[14 Jul 2009 16:06] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78660

2844 Chuck Bell	2009-07-14 [merge]
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      PROBLEM: The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        Updated result file with additional test cases.
     @ mysql-test/suite/backup/r/backup_security_check.result
        New result file.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test file and added new test cases.
     @ mysql-test/suite/backup/t/backup_security_check.test
        New test to check that backup images contain all objects in the backup
        image or fail. This test can detect if a regression has occurred in 
        the object checking (counting) mechanism introduced in this patch.
     @ sql/backup/backup_info.cc
        Added call to check user access for each database to ensure the
        user has visibility to all objects. 
        Added call to check user access for all databases in the event a
        BACKUP DATABASE * command is issued.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user has visibility on all objects
        for a given database and has visibility for all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[15 Jul 2009 14:22] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78739

2844 Chuck Bell	2009-07-15 [merge]
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      PROBLEM: The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        Updated result file with additional test cases.
     @ mysql-test/suite/backup/r/backup_security_check.result
         New result file.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test file and added new test cases.
     @ mysql-test/suite/backup/t/backup_security_check.test
        New test to check that backup images contain all objects in the backup
        image or fail. This test can detect if a regression has occurred in 
        the object checking (counting) mechanism introduced in this patch.
     @ sql/backup/backup_info.cc
        Added call to check user access for each database to ensure the
        user has visibility to all objects. 
        Added call to check user access for all databases in the event a
        BACKUP DATABASE * command is issued.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user has visibility on all objects
        for a given database and has visibility for all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[15 Jul 2009 16:44] Rafal Somla
Good to push.
[15 Jul 2009 19:53] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/78785

2845 Chuck Bell	2009-07-15 [merge]
      BUG#45889 : BACKUP DATABASE command should include all objects 
      
      PROBLEM: The code currently silently succeeds if a user is not able to read
      all of the objects and their metadata in the database and, in the case of
      BACKUP DATABASE *, is not able to read all of the databases.
      
      This is an error because the BACKUP DATABASE command has been clarified
      to error if a user cannot read all of the objects and their metadata
      all all of the databases listed.
      
      This patch implements a solution to produce an error if either of the
      following are true:
      
      * The user cannot read (SELECT) all of the databases when issuing a
        BACKUP DATABASE * command.
      * The user cannot read (SELECT) all of the objects in a database
        and read their metadata when issuing a BACKUP DATABASE <list> command.
     @ mysql-test/suite/backup/r/backup_security.result
        Updated result file with additional test cases.
     @ mysql-test/suite/backup/r/backup_security_check.result
        New result file.
     @ mysql-test/suite/backup/t/backup_security.test
        Reorganized test file and added new test cases.
     @ mysql-test/suite/backup/t/backup_security_check.test
        New test to check that backup images contain all objects in the backup
        image or fail. This test can detect if a regression has occurred in 
        the object checking (counting) mechanism introduced in this patch.
     @ sql/backup/backup_info.cc
        Added call to check user access for each database to ensure the
        user has visibility to all objects. 
        Added call to check user access for all databases in the event a
        BACKUP DATABASE * command is issued.
     @ sql/share/errmsg-utf8.txt
        New error messages.
     @ sql/share/errmsg.txt
        New error messages.
     @ sql/si_objects.cc
        Added helper methods to check to see if user has visibility on all objects
        for a given database and has visibility for all databases except 
        mysql and information_schema.
     @ sql/si_objects.h
        Declaration for new methods.
        Clarification in comments.
[6 Aug 2009 8:27] Bugs System
Pushed into 5.4.4-alpha (revid:alik@sun.com-20090806082225-qssc912qdv1mm6xv) (version source revid:ingo.struewing@sun.com-20090720094244-4ue59bdysxrrm095) (merge vers: 5.4.4-alpha) (pib:11)
[6 Aug 2009 19:44] Paul DuBois
Not in any released version. No changelog entry needed.