From 141e79616303320289b33323cedbcdbe2d78c445 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Fri, 30 Oct 2009 14:28:33 -0600 Subject: [PATCH 1/4] Bug#35106: mysql_secure_installation fails on Windows, missing "use Term::ReadKey" Add the missing module import. Also, while here, fix a few glaring problems with the script, and ensure that it behaves properly. It seems this script may have never been working correctly (e.g., reading password didn't chomp() the result, so password was set with \n at the end; comparing the re-typed password to original was done with inverted test). Add END { cleanup(); } block to ensure the script removes temporary working files. Add SIG{INT} / SIG{QUIT} handler. Do a bit of reorganization to make the code easier to understand. Limit failed connection attempts to 3. Use ./bin/mysql if it exists, and then fall back on mysql in PATH (before it assumed 'mysql' in the path). Print a nicer error if 'mysql' can't be called. This has been tested on Windows (ActivePerl from cmd.exe, no cygwin needed) and Linux. --- scripts/mysql_secure_installation.pl.in | 175 +++++++++++++----------- 1 file changed, 95 insertions(+), 80 deletions(-) diff --git a/scripts/mysql_secure_installation.pl.in b/scripts/mysql_secure_installation.pl.in index 4eeb50e6d2f..281f3558808 100755 --- a/scripts/mysql_secure_installation.pl.in +++ b/scripts/mysql_secure_installation.pl.in @@ -17,17 +17,42 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA use Fcntl; +use File::Spec; +use if $^O eq 'MSWin32', 'Term::ReadKey' => qw/ReadMode/; use strict; my $config = ".my.cnf.$$"; my $command = ".mysql.$$"; my $hadpass = 0; - -# FIXME -# trap "interrupt" 2 - +my $mysql; # How to call the mysql client my $rootpass = ""; + +$SIG{QUIT} = $SIG{INT} = sub { + print "\nAborting!\n\n"; + echo_on(); + cleanup(); + exit 1; +}; + + +END { + # Remove temporary files, even if exiting via die(), etc. + cleanup(); +} + + +sub read_without_echo { + my ($prompt) = @_; + print $prompt; + echo_off(); + my $answer = ; + echo_on(); + print "\n"; + chomp($answer); + return $answer; +} + sub echo_on { if ($^O eq 'MSWin32') { ReadMode('normal'); @@ -55,6 +80,25 @@ sub write_file { } sub prepare { + # Locate the mysql client; look in current directory first, then + # in path + our $SAVEERR; # Suppress Perl warning message + open SAVEERR, ">& STDERR"; + close STDERR; + for my $m (File::Spec->catfile('bin', 'mysql'), 'mysql') { + # mysql --version should always work + qx($m --no-defaults --version); + next unless $? == 0; + + $mysql = $m; + last; + } + open STDERR, ">& SAVEERR"; + + die "Can't find a 'mysql' client in PATH or ./bin\n" + unless $mysql; + + # Create safe files to avoid leaking info to other users foreach my $file ( $config, $command ) { next if -f $file; # Already exists local *FILE; @@ -67,8 +111,9 @@ sub prepare { sub do_query { my $query = shift; write_file($command, $query); - system("mysql --defaults-file=$config < $command"); - return $?; + my $rv = system("$mysql --defaults-file=$config < $command"); + die "Failed to execute mysql client '$mysql'\n" if $rv == -1; + return ($rv == 0 ? 1 : undef); } sub make_config { @@ -82,12 +127,9 @@ sub make_config { } sub get_root_password { - my $status = 1; - while ( $status == 1 ) { - echo_off(); - print "Enter current password for root (enter for none): "; - my $password = ; - echo_on(); + my $attempts = 3; + for (;;) { + my $password = read_without_echo("Enter current password for root (enter for none): "); if ( $password ) { $hadpass = 1; } else { @@ -95,64 +137,56 @@ sub get_root_password { } $rootpass = $password; make_config($rootpass); - do_query(""); - $status = $?; + last if do_query(""); + + die "Unable to connect to the server as root user, giving up.\n" + if --$attempts == 0; } print "OK, successfully used password, moving on...\n\n"; } sub set_root_password { - echo_off(); - print "New password: "; - my $password1 = ; - print "\nRe-enter new password: "; - my $password2 = ; - print "\n"; - echo_on(); + my $password1; + for (;;) { + $password1 = read_without_echo("New password: "); - if ( $password1 eq $password2 ) { - print "Sorry, passwords do not match.\n\n"; - return 1; - } - - if ( !$password1 ) { - print "Sorry, you can't use an empty password here.\n\n"; - return 1; - } - - do_query("UPDATE mysql.user SET Password=PASSWORD('$password1') WHERE User='root';"); - if ( $? == 0 ) { - print "Password updated successfully!\n"; - print "Reloading privilege tables..\n"; - if ( !reload_privilege_tables() ) { - exit 1; + if ( !$password1 ) { + print "Sorry, you can't use an empty password here.\n\n"; + next; } - print "\n"; - $rootpass = $password1; - make_config($rootpass); - } else { - print "Password update failed!\n"; - exit 1; + + my $password2 = read_without_echo("Re-enter new password: "); + + if ( $password1 ne $password2 ) { + print "Sorry, passwords do not match.\n\n"; + next; + } + + last; } - return 0; + # FIXME: Quote password1 properly for SQL + do_query("UPDATE mysql.user SET Password=PASSWORD('$password1') WHERE User='root';") + or die "Password update failed!\n"; + + print "Password updated successfully!\n"; + print "Reloading privilege tables..\n"; + reload_privilege_tables() + or die "Can not continue.\n"; + + print "\n"; + $rootpass = $password1; + make_config($rootpass); } sub remove_anonymous_users { - do_query("DELETE FROM mysql.user WHERE User='';"); - if ( $? == 0 ) { - print " ... Success!\n"; - } else { - print " ... Failed!\n"; - exit 1; - } - - return 0; + do_query("DELETE FROM mysql.user WHERE User='';") + or die print " ... Failed!\n"; + print " ... Success!\n"; } sub remove_remote_root { - do_query("DELETE FROM mysql.user WHERE User='root' AND Host!='localhost';"); - if ( $? == 0 ) { + if (do_query("DELETE FROM mysql.user WHERE User='root' AND Host!='localhost';")) { print " ... Success!\n"; } else { print " ... Failed!\n"; @@ -161,44 +195,31 @@ sub remove_remote_root { sub remove_test_database { print " - Dropping test database...\n"; - do_query("DROP DATABASE test;"); - if ( $? == 0 ) { + if (do_query("DROP DATABASE test;")) { print " ... Success!\n"; } else { print " ... Failed! Not critical, keep moving...\n"; } print " - Removing privileges on test database...\n"; - do_query("DELETE FROM mysql.db WHERE Db='test' OR Db='test\\_%'"); - if ( $? == 0 ) { + if (do_query("DELETE FROM mysql.db WHERE Db='test' OR Db='test\\_%'")) { print " ... Success!\n"; } else { print " ... Failed! Not critical, keep moving...\n"; } - - return 0; } sub reload_privilege_tables { - do_query("FLUSH PRIVILEGES;"); - if ( $? == 0 ) { + if (do_query("FLUSH PRIVILEGES;")) { print " ... Success!\n"; - return 0; + return 1; } else { print " ... Failed!\n"; - return 1; + return undef; } } -sub interrupt { - print "\nAborting!\n\n"; - cleanup(); - echo_on(); - exit 1; -} - sub cleanup { - print "Cleaning up...\n"; unlink($config,$command); } @@ -242,11 +263,7 @@ my $reply = ; if ( $reply =~ /n/i ) { print " ... skipping.\n"; } else { - my $status = 1; - while ( $status == 1 ) { - set_root_password(); - $status = $?; - } + set_root_password(); } print "\n"; @@ -334,8 +351,6 @@ if ( $reply =~ /n/i ) { } print "\n"; -cleanup(); - print < Date: Tue, 3 Nov 2009 13:32:12 -0700 Subject: [PATCH 2/4] Bug#48086: mysql_secure_installation does NOT work on Solaris Remove a bash-ism (if ! ...). --- scripts/mysql_secure_installation.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/mysql_secure_installation.sh b/scripts/mysql_secure_installation.sh index 6c2d88d6d29..597f6cac83f 100644 --- a/scripts/mysql_secure_installation.sh +++ b/scripts/mysql_secure_installation.sh @@ -98,9 +98,7 @@ set_root_password() { if [ $? -eq 0 ]; then echo "Password updated successfully!" echo "Reloading privilege tables.." - if ! reload_privilege_tables; then - exit 1 - fi + reload_privilege_tables || exit 1 echo rootpass=$password1 make_config From e29b7ef5b8f663c04d7a40a575a6ff8c231c9433 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 3 Nov 2009 13:50:28 -0700 Subject: [PATCH 3/4] Bug#48031: mysql_secure_installation -- bash bug regarding passwords with special chars This script failed when the user tried passwords with multiple spaces, \, # or ' characters. Now proper escaping and quoting is used in all contexts. This problem occurs in the Perl version of this script, too, so fix it in both places. --- scripts/mysql_secure_installation.pl.in | 24 +++++++++++++++++--- scripts/mysql_secure_installation.sh | 30 ++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/scripts/mysql_secure_installation.pl.in b/scripts/mysql_secure_installation.pl.in index 281f3558808..255416763ef 100755 --- a/scripts/mysql_secure_installation.pl.in +++ b/scripts/mysql_secure_installation.pl.in @@ -108,6 +108,23 @@ sub prepare { } } +# Simple escape mechanism (\-escape any ' and \), suitable for two contexts: +# - single-quoted SQL strings +# - single-quoted option values on the right hand side of = in my.cnf +# +# These two contexts don't handle escapes identically. SQL strings allow +# quoting any character (\C => C, for any C), but my.cnf parsing allows +# quoting only \, ' or ". For example, password='a\b' quotes a 3-character +# string in my.cnf, but a 2-character string in SQL. +# +# This simple escape works correctly in both places. +sub basic_single_escape { + my ($str) = @_; + # Inside a character class, \ is not special; this escapes both \ and ' + $str =~ s/([\'])/\\$1/g; + return $str; +} + sub do_query { my $query = shift; write_file($command, $query); @@ -119,11 +136,12 @@ sub do_query { sub make_config { my $password = shift; + my $esc_pass = basic_single_escape($rootpass); write_file($config, "# mysql_secure_installation config file", "[mysql]", "user=root", - "password=$rootpass"); + "password='$esc_pass'"); } sub get_root_password { @@ -165,8 +183,8 @@ sub set_root_password { last; } - # FIXME: Quote password1 properly for SQL - do_query("UPDATE mysql.user SET Password=PASSWORD('$password1') WHERE User='root';") + my $esc_pass = basic_single_escape($password1); + do_query("UPDATE mysql.user SET Password=PASSWORD('$esc_pass') WHERE User='root';") or die "Password update failed!\n"; print "Password updated successfully!\n"; diff --git a/scripts/mysql_secure_installation.sh b/scripts/mysql_secure_installation.sh index 597f6cac83f..25d6343ee2c 100644 --- a/scripts/mysql_secure_installation.sh +++ b/scripts/mysql_secure_installation.sh @@ -38,16 +38,39 @@ prepare() { } do_query() { - echo $1 >$command + echo "$1" >$command + #sed 's,^,> ,' < $command # Debugging mysql --defaults-file=$config <$command return $? } +# Simple escape mechanism (\-escape any ' and \), suitable for two contexts: +# - single-quoted SQL strings +# - single-quoted option values on the right hand side of = in my.cnf +# +# These two contexts don't handle escapes identically. SQL strings allow +# quoting any character (\C => C, for any C), but my.cnf parsing allows +# quoting only \, ' or ". For example, password='a\b' quotes a 3-character +# string in my.cnf, but a 2-character string in SQL. +# +# This simple escape works correctly in both places. +basic_single_escape () { + # The quoting on this sed command is a bit complex. Single-quoted strings + # don't allow *any* escape mechanism, so they cannot contain a single + # quote. The string sed gets (as argv[1]) is: s/\(['\]\)/\\\1/g + # + # Inside a character class, \ and ' are not special, so the ['\] character + # class is balanced and contains two characters. + echo "$1" | sed 's/\(['"'"'\]\)/\\\1/g' +} + make_config() { echo "# mysql_secure_installation config file" >$config echo "[mysql]" >>$config echo "user=root" >>$config - echo "password=$rootpass" >>$config + esc_pass=`basic_single_escape "$rootpass"` + echo "password='$esc_pass'" >>$config + #sed 's,^,> ,' < $config # Debugging } get_root_password() { @@ -94,7 +117,8 @@ set_root_password() { return 1 fi - do_query "UPDATE mysql.user SET Password=PASSWORD('$password1') WHERE User='root';" + esc_pass=`basic_single_escape "$password1"` + do_query "UPDATE mysql.user SET Password=PASSWORD('$esc_pass') WHERE User='root';" if [ $? -eq 0 ]; then echo "Password updated successfully!" echo "Reloading privilege tables.." From 345054c916672dcfb51492d68fc0d00eb999d430 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 3 Nov 2009 14:34:01 -0700 Subject: [PATCH 4/4] Add a few comments to clarify do_query() return values in mysql_secure_installation.pl --- scripts/mysql_secure_installation.pl.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/mysql_secure_installation.pl.in b/scripts/mysql_secure_installation.pl.in index 255416763ef..25339f9b916 100755 --- a/scripts/mysql_secure_installation.pl.in +++ b/scripts/mysql_secure_installation.pl.in @@ -129,7 +129,11 @@ sub do_query { my $query = shift; write_file($command, $query); my $rv = system("$mysql --defaults-file=$config < $command"); + # system() returns -1 if exec fails (e.g., command not found, etc.); die + # in this case because nothing is going to work die "Failed to execute mysql client '$mysql'\n" if $rv == -1; + # Return true if query executed OK, or false if there was some problem + # (for example, SQL error or wrong password) return ($rv == 0 ? 1 : undef); }