diff --git a/mysql/backup_db.cgi b/mysql/backup_db.cgi index 309068b5d..2ed0c8f57 100755 --- a/mysql/backup_db.cgi +++ b/mysql/backup_db.cgi @@ -53,14 +53,17 @@ if (!$in{'save'} || $in{'sched'}) { push(@compat, split(/\0/, $in{'options'})); $cron = !$module_info{'usermin'} && $access{'buser'} eq 'root' && !$access{'user'} && &foreign_installed("cron"); +$commands = &can_run_backup_commands($cron); $cmode = 0; -if ($cron) { +if ($commands) { $config{'backup_before_'.$in{'db'}} = $in{'before'}; $config{'backup_after_'.$in{'db'}} = $in{'after'}; if ($in{'all'}) { $config{'backup_cmode_'.$in{'db'}} = $in{'cmode'}; $cmode = $in{'cmode'}; } + } +if ($cron) { $config{'backup_email_'.$in{'db'}} = $in{'email'}; $config{'backup_notify_'.$in{'db'}} = $in{'notify'}; @@ -193,7 +196,7 @@ if (!$in{'save'}) { &html_escape($file)),"
\n"; next; } - if ($cron && $cmode == 0) { + if ($commands && $cmode == 0) { # Run and check before-backup command (for one DB) $bok = &execute_before($db, STDOUT, 1, $file, $in{'all'} ? undef : $db, @@ -220,7 +223,7 @@ if (!$in{'save'}) { } &execute_after($db, STDOUT, 1, $file, $in{'all'} ? undef : $db, $access{'buser'}) - if ($cron && $cmode == 0); + if ($commands && $cmode == 0); if ($in{'dest'}) { # Sent to browser diff --git a/mysql/backup_form.cgi b/mysql/backup_form.cgi index 4e9cac733..cda87e66a 100755 --- a/mysql/backup_form.cgi +++ b/mysql/backup_form.cgi @@ -28,6 +28,7 @@ if (!-x $cmd) { $cron = !$module_info{'usermin'} && $access{'buser'} eq 'root' && !$access{'user'} && &foreign_installed("cron"); +$commands = &can_run_backup_commands($cron); if ($in{'all'}) { print "$text{'backup_desc3'}\n"; } @@ -156,7 +157,7 @@ $b = $c{'backup_parameters_'.$in{'db'}}; print &ui_table_row($text{'backup_parameters'}, &ui_textbox("parameters", $b, 60)); -if ($cron) { +if ($commands) { # Show before/after commands $b = $c{'backup_before_'.$in{'db'}}; print &ui_table_row($text{'backup_before'}, @@ -175,7 +176,10 @@ if ($cron) { [ 1, $text{'backup_cmode1'} ] ])); } - print &ui_hidden_table_end("opts"); + } +print &ui_hidden_table_end("opts"); + +if ($cron) { print &ui_hidden_table_start($text{'backup_header3'}, "width=100%", 2, "sched", 1, [ "width=30%" ]); @@ -211,9 +215,6 @@ if ($cron) { print &ui_hidden_table_end("sched"); } -else { - print &ui_hidden_table_end("opts"); - } if ($cron) { print &ui_form_end([ [ "backup", $text{'backup_ok'} ], diff --git a/mysql/mysql-lib.pl b/mysql/mysql-lib.pl index 15fac1e0f..3fbc6d14c 100755 --- a/mysql/mysql-lib.pl +++ b/mysql/mysql-lib.pl @@ -1023,6 +1023,17 @@ if ($config{'webmin_subs'}) { return $rv; } +# can_run_backup_commands(cron) +# Returns true if this request can configure and run backup commands. Cron +# access already permits root commands, while other Webmin users are limited +# to commands run as their configured non-root backup user. +sub can_run_backup_commands +{ +my ($cron) = @_; +return !$module_info{'usermin'} && + ($cron || $access{'buser'} && $access{'buser'} ne 'root'); +} + # execute_before(db, handle, escape, path, db-for-config, [&as-user]) # Executes the before-backup command for some DB, and sends output to the # given file handle. Returns 1 if the command succeeds, or 0 on failure diff --git a/t/mysql-lib.t b/t/mysql-lib.t index bde1e9329..26b476902 100644 --- a/t/mysql-lib.t +++ b/t/mysql-lib.t @@ -136,6 +136,51 @@ subtest 'sandboxed SQL files use direct input' => sub { 'the sandboxed command does not use source'); }; +subtest 'backup commands honor the backup user privilege boundary' => sub { + local %main::module_info; + local %main::access = ( 'buser' => 'root' ); + ok(!main::can_run_backup_commands(0), + 'root commands require existing cron eligibility'); + ok(main::can_run_backup_commands(1), + 'cron-eligible users can run root commands'); + + $main::access{'buser'} = 'backupuser'; + ok(main::can_run_backup_commands(0), + 'Webmin backups can run commands as a non-root backup user'); + $main::module_info{'usermin'} = 1; + ok(!main::can_run_backup_commands(1), + 'Usermin cannot gain backup command access'); +}; + +subtest 'backup commands run as the non-root backup user' => sub { + no warnings 'redefine'; + local $main::config{'backup_before_testdb'} = 'before-command'; + local $main::config{'backup_after_testdb'} = 'after-command'; + my @commands; + local *main::command_as_user = sub { + my ($user, $background, $command) = @_; + is($background, 0, 'backup command remains synchronous'); + return "as-$user $command"; + }; + local *main::execute_command = sub { + push(@commands, $_[0]); + return 0; + }; + + ok(main::execute_before('testdb', undef, 0, '/tmp/test.sql', + 'testdb', 'backupuser'), + 'before command succeeds'); + ok(main::execute_after('testdb', undef, 0, '/tmp/test.sql', + 'testdb', 'backupuser'), + 'after command succeeds'); + is_deeply(\@commands, + [ 'as-backupuser before-command', + 'as-backupuser after-command' ], + 'both commands are wrapped for the backup user'); + is($ENV{'BACKUP_FILE'}, '/tmp/test.sql', + 'backup commands receive the destination path'); +}; + subtest 'binary log retention display round-trips exactly' => sub { # The page shows seconds as days, and saving converts the shown days # back with int(days * 86400 + 0.5), which must reproduce the exact