Skip to content

Commit 426aca8

Browse files
kjarirkjarir
committed
MDEV-10112: mysql_secure_installation should use DDL for Galera compatibility
Refactor mysql_secure_installation to use DDL statements (ALTER USER, DROP USER, REVOKE) instead of direct DML on system tables. This ensures proper replication in Galera clusters. - Fixes ALTER USER syntax for multiple root accounts by correctly associating authentication options with each user. - Fixes shell escaping bugs for dynamic SQL by using QUOTE(User) and QUOTE(Host). - Improves code legibility by sub-dividing long SQL strings strictly under 80 characters and indenting SET commands within here-docs. - Refactors tests per mentor (gkodinov) feedback: - Removed redundant pre-cleanup blocks in favor of fresh MTR workers. - Used isolated database 'msi_test_db' and dummy accounts. - Switched to unconditional DROP statements in cleanup to validate state. - Removed unnecessary FLUSH PRIVILEGES. - Adds galera.galera_secure_installation cluster test to verify replication of security changes across nodes. - Fixes a hang in automated tests by correctly handling the initial root password prompt with an empty input line.
1 parent 52cbcf2 commit 426aca8

5 files changed

Lines changed: 207 additions & 80 deletions

File tree

‎mysql-test/main/mysql_secure_installation.result‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
1-
# restart: --datadir=MYSQLTEST_VARDIR/tmp/msi_datadir
2-
# Verify: anonymous users removed
3-
SELECT user, host FROM mysql.global_priv WHERE user = '' ORDER BY host;
1+
#
2+
# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
3+
# instead of DML for Galera compatibility
4+
#
5+
#
6+
# Setup dummy accounts and database to be secured
7+
#
8+
CREATE USER 'root'@'remote_host' IDENTIFIED BY 'pass';
9+
CREATE USER 'test_user'@'localhost' IDENTIFIED BY 'pass';
10+
CREATE DATABASE msi_test_db;
11+
GRANT ALL PRIVILEGES ON msi_test_db.* TO 'test_user'@'localhost';
12+
GRANT ALL PRIVILEGES ON test.* TO 'test_user'@'localhost';
13+
# Verify: remote root account removed
14+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
415
user host
5-
# Verify: only local root accounts remain
6-
SELECT user, host FROM mysql.global_priv
7-
WHERE user = 'root' ORDER BY host;
8-
user host
9-
root 127.0.0.1
10-
root ::1
11-
root localhost
1216
# Verify: test database removed
1317
SHOW DATABASES LIKE 'test';
1418
Database (test)
15-
# Verify: no test DB privileges remain in mysql.db
16-
SELECT user, host, db FROM mysql.db
17-
WHERE db = 'test' OR db LIKE 'test\\_%'
18-
ORDER BY user, host, db;
19+
# Verify: test DB privileges removed from mysql.db
20+
SELECT user, host, db FROM mysql.db WHERE db = 'test' ORDER BY user, host, db;
1921
user host db
20-
# restart
22+
# Cleanup
23+
DROP USER 'test_user'@'localhost';
24+
DROP DATABASE msi_test_db;

‎mysql-test/main/mysql_secure_installation.test‎

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,38 @@
1-
# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
2-
# instead of DML for Galera compatibility
3-
#
4-
# This test verifies that mysql_secure_installation works correctly
5-
# after being converted from DML (UPDATE/DELETE on system tables) to
6-
# DDL (ALTER USER, DROP USER, REVOKE, etc).
7-
81
--source include/not_windows.inc
92
--source include/not_embedded.inc
3+
--source include/save_sys_tables.inc
4+
5+
--echo #
6+
--echo # MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
7+
--echo # instead of DML for Galera compatibility
8+
--echo #
9+
10+
--echo #
11+
--echo # Setup dummy accounts and database to be secured
12+
--echo #
13+
CREATE USER 'root'@'remote_host' IDENTIFIED BY 'pass';
14+
CREATE USER 'test_user'@'localhost' IDENTIFIED BY 'pass';
1015

