MDEV-10112: mysql_secure_installation should use GRANT, REVOKE, etc for galera support by kjarir · Pull Request #4775 · MariaDB/server

kjarir · 2026-03-10T14:11:37Z

Problem

mysql_secure_installation used direct DML (UPDATE, DELETE) on mysql.global_priv
and mysql.db — both MyISAM tables not replicated by Galera. Running the script on
one cluster node left all other nodes unsecured.

See: https://jira.mariadb.org/browse/MDEV-10112

Solution

Replace all DML with DDL (ALTER USER, DROP USER, REVOKE) that Galera replicates
correctly via Total Order Isolation (TOI).

Changes (scripts/mysql_secure_installation.sh)

Function	Before	After
set_root_password	UPDATE mysql.global_priv SET priv=json_set(...)	ALTER USER ... IDENTIFIED BY '...'
unix_socket enable	UPDATE mysql.global_priv SET priv=json_set(...)	INSTALL SONAME 'auth_socket' + ALTER USER ... IDENTIFIED VIA ... OR unix_socket
remove_anonymous_users	DELETE FROM mysql.global_priv WHERE User=''	DROP USER IF EXISTS ''@'host', ...
remove_remote_root	DELETE FROM mysql.global_priv WHERE Host NOT IN (...)	DROP USER IF EXISTS 'root'@'remotehost', ...
remove_test_database (privileges)	DELETE FROM mysql.db WHERE Db='test'...	REVOKE ALL PRIVILEGES ON test.* FROM ... + FLUSH PRIVILEGES

For bulk operations, PREPARE / EXECUTE with GROUP_CONCAT dynamically builds a
single DDL statement handling all accounts at once.

Note: DROP USER automatically removes all privilege table entries including
mysql.db rows, so anonymous users' test DB grants are cleaned up in remove_anonymous_users.

grooverdan · 2026-03-11T00:24:27Z

Some previous work with test cases that was rejected because it was too invasive.

8977ad6 hopefully some ideas are salvageable on tests.

gkodinov

Thank you for your contribution! This is a preliminary review.

gkodinov · 2026-03-11T09:46:05Z

mysql-test/main/mysql_secure_installation.result

+performance_schema
+sys
+test
+BINDIR/scripts/mysql_secure_installation: Deprecated program name. It will be removed in a future release, use 'mariadb-secure-installation' instead


please do that!

I updated mariadb-test-run.pl to prioritize the modern mariadb-secure-installation binary. This ensures the test runs against the correct name and eliminates the deprecation warning from the test results.

gkodinov · 2026-03-11T09:46:50Z

mysql-test/main/mysql_secure_installation.result

@@ -0,0 +1,96 @@
+SELECT user, host FROM mysql.global_priv


do you need these SELECTs?

I removed all the initial SELECT and SHOW DATABASES queries from the beginning of the test. The test now only records verification queries after the script has executed, keeping the results focused and clean.

gkodinov · 2026-03-11T09:47:37Z

mysql-test/main/mysql_secure_installation.test

+--let $hostname= `SELECT LOWER(@@hostname)`
+
+#
+# Record initial state: verify root accounts and databases exist before


I'd limit the SELECTs to output only the things you need checked. And think twice if they really are needed for the test: the initial state is what it is. mysqltest guarantees this.

I have completely removed the SELECT statements from the beginning of the test. Since MTR guarantees a consistent initial state for every test case, these "before" checks were redundant. I have now limited the SELECT queries to only the minimal set at the very end of the test required to verify that the anonymous users, remote root accounts, and test database were successfully removed by the script.

gkodinov · 2026-03-11T09:51:32Z

mysql-test/main/mysql_secure_installation.test

+# --basedir is needed so the script can find my_print_defaults and mariadb
+# from the build tree (extra/ and client/ directories).
+--replace_result $MYSQL_BINDIR BINDIR
+--exec $MYSQL_SECURE_INSTALLATION --basedir=$MYSQL_BINDIR -S $MASTER_MYSOCK < $MYSQLTEST_VARDIR/tmp/mysql_secure_installation_input.txt 2>&1


