Opened 5 years ago

Last modified 5 years ago

#908 new Bug / Defect

inconsistent handling of password string input with LF at the end

Reported by: xTom Owned by:
Priority: minor Milestone:
Component: Windows GUI Version: OpenVPN 2.4.3 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:


adding 0x0a to a key password in the UI Password Dialog will accept the password but prevent the openvpn client from proceeding:

Fri Jun 30 17:26:43 2017 MANAGEMENT: CMD 'hold release'
-password entered-
Fri Jun 30 17:26:53 2017 MANAGEMENT: CMD 'password [...]'
Fri Jun 30 17:26:53 2017 MANAGEMENT: CMD '"'
-now client hangs and does nothing-

The change password dialog will reject the same password (with 0x0a added) as wrong.

So string handling is probably different at these two places.

This appears to be so since at least 2.3.8 (the oldest version i tested) up to 2.4.3.

I stumbled upon this by creating a key/cert pair with a random password on a Linux system, putting the key password on Linux in a textfile via copy and paste (without explicit newline at the end of the password) and using this same file in windows notepad to copy and paste the password into the dialog.
This way I added the invisible 0x0a to the password in Windows and the client - managed via the GUI just hangs (see above) but runs fine in a CMD window. And the password change dialog refuses the pasted password as wrong.

This way you can also add additional commands to be sent to the management interface of the client. I don't know if that could be a serious issue unless the same routines are used in other places.
At least string handling should be the same for password input and password change dialog.



Change History (3)

comment:1 Changed 5 years ago by Selva Nair

The GUI takes the unicode text as entered into the dialog, converts it into utf8 and send to the management interface. No info should be lost in there. However, the management interface uses '\n' as a command delimiter, so openvpn will strip the embedded '\n' and treat the text up to there as the password. The rest of the password would get parsed as a new input but its unlikely that should cause the client to hang..

Will try to reproduce this -- could you provide a "password" string (post it in ,say, hex) that causes the client to hang? I am not sure exactly what happens when a binary string is cut and pasted into a Windows dialog that expects unicode text.. There could be some mangling going on there too.

As for the password change code, its so outdated and does a custom "unicode-to-ascii" conversion that is not necessarily consistent with the rest. The code is lacking in other respects too (inline keys are not supported, multiple key specifications are not correctly handled etc.) so a quick cleanup is not going to happen. That said, if the password uses only 7 bit ascii characters (but no embedded NUL) it should interpret it correctly, I think..

comment:2 Changed 5 years ago by xTom

It causes a hanging client (and it took me a whole day to figure the problem).

To reproduce, change your key password to 12345678

Then, on Linux:
$ printf "12345678\n" > p.txt
$ hd p.txt
00000000 31 32 33 34 35 36 37 38 0a |12345678.|

copy p.txt to windows. Open with notepad (Win7 here), doubleclick to mark the text, copy & paste when to password prompt (if you look closely there'll be 9 characters pasted) - Password will be accepted, but client will hang.

another example:
$ printf "12345678\nsome\nmore" > pwmore.txt
$ hd pwmore.txt
00000000 31 32 33 34 35 36 37 38 0a 73 6f 6d 65 0a 6d 6f ||
00000010 72 65 |re|

notepad will show this as 12345678somemore
copy & paste will give you a hanging client again with:
Fri Jul 07 19:44:52 2017 MANAGEMENT: CMD 'hold release'
Fri Jul 07 19:45:06 2017 MANAGEMENT: CMD 'password [...]'
Fri Jul 07 19:45:06 2017 MANAGEMENT: CMD 'some'
Fri Jul 07 19:45:06 2017 MANAGEMENT: CMD 'more"'

I don't think this special scenario has a huge security impact (for this case). But string handling, especially in security related software, should probably be better.

And at least in this regard, the password change code does it better.

comment:3 Changed 5 years ago by Selva Nair

Embedded '\n' in passwords is now rejected by the GUI with an error message. See #958

Note: See TracTickets for help on using tickets.