11-
# To avoid affecting other tests, we run this test on a temporary datadir
12-
# using MTR's official restart mechanism.
13-
--let $MSI_DATADIR= $MYSQLTEST_VARDIR/tmp/msi_datadir
14-
--mkdir $MSI_DATADIR
15-
--exec cp -R $MYSQLTEST_VARDIR/install.db/. $MSI_DATADIR/
16-
--exec chmod -R 700 $MSI_DATADIR
16+
# 'test' is a special database in MTR, ensuring it exists for the script
17+
--disable_warnings
18+
CREATE DATABASE IF NOT EXISTS test;
19+
--enable_warnings
1720

18-
--let $old_restart_parameters= $restart_parameters
19-
--let $restart_parameters= --datadir=$MSI_DATADIR
20-
--source include/restart_mysqld.inc
21+
# Create a custom database to ensure we don't interfere with other tests
22+
CREATE DATABASE msi_test_db;
23+
GRANT ALL PRIVILEGES ON msi_test_db.* TO 'test_user'@'localhost';
2124

22-
#
23-
# Scripted answers for mariadb-secure-installation
24-
#
25+
# Verify the script's REVOKE logic by granting on 'test'
26+
GRANT ALL PRIVILEGES ON test.* TO 'test_user'@'localhost';
27+
28+
# Prepare input for the script
29+
# 1. [Empty] - current root password
30+
# 2. n - Enable unix_socket?
31+
# 3. n - Set root password?
32+
# 4. Y - Remove anonymous users?
33+
# 5. Y - Disallow root login remotely?
34+
# 6. Y - Remove test database?
35+
# 7. Y - Reload privilege tables?
2536
--write_file $MYSQLTEST_VARDIR/tmp/msi_input.txt
2637

2738
n
@@ -32,35 +43,24 @@ Y
3243
Y
3344
EOF
3445

35-
# --basedir is needed so the script can find my_print_defaults and mariadb
36-
# from the build tree (extra/ and client/ directories).
37-
# Output redirected to log file to keep result file clean.
46+
# Run the script
3847
--exec $MYSQL_SECURE_INSTALLATION --basedir=$MYSQL_BINDIR -S $MASTER_MYSOCK < $MYSQLTEST_VARDIR/tmp/msi_input.txt > $MYSQLTEST_VARDIR/tmp/msi.log 2>&1
3948

4049
--remove_file $MYSQLTEST_VARDIR/tmp/msi_input.txt
4150
--remove_file $MYSQLTEST_VARDIR/tmp/msi.log
4251

43-
#
44-
# Verify the results
45-
#
46-
--echo # Verify: anonymous users removed
47-
SELECT user, host FROM mysql.global_priv WHERE user = '' ORDER BY host;
48-
49-
--echo # Verify: only local root accounts remain
50-
SELECT user, host FROM mysql.global_priv
51-
WHERE user = 'root' ORDER BY host;
52+
--echo # Verify: remote root account removed
53+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
5254

5355
--echo # Verify: test database removed
5456
SHOW DATABASES LIKE 'test';
5557

56-
--echo # Verify: no test DB privileges remain in mysql.db
57-
SELECT user, host, db FROM mysql.db
58-
WHERE db = 'test' OR db LIKE 'test\\_%'
59-
ORDER BY user, host, db;
58+
--echo # Verify: test DB privileges removed from mysql.db
59+
SELECT user, host, db FROM mysql.db WHERE db = 'test' ORDER BY user, host, db;
60+
61+
# Cleanup
62+
# The script already dropped 'root'@'remote_host' and 'test' DB.
63+
DROP USER 'test_user'@'localhost';
64+
DROP DATABASE msi_test_db;
6065

61-
#
62-
# Clean up: restore the original server state
63-
#
64-
--let $restart_parameters= $old_restart_parameters
65-
--source include/restart_mysqld.inc
66-
--rmdir $MSI_DATADIR
66+
--source include/restore_sys_tables.inc

‎mysql-test/suite/galera/r/galera_secure_installation.result‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#
2+
# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
3+
# instead of DML for Galera compatibility
4+
#
5+
# On node_1
6+
#
7+
# Setup remote root account to be removed
8+
#
9+
CREATE USER 'root'@'remote_host' IDENTIFIED BY 'pass';
10+
GRANT ALL ON *.* TO 'root'@'remote_host';
11+
# On node_2
12+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
13+
user host
14+
root remote_host
15+
SHOW DATABASES LIKE 'test';
16+
Database (test)
17+
test
18+
# Verify on node_1: remote root is gone
19+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
20+
user host
21+
# Verify on node_1: test database is gone
22+
SHOW DATABASES LIKE 'test';
23+
Database (test)
24+
# Now verify on node_2 (replication check)
25+
# Verify on node_2: remote root is gone (replicated)
26+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
27+
user host
28+
# Verify on node_2: test database is gone (replicated)
29+
SHOW DATABASES LIKE 'test';
30+
Database (test)