I'd redirect the output to some file that is removed at the end. No need sanitize mariadb_secure_installation's output IMHO. This is not what you're testing.

I modified the test to redirect the script's interactive output to a temporary log file (msi.log), which is deleted at the end of the test. This stops the script's text from cluttering the result file.

gkodinov · 2026-03-11T09:55:02Z

mysql-test/main/mysql_secure_installation.test

+# This approach (kill server, replace datadir, restart) is adapted from
+# commit 8977ad6 -- the previous attempt at this MDEV.
+#
+--let MYSQLD_DATADIR= `select @@datadir`


Not sure the results of mysql_install_db is what mysqltest expects.

I'd avoid this by re-creating the missing databases. Or starting the server in a separate data directory before doing mysql_secure_installation.

I refactored the test to run on a isolated, temporary data directory using MTR's official restart_mysqld.inc mechanism. This provides a clean MariaDB system state for the script to run against without being "invasive" to the main MTR data directory.

gkodinov · 2026-03-11T10:17:12Z

scripts/mysql_secure_installation.sh


    esc_pass=`basic_single_escape "$password1"`
-    do_query "UPDATE mysql.global_priv SET priv=json_set(priv, '$.plugin', 'mysql_native_password', '$.authentication_string', PASSWORD('$esc_pass')) WHERE User='root';"
+    do_query "SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\'')) FROM mysql.global_priv WHERE User='root'), 'root@localhost'); SET @str = CONCAT('ALTER USER ', @str, ' IDENTIFIED BY \'$esc_pass\''); PREPARE stmt FROM @str; EXECUTE stmt; DEALLOCATE PREPARE stmt;"


This is hard to read and maintain.

Shell has a way to do multi-line stirng literals, e.g.:

#!/bin/sh foo=" a b " echo $foo

I reformatted all the SQL queries (ALTER USER, DROP USER, REVOKE) in the shell script to use multi-line string literals. This makes the code much cleaner and easier to maintain.

gkodinov · 2026-03-11T10:18:54Z

scripts/mysql_secure_installation.sh

    else
 	echo " ... Failed!  Not critical, keep moving..."
    fi
+    do_query "FLUSH PRIVILEGES;"


ACL statements maintain the cache as well as the disk image. If you use these there should be no need for FLUSH PRIVILEGES.

I removed the FLUSH PRIVILEGES call from the remove_test_database function. Since we are now using REVOKE (DDL) instead of DELETE (DML), the privilege cache is updated automatically.

gkodinov · 2026-03-11T10:19:45Z

scripts/mysql_secure_installation.sh

 else
  emptypass=0
-  do_query "UPDATE mysql.global_priv SET priv=json_set(priv, '$.password_last_changed', UNIX_TIMESTAMP(), '$.plugin', 'mysql_native_password', '$.authentication_string', 'invalid', '$.auth_or', json_array(json_object(), json_object('plugin', 'unix_socket'))) WHERE User='root';"
+  do_query "INSTALL SONAME 'auth_socket';"


why do you need this?

I have removed the INSTALL SONAME command. The script now assumes the unix_socket plugin is either already present or handled externally, which simplifies the script logic as requested.

