#At file:///localhome/jl208045/mysql/mysql-6.0-backup-34171/
2679 Jorgen Loland 2008-08-21
Bug#34171 - "Backup: ignoring --secure-file-priv"
Before patch: Backup code ignored secure-file-priv option,
allowing backup to write to any location
After patch: Backup and restore are not allowed to write or read
outside path in secure-file-priv option if option has been
specified.
added:
mysql-test/r/backup_securefilepriv.result
mysql-test/t/backup_backupdir-master.opt
mysql-test/t/backup_securefilepriv-master.opt
mysql-test/t/backup_securefilepriv.test
modified:
mysql-test/lib/mtr_report.pl
mysql-test/r/backup_backupdir.result
mysql-test/r/backup_errors.result
sql/backup/stream.cc
sql/backup/stream.h
sql/share/errmsg.txt
per-file comments:
mysql-test/lib/mtr_report.pl
Added expected error for backup_securefilepriv
mysql-test/r/backup_backupdir.result
Result modified to handle slightly changed error message.
mysql-test/r/backup_errors.result
Result modified to handle slightly changed error message.
mysql-test/r/backup_securefilepriv.result
New result file for secure-file-priv backup tests
mysql-test/t/backup_backupdir-master.opt
Sets secure-file-priv to / for backup_backupdir test
mysql-test/t/backup_securefilepriv-master.opt
Sets secure-file-priv option for backup_securefilepriv test
mysql-test/t/backup_securefilepriv.test
New test file for secure-file-priv backup tests
sql/backup/stream.cc
Adds check for secure-file-priv option when opening backup/restore streams. Also some doc-bug fixes for Stream::prepare_path
sql/backup/stream.h
Added private method test_secure_file_priv_access
sql/share/errmsg.txt
Changed ER_BACKUP_WRITE_LOC to include secure-file-priv in err message
=== modified file 'mysql-test/lib/mtr_report.pl'
--- a/mysql-test/lib/mtr_report.pl 2008-08-20 13:23:10 +0000
+++ b/mysql-test/lib/mtr_report.pl 2008-08-21 08:38:12 +0000
@@ -357,6 +357,12 @@ sub mtr_report_stats ($) {
(
/Restore: Tablespace .* needed by tables being restored has changed on the server/
) or
+
+ # The securefilepriv test triggers error below on purpose
+ ($testname eq 'main.backup_securefilepriv') and
+ (
+ /Backup: Can't write to backup location/
+ ) or
# The views test triggers errors below on purpose
($testname eq 'main.backup_views') and
=== modified file 'mysql-test/r/backup_backupdir.result'
--- a/mysql-test/r/backup_backupdir.result 2008-08-08 17:21:31 +0000
+++ b/mysql-test/r/backup_backupdir.result 2008-08-21 08:38:12 +0000
@@ -42,10 +42,10 @@ backup_id
Ensure backup image file went to the correct location
Try a backup to an invalid relative path.
BACKUP DATABASE bup_backupdir TO '../../../../../../../../../../../../../../../../../../bup_backupdir5.bak';
-ERROR HY000: Can't write to backup location '../../../../../../../../../../../../../../../../../../bup_backup' (file already exists?)
+ERROR HY000: Can't write to backup location '../../../../../../../../../../../../../../../../../../bup_backup' (file already exists or running with --secure-file-priv option?)
Try a backup to an invalid hard path.
BACKUP DATABASE bup_backupdir TO '/dev/not/there/either/bup_backupdir6.bak';
-ERROR HY000: Can't write to backup location '/dev/not/there/either/bup_backupdir6.bak' (file already exists?)
+ERROR HY000: Can't write to backup location '/dev/not/there/either/bup_backupdir6.bak' (file already exists or running with --secure-file-priv option?)
SET @@global.backupdir = 'This_is_really_stupid/not/there/at/all';
Warnings:
Warning 1725 The path specified for the system variable backupdir cannot be accessed or is invalid. ref: This_is_really_stupid/not/there/at/all
=== modified file 'mysql-test/r/backup_errors.result'
--- a/mysql-test/r/backup_errors.result 2008-03-05 17:48:12 +0000
+++ b/mysql-test/r/backup_errors.result 2008-08-21 08:38:12 +0000
@@ -8,12 +8,12 @@ CREATE TABLE bdb.t1(a int) ENGINE=MEMORY
BACKUP DATABASE adb TO '';
ERROR HY000: Malformed file path ''
BACKUP DATABASE adb TO "bdb/t1.frm";
-ERROR HY000: Can't write to backup location 'bdb/t1.frm' (file already exists?)
+ERROR HY000: Can't write to backup location 'bdb/t1.frm' (file already exists or running with --secure-file-priv option?)
BACKUP DATABASE adb TO "test.bak";
backup_id
#
BACKUP DATABASE adb TO "test.bak";
-ERROR HY000: Can't write to backup location 'test.bak' (file already exists?)
+ERROR HY000: Can't write to backup location 'test.bak' (file already exists or running with --secure-file-priv option?)
DROP DATABASE IF EXISTS foo;
DROP DATABASE IF EXISTS bar;
BACKUP DATABASE foo TO 'test.bak';
=== added file 'mysql-test/r/backup_securefilepriv.result'
--- a/mysql-test/r/backup_securefilepriv.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/backup_securefilepriv.result 2008-08-21 08:38:12 +0000
@@ -0,0 +1,63 @@
+Initializing tests
+Create directories for backup images
+Creating database and populating tables
+CREATE DATABASE sfp_db;
+CREATE TABLE sfp_db.t1 (i int);
+INSERT INTO sfp_db.t1 VALUES (1);
+INSERT INTO sfp_db.t1 VALUES (2);
+INSERT INTO sfp_db.t1 VALUES (3);
+INSERT INTO sfp_db.t1 VALUES (4);
+INSERT INTO sfp_db.t1 VALUES (5);
+INSERT INTO sfp_db.t1 VALUES (10);
+INSERT INTO sfp_db.t1 VALUES (15);
+INSERT INTO sfp_db.t1 VALUES (100);
+
+Starting tests
+
+Backup to secure_file_priv dir
+(MYSQLTEST_VARDIR/master-data/sfp_path)
+BACKUP DATABASE sfp_db TO 'sfp_path/bup_sfp1.bak';
+backup_id
+#
+Ensure backup image file went to the correct location
+
+Backup to subpath of secure_file_priv
+(MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+BACKUP DATABASE sfp_db TO 'sfp_path/subpath/bup_sfp2.bak';
+backup_id
+#
+Ensure backup image file went to the correct location
+
+Change backupdir to sfp_path/subpath
+(MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+SET @@global.backupdir = 'sfp_path/subpath';
+
+Backup to secure_file_priv subpath, no dir in backup file name
+(MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+BACKUP DATABASE sfp_db TO 'bup_sfp3.bak';
+backup_id
+#
+Ensure backup image file went to the correct location
+
+Backup to secure_file_priv, relative path in backup file name
+(MYSQLTEST_VARDIR/master-data/sfp_path)
+BACKUP DATABASE sfp_db TO '../bup_sfp4.bak';
+backup_id
+#
+Ensure backup image file went to the correct location
+
+Backup to relative path below secure_file_priv path should fail
+BACKUP DATABASE sfp_db TO '../../bup_sfp_fail1.bak';
+ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement
+
+Reset backupdir to MYSQLTEST_VARDIR/master-data/
+SET @@global.backupdir = @@global.datadir;
+
+Backup to other path than secure_file_priv should fail
+BACKUP DATABASE sfp_db TO 'bup_sfp_fail2.bak';
+ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement
+
+Cleanup
+
+DROP TABLE sfp_db.t1;
+DROP DATABASE sfp_db;
=== added file 'mysql-test/t/backup_backupdir-master.opt'
--- a/mysql-test/t/backup_backupdir-master.opt 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/backup_backupdir-master.opt 2008-08-21 08:38:12 +0000
@@ -0,0 +1 @@
+--secure-file-priv=/
=== added file 'mysql-test/t/backup_securefilepriv-master.opt'
--- a/mysql-test/t/backup_securefilepriv-master.opt 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/backup_securefilepriv-master.opt 2008-08-21 08:38:12 +0000
@@ -0,0 +1 @@
+--secure-file-priv=$MYSQLTEST_VARDIR/master-data/sfp_path
=== added file 'mysql-test/t/backup_securefilepriv.test'
--- a/mysql-test/t/backup_securefilepriv.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/backup_securefilepriv.test 2008-08-21 08:38:12 +0000
@@ -0,0 +1,118 @@
+#
+# Purpose: Backup images should only be allowed to be written to the
+# path specified by secure_file_priv or a sub-path of it.
+#
+# See backup_securefilepriv-master.opt for secure_file_priv command line option
+#
+# @@backupdir is MYSQLTEST_VARDIR/master-data/
+# @@secure_file_priv is MYSQLTEST_VARDIR/master-data/sfp_path/
+
+--echo Initializing tests
+
+--error 0,1,2
+rmdir $MYSQLTEST_VARDIR/master-data/sfp_path/subpath;
+--error 0,1,2
+rmdir $MYSQLTEST_VARDIR/master-data/sfp_path;
+
+--echo Create directories for backup images
+mkdir $MYSQLTEST_VARDIR/master-data/sfp_path;
+mkdir $MYSQLTEST_VARDIR/master-data/sfp_path/subpath;
+
+--echo Creating database and populating tables
+
+CREATE DATABASE sfp_db;
+
+CREATE TABLE sfp_db.t1 (i int);
+
+INSERT INTO sfp_db.t1 VALUES (1);
+INSERT INTO sfp_db.t1 VALUES (2);
+INSERT INTO sfp_db.t1 VALUES (3);
+INSERT INTO sfp_db.t1 VALUES (4);
+INSERT INTO sfp_db.t1 VALUES (5);
+INSERT INTO sfp_db.t1 VALUES (10);
+INSERT INTO sfp_db.t1 VALUES (15);
+INSERT INTO sfp_db.t1 VALUES (100);
+
+--echo
+--echo Starting tests
+
+--echo
+--echo Backup to secure_file_priv dir
+--echo (MYSQLTEST_VARDIR/master-data/sfp_path)
+--replace_column 1 #
+BACKUP DATABASE sfp_db TO 'sfp_path/bup_sfp1.bak';
+
+--echo Ensure backup image file went to the correct location
+--file_exists $MYSQLTEST_VARDIR/master-data/sfp_path/bup_sfp1.bak
+
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/sfp_path/bup_sfp1.bak
+
+--echo
+--echo Backup to subpath of secure_file_priv
+--echo (MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+--replace_column 1 #
+BACKUP DATABASE sfp_db TO 'sfp_path/subpath/bup_sfp2.bak';
+
+--echo Ensure backup image file went to the correct location
+--file_exists $MYSQLTEST_VARDIR/master-data/sfp_path/subpath/bup_sfp2.bak
+
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/sfp_path/subpath/bup_sfp2.bak
+
+--echo
+--echo Change backupdir to sfp_path/subpath
+--echo (MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+SET @@global.backupdir = 'sfp_path/subpath';
+
+--echo
+--echo Backup to secure_file_priv subpath, no dir in backup file name
+--echo (MYSQLTEST_VARDIR/master-data/sfp_path/subpath)
+--replace_column 1 #
+BACKUP DATABASE sfp_db TO 'bup_sfp3.bak';
+
+--echo Ensure backup image file went to the correct location
+--file_exists $MYSQLTEST_VARDIR/master-data/sfp_path/subpath/bup_sfp3.bak
+
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/sfp_path/subpath/bup_sfp3.bak
+
+--echo
+--echo Backup to secure_file_priv, relative path in backup file name
+--echo (MYSQLTEST_VARDIR/master-data/sfp_path)
+--replace_column 1 #
+BACKUP DATABASE sfp_db TO '../bup_sfp4.bak';
+
+--echo Ensure backup image file went to the correct location
+--file_exists $MYSQLTEST_VARDIR/master-data/sfp_path/bup_sfp4.bak
+
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/sfp_path/bup_sfp4.bak
+
+# Tests that fail
+
+--echo
+--echo Backup to relative path below secure_file_priv path should fail
+--error ER_OPTION_PREVENTS_STATEMENT
+BACKUP DATABASE sfp_db TO '../../bup_sfp_fail1.bak';
+
+--echo
+--echo Reset backupdir to MYSQLTEST_VARDIR/master-data/
+SET @@global.backupdir = @@global.datadir;
+
+--echo
+--echo Backup to other path than secure_file_priv should fail
+--error ER_OPTION_PREVENTS_STATEMENT
+BACKUP DATABASE sfp_db TO 'bup_sfp_fail2.bak';
+
+--echo
+--echo Cleanup
+--echo
+
+DROP TABLE sfp_db.t1;
+DROP DATABASE sfp_db;
+
+--error 0,1,2
+rmdir $MYSQLTEST_VARDIR/master-data/sfp_path/subpath;
+--error 0,1,2
+rmdir $MYSQLTEST_VARDIR/master-data/sfp_path;
=== modified file 'sql/backup/stream.cc'
--- a/sql/backup/stream.cc 2008-08-08 17:21:31 +0000
+++ b/sql/backup/stream.cc 2008-08-21 08:38:12 +0000
@@ -218,6 +218,9 @@ Stream::Stream(Logger &log, ::String *ba
For example, if backupdir = '/dev/tmp' and orig_log = '../backup.bak', the
combined path is = '/dev/backup.bak'.
+ Note: fn_format may likely be used instead of this function - will
+ not be changed unless bugs are found.
+
@returns
0 if success
1 if cannot be combined. Note: m_path is set to '' when this occurs to
@@ -288,6 +291,23 @@ int Stream::prepare_path(::String *backu
{
int path_len= 0;
+
+ // If backupdir is not a hard path, replace it with the full path
+ // name. Full path is needed to check secure_file_priv.
+ if (!test_if_hard_path((*backupdir).c_ptr())) {
+ char full_path[FN_LEN];
+ /*
+ MY_UNPACK_FILENAME -> "~/" will be changed to full path
+ MY_RELATIVE_PATH -> path is constructed from the path in both
+ arguments, including that every "../" in backupdir removes
+ deepest dir from mysql_real_data_home
+ */
+ fn_format(full_path, (*backupdir).c_ptr(), mysql_real_data_home, "",
+ MY_UNPACK_FILENAME|MY_RELATIVE_PATH);
+
+ (*backupdir)= (String)full_path;
+ }
+
/*
Prepare the path using the backupdir iff no relative path
or no hard path included.
@@ -301,9 +321,9 @@ int Stream::prepare_path(::String *backu
Make relative to backupdir.
Example BACKUP DATATBASE ... TO '../monthly/dec.bak'
- If backupdir = '/dev/daily/backup' then the
+ If backupdir = '/dev/daily' then the
calculated path becomes
- '/dev/monthly/backup/dec.bak'
+ '/dev/monthly/dec.bak'
*/
char new_path[FN_LEN];
if (make_relative_path(new_path, orig_loc.str, backupdir))
@@ -316,27 +336,31 @@ int Stream::prepare_path(::String *backu
else if (!test_if_hard_path(orig_loc.str))
{
/*
- Case 2: Backup image file name has hard path.
+ Case 2: Backup image file name has no path or has a subpath.
- Example BACKUP DATATBASE ... TO '/dev/dec.bak'
- If backupdir = '/dev/daily/backup' then the
+ Example BACKUP DATABASE ... TO 'week2.bak'
+ If backupdir = '/dev/weekly/' then the
calculated path becomes
- '/dev/dec.bak'
+ '/dev/weekly/week2.bak'
+ Example BACKUP DATABASE ... TO 'jan/day1.bak'
+ If backupdir = '/dev/monthly/' then the
+ calculated path becomes
+ '/dev/monthly/jan/day1.bak'
*/
path_len=backupdir->length() + orig_loc.length + 1;
m_path.alloc(path_len);
fn_format(m_path.c_ptr(), orig_loc.str, backupdir->c_ptr(), "",
- MY_UNPACK_FILENAME);
+ MY_UNPACK_FILENAME|MY_RELATIVE_PATH);
}
else
{
/*
- Case 3: Backup image file name has no path.
+ Case 3: Backup image file name has hard path.
- Example BACKUP DATATBASE ... TO 'week2.bak'
- If backupdir = '/dev/weekly/backup' then the
+ Example BACKUP DATATBASE ... TO '/dev/dec.bak'
+ If backupdir = '/dev/daily/backup' then the
calculated path becomes
- '/dev/weekly/week2.bak'
+ '/dev/dec.bak'
*/
path_len= orig_loc.length + 1;
m_path.alloc(path_len);
@@ -347,9 +371,35 @@ int Stream::prepare_path(::String *backu
return 0;
}
+/**
+ Check if secure_file_priv variable has been set and if so, whether
+ or not backup tries to write to the path (or a sub-path) specified
+ by secure_file_priv.
+
+ Reports error ER_OPTION_PREVENTS_STATEMENT if backup tries to write
+ to a different path than specified by secure_file_priv.
+
+ @retval TRUE backup is allowed to write to this path
+ @retval FALSE backup is not allowed to write to this path. Side
+ effect: error is reported
+*/
+bool Stream::test_secure_file_priv_access(char *path) {
+ bool has_access = !opt_secure_file_priv || // option not specified, or
+ !strncmp(opt_secure_file_priv, path, // path is (subpath of)
+ strlen(opt_secure_file_priv)); // opt_secure_file_priv
+ if (!has_access) {
+ /* Write only allowed to dir or subdir specified by secure_file_priv */
+ my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");
+ }
+ return has_access;
+}
+
bool Stream::open()
{
close();
+ if (!test_secure_file_priv_access(m_path.c_ptr())) {
+ return FALSE;
+ }
m_fd= my_open(m_path.c_ptr(), m_flags, MYF(0));
return m_fd >= 0;
}
=== modified file 'sql/backup/stream.h'
--- a/sql/backup/stream.h 2008-08-08 17:21:31 +0000
+++ b/sql/backup/stream.h 2008-08-21 08:38:12 +0000
@@ -109,7 +109,7 @@ private:
::String *backupdir);
int prepare_path(::String *backupdir,
LEX_STRING orig_loc);
-
+ bool test_secure_file_priv_access(char *path);
};
/// Used to write to backup stream.
=== modified file 'sql/share/errmsg.txt'
--- a/sql/share/errmsg.txt 2008-08-20 13:23:10 +0000
+++ b/sql/share/errmsg.txt 2008-08-21 08:38:12 +0000
@@ -6177,7 +6177,7 @@ ER_BACKUP_INVALID_LOC
ER_BACKUP_READ_LOC
eng "Can't read backup location '%-.64s'"
ER_BACKUP_WRITE_LOC
- eng "Can't write to backup location '%-.64s' (file already exists?)"
+ eng "Can't write to backup location '%-.64s' (file already exists or running with --secure-file-priv option?)"
ER_BACKUP_LIST_DBS
eng "Can't enumerate server databases"
ER_BACKUP_LIST_TABLES