‎mysql-test/suite/galera/t/galera_secure_installation.test‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--source include/galera_cluster.inc
2+
--source include/have_innodb.inc
3+
4+
--echo #
5+
--echo # MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
6+
--echo # instead of DML for Galera compatibility
7+
--echo #
8+
9+
--connection node_1
10+
--echo #
11+
--echo # Setup remote root account to be removed
12+
--echo #
13+
CREATE USER 'root'@'remote_host' IDENTIFIED BY 'pass';
14+
GRANT ALL ON *.* TO 'root'@'remote_host';
15+
16+
# 'test' is a special database in MTR, ensuring it exists for the script
17+
--disable_warnings
18+
CREATE DATABASE IF NOT EXISTS test;
19+
--enable_warnings
20+
21+
--connection node_2
22+
--let $wait_condition = SELECT COUNT(*) = 1 FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host'
23+
--source include/wait_condition.inc
24+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
25+
SHOW DATABASES LIKE 'test';
26+
27+
--connection node_1
28+
# Prepare input:
29+
# 1. [Empty] - current root password
30+
# 2. n - Enable unix_socket?
31+
# 3. n - Set root password?
32+
# 4. n - Remove anonymous users?
33+
# 5. Y - Disallow root login remotely?
34+
# 6. Y - Remove test database?
35+
# 7. Y - Reload privilege tables?
36+
37+
--write_file $MYSQLTEST_VARDIR/tmp/msi_galera_input.txt
38+
39+
n
40+
n
41+
n
42+
Y
43+
Y
44+
Y
45+
EOF
46+
47+
--exec $MYSQL_SECURE_INSTALLATION --basedir=$MYSQL_BINDIR -S $MASTER_MYSOCK < $MYSQLTEST_VARDIR/tmp/msi_galera_input.txt > $MYSQLTEST_VARDIR/tmp/msi_galera.log 2>&1
48+
49+
--remove_file $MYSQLTEST_VARDIR/tmp/msi_galera_input.txt
50+
--remove_file $MYSQLTEST_VARDIR/tmp/msi_galera.log
51+
52+
--echo # Verify on node_1: remote root is gone
53+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
54+
--echo # Verify on node_1: test database is gone
55+
SHOW DATABASES LIKE 'test';
56+
57+
--echo # Now verify on node_2 (replication check)
58+
--connection node_2
59+
--let $wait_condition = SELECT COUNT(*) = 0 FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host'
60+
--source include/wait_condition.inc
61+
--echo # Verify on node_2: remote root is gone (replicated)
62+
SELECT user, host FROM mysql.global_priv WHERE user = 'root' AND host = 'remote_host';
63+
--echo # Verify on node_2: test database is gone (replicated)
64+
SHOW DATABASES LIKE 'test';
65+
66+
--source include/galera_end.inc

‎scripts/mysql_secure_installation.sh‎

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -317,11 +317,16 @@ set_root_password() {
317317

318318
esc_pass=`basic_single_escape "$password1"`
319319
query="
320-
SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\'')) FROM mysql.global_priv WHERE User='root'), 'root@localhost');
321-
SET @str = CONCAT('ALTER USER ', @str, ' IDENTIFIED BY \'$esc_pass\'');
322-
PREPARE stmt FROM @str;
323-
EXECUTE stmt;
324-
DEALLOCATE PREPARE stmt;"
320+
SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT(QUOTE(User),
321+
'@',
322+
QUOTE(Host),
323+
' IDENTIFIED BY ',
324+
'\'$esc_pass\''))
325+
FROM mysql.global_priv
326+
WHERE User='root'),
327+
'\'root\'@\'localhost\' IDENTIFIED BY \'$esc_pass\'');
328+
SET @str = CONCAT('ALTER USER ', @str);
329+
EXECUTE IMMEDIATE @str;"
325330
do_query "$query"
326331
if [ $? -eq 0 ]; then
327332
echo "Password updated successfully!"
@@ -343,11 +348,15 @@ DEALLOCATE PREPARE stmt;"
343348

