Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openmediavault-luksencryption add password does not accept blanks (probably quoting problem, security-relevant) #48

Open
meyergru opened this issue May 16, 2023 · 4 comments

Comments

@meyergru
Copy link

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

https://forum.openmediavault.org/index.php?thread/47819-bug-report-openmediavault-luksencryption-add-password-does-not-accept-blanks-pro/

openmediavault Server (please complete the following information):

  • OS version: Linux sixwd 6.1.0-0.deb11.6-amd64 Merge keyfile branch #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

@meyergru meyergru changed the title openmediavault-luksencryption add password does not accept blanks (probably quoting problem) openmediavault-luksencryption add password does not accept blanks (probably quoting problem, security-relevant) May 16, 2023
@ryecoaaron
Copy link
Member

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.

@DonkeeeyKong
Copy link

DonkeeeyKong commented Aug 6, 2024

I experienced the same problem:

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 'oldpassword!' | cryptsetup luksAddKey -q  '/dev/nvme0n1'  <(echo -n 'the new password with spaces')' 2>&1' with exit code '2': new: -c: line 2: unexpected EOF while looking for matching `)'

OMV\Exception: 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 'oldpassword!' | cryptsetup luksAddKey -q  '/dev/nvme0n1'  <(echo -n 'the new password with spaces')' 2>&1' with exit code '2': new: -c: line 2: unexpected EOF while looking for matching `)' in /usr/share/openmediavault/engined/rpc/luks.inc:616
Stack trace:
#0 [internal function]: OMVRpcServiceLuksMgmt->addContainerKey()
#1 /usr/share/php/openmediavault/rpc/serviceabstract.inc(122): call_user_func_array()
#2 /usr/share/php/openmediavault/rpc/rpc.inc(86): OMV\Rpc\ServiceAbstract->callMethod()
#3 /usr/sbin/omv-engined(544): OMV\Rpc\Rpc::call()
#4 {main}

If I understand the log correctly: Could it be that the cryptsetup command is called via /bin/bash -c 'command ...':
/bin/bash -c 'echo -n 'oldpassword!' | cryptsetup luksAddKey -q '/dev/nvme0n1' <(echo -n 'the new password with spaces')' 2>&1, so the actual command is quoted – thus unquoting the passwords?

@DonkeeeyKong
Copy link

Yep. This seems to be the problem: I just made a bash script with the command from the log entry and ran it through shellcheck:

 /bin/bash -c 'echo -n 'oldpassword!' | cryptsetup luksAddKey -q  '/dev/nvme0n1'  <(echo -n 'the new password with spaces')' 2>&1
                        ^---------^ SC2026 (info): This word is outside of quotes. Did you intend to 'nest '"'single quotes'"' instead'? 
                                                                                             ^-^ SC2026 (info): This word is outside of quotes. Did you intend to 'nest '"'single quotes'"' instead'? 

For more information:
  https://www.shellcheck.net/wiki/SC2026 -- This word is outside of quotes. D...

@DonkeeeyKong
Copy link

DonkeeeyKong commented Aug 6, 2024

A workaround for now seems to be to enter the passphrase in double quotes: "this passphrase gets accepted"
I was also able to successfully test a passphrase containing blanks that I set up using CLI (without double quotes) to be accepted by the web GUI when entered within double quotes.

@meyergru: Are you sure this gets logged? I also don't want my passwords to be safed in logs and tried to delete them but I couldn't find any logfile nor anything in journalctl that would contain this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants