pqarmitage
commented
1 month ago @corubba Many thanks for the detailed description of this problem. This issue was caused by two problems: i) the greedy parsing of \xNN hex strings, and ii) the double evaluation of escaped characters.
Commit ecdb339 resolves the parsing of hex escape sequences, and commit af7aa8b resolves the double parsing of escape sequences.
During the resolution of these issues, I also found a problem of inappropriate logging of "unmatched quotes" warnings, which are resolved by commit 055a7b1. I also discovered that the parsing of /etc/iproute/rt* files was not ideal, and this is resolved by commit c875649.
Many thanks for reporting these issues.
corubba
Describe the bug
During the parsing of the config file, the quoted string value of the
misc_pathoption is unescaped twice, resulting in behaviour violating the documented escaping rules. Additionally, the "greedy" handling of the hexadecimal\xXXescape sequence makes it potentially discard characters.To Reproduce
1) Create the to-be-checked file with
touch /tmp/my\\x20file(the shell requires the one backslash to be escaped) 2) Doublecheck it's correctly created; there should be a backslash in the name3) Start keepalived using the (minimal) config from below 3) Wait for the healthcheck command to be executed 3) See the command and healthcheck failing, while it should succeed
Expected behavior
The quoted string value of the
misc_pathoption is unescaped/used following the documented rules.Keepalived version
Initially encountered in v2.2.7, still reproducable with the current
masterbranch.Distro:
Details of any containerisation or hosted service (e.g. AWS)
Runs on bare metal, no container or hosted service.
Configuration file:
Notify and track scripts
None.
System Log entries
The output contains additional crude debug output I added to better show the problem. The strings are printed first plain character-wise, and then below character-wise their hexadecimal value. See "Additional context" below for a more detailed description.
Did keepalived coredump?
No.
Additional context
When parsing the config file, the
alloc_strvec_quoted_escaped()function is called twice, first inmisc_path_handler()and then shortly after again inset_script_params_array()(which is called frommisc_path_handler()too). Because of this, the value gets unescaped twice: The first time themy\\x20filegets turned intomy\x20file, which gets turned intomyilewith the invisible "shift in" character0x0fbetween theyandi; and this is what ends up in the command that gets executed as healthcheck.The astute reader may wonder where the
0x0fcomes from when the original escape sequence was\x20. When the parser encounters the\xsequence, it will continue to read characters until it finds a non-hexadecimal one (aka not[0-9a-fA-F]); so in this case it reads\x20fand stops only when it finds thei. Because the final value of the sequence is stored in a 8 bitchar(and a signed one at that!) and shifted by 4 bit for every hexadecimal character, the2is "shifted out" and effectively discarded when reading the third characterf. For any hexadecimal sequence only the last two characters survive, the sequences\x12and\x0011fff773312will result in the same final value0x12. To this end, the octal escape sequence\NNNbehaves as one would expect because it reads exactly three numbers following the backslash.Since the value is unescaped twice, the workaround I found (and am using for the moment) is to just escape it twice too. Using the line
misc_path "/bin/test -f /tmp/my\\\\x20file"(that is with four instead of two backslashes) in the config makes the healthcheck work as wanted. The first unescaping will turnmy\\\\x20fileintomy\\x20file, and the second then intomy\x20file. Below the matching console output.The example here using the
\x20(which is a space) was choosen to be minimal and easily reproducible. The original motivation for executing a command with an embedded backslash escape sequence is for checking the status of an systemd unit using something like/bin/systemctl --system is-active systemd-nspawn@My\x20Container\x201.service. See the systemd unit name escaping for more info.