…era compatibility The script previously used direct DML statements (UPDATE, DELETE, INSERT) on MyISAM system tables (mysql.global_priv, mysql.db) which are not replicated by Galera clusters. Running the script on one node would leave other cluster nodes unsecured. Replace all such DML with Galera-safe DDL equivalents: 1. set_root_password: Before: UPDATE mysql.global_priv SET priv=json_set(...) WHERE User='root' After: ALTER USER ... IDENTIFIED BY '...' Uses PREPARE/EXECUTE to handle all root accounts across all hosts. 2. enable unix_socket authentication: Before: UPDATE mysql.global_priv SET priv=json_set(...) WHERE User='root' After: INSTALL SONAME 'auth_socket' (ensure plugin is loaded) ALTER USER ... IDENTIFIED VIA mysql_native_password USING 'invalid' OR unix_socket Uses PREPARE/EXECUTE to handle all root accounts across all hosts. 3. remove_anonymous_users: Before: DELETE FROM mysql.global_priv WHERE User='' After: DROP USER IF EXISTS ''@'host1', ''@'host2', ... Uses SELECT/GROUP_CONCAT to build the list of anonymous accounts dynamically. DROP USER automatically removes all privilege table entries for the dropped users, including mysql.db rows. 4. remove_remote_root: Before: DELETE FROM mysql.global_priv WHERE User='root' AND Host NOT IN (...) After: DROP USER IF EXISTS 'root'@'remotehost', ... Uses SELECT/GROUP_CONCAT to build the list of remote root accounts. 5. remove_test_database privileges: Before: DELETE FROM mysql.db WHERE Db='test' OR Db='test\_%' After: REVOKE ALL PRIVILEGES ON test.* FROM non-anonymous users in mysql.db, followed by FLUSH PRIVILEGES to purge stale cache entries. Anonymous user entries are already cleaned up in step 3, since DROP USER automatically removes all mysql.db rows for the dropped user. All five changes use PREPARE/EXECUTE with GROUP_CONCAT-generated DDL to handle cases where multiple rows need to be acted on in a single statement. Also added: - MTR test case 'main.mysql_secure_installation' to verify the script works as expected and that the new DDL statements are correctly generated and executed. - Support in 'mariadb-test-run.pl' to locate the 'mysql_secure_installation' script during tests.

kjarir · 2026-03-11T14:36:40Z

Thank you for the feedback! I've updated the PR to address all your comments: refactored queries into multi-line strings for readability, removed redundant FLUSH PRIVILEGES/INSTALL SONAME calls, and refactored the test to use a non-invasive temporary datadir with cleaner output. Ready for review.

grooverdan · 2026-03-13T05:04:01Z

scripts/mysql_secure_installation.sh

+SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ', GROUP_CONCAT(CONCAT('\'', User, '\'@\'', Host, '\''))), '') FROM mysql.global_priv WHERE User='');
+SET @str = IF(@str = '', 'DO 1', @str);
+PREPARE stmt FROM @str;
+EXECUTE stmt;


https://mariadb.com/docs/server/reference/sql-statements/prepared-statements/execute-immediate as a short cut.

grooverdan · 2026-03-13T05:06:19Z

mysql-test/main/mysql_secure_installation.test

+
+# To avoid affecting other tests, we run this test on a temporary datadir
+# using MTR's official restart mechanism.
+--let $MSI_DATADIR= $MYSQLTEST_VARDIR/tmp/msi_datadir


These assumptions aren't valid with mtr --parallel=2 is used which is why there's so many CI failures.

Do you really need a new datadir? Or is there some backup/restore mechanism that can be used?

grooverdan · 2026-03-13T05:07:16Z

mysql-test/main/mysql_secure_installation.test

@@ -0,0 +1,66 @@
+# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)


this should be echoed out.

grooverdan · 2026-03-13T05:08:08Z

mysql-test/main/mysql_secure_installation.test

@@ -0,0 +1,66 @@
+# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
+#             instead of DML for Galera compatibility


If its for galera compatibility, maybe look at one of the galera tests, and validate the security by looking at another node.

grooverdan · 2026-03-13T05:08:34Z

mysql-test/main/mysql_secure_installation.test

+# after being converted from DML (UPDATE/DELETE on system tables) to
+# DDL (ALTER USER, DROP USER, REVOKE, etc).
+
+--source include/not_windows.inc


this would be at the top. However many galera tests would be doing these already.

gkodinov · 2026-03-13T08:45:54Z

mysql-test/main/mysql_secure_installation.test

+--echo #
+--echo # Setup dummy accounts and database to be secured
+--echo #
+--disable_warnings


don't try to pre- cleanup please. Each MTR test should clean up after itself. This way you are sure to get a clean slate every time.

gkodinov · 2026-03-13T08:47:07Z

mysql-test/main/mysql_secure_installation.test

+Y
+EOF
+
+# The script drops 'test' explicitly if it exists. We ensure it exists.


afaik test is a "special" database in mtr testing. I'd use a different name and make sure it's dropped after that.

gkodinov · 2026-03-13T08:49:35Z

mysql-test/main/mysql_secure_installation.test

+DROP DATABASE IF EXISTS secure_test;
+DROP DATABASE IF EXISTS test;
+--enable_warnings
+FLUSH PRIVILEGES;


why the "FLUSH PRIVILEGES" ?

gkodinov · 2026-03-13T08:50:53Z

mysql-test/main/mysql_secure_installation.test

+
+# Cleanup any remaining state
+--disable_warnings
+DROP USER IF EXISTS 'root'@'remote_host';


do not use IF EXISTS In scripts unless you really want to test it please. DROP the user unconditionally if you expect that it should exist (that's the case here I guess). If it doesn't then you'll get a nice error instead of sweeping this under the carpet.

gkodinov · 2026-03-13T08:51:14Z

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

+--echo # This test verifies that changes made by mysql_secure_installation
+--echo # on one node are correctly replicated to other nodes in the cluster.
+
+--connection node_1


same remarks as the other test.

gkodinov · 2026-03-13T08:53:55Z

scripts/mysql_secure_installation.sh

    esc_pass=`basic_single_escape "$password1"`
-    do_query "UPDATE mysql.global_priv SET priv=json_set(priv, '$.plugin', 'mysql_native_password', '$.authentication_string', PASSWORD('$esc_pass')) WHERE User='root';"
+    query="
+SET @str = IFNULL((SELECT GROUP_CONCAT(CONCAT(QUOTE(User), '@', QUOTE(Host))) FROM mysql.global_priv WHERE User='root'), '\'root\'@\'localhost\'');


I'd further sub-divide that. We have a recommendation to not exceed 80 symbols per line.

gkodinov · 2026-03-13T08:57:29Z

scripts/mysql_secure_installation.sh

 remove_anonymous_users() {
-    do_query "DELETE FROM mysql.global_priv WHERE User='';"
+    query="
+SET @str = (SELECT IFNULL(CONCAT('DROP USER IF EXISTS ', GROUP_CONCAT(CONCAT(QUOTE(User), '@', QUOTE(Host)))), '') FROM mysql.global_priv WHERE User='');


I'd offset the SET command. SQL can deal with spaces.

…tibility 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.

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch from 03ab357 to bd56471 Compare March 10, 2026 14:13

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch from bd56471 to 653b89e Compare March 11, 2026 01:06

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 11, 2026

gkodinov changed the title ~~MDEV - 10112 mysql_secure_installation should use GRANT, REVOKE, etc for galera support~~ MDEV-10112: mysql_secure_installation should use GRANT, REVOKE, etc for galera support Mar 11, 2026

gkodinov requested changes Mar 11, 2026

View reviewed changes

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch from 653b89e to d598478 Compare March 11, 2026 14:27

kjarir added 2 commits March 11, 2026 20:06

Merge branch '11.4' into MDEV-10112-galera-safe-secure-installation

bb84567

Merge branch '11.4' into MDEV-10112-galera-safe-secure-installation

52cbcf2

kjarir requested a review from gkodinov March 12, 2026 00:14

grooverdan requested changes Mar 13, 2026

View reviewed changes

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch 6 times, most recently from 6386869 to 8de0e39 Compare March 13, 2026 08:58

gkodinov requested changes Mar 13, 2026

View reviewed changes

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch from 8de0e39 to f24b619 Compare March 13, 2026 09:04

kjarir force-pushed the MDEV-10112-galera-safe-secure-installation branch from f24b619 to 426aca8 Compare March 13, 2026 09:30

		@@ -0,0 +1,66 @@
		# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)

		@@ -0,0 +1,66 @@
		# MDEV-10112: mysql_secure_installation should use DDL (GRANT, REVOKE, etc)
		# instead of DML for Galera compatibility

Uh oh!

Conversation

kjarir commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Changes (scripts/mysql_secure_installation.sh)

Uh oh!

grooverdan commented Mar 11, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjarir commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

kjarir commented Mar 10, 2026 •

edited

Loading