344349
remove_anonymous_users() {
345350
query="
346-
SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ', GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\''))), '') FROM mysql.global_priv WHERE User='');
347-
SET @str = IF(@str = '', 'DO 1', @str);
348-
PREPARE stmt FROM @str;
349-
EXECUTE stmt;
350-
DEALLOCATE PREPARE stmt;"
351+
SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ',
352+
GROUP_CONCAT(CONCAT(QUOTE(User),
353+
'@',
354+
QUOTE(Host)))),
355+
'')
356+
FROM mysql.global_priv
357+
WHERE User='');
358+
SET @str = IF(@str = '', 'DO 1', @str);
359+
EXECUTE IMMEDIATE @str;"
351360
do_query "$query"
352361
if [ $? -eq 0 ]; then
353362
echo " ... Success!"
@@ -361,11 +370,16 @@ DEALLOCATE PREPARE stmt;"
361370

362371
remove_remote_root() {
363372
query="
364-
SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ', GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\''))), '') FROM mysql.global_priv WHERE User='root' AND Host NOT IN ('localhost', '127.0.0.1', '::1'));
365-
SET @str = IF(@str = '', 'DO 1', @str);
366-
PREPARE stmt FROM @str;
367-
EXECUTE stmt;
368-
DEALLOCATE PREPARE stmt;"
373+
SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ',
374+
GROUP_CONCAT(CONCAT(QUOTE(User),
375+
'@',
376+
QUOTE(Host)))),
377+
'')
378+
FROM mysql.global_priv
379+
WHERE User='root' AND
380+
Host NOT IN ('localhost', '127.0.0.1', '::1'));
381+
SET @str = IF(@str = '', 'DO 1', @str);
382+
EXECUTE IMMEDIATE @str;"
369383
do_query "$query"
370384
if [ $? -eq 0 ]; then
371385
echo " ... Success!"
@@ -389,10 +403,15 @@ remove_test_database() {
389403
# via DROP USER, which also removes their mysql.db rows automatically.
390404
# FLUSH PRIVILEGES below clears any remaining stale cache entries.
391405
query="
392-
SET @str = (SELECT IFNULL(CONCAT('REVOKE ALL PRIVILEGES ON \`test\`.* FROM ', GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\''))), 'DO 1') FROM mysql.db WHERE Db='test' AND User <> '');
393-
PREPARE stmt FROM @str;
394-
EXECUTE stmt;
395-
DEALLOCATE PREPARE stmt;"
406+
SET @str = (SELECT IFNULL(CONCAT('REVOKE ALL PRIVILEGES ON \`test\`.* ',
407+
'FROM ',
408+
GROUP_CONCAT(CONCAT(QUOTE(User),
409+
'@',
410+
QUOTE(Host)))),
411+
'DO 1')
412+
FROM mysql.db
413+
WHERE Db='test' AND User <> '');
414+
EXECUTE IMMEDIATE @str;"
396415
do_query "$query"
397416

398417
return 0
@@ -471,11 +490,19 @@ if [ "$reply" = "n" ]; then
471490
else
472491
emptypass=0
473492
query="
474-
SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\'')) FROM mysql.global_priv WHERE User='root'), 'root@localhost');
475-
SET @str = CONCAT('ALTER USER ', @str, ' IDENTIFIED VIA mysql_native_password USING \'invalid\' OR unix_socket');
476-
PREPARE stmt FROM @str;
477-
EXECUTE stmt;
478-
DEALLOCATE PREPARE stmt;"
493+
SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT(QUOTE(User),
494+
'@',
495+
QUOTE(Host),
496+
' IDENTIFIED VIA ',
497+
'mysql_native_password ',
498+
'USING \'invalid\' ',
499+
'OR unix_socket'))
500+
FROM mysql.global_priv
501+
WHERE User='root'),
502+
'\'root\'@\'localhost\' IDENTIFIED VIA '
503+
'mysql_native_password USING \'invalid\' OR unix_socket');
504+
SET @str = CONCAT('ALTER USER ', @str);
505+
EXECUTE IMMEDIATE @str;"
479506
do_query "$query"
480507
if [ $? -eq 0 ]; then
481508
echo "Enabled successfully!"

0 commit comments

Comments
 (0)