MDEV-39648: wsrep_sst_rsync.sh: apply safe() to joiner-supplied parameters#5092
MDEV-39648: wsrep_sst_rsync.sh: apply safe() to joiner-supplied parameters#5092hemantdangi-gc wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the security of the wsrep_sst_rsync.sh script by sanitizing WSREP_SST_OPT_REMOTE_USER and WSREP_SST_OPT_REMOTE_PSWD using the safe function, and adds a regression test for shell-unsafe certificate Common Names. Feedback was provided regarding insecure error handling: since the safe function is called within subshells, its exit status is currently ignored by the main script. This could lead to the script continuing with incomplete or malformed values. It is recommended to explicitly check the exit status of these assignments to ensure the script aborts correctly on validation failure.
…eters Issue: wsrep_sst_rsync.sh interpolated WSREP_SST_OPT_REMOTE_USER and WSREP_SST_OPT_REMOTE_PSWD verbatim. Because both values originate from the joiner side of the SST request, a newline in either could splice an extra directive into the donor-written stunnel.conf (silently downgrading peer-cert verification) or an extra line into the rsync magic file. MDEV-39413 had introduced safe() for the same threat class in wsrep_sst_mariabackup but did not extend it to the rsync script. Solution: Routing the rsync interpolations through safe() closes the gap, and extending safe() to also reject tab and newline ensures multi-line values cannot survive into a config-file heredoc.
hemantdangi-gc
force-pushed
the
10.6-MDEV-39648
branch
from
2ce7061 to
14291eb
Compare
|
@vuvova Both gemini issue are handled and closed. Please review if it's ok to be pushed? |
|
will push after fixing a typo |
4 participants
Issue:
wsrep_sst_rsync.sh interpolated WSREP_SST_OPT_REMOTE_USER and WSREP_SST_OPT_REMOTE_PSWD verbatim. Because both values originate from the joiner side of the SST request, a newline in either could splice an extra directive into the donor-written stunnel.conf (silently downgrading peer-cert verification) or an extra line into the rsync magic file. MDEV-39413 had introduced safe() for the same threat class in wsrep_sst_mariabackup but did not extend it to the rsync script.
Solution:
Routing the rsync interpolations through safe() closes the gap, and extending safe() to also reject tab and newline ensures multi-line values cannot survive into a config-file heredoc.