From 55da6647c0545cac0fa3178c2aafcf86b3e2f830 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 18 Aug 2026 15:53:09 +0530 Subject: [PATCH 1/7] MDEV-40151: Galera SST: reject shell-unsafe datadir/socket before sh -c Issue: mysqld builds the wsrep_sst_ command line with datadir and the socket path single-quoted, then runs it via sh -c. A single quote in either value breaks out of the quoting and runs the remainder as a shell command. Solution: Add an allowlist validator, wsrep_path_char(), and reject datadir, the socket path, and the defaults-file/suffix values before the command line is built. --- .../r/galera_sst_datadir_injection.result | 28 ++++++ .../galera/t/galera_sst_datadir_injection.cnf | 5 + .../t/galera_sst_datadir_injection.test | 97 +++++++++++++++++++ sql/wsrep_sst.cc | 74 +++++++++++++- sql/wsrep_utils.cc | 12 +++ sql/wsrep_utils.h | 1 + 6 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_sst_datadir_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_injection.test diff --git a/mysql-test/suite/galera/r/galera_sst_datadir_injection.result b/mysql-test/suite/galera/r/galera_sst_datadir_injection.result new file mode 100644 index 0000000000000..00a3c04412d1e --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_datadir_injection.result @@ -0,0 +1,28 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +FOUND 1 /wsrep_sst_rsync/ in mysqld.1.err +connection node_1; +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +connection node_2; +FOUND 1 /unsafe for shell interpolation/ in mysqld.2.err +datadir injection prevented +connection node_2; +call mtr.add_suppression('Illegal character in variable'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); +call mtr.add_suppression('Cleanup after exit with status'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf b/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_injection.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_injection.test b/mysql-test/suite/galera/t/galera_sst_datadir_injection.test new file mode 100644 index 0000000000000..1b0d9e8ef1937 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_injection.test @@ -0,0 +1,97 @@ +# +# mysqld must reject a datadir containing shell metacharacters before it +# builds the SST command line that it runs via sh -c. +# +# Steps: +# 1. Bring up a 2-node rsync-SST cluster. +# 2. Move node 2's datadir to a name carrying a "touch " payload and +# force a full SST. +# 3. Assert node 2 refuses the datadir and the payload did not run. +# 4. Restore the datadir and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +SELECT 1; + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +# sanity: initial SST used rsync +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = wsrep_sst_rsync +--source include/search_pattern_in_file.inc + +--connection node_1 +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +# move the datadir to a name carrying a shell-injection payload +perl; + use strict; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $marker = "$tmp/datadir_inj_marker"; + my $ddir = "$tmp/dd_inj'&touch $marker&'x"; + my $cnf = "$vardir/my.cnf"; + unlink $marker; + (my $parent = $ddir) =~ s{/[^/]+$}{}; + mkpath($parent); + rename("$vardir/mysqld.2/data", $ddir) or die "rename datadir: $!"; + unlink "$ddir/grastate.dat"; + open(my $sz, '>', "$tmp/inj_cnf_size") or die $!; print $sz -s $cnf; close $sz; + open(my $fh, '>>', $cnf) or die $!; print $fh "[mysqld.2]\ndatadir=\"$ddir\"\n"; close $fh; +EOF + +--connection node_2 +--error 1,134 +--exec $MYSQLD_LAST_CMD + +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = unsafe for shell interpolation +--source include/search_pattern_in_file.inc + +perl; + die "FAIL: marker created - datadir injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/datadir_inj_marker"; + print "datadir injection prevented\n"; +EOF + +# restore datadir and rejoin +perl; + use strict; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $cnf = "$vardir/my.cnf"; + open(my $sz, '<', "$tmp/inj_cnf_size") or die $!; my $orig = <$sz>; close $sz; + truncate($cnf, $orig) or die "truncate: $!"; + unlink "$tmp/datadir_inj_marker", "$tmp/inj_cnf_size"; + rename("$tmp/dd_inj'&touch $tmp/datadir_inj_marker&'x", "$vardir/mysqld.2/data") + or die "restore datadir: $!"; +EOF + +--connection node_2 +--source include/start_mysqld.inc +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +# suppress the rejected-SST noise on the final node_2 instance +call mtr.add_suppression('Illegal character in variable'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Failed to read .* from: wsrep_sst_rsync'); +call mtr.add_suppression('Failed to prepare for .* SST'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('SST request callback failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); +call mtr.add_suppression('Cleanup after exit with status'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index b0a0b2cf56668..febda0cdc86fb 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -207,8 +207,22 @@ void wsrep_set_data_home_dir(const char *data_dir) data_home_dir= (data_dir && *data_dir) ? data_dir : NULL; } -static void make_wsrep_defaults_file() +/* Returns false if a defaults file path or the group suffix is unsafe + for the single-quoted interpolation below. */ +static bool make_wsrep_defaults_file() { + if ((my_defaults_file && + wsrep_check_request_str(my_defaults_file, wsrep_path_char, true)) || + (my_defaults_extra_file && + wsrep_check_request_str(my_defaults_extra_file, wsrep_path_char, true)) || + (my_defaults_group_suffix && + wsrep_check_request_str(my_defaults_group_suffix, wsrep_path_char, true))) + { + WSREP_ERROR("Refusing SST: defaults file/suffix has characters unsafe " + "for shell interpolation."); + return false; + } + if (!wsrep_defaults_file[0]) { char *ptr= wsrep_defaults_file; @@ -225,6 +239,7 @@ static void make_wsrep_defaults_file() ptr= strxnmov(ptr, end - ptr, WSREP_SST_OPT_CONF_SUFFIX, " '", my_defaults_group_suffix, "' ", NULL); } + return true; } @@ -1245,7 +1260,24 @@ static ssize_t sst_prepare_other (const char* method, return ret; } - make_wsrep_defaults_file(); + /* datadir is interpolated single-quoted into the command line below. + innodb-data-home-dir goes through the environment instead, so it does + not need this check. */ + if (wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true)) + { + WSREP_ERROR("Refusing SST: datadir has characters unsafe for shell " + "interpolation."); + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } + + if (!make_wsrep_defaults_file()) + { + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } ret= snprintf (cmd_str(), cmd_len, "wsrep_sst_%s " @@ -1626,6 +1658,22 @@ static int sst_donate_mysqldump (const char* addr, return -ENOMEM; } + /* Checked before queries are rejected below, so that a refused SST does + not leave the donor rejecting queries with no way back. */ + if ((mysqld_unix_port && + wsrep_check_request_str(mysqld_unix_port, wsrep_path_char, true)) || + wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true)) + { + WSREP_ERROR("Refusing SST: socket or datadir has characters unsafe for " + "shell interpolation."); + return -EINVAL; + } + + if (!make_wsrep_defaults_file()) + { + return -EINVAL; + } + /* we enable new client connections so that mysqldump donation can connect in, but we reject local connections from modifyingcdata during SST, to keep @@ -1633,8 +1681,6 @@ static int sst_donate_mysqldump (const char* addr, */ if (!bypass && wsrep_sst_donor_rejects_queries) sst_reject_queries(TRUE); - make_wsrep_defaults_file(); - std::ostringstream uuid_oss; uuid_oss << gtid.id(); int ret= snprintf (cmd_str(), cmd_len, @@ -2071,7 +2117,25 @@ static int sst_donate_other (const char* method, return ret; } - make_wsrep_defaults_file(); + /* socket path and datadir are interpolated single-quoted into the + command line below. */ + if ((mysqld_unix_port && + wsrep_check_request_str(mysqld_unix_port, wsrep_path_char, true)) || + wsrep_check_request_str(mysql_real_data_home, wsrep_path_char, true)) + { + WSREP_ERROR("Refusing SST: socket or datadir has characters unsafe for " + "shell interpolation."); + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } + + if (!make_wsrep_defaults_file()) + { + my_free(binlog_opt_val); + my_free(binlog_index_opt_val); + return -EINVAL; + } std::ostringstream uuid_oss; uuid_oss << gtid.id(); diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index 83fc00ba2f56a..3a6ca931f08fe 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -633,6 +633,18 @@ bool wsrep_names_list(const unsigned char c) return wsrep_address_char(c) || wsrep_comma_char(c); } +/* return true if character is valid in a path and safe to interpolate + into a single-quoted shell string. Allowlist: everything else, including + every shell metacharacter, is rejected. UTF-8 bytes are always allowed. */ +bool wsrep_path_char(const unsigned char c) +{ + if (c >= 0x80) return true; + return wsrep_filename_char(c) || + (c == '/') || (c == '\\') || /* POSIX / Windows separators */ + (c == ':') || (c == ' ') || /* Windows drive letter; space */ + (c == '+') || (c == '=') || (c == ',') || (c == '@') || (c == '%'); +} + bool wsrep_check_request_str(const char* const str, bool (*check) (const unsigned char), bool log_warn) diff --git a/sql/wsrep_utils.h b/sql/wsrep_utils.h index 0cc70c1fc7d6a..e628e957389fa 100644 --- a/sql/wsrep_utils.h +++ b/sql/wsrep_utils.h @@ -450,6 +450,7 @@ bool wsrep_filename_char(const unsigned char c); bool wsrep_comma_char(const unsigned char c); bool wsrep_address_char(const unsigned char c); bool wsrep_names_list(const unsigned char c); +bool wsrep_path_char(const unsigned char c); bool wsrep_check_request_str(const char* const str, bool (*check) (const unsigned char), bool log_warn = true); From 05172027ca676643fcfb97bbc58da3eb91b6f138 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 18 Aug 2026 18:33:36 +0530 Subject: [PATCH 2/7] MDEV-40151: Galera SST: escape backquotes in forwarded mysqld arguments Issue: copy_orig_argv() appends mysqld's own arguments to the SST command line that is run via sh -c. A backquote only caused the argument to be enclosed in double quotes, which does not stop command substitution, so a backquote in any forwarded argument ran as a shell command. Solution: Escape the backquote with a backslash instead of relying on the enclosing double quotes, which also keeps it literal when the argument is not quoted. --- .../r/galera_sst_mysqld_args_injection.result | 18 ++++++ .../t/galera_sst_mysqld_args_injection.cnf | 5 ++ .../t/galera_sst_mysqld_args_injection.test | 64 +++++++++++++++++++ sql/wsrep_sst.cc | 16 ++--- 4 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_sst_mysqld_args_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.test diff --git a/mysql-test/suite/galera/r/galera_sst_mysqld_args_injection.result b/mysql-test/suite/galera/r/galera_sst_mysqld_args_injection.result new file mode 100644 index 0000000000000..bb096a45ceb3b --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_mysqld_args_injection.result @@ -0,0 +1,18 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +connection node_1; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +backquote injection prevented +connection node_2; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); +call mtr.add_suppression('InnoDB: Cannot open .* for writing'); diff --git a/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.cnf b/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.test b/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.test new file mode 100644 index 0000000000000..a8aca32e42229 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_mysqld_args_injection.test @@ -0,0 +1,64 @@ +# +# mysqld appends its own arguments to the SST command line that it runs via +# sh -c, so a backquote in one of them must not be executed. +# +# Steps: +# 1. Bring up a 2-node rsync-SST cluster. +# 2. Force a full SST on node_2, restarting it with an argument whose value +# contains a backquoted redirection that would create a marker file. It +# has to be on the command line: my.cnf values are not forwarded this way. +# 3. Assert node_2 joins and the marker was not created. +# 4. Restart node_2 cleanly and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +SELECT 1; + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +--connection node_1 +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +perl; + unlink "$ENV{MYSQL_TMP_DIR}/mysqld_args_inj_marker"; +EOF + +# ">file" needs no space, so the payload survives being split on whitespace +--let $start_mysqld_params = --innodb-buffer-pool-filename=x`>$MYSQL_TMP_DIR/mysqld_args_inj_marker`y +--source include/start_mysqld.inc +--let $start_mysqld_params = + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +perl; + die "FAIL: marker created - backquote in a forwarded mysqld argument was executed\n" + if -e "$ENV{MYSQL_TMP_DIR}/mysqld_args_inj_marker"; + print "backquote injection prevented\n"; +EOF + +# restart node_2 cleanly (no payload) and rejoin +--connection node_2 +--source include/shutdown_mysqld.inc +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +--source include/start_mysqld.inc + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); +# InnoDB took the payload for a file name, which is the point of the test +call mtr.add_suppression('InnoDB: Cannot open .* for writing'); diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index febda0cdc86fb..09a674bf3f99f 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -953,24 +953,24 @@ static int sst_append_env_var(wsp::env& env, #define IS_REQ_ESCAPING(c) (c == '""' || c == '%') #else /* - Space, single quote, ampersand, backquote, and I/O redirection + Space, single quote, ampersand, semicolon and I/O redirection characters require text to be enclosed in double quotes. The - semicolon is used to separate shell commands, so it must be - enclosed in double quotes as well: + backquote is escaped instead (see IS_REQ_ESCAPING), since double + quotes do not stop command substitution: */ #define IS_SPECIAL(c) \ - (isspace(c) || c == '\'' || c == '&' || c == '`' || c == '|' || \ + (isspace(c) || c == '\'' || c == '&' || c == '|' || \ c == '>' || c == '<' || c == ';') /* Inside values, characters are interpreted as in parameter names: */ #define IS_SPECIAL_V(c) IS_SPECIAL(c) /* - Double quotation mark and backslash characters require - backslash prefixing, the dollar symbol is used to substitute - a variable value, therefore it also requires escaping: + Double quotation mark and backslash require backslash prefixing. + The dollar symbol and the backquote substitute a value or a + command, so they require escaping as well: */ -#define IS_REQ_ESCAPING(c) (c == '"' || c == '\\' || c == '$') +#define IS_REQ_ESCAPING(c) (c == '"' || c == '\\' || c == '$' || c == '`') #endif static size_t estimate_cmd_len (bool* extra_args) From fd80f7840b003ee7c2c3e2a6152de10e849e601c Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 18 Aug 2026 16:11:38 +0530 Subject: [PATCH 3/7] MDEV-40151: Galera SST: build mariadb-backup command from arrays Issue: INNOEXTRA embedded aria-log-dir-path, innodb-data-home-dir, innodb-log-group-home-dir, innodb-undo-directory, innodb-buffer-pool-filename and innodb-buffer-pool-size single-quoted into a string; INNOAPPLY/INNOMOVE/INNOBACKUP embedded that string and were run via eval. A single quote in any of the six values broke out of the quoting and ran as a shell command. Solution: Build INNOEXTRA and the mariadb-backup commands as bash arrays so each value is a literal argv element, and drop the eval from setup_commands and its callers. The forwarded --mysqld-args, the [sst] inno-*-opts, the defaults file and the tmpdir are converted the same way, since they are assembled in the same eval'd command and any eval left there would still be the injection mechanism the sink relies on. WSREP_SST_OPT_MYSQLD keeps its historical string format for other scripts that source wsrep_sst_common, now with embedded quotes escaped. --- .../r/galera_sst_buffer_pool_injection.result | 18 ++ .../t/galera_sst_buffer_pool_injection.cnf | 6 + .../t/galera_sst_buffer_pool_injection.test | 76 ++++++++ scripts/wsrep_sst_common.sh | 107 ++++++----- scripts/wsrep_sst_mariabackup.sh | 176 ++++++++++++++---- 5 files changed, 301 insertions(+), 82 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test diff --git a/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result b/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result new file mode 100644 index 0000000000000..8ccfd46365a89 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_buffer_pool_injection.result @@ -0,0 +1,18 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +connection node_1; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +buffer-pool injection prevented +FOUND 1 /Running: .*mariadb-backup --prepare/ in mysqld.2.err +connection node_2; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf new file mode 100644 index 0000000000000..094858fa9eaea --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.cnf @@ -0,0 +1,6 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=mariabackup +wsrep_sst_auth="root:" +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test new file mode 100644 index 0000000000000..999069027ba70 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_buffer_pool_injection.test @@ -0,0 +1,76 @@ +# +# mariabackup SST must pass innodb-buffer-pool-filename to mariadb-backup as a +# literal argument, so that shell metacharacters in it are not executed. +# +# Steps: +# 1. Bring up a 2-node mariabackup-SST cluster. +# 2. Force a full SST on node_2, restarting it with an +# innodb-buffer-pool-filename carrying a "touch " payload. It has +# to be on the command line: the SST script has no config-file fallback +# for this option. +# 3. Assert node_2 joins, the payload did not run, and the mariadb-backup +# command line is still logged. +# 4. Restart node_2 cleanly and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc +--source include/have_mariabackup.inc + +SELECT 1; + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +--connection node_1 +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +perl; + unlink "$ENV{MYSQL_TMP_DIR}/bp_inj_marker"; +EOF + +# $start_mysqld_params is split on whitespace with no quote handling, so the +# payload must be a single space-free token; ">file" (a bare redirection, +# needs no command name or space) is used as the marker-creation payload. +--let $start_mysqld_params = --innodb-buffer-pool-filename=x'>$MYSQL_TMP_DIR/bp_inj_marker&'y +--source include/start_mysqld.inc +--let $start_mysqld_params = + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +perl; + die "FAIL: marker created - buffer-pool injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/bp_inj_marker"; + print "buffer-pool injection prevented\n"; +EOF + +# SST failures are diagnosed from the logged command line +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = Running: .*mariadb-backup --prepare +--source include/search_pattern_in_file.inc + +# restart node_2 cleanly (no malicious arg) and rejoin +--connection node_2 +--source include/shutdown_mysqld.inc +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +--source include/start_mysqld.inc + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +perl; + unlink "$ENV{MYSQL_TMP_DIR}/bp_inj_marker"; +EOF + +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/scripts/wsrep_sst_common.sh b/scripts/wsrep_sst_common.sh index 2bbbdb7ba1078..730f7b74b2f31 100644 --- a/scripts/wsrep_sst_common.sh +++ b/scripts/wsrep_sst_common.sh @@ -208,6 +208,8 @@ WSREP_SST_OPT_EXTRA_DEFAULTS="" WSREP_SST_OPT_SUFFIX_DEFAULT="" WSREP_SST_OPT_SUFFIX_VALUE="" WSREP_SST_OPT_MYSQLD="" +# Array form: each argument is a literal argv element, never re-parsed. +WSREP_SST_OPT_MYSQLD_ARR=() WSREP_SST_OPT_PORT="" WSREP_SST_OPT_ADDR="" WSREP_SST_OPT_ADDR_PORT="" @@ -220,7 +222,8 @@ INNODB_UNDO_DIR=$(trim_dir "${INNODB_UNDO_DIR:-}") INNODB_BUFFER_POOL="" INNODB_BUFFER_POOL_SIZE="" INNODB_FORCE_RECOVERY="" -INNOEXTRA="" +# An array, so that the path values in it stay literal argv elements: +INNOEXTRA=() while [ $# -gt 0 ]; do case "$1" in @@ -435,7 +438,7 @@ case "$1" in shift ;; '--mysqld-args') - original_cmd="" + WSREP_SST_OPT_MYSQLD_ARR=() shift cmd_tail=0 while [ $# -gt 0 ]; do @@ -444,11 +447,11 @@ case "$1" in if [ -z "$lname" ]; then shift if [ $# -gt 0 ]; then - # copy "--" to the output string: - original_cmd="$original_cmd --" + # copy "--" to the output list: + WSREP_SST_OPT_MYSQLD_ARR+=("--") # All other arguments must be copied unchanged: while [ $# -gt 0 ]; do - original_cmd="$original_cmd '$1'" + WSREP_SST_OPT_MYSQLD_ARR+=("$1") shift done fi @@ -461,7 +464,7 @@ case "$1" in # options list, starting with "-": options="${1#-}" if [ "$options" != "$1" -a -n "$options" ]; then - slist="" + sflags="" while [ -n "$options" ]; do # Let's separate the first character as the current # option name: @@ -510,48 +513,43 @@ case "$1" in elif [ "$option" != 'u' -a \ "$option" != 'P' ] then - if [ $cmd_tail -ne 0 ]; then - option="$option --" - fi + # Short option with a value: flush the flags + # collected so far, then push the option and + # its value as separate elements. if [ -z "$value" ]; then - slist="$slist$option" - elif [ -z "$slist" ]; then - slist="$option '$value'" + sflags="$sflags$option" else - slist="$slist -$option '$value'" + [ -n "$sflags" ] && { WSREP_SST_OPT_MYSQLD_ARR+=("-$sflags"); sflags=""; } + WSREP_SST_OPT_MYSQLD_ARR+=("-$option") + [ $cmd_tail -ne 0 ] && WSREP_SST_OPT_MYSQLD_ARR+=("--") + WSREP_SST_OPT_MYSQLD_ARR+=("$value") fi break fi + # extracted (h) or dropped (u/P) option: if [ $cmd_tail -ne 0 ]; then - if [ -n "$slist" ]; then - slist="$slist --" - else - slist='-' - fi + [ -n "$sflags" ] && { WSREP_SST_OPT_MYSQLD_ARR+=("-$sflags"); sflags=""; } + WSREP_SST_OPT_MYSQLD_ARR+=("--") fi break else - slist="$slist$option" + sflags="$sflags$option" fi options="$value" done - if [ -n "$slist" ]; then - original_cmd="$original_cmd -$slist" - fi + [ -n "$sflags" ] && WSREP_SST_OPT_MYSQLD_ARR+=("-$sflags") elif [ -z "$options" ]; then - # We found an minus sign without any characters after it: - original_cmd="$original_cmd -" + # a minus sign without any characters after it: + WSREP_SST_OPT_MYSQLD_ARR+=("-") else - # We found a value that does not start with a minus - - # it is a positional argument or the value of previous - # option. Copy it to output string (as is): - original_cmd="$original_cmd '$1'" + # positional argument or value of previous option (as is): + WSREP_SST_OPT_MYSQLD_ARR+=("$1") fi shift if [ $cmd_tail -ne 0 ]; then # All other arguments must be copied unchanged: while [ $# -gt 0 ]; do - original_cmd="$original_cmd '$1'" + WSREP_SST_OPT_MYSQLD_ARR+=("$1") shift done break @@ -659,12 +657,11 @@ case "$1" in ;; esac if [ $skip_mysqld_arg -eq 0 ]; then - original_cmd="$original_cmd '$1'" + WSREP_SST_OPT_MYSQLD_ARR+=("$1") fi fi shift done - WSREP_SST_OPT_MYSQLD="${original_cmd# *}" break ;; *) # Must be command usage @@ -755,41 +752,45 @@ fi # Reconstructing the command line arguments that control the innodb # and binlog options: if [ -n "$WSREP_SST_OPT_LOG_BASENAME" ]; then - if [ -n "$WSREP_SST_OPT_MYSQLD" ]; then - WSREP_SST_OPT_MYSQLD="--log-basename='$WSREP_SST_OPT_LOG_BASENAME' $WSREP_SST_OPT_MYSQLD" - else - WSREP_SST_OPT_MYSQLD="--log-basename='$WSREP_SST_OPT_LOG_BASENAME'" - fi + WSREP_SST_OPT_MYSQLD_ARR=(--log-basename="$WSREP_SST_OPT_LOG_BASENAME" \ + ${WSREP_SST_OPT_MYSQLD_ARR[@]+"${WSREP_SST_OPT_MYSQLD_ARR[@]}"}) fi if [ -n "$ARIA_LOG_DIR" ]; then - INNOEXTRA="$INNOEXTRA --aria-log-dir-path='$ARIA_LOG_DIR'" + INNOEXTRA+=(--aria-log-dir-path="$ARIA_LOG_DIR") fi if [ -n "$INNODB_DATA_HOME_DIR" ]; then - INNOEXTRA="$INNOEXTRA --innodb-data-home-dir='$INNODB_DATA_HOME_DIR'" + INNOEXTRA+=(--innodb-data-home-dir="$INNODB_DATA_HOME_DIR") fi if [ -n "$INNODB_LOG_GROUP_HOME" ]; then - INNOEXTRA="$INNOEXTRA --innodb-log-group-home-dir='$INNODB_LOG_GROUP_HOME'" + INNOEXTRA+=(--innodb-log-group-home-dir="$INNODB_LOG_GROUP_HOME") fi if [ -n "$INNODB_UNDO_DIR" ]; then - INNOEXTRA="$INNOEXTRA --innodb-undo-directory='$INNODB_UNDO_DIR'" + INNOEXTRA+=(--innodb-undo-directory="$INNODB_UNDO_DIR") fi if [ -n "$INNODB_BUFFER_POOL" ]; then - INNOEXTRA="$INNOEXTRA --innodb-buffer-pool-filename='$INNODB_BUFFER_POOL'" + INNOEXTRA+=(--innodb-buffer-pool-filename="$INNODB_BUFFER_POOL") fi if [ -n "$INNODB_BUFFER_POOL_SIZE" ]; then - INNOEXTRA="$INNOEXTRA --innodb-buffer-pool-size='$INNODB_BUFFER_POOL_SIZE'" + INNOEXTRA+=(--innodb-buffer-pool-size="$INNODB_BUFFER_POOL_SIZE") fi if [ -n "$WSREP_SST_OPT_BINLOG" ]; then - INNOEXTRA="$INNOEXTRA --log-bin='$WSREP_SST_OPT_BINLOG'" + INNOEXTRA+=(--log-bin="$WSREP_SST_OPT_BINLOG") if [ -n "$WSREP_SST_OPT_BINLOG_INDEX" ]; then - if [ -n "$WSREP_SST_OPT_MYSQLD" ]; then - WSREP_SST_OPT_MYSQLD="--log-bin-index='$WSREP_SST_OPT_BINLOG_INDEX' $WSREP_SST_OPT_MYSQLD" - else - WSREP_SST_OPT_MYSQLD="--log-bin-index='$WSREP_SST_OPT_BINLOG_INDEX'" - fi + WSREP_SST_OPT_MYSQLD_ARR=(--log-bin-index="$WSREP_SST_OPT_BINLOG_INDEX" \ + ${WSREP_SST_OPT_MYSQLD_ARR[@]+"${WSREP_SST_OPT_MYSQLD_ARR[@]}"}) fi fi +# Nothing here uses the string form, the array above is what gets executed, +# but other scripts sourcing this file expect the historical format: every +# argument single quoted. Embedded quotes are escaped, unlike historically, +# so that the result survives being evaluated. +for _mysqld_arg in ${WSREP_SST_OPT_MYSQLD_ARR[@]+"${WSREP_SST_OPT_MYSQLD_ARR[@]}"} +do + WSREP_SST_OPT_MYSQLD="$WSREP_SST_OPT_MYSQLD${WSREP_SST_OPT_MYSQLD:+ }'${_mysqld_arg//\'/\'\\\'\'}'" +done +unset _mysqld_arg + readonly INNODB_FORCE_RECOVERY readonly WSREP_SST_OPT_MYSQLD @@ -919,6 +920,16 @@ wsrep_defaults="$wsrep_defaults${WSREP_SST_OPT_SUFFIX_DEFAULT:+ }$WSREP_SST_OPT_ readonly WSREP_SST_OPT_CONF_UNQUOTED="${wsrep_defaults:+ }$wsrep_defaults" +# Array form of the above, for callers passing them as literal argv elements. +# Each value is already a single "--opt=value" token. +WSREP_SST_OPT_CONF_ARR=() +[ -n "$WSREP_SST_OPT_DEFAULT" ] && \ + WSREP_SST_OPT_CONF_ARR+=("$WSREP_SST_OPT_DEFAULT") +[ -n "$WSREP_SST_OPT_EXTRA_DEFAULT" ] && \ + WSREP_SST_OPT_CONF_ARR+=("$WSREP_SST_OPT_EXTRA_DEFAULT") +[ -n "$WSREP_SST_OPT_SUFFIX_DEFAULT" ] && \ + WSREP_SST_OPT_CONF_ARR+=("$WSREP_SST_OPT_SUFFIX_DEFAULT") + # # User can specify mariabackup specific settings that will be used during sst # process like encryption, etc. Parse such configuration option. diff --git a/scripts/wsrep_sst_mariabackup.sh b/scripts/wsrep_sst_mariabackup.sh index 7515ee029b383..619dfe1446819 100644 --- a/scripts/wsrep_sst_mariabackup.sh +++ b/scripts/wsrep_sst_mariabackup.sh @@ -81,7 +81,6 @@ uextra=0 disver="" STATDIR="" -tmpopts="" itmpdir="" xtmpdir="" @@ -110,6 +109,9 @@ INNOAPPLYLOG="$DATA/mariabackup.prepare.log" INNOMOVELOG="$DATA/mariabackup.move.log" INNOBACKUPLOG="$DATA/mariabackup.backup.log" +# Where mariadb-backup stderr goes: 'file' (the logs above) or 'logger': +INNO_LOG_MODE='file' + timeit() { local stage="$1" @@ -135,6 +137,31 @@ timeit() return $extcode } +# Like timeit, but runs a function instead of eval'ing a string. +run_timed() +{ + local stage="$1" + shift + local x1 x2 took extcode + + if [ $ttime -eq 1 ]; then + x1=$(date +%s) + fi + + wsrep_log_info "Running $stage" + "$@" + extcode=$? + + if [ $ttime -eq 1 ]; then + x2=$(date +%s) + took=$(( x2-x1 )) + wsrep_log_info "NOTE: $stage took $took seconds" + totime=$(( totime+took )) + fi + + return $extcode +} + get_keys() { # $encrypt -eq 1 is for internal purposes only @@ -549,6 +576,28 @@ adjust_progress() bkgroups='sst|xtrabackup|mariabackup' encgroups="--mysqld|$bkgroups" +# Split a configuration option string into the named array. Splitting is on +# whitespace only, so a value containing a space cannot be expressed - warn +# instead of silently splitting such a value in two. +split_cnf_opts() +{ + local _arr="$1" + local _name="$2" + local _value="$3" + + [ -z "$_value" ] && return 0 + + case "$_value" in + *[\'\"]*) + wsrep_log_warning "Quotes in '$_name' are not interpreted:" \ + "the value is split on whitespace, so an option" \ + "value containing a space is not supported." + ;; + esac + + read -ra "$_arr" <<< "$_value" +} + read_cnf() { sfmt=$(parse_cnf sst streamfmt 'mbstream') @@ -615,9 +664,11 @@ read_cnf() rlimit=$(parse_cnf sst rlimit) uextra=$(parse_cnf sst use-extra 0) speciald=$(parse_cnf sst 'sst-special-dirs' 1) - iopts=$(parse_cnf "$bkgroups" 'inno-backup-opts') - iapts=$(parse_cnf "$bkgroups" 'inno-apply-opts') - impts=$(parse_cnf "$bkgroups" 'inno-move-opts') + # tokenized into arrays, so that they stay literal argv elements: + iopts=(); iapts=(); impts=() + split_cnf_opts iopts 'inno-backup-opts' "$(parse_cnf "$bkgroups" 'inno-backup-opts')" + split_cnf_opts iapts 'inno-apply-opts' "$(parse_cnf "$bkgroups" 'inno-apply-opts')" + split_cnf_opts impts 'inno-move-opts' "$(parse_cnf "$bkgroups" 'inno-move-opts')" use_memory=$(parse_cnf "$bkgroups" 'use-memory') if [ -z "$use_memory" ]; then if [ -n "$INNODB_BUFFER_POOL_SIZE" ]; then @@ -797,7 +848,7 @@ check_extra() # mariadb-backup works only locally. # Hence, setting host to 127.0.0.1 unconditionally: wsrep_log_info "SST through extra_port $eport" - INNOEXTRA="$INNOEXTRA --host=127.0.0.1 --port=$eport" + INNOEXTRA+=(--host=127.0.0.1 --port="$eport") use_socket=0 else wsrep_log_error "Extra port $eport null, failing" @@ -808,7 +859,7 @@ check_extra() fi fi if [ $use_socket -eq 1 -a -n "$WSREP_SST_OPT_SOCKET" ]; then - INNOEXTRA="$INNOEXTRA --socket='$WSREP_SST_OPT_SOCKET'" + INNOEXTRA+=(--socket="$WSREP_SST_OPT_SOCKET") fi } @@ -995,9 +1046,7 @@ if [ $ssyslog -eq 1 ]; then else wsrep_log_error "logger not in path: $PATH. Ignoring" fi - INNOAPPLY="2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-apply" - INNOMOVE="2>&1 | logger -p daemon.err -t ${ssystag}innobackupex-move" - INNOBACKUP="2> >(logger -p daemon.err -t ${ssystag}innobackupex-backup)" + INNO_LOG_MODE='logger' else if [ $sstlogarchive -eq 1 ] then @@ -1053,27 +1102,87 @@ else wsrep_log_warning "Failed to archive log file ('$newfile')" fi fi - INNOAPPLY="> '$INNOAPPLYLOG' 2>&1" - INNOMOVE="> '$INNOMOVELOG' 2>&1" - INNOBACKUP="2> '$INNOBACKUPLOG'" + INNO_LOG_MODE='file' fi +# Build the mariadb-backup commands as arrays (globals, run by run_inno*). +# Every value stays a literal argv element, so nothing is re-parsed. setup_commands() { - local mysqld_args="" - if [ -n "$WSREP_SST_OPT_MYSQLD" ]; then - mysqld_args=" --mysqld-args $WSREP_SST_OPT_MYSQLD" - fi - local recovery="" - if [ -n "$INNODB_FORCE_RECOVERY" ]; then - recovery=" --innodb-force-recovery=$INNODB_FORCE_RECOVERY" + local -a mysqld_args=() + if [ ${#WSREP_SST_OPT_MYSQLD_ARR[@]} -ne 0 ]; then + mysqld_args=(--mysqld-args "${WSREP_SST_OPT_MYSQLD_ARR[@]}") fi if [ -n "$use_memory" ]; then - INNOEXTRA="$INNOEXTRA --use-memory=$use_memory" + INNOEXTRA+=(--use-memory="$use_memory") + fi + + INNOAPPLY_CMD=("$BACKUP_BIN" --prepare) + [ -n "$disver" ] && INNOAPPLY_CMD+=(--no-version-check) + [ -n "$INNODB_FORCE_RECOVERY" ] && \ + INNOAPPLY_CMD+=(--innodb-force-recovery="$INNODB_FORCE_RECOVERY") + INNOAPPLY_CMD+=(${iapts[@]+"${iapts[@]}"}) + INNOAPPLY_CMD+=(${INNOEXTRA[@]+"${INNOEXTRA[@]}"} \ + --target-dir="$DATA" --datadir="$DATA" \ + ${mysqld_args[@]+"${mysqld_args[@]}"}) + + INNOMOVE_CMD=("$BACKUP_BIN" \ + ${WSREP_SST_OPT_CONF_ARR[@]+"${WSREP_SST_OPT_CONF_ARR[@]}"}) + INNOMOVE_CMD+=(--move-back) + [ -n "$disver" ] && INNOMOVE_CMD+=(--no-version-check) + INNOMOVE_CMD+=(${impts[@]+"${impts[@]}"}) + INNOMOVE_CMD+=(${INNOEXTRA[@]+"${INNOEXTRA[@]}"} \ + --galera-info --force-non-empty-directories \ + --target-dir="$DATA" --datadir="${TDATA:-$DATA}") + + INNOBACKUP_CMD=("$BACKUP_BIN" \ + ${WSREP_SST_OPT_CONF_ARR[@]+"${WSREP_SST_OPT_CONF_ARR[@]}"}) + INNOBACKUP_CMD+=(--backup) + [ -n "$disver" ] && INNOBACKUP_CMD+=(--no-version-check) + INNOBACKUP_CMD+=(${iopts[@]+"${iopts[@]}"}) + [ -n "$xtmpdir" ] && INNOBACKUP_CMD+=(--tmpdir="$xtmpdir") + INNOBACKUP_CMD+=(${INNOEXTRA[@]+"${INNOEXTRA[@]}"} \ + --galera-info --stream="$sfmt" \ + --target-dir="$itmpdir" --datadir="$DATA" \ + ${mysqld_args[@]+"${mysqld_args[@]}"}) +} + +# Run the command arrays with real redirection, never through eval. +run_innoapply() +{ + wsrep_log_info "Running: ${INNOAPPLY_CMD[*]}" + if [ "$INNO_LOG_MODE" = 'logger' ]; then + "${INNOAPPLY_CMD[@]}" 2>&1 | \ + logger -p daemon.err -t "${ssystag}innobackupex-apply" + else + "${INNOAPPLY_CMD[@]}" > "$INNOAPPLYLOG" 2>&1 + fi +} + +run_innomove() +{ + wsrep_log_info "Running: ${INNOMOVE_CMD[*]}" + if [ "$INNO_LOG_MODE" = 'logger' ]; then + "${INNOMOVE_CMD[@]}" 2>&1 | \ + logger -p daemon.err -t "${ssystag}innobackupex-move" + else + "${INNOMOVE_CMD[@]}" > "$INNOMOVELOG" 2>&1 + fi +} + +# Stream the backup to the transfer command. $tcmd carries no path parameter +# and stays a string; PIPESTATUS still gives [backup, transfer]. +run_innobackup() +{ + wsrep_log_info "Running: ${INNOBACKUP_CMD[*]} | $tcmd" + if [ "$INNO_LOG_MODE" = 'logger' ]; then + "${INNOBACKUP_CMD[@]}" \ + 2> >(logger -p daemon.err -t "${ssystag}innobackupex-backup") | \ + eval "$tcmd" + else + "${INNOBACKUP_CMD[@]}" 2> "$INNOBACKUPLOG" | eval "$tcmd" fi - INNOAPPLY="$BACKUP_BIN --prepare$disver$recovery${iapts:+ }$iapts$INNOEXTRA --target-dir='$DATA' --datadir='$DATA'$mysqld_args $INNOAPPLY" - INNOMOVE="$BACKUP_BIN$WSREP_SST_OPT_CONF --move-back$disver${impts:+ }$impts$INNOEXTRA --galera-info --force-non-empty-directories --target-dir='$DATA' --datadir='${TDATA:-$DATA}' $INNOMOVE" - INNOBACKUP="$BACKUP_BIN$WSREP_SST_OPT_CONF --backup$disver${iopts:+ }$iopts$tmpopts$INNOEXTRA --galera-info --stream=$sfmt --target-dir='$itmpdir' --datadir='$DATA'$mysqld_args $INNOBACKUP" + RC=( "${PIPESTATUS[@]}" ) } send_magic() @@ -1118,13 +1227,12 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then fi wsrep_log_info "Using '$xtmpdir' as mariadb-backup temporary directory" - tmpopts=" --tmpdir='$xtmpdir'" wsrep_log_info "Using '$itmpdir' as mariadb-backup working directory" if [ -n "$WSREP_SST_OPT_USER" ]; then WSREP_SST_OPT_USER_SAFE="$(safe WSREP_SST_OPT_USER)" - INNOEXTRA="$INNOEXTRA --user='$WSREP_SST_OPT_USER_SAFE'" + INNOEXTRA+=(--user="$WSREP_SST_OPT_USER_SAFE") fi if [ -n "$WSREP_SST_OPT_PSWD" ]; then @@ -1188,28 +1296,28 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then tcmd="$ecmd | $tcmd" fi - iopts="--databases-exclude='lost+found'${iopts:+ }$iopts" + iopts=(--databases-exclude=lost+found ${iopts[@]+"${iopts[@]}"}) # if compression is enabled for backup files, then add the # appropriate options to the mariadb-backup command line: if [ "$compress" != 'none' ]; then - iopts="--compress${compress:+=$compress}${iopts:+ }$iopts" + iopts=("--compress${compress:+=$compress}" ${iopts[@]+"${iopts[@]}"}) if [ -n "$compress_threads" ]; then - iopts="--compress-threads=$compress_threads${iopts:+ }$iopts" + iopts=("--compress-threads=$compress_threads" ${iopts[@]+"${iopts[@]}"}) fi if [ -n "$compress_chunk" ]; then - iopts="--compress-chunk-size=$compress_chunk${iopts:+ }$iopts" + iopts=("--compress-chunk-size=$compress_chunk" ${iopts[@]+"${iopts[@]}"}) fi fi if [ -n "$backup_threads" ]; then - iopts="--parallel=$backup_threads${iopts:+ }$iopts" + iopts=("--parallel=$backup_threads" ${iopts[@]+"${iopts[@]}"}) fi setup_commands set +e - timeit "$stagemsg-SST" "$INNOBACKUP | $tcmd; RC=( "\${PIPESTATUS[@]}" )" + run_timed "$stagemsg-SST" run_innobackup set -e if [ ${RC[0]} -ne 0 ]; then @@ -1257,7 +1365,7 @@ else # joiner [ -n "$SST_PROGRESS_FILE" ] && touch "$SST_PROGRESS_FILE" if [ -n "$backup_threads" ]; then - impts="--parallel=$backup_threads${impts:+ }$impts" + impts=("--parallel=$backup_threads" ${impts[@]+"${impts[@]}"}) fi stagemsg='Joiner-Recv' @@ -1445,7 +1553,7 @@ else # joiner wsrep_log_info "Preparing the backup at $DATA" setup_commands - timeit 'mariadb-backup prepare stage' "$INNOAPPLY" + run_timed 'mariadb-backup prepare stage' run_innoapply if [ $? -ne 0 ]; then wsrep_log_error "mariadb-backup apply finished with errors." \ "Check syslog or '$INNOAPPLYLOG' for details." @@ -1507,7 +1615,7 @@ else # joiner DONOR_MAGIC_FILE="$TDATA/$DONOR_INFO_FILE" wsrep_log_info "Moving the backup to $TDATA" - timeit 'mariadb-backup move stage' "$INNOMOVE" + run_timed 'mariadb-backup move stage' run_innomove if [ $? -eq 0 ]; then wsrep_log_info "Move successful, removing $DATA" rm -rf "$DATA" From cd09dda2193b7b9cf86d296f049e2f29b854cb46 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 18 Aug 2026 16:14:07 +0530 Subject: [PATCH 4/7] MDEV-40151: Galera SST: reject a newline in rsyncd.conf path values Issue: datadir and the InnoDB/Aria directory values are written unquoted into the generated rsyncd.conf as "path = " lines. A directory name containing a newline splices in an extra rsync daemon directive. Solution: Reject a newline or carriage return in those values before writing the config file. The config format has no quoting, so this is checked directly rather than passed through a shell construct. --- .../r/galera_sst_rsyncd_conf_injection.result | 4 ++ .../t/galera_sst_rsyncd_conf_injection.cnf | 5 +++ .../t/galera_sst_rsyncd_conf_injection.test | 42 +++++++++++++++++++ scripts/wsrep_sst_common.sh | 11 +++++ scripts/wsrep_sst_rsync.sh | 5 +++ 5 files changed, 67 insertions(+) create mode 100644 mysql-test/suite/galera/r/galera_sst_rsyncd_conf_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.test diff --git a/mysql-test/suite/galera/r/galera_sst_rsyncd_conf_injection.result b/mysql-test/suite/galera/r/galera_sst_rsyncd_conf_injection.result new file mode 100644 index 0000000000000..6abef58f0f9bb --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_rsyncd_conf_injection.result @@ -0,0 +1,4 @@ +connection node_2; +connection node_1; +REJECTED_newline +PASS_clean diff --git a/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.cnf b/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.test b/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.test new file mode 100644 index 0000000000000..eb327972524a6 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_rsyncd_conf_injection.test @@ -0,0 +1,42 @@ +# +# A directory name containing a newline must be rejected before it is written +# into the generated rsyncd.conf, where it would splice in a directive. +# +# check_conf_value() is called directly rather than through a real SST: a +# newline cannot be delivered end to end here, my.cnf is line oriented and +# the restart parameters are split on whitespace. +# +# Steps: +# 1. Extract check_conf_value() from the installed wsrep_sst_common. +# 2. Assert it rejects a value containing a newline. +# 3. Assert it accepts an ordinary value. +# + +--source include/galera_cluster.inc + +perl; + my $tmp = $ENV{MYSQL_TMP_DIR}; + open(my $fh, '>', "$tmp/check_conf_value.sh") or die $!; + print $fh <<'SCRIPT'; +#!/usr/bin/env bash +D=$(dirname "$(command -v wsrep_sst_rsync)") +eval "$(sed -n '/^check_conf_value()/,/^}/p' "$D/wsrep_sst_common")" +wsrep_log_error() { :; } + +FOO=$'good\nvalue' +( check_conf_value FOO ) +[ $? -eq 22 ] && echo REJECTED_newline || echo UNEXPECTED_PASS_newline + +BAR="a clean value" +( check_conf_value BAR ) +[ $? -eq 0 ] && echo PASS_clean || echo UNEXPECTED_REJECT_clean +SCRIPT + close $fh; + chmod 0755, "$tmp/check_conf_value.sh"; +EOF + +--exec $MYSQL_TMP_DIR/check_conf_value.sh + +perl; + unlink "$ENV{MYSQL_TMP_DIR}/check_conf_value.sh"; +EOF diff --git a/scripts/wsrep_sst_common.sh b/scripts/wsrep_sst_common.sh index 730f7b74b2f31..64ef67096b46d 100644 --- a/scripts/wsrep_sst_common.sh +++ b/scripts/wsrep_sst_common.sh @@ -38,6 +38,17 @@ safe() echo "${!1}" } +# A value written into a config file (rsyncd.conf) must not contain a newline: +# that format has no quoting, so a newline would inject a directive. Only +# newlines are rejected, every other character is valid in a path. +check_conf_value() +{ + if [[ "${!1}" == *$'\n'* || "${!1}" == *$'\r'* ]]; then + wsrep_log_error "Refusing SST: newline character in $1" + exit 22 + fi +} + commandex() { if [ -n "$BASH_VERSION" ]; then diff --git a/scripts/wsrep_sst_rsync.sh b/scripts/wsrep_sst_rsync.sh index 4146255c747f2..f2bfa7192fdd1 100644 --- a/scripts/wsrep_sst_rsync.sh +++ b/scripts/wsrep_sst_rsync.sh @@ -702,6 +702,11 @@ else # joiner SILENT="" fi +# these become "path =" directives below, a newline would inject a directive +for _conf_var in DATA ib_log_dir ib_home_dir ib_undo_dir ar_log_dir; do + check_conf_value "$_conf_var" +done + cat << EOF > "$RSYNC_CONF" pid file = $RSYNC_PID use chroot = no From 902249f9ed6f4504ef6316a8b1a831ea93966e1e Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 18 Aug 2026 15:54:47 +0530 Subject: [PATCH 5/7] MDEV-40151: Galera SST: build rsync donor FILTER from an array Issue: FILTER embedded directory values single-quoted into a string that was run via eval. On the donor these values always equal the datadir (create_dirs(), which resolves them from the individual InnoDB/Aria directory options, only runs on the joiner), so this is not independently reachable given the datadir check added for the mysqld sh -c sink, but the construct is still the same shell-injection pattern the SST scripts should not use anywhere. Solution: Build FILTER as a bash array so each value is a literal argv element, and call rsync directly instead of through eval. --- .../galera/r/galera_sst_datadir_spaces.result | 21 ++++++ .../galera/t/galera_sst_datadir_spaces.cnf | 5 ++ .../galera/t/galera_sst_datadir_spaces.test | 75 +++++++++++++++++++ scripts/wsrep_sst_rsync.sh | 31 ++++---- 4 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_sst_datadir_spaces.result create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_spaces.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_datadir_spaces.test diff --git a/mysql-test/suite/galera/r/galera_sst_datadir_spaces.result b/mysql-test/suite/galera/r/galera_sst_datadir_spaces.result new file mode 100644 index 0000000000000..b21426dc0f289 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_datadir_spaces.result @@ -0,0 +1,21 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +connection node_1; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +connection node_2; +connection node_2; +SELECT 1; +1 +1 +connection node_2; +connection node_2; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_spaces.cnf b/mysql-test/suite/galera/t/galera_sst_datadir_spaces.cnf new file mode 100644 index 0000000000000..00992b9b1497b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_spaces.cnf @@ -0,0 +1,5 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_datadir_spaces.test b/mysql-test/suite/galera/t/galera_sst_datadir_spaces.test new file mode 100644 index 0000000000000..829eac8438760 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_datadir_spaces.test @@ -0,0 +1,75 @@ +# +# A space in the datadir is legitimate and must still SST successfully, so +# that the array conversion does not regress ordinary paths. +# +# Steps: +# 1. Bring up a 2-node rsync-SST cluster. +# 2. Relocate node_2's datadir to a path containing a space; force a full SST. +# 3. Assert node_2 joins successfully. +# 4. Restore the datadir and rejoin. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +SELECT 1; + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +--connection node_1 +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); + +--connection node_2 +--source include/shutdown_mysqld.inc + +# move the datadir to a path containing a space +perl; + use strict; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $ddir = "$tmp/dd spaces x"; + my $cnf = "$vardir/my.cnf"; + (my $parent = $ddir) =~ s{/[^/]+$}{}; + mkpath($parent); + rename("$vardir/mysqld.2/data", $ddir) or die "rename datadir: $!"; + unlink "$ddir/grastate.dat"; + open(my $sz, '>', "$tmp/inj_cnf_size") or die $!; print $sz -s $cnf; close $sz; + open(my $fh, '>>', $cnf) or die $!; print $fh "[mysqld.2]\ndatadir=\"$ddir\"\n"; close $fh; +EOF + +--connection node_2 +--source include/start_mysqld.inc + +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +SELECT 1; + +# restore the datadir and rejoin +--connection node_2 +--source include/shutdown_mysqld.inc +perl; + use strict; + my $tmp = $ENV{MYSQL_TMP_DIR}; + my $vardir = $ENV{MYSQLTEST_VARDIR}; + my $cnf = "$vardir/my.cnf"; + open(my $sz, '<', "$tmp/inj_cnf_size") or die $!; my $orig = <$sz>; close $sz; + truncate($cnf, $orig) or die "truncate: $!"; + unlink "$tmp/inj_cnf_size"; + rename("$tmp/dd spaces x", "$vardir/mysqld.2/data") or die "restore datadir: $!"; +EOF + +--connection node_2 +--source include/start_mysqld.inc +--let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' +--source include/wait_condition.inc + +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST .* failed'); diff --git a/scripts/wsrep_sst_rsync.sh b/scripts/wsrep_sst_rsync.sh index f2bfa7192fdd1..ce20c7937b414 100644 --- a/scripts/wsrep_sst_rsync.sh +++ b/scripts/wsrep_sst_rsync.sh @@ -477,8 +477,9 @@ EOF # --exclude grastate.txt --exclude '*.pem' \ # --exclude '*.[0-9][0-9][0-9][0-9][0-9][0-9]' --exclude '*.index') -# New filter - exclude everything except dirs (schemas) and innodb files -FILTER="-f '- /lost+found' +# New filter - exclude everything except dirs (schemas) and innodb files. +# An array, so that the directory values stay literal argv elements: +FILTER=(-f '- /lost+found' -f '- /.zfs' -f '- /.fseventsd' -f '- /.Trashes' @@ -486,22 +487,26 @@ FILTER="-f '- /lost+found' -f '- /.conf' -f '- /.snapshot/' -f '+ /wsrep_sst_binlog.tar' - -f '- $ib_home_dir/ib_lru_dump' - -f '- $ib_home_dir/ibdata*' - -f '- $ib_undo_dir/undo*' - -f '- $ib_log_dir/ib_logfile[0-9]*' - -f '- $ar_log_dir/aria_log_control' - -f '- $ar_log_dir/aria_log.*' + -f "- $ib_home_dir/ib_lru_dump" + -f "- $ib_home_dir/ibdata*" + -f "- $ib_undo_dir/undo*" + -f "- $ib_log_dir/ib_logfile[0-9]*" + -f "- $ar_log_dir/aria_log_control" + -f "- $ar_log_dir/aria_log.*" -f '+ /*/' - -f '- /*'" + -f '- /*') # first, the normal directories, so that we can detect - # incompatible protocol: - eval rsync ${STUNNEL:+"--rsh='$STUNNEL'"} \ + # incompatible protocol. Called directly, so the paths stay verbatim: + RSH_OPT=() + [ -n "$STUNNEL" ] && RSH_OPT=(--rsh="$STUNNEL") + # the guard is needed: expanding an empty array under "set -u" + # aborts on bash before 4.4 + rsync ${RSH_OPT[@]+"${RSH_OPT[@]}"} \ --owner --group --perms --links --specials \ --ignore-times --inplace --dirs --delete --quiet \ - $WHOLE_FILE_OPT $FILTER "'$DATA/'" \ - "'rsync://$WSREP_SST_OPT_ADDR'" >&2 || RC=$? + $WHOLE_FILE_OPT "${FILTER[@]}" "$DATA/" \ + "rsync://$WSREP_SST_OPT_ADDR" >&2 || RC=$? if [ $RC -ne 0 ]; then wsrep_log_error "rsync returned code $RC:" From 99226e2c15e1802f758f989bf3b21b0416882844 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Wed, 19 Aug 2026 10:42:30 +0530 Subject: [PATCH 6/7] MDEV-40151: Galera SST: add donor-side datadir injection test Issue: sst_donate_other()'s shell-unsafe datadir/socket check (added in a prior commit) only had joiner-side test coverage; the donor path was untested. Solution: add a regression test that forces node_1 to act as donor with a malicious datadir, confirming sst_donate_other() rejects it. --- .../galera_sst_donor_datadir_injection.result | 20 +++++ .../t/galera_sst_donor_datadir_injection.cnf | 19 +++++ .../t/galera_sst_donor_datadir_injection.test | 82 +++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 mysql-test/suite/galera/r/galera_sst_donor_datadir_injection.result create mode 100644 mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.test diff --git a/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection.result b/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection.result new file mode 100644 index 0000000000000..6bb6d4c00ff2f --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection.result @@ -0,0 +1,20 @@ +connection node_2; +connection node_1; +connection node_1; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST failed'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Illegal character in variable'); +connection node_2; +FOUND 1 /unsafe for shell interpolation/ in mysqld.1.err +donor-side datadir injection prevented +FOUND 1 /Parent mysqld process .* terminated unexpectedly/ in mysqld.2.err +# restart +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); diff --git a/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.cnf b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.cnf new file mode 100644 index 0000000000000..c83d696b13fa4 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.cnf @@ -0,0 +1,19 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_sst_method=rsync +wsrep_debug=1 + +# Both nodes bootstrap as their own standalone single-member cluster, so +# MTR's own startup does not depend on either being able to reach the other +# (node_1's datadir below makes it permanently unable to serve as donor). +# The test body then explicitly joins node_2 to node_1. +[mysqld.1] +wsrep_cluster_address=gcomm:// +# node_1 is the cluster's bootstrap member, so it never itself receives an +# SST - only sst_donate_other() ever validates its datadir, when it serves +# as donor. Give it a malicious datadir from its very first boot. +datadir=@ENV.MYSQL_TMP_DIR/donor_dd_inj'&touch @ENV.MYSQL_TMP_DIR/donor_dd_inj_marker&'x + +[mysqld.2] +wsrep_cluster_address=gcomm:// diff --git a/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.test b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.test new file mode 100644 index 0000000000000..12de366eeb748 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection.test @@ -0,0 +1,82 @@ +# +# mysqld must reject a shell-unsafe datadir on the DONOR side too: +# sst_donate_other() single-quotes it into the SST command line, a separate +# check from the one sst_prepare_other() applies on the joiner. +# +# Both nodes bootstrap as independent single-member clusters (so neither +# depends on the other to start, and node_1's datadir - malicious from its +# very first boot - is never itself checked by sst_prepare_other(), since a +# bootstrap member never receives an SST). node_2 is then explicitly joined +# to node_1, so node_1 is selected as donor and sst_donate_other() runs. +# +# Steps: +# 1. Pre-create the malicious directory (must exist before node_1 boots). +# 2. Bring up node_1 and node_2 as independent single-member clusters. +# 3. Join node_2 to node_1, forcing a full SST with node_1 as donor. +# 4. Assert node_1 refuses to serve as donor and the payload did not run. +# 5. Restart node_2 standalone again, leaving both nodes up for cleanup. +# + +perl; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + unlink "$tmp/donor_dd_inj_marker"; + mkpath("$tmp/donor_dd_inj'&touch $tmp/donor_dd_inj_marker&'x"); +EOF + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +--connection node_1 +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST failed'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Illegal character in variable'); +--let $node_1_address = `SELECT @@wsrep_node_address` + +# join node_2 to node_1, forcing a full SST with node_1 as donor. node_2 +# aborts once its only donor candidate refuses, so this blocks for exit +# rather than using start_mysqld.inc, which expects the restart to succeed. +--connection node_2 +--source include/shutdown_mysqld.inc +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +--error 1,134 +--exec $MYSQLD_LAST_CMD --wsrep_cluster_address=gcomm://$node_1_address + +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = unsafe for shell interpolation +--source include/search_pattern_in_file.inc + +perl; + die "FAIL: marker created - donor-side datadir injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/donor_dd_inj_marker"; + print "donor-side datadir injection prevented\n"; +EOF + +# node_2's orphaned joiner-side SST script logs its own error a moment +# after mysqld exits; wait for it so it lands within this test's log +# window (and gets suppressed) instead of the next test's. +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.2.err +--let SEARCH_PATTERN = Parent mysqld process .* terminated unexpectedly +--let SEARCH_WAIT = FOUND +--source include/search_pattern_in_file.inc + +# node_1's datadir stays permanently unsafe, so bring node_2 back up +# standalone rather than retrying the join. +perl; + unlink "$ENV{MYSQLTEST_VARDIR}/mysqld.2/data/grastate.dat"; +EOF +--let $start_mysqld_params = --wsrep-cluster-address=gcomm:// +--source $MYSQL_TEST_DIR/include/start_mysqld.inc +--let $start_mysqld_params = + +# suppressions are per-server and the restart above wiped node_2's, so +# add them again now, against the freshly restarted process. +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); From f426ca10b7641e18d41ac3c42d6d9ca5a4869566 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Wed, 19 Aug 2026 10:52:34 +0530 Subject: [PATCH 7/7] MDEV-40151: Galera SST: add mysqldump donor-side datadir injection test Issue: sst_donate_mysqldump()'s shell-unsafe datadir/socket check has its own copy of the validation logic used elsewhere, but had no test coverage. Solution: add a regression test that forces node_1 to donate via mysqldump with a malicious datadir, confirming sst_donate_mysqldump() rejects it. --- ...t_donor_datadir_injection_mysqldump.result | 26 ++++++ ..._sst_donor_datadir_injection_mysqldump.cnf | 15 ++++ ...sst_donor_datadir_injection_mysqldump.test | 87 +++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 mysql-test/suite/galera/r/galera_sst_donor_datadir_injection_mysqldump.result create mode 100644 mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.cnf create mode 100644 mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.test diff --git a/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection_mysqldump.result b/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection_mysqldump.result new file mode 100644 index 0000000000000..e53c24542bb4a --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_donor_datadir_injection_mysqldump.result @@ -0,0 +1,26 @@ +connection node_2; +connection node_1; +connection node_1; +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST failed'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Illegal character in variable'); +CREATE USER 'sst'; +GRANT ALL PRIVILEGES ON *.* TO 'sst'; +SET GLOBAL wsrep_sst_auth = 'sst:'; +connection node_2; +FOUND 1 /unsafe for shell interpolation/ in mysqld.1.err +donor-side datadir injection prevented +connection node_1; +SET GLOBAL wsrep_sst_auth = ''; +DROP USER 'sst'; +connection node_2; +# restart +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); diff --git a/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.cnf b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.cnf new file mode 100644 index 0000000000000..499a98c7245fa --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.cnf @@ -0,0 +1,15 @@ +!include ../galera_2nodes.cnf + +[mysqld] +wsrep_debug=1 + +# Both nodes bootstrap as their own standalone single-member cluster (see +# galera_sst_donor_datadir_injection.cnf for why). node_2 is later joined +# to node_1 with wsrep_sst_method=mysqldump, forcing sst_donate_mysqldump() +# to run on node_1. +[mysqld.1] +wsrep_cluster_address=gcomm:// +datadir=@ENV.MYSQL_TMP_DIR/donor_dd_inj_md'&touch @ENV.MYSQL_TMP_DIR/donor_dd_inj_md_marker&'x + +[mysqld.2] +wsrep_cluster_address=gcomm:// diff --git a/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.test b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.test new file mode 100644 index 0000000000000..455f956e7b828 --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_donor_datadir_injection_mysqldump.test @@ -0,0 +1,87 @@ +# +# mysqld must reject a shell-unsafe datadir on the mysqldump SST donor path +# too: sst_donate_mysqldump() has its own copy of the check +# sst_donate_other() and sst_prepare_other() apply. +# +# Both nodes bootstrap as independent single-member clusters (see +# galera_sst_donor_datadir_injection.test for why). node_2 is then +# explicitly joined to node_1 with wsrep_sst_method=mysqldump, so node_1 +# is selected as donor and sst_donate_mysqldump() runs. +# +# Steps: +# 1. Pre-create the malicious directory (must exist before node_1 boots). +# 2. Bring up node_1 and node_2 as independent single-member clusters. +# 3. Create the SST user mysqldump needs on node_1. +# 4. Join node_2 to node_1 via mysqldump, forcing node_1 to donate. +# 5. Assert node_1 refuses to serve as donor and the payload did not run. +# 6. Restart node_2 standalone again, leaving both nodes up for cleanup. +# + +perl; + use File::Path; + my $tmp = $ENV{MYSQL_TMP_DIR}; + unlink "$tmp/donor_dd_inj_md_marker"; + mkpath("$tmp/donor_dd_inj_md'&touch $tmp/donor_dd_inj_md_marker&'x"); +EOF + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +--connection node_1 +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('SST failed'); +call mtr.add_suppression('SST preparation failed'); +call mtr.add_suppression('Will never receive state. Need to abort'); +call mtr.add_suppression('unsafe for shell interpolation'); +call mtr.add_suppression('Illegal character in variable'); +CREATE USER 'sst'; +GRANT ALL PRIVILEGES ON *.* TO 'sst'; +SET GLOBAL wsrep_sst_auth = 'sst:'; +--let $node_1_address = `SELECT @@wsrep_node_address` + +# join node_2 to node_1 via mysqldump, forcing node_1 to donate. node_2 +# aborts once its only donor candidate refuses, so this blocks for exit +# rather than using start_mysqld.inc, which expects the restart to succeed. +--connection node_2 +--source include/shutdown_mysqld.inc +--remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat +--error 1,134 +--exec $MYSQLD_LAST_CMD --wsrep_cluster_address=gcomm://$node_1_address --wsrep_sst_method=mysqldump --wsrep_sst_auth=sst: --wsrep-sst-receive-address=127.0.0.1:$NODE_MYPORT_2 + +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = unsafe for shell interpolation +--source include/search_pattern_in_file.inc + +perl; + die "FAIL: marker created - donor-side datadir injection was NOT prevented\n" + if -e "$ENV{MYSQL_TMP_DIR}/donor_dd_inj_md_marker"; + print "donor-side datadir injection prevented\n"; +EOF + +# unlike rsync/mariabackup, sst_prepare_mysqldump() runs no external SST +# script on the joiner, so there is no orphaned-script log line to wait +# for here. + +# undo the SST user/auth so MTR's end-of-test state check is happy. +--connection node_1 +SET GLOBAL wsrep_sst_auth = ''; +DROP USER 'sst'; + +# node_1's datadir stays permanently unsafe, so bring node_2 back up +# standalone rather than retrying the join. +--connection node_2 +perl; + unlink "$ENV{MYSQLTEST_VARDIR}/mysqld.2/data/grastate.dat"; +EOF +--let $start_mysqld_params = --wsrep-cluster-address=gcomm:// +--source $MYSQL_TEST_DIR/include/start_mysqld.inc +--let $start_mysqld_params = + +# suppressions are per-server and the restart above wiped node_2's, so +# add them again now, against the freshly restarted process. +call mtr.add_suppression('Process completed with error'); +call mtr.add_suppression('WSREP_SST:'); +call mtr.add_suppression('State transfer to .* failed'); +call mtr.add_suppression('Will never receive state. Need to abort');