openmediavault-luksencryption add password does not accept blanks (probably quoting problem, security-relevant)
meyergru opened this issue · 4 comments
Describe the bug
When I try to add a password via the GUI that contains blanks, I get an error like this:
500 - Internal Server Error
Unable to add the key to the encrypted device: Failed to execute command 'export PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin; export LANG=C.UTF-8; export LANGUAGE=; /bin/bash -c 'echo -n 'Altes Passwort' | cryptsetup luksAddKey -q '/dev/md0' <(echo -n 'Neues Passwort')' 2>&1' with exit code '2': Passwort): -c: line 1: unexpected EOF while looking for matching `)' Passwort): -c: line 2: syntax error: unexpected end of file
To show and or protocol such an error is bad (tm), since that line gets logged (probably also remotely, if enabled) and thus renders all passwords insecure.
Also, the quoting is probably incorrect since such errors do not occur when they consist only of numbers and letters. I suspect that it coughs on other special characters as well.
To Reproduce
Use the GUI to add a password. Choose a password with a blank in it.
Expected behavior
No error. And if there is an error, no log entry with the cleartext passwords in it.
Reference to Forum
openmediavault Server (please complete the following information):
- OS version: Linux sixwd 6.1.0-0.deb11.6-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.15-1~bpo11+1 (2023-03-16) x86_64 GNU/Linux
- openmediavault version: 6.3.11-2
Client (please complete the following information):
none
The passphrase should be escaped - https://github.com/OpenMediaVault-Plugin-Developers/openmediavault-luksencryption/blob/master/usr/share/php/openmediavault/system/storage/luks/container.inc#L512. I will need to setup a test system to test this. I don't currently have much time to do that though. Feel free to send a pull request.
I suspect that it coughs on other special characters as well.
It shouldn't.