Bug #4177
closed
Bug in OpenVPN user/pass auth
Added by Anonymous over 9 years ago. Updated about 9 years ago.
100%
Description
As stated in https://forum.pfsense.org/index.php?topic=85311
OpenVPN user/pass auth fails if passwords end on special characters.
Same password is accepted for user login on GUI (local user auth as backend).
Updated by Phillip Davis over 9 years ago
I think Ermal was fixing some of that. Give an example of final char/s that still do not work on latest snapshots - then it will be easy to verify and, hopefully, to fix.
Updated by Anonymous over 9 years ago
For instance if the password ends with a +
Updated by
Updated by Chris Buechler over 9 years ago
- Priority changed from Normal to High
the problem here is how ovpn_auth_verify passes the password to openvpn.auth-user.php. The latter does a urldecode, which will break for things like + and possibly other things. It seems like a reasonable fix to just base64 it in ovpn_auth_verify by replacing that sed with 'openssl enc -base64', then s/urldecode/base64_decode/ in openvpn.auth-user. I changed it to do just that, but somehow it's still failing. I'll revisit tomorrow unless Ermal beats me to it.
Updated by Ermal Luçi over 9 years ago
- Status changed from Confirmed to Feedback
base64 is a better solution, implemented now.
Updated by Ermal Luçi over 9 years ago
- % Done changed from 0 to 100
Applied in changeset e821f30e7dd50285cf0c590d205409bb53cf3d6a.
Updated by Phillip Davis over 9 years ago
Ermal LUÇI - that fix only got applied to master. It needs to be in RELENG_2_2 also.
Updated by Ermal Luçi over 9 years ago
Applied in changeset 30656f66407ab42c6f42e9552371090ca84165bb.
Updated by Chris Buechler over 9 years ago
- Status changed from Feedback to Confirmed
That's functionally equivalent to what I was trying, which seems like it should work and does fix at least part of the problem, but auth still ultimately fails with that change which is why I hadn't committed it.
Ermal - check 22vpntest host. user vpntestuser, OpenVPN server instance "UDP User/Pass/Cert Local Auth test VPN". Passwords in Lastpass.
That's the most recent stock snapshot available at this time, and includes all the changes here. I added debug logging in openvpn.auth-user which does a log_error with the user/pass at that point (see openvpn.log). It's correct there now, but OpenVPN auth still fails for that user. Go to Diag>Auth, and it works fine. Change it to something without the trailing + and it works with OpenVPN.
Updated by Ermal Luçi over 9 years ago
- Status changed from Confirmed to Feedback
Works for me now.
The issue was related to just echo introducing a \n at the end.
Updated by Ermal Luçi over 9 years ago
Applied in changeset 21165e6455f1402eb6b319dd515a6b43f0bb0e04.
Updated by Ermal Luçi over 9 years ago
Applied in changeset 5cd24cf110ed3b5919922de4437ea3344f54eaea.
Updated by Maciej Blachnio about 9 years ago
Hi,
I've stumbled upon a special case where my client invented a super complicated password and got what he had coming. AUTH_FAILED.
After some trial and error I came to a conclusion that something is not being base encoded properly.
It seems that the below base64 nesting does the job though.
If you confirm and implement this in the official release, as mine is not so official anymore :)
Thanks
Smirk
Example:
Plaintext password with special characters: RJN3p~(^wj.W?6A
One level of base64 encoding ("+" inside and AUTH_FAILED in OVPN):
[root@test]# echo -n "RJN3p~(^wj.W?6A"|openssl enc -base64
UkpOM3B+KF53ai5XPzZB
Two levels (auth success):
[root@test]# echo -n "RJN3p~(^wj.W?6A"|openssl enc -base64|openssl enc -base64
VWtwT00zQitLRjUzYWk1WFB6WkIK
Base64 nesting / possible resolution:
[2.2-RELEASE][admin@pfSense.localdomain]/etc: diff /usr/local/sbin/ovpn_auth_verify /usr/local/sbin/ovpn_auth_verify.bak
7c7
< password=$(echo n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g'| openssl enc -base64)
--
password=$(echo -n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g')
[2.2-RELEASE][admin@pfSense.localdomain]/etc: diff /etc/inc/openvpn.auth-user.php /etc/inc/openvpn.auth-user.php.bak
88c88
< $password = base64_decode(base64_decode(str_replace('%3D', '=', $_GET['password'])));
---
$password = base64_decode(str_replace('%3D', '=', $_GET['password']));
Updated by Maciej Blachnio about 9 years ago
Sorry for the mess.
The diff below should have looked like this:
[2.2-RELEASE][admin@pfSense.localdomain]/etc: diff /usr/local/sbin/ovpn_auth_verify /usr/local/sbin/ovpn_auth_verify.bak
7c7
< password=$(echo -n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g'| openssl enc -base64)
---
> password=$(echo -n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g')
[2.2-RELEASE][admin@pfSense.localdomain]/etc: diff /etc/inc/openvpn.auth-user.php /etc/inc/openvpn.auth-user.php.bak
88c88
< $password = base64_decode(base64_decode(str_replace('%3D', '=', $_GET['password'])));
---
> $password = base64_decode(str_replace('%3D', '=', $_GET['password']));
Updated by Dave Crane about 9 years ago
The extra base64 encoding is not needed. The problem is that base64 can produce three non-alphanum characters: =, + and /.
The line in ovpn_auth_verify should read:
password=$(echo -n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g' | sed -e 's/+/%2B/g' | sed -e 's_/_%2F_g')
Updated by Maciej Blachnio about 9 years ago
True. Why did I not think of that :)
Anyway, I believe this should be commited somewhere in a new release.
Regards
Smirk
Updated by Maciej Blachnio about 9 years ago
Maybe to shorten it:
/usr/local/sbin/ovpn_auth_verify might read:
password=$(echo -n "${password}" | openssl enc -base64 | sed -e 's/=/%3D/g;s/+/%2B/g;s_/_%2F_g')
and /etc/inc/openvpn.auth-user.php might read:
$chars = array("=","+","/");
$repls = array("%3D","%2B","%2F");
$password = base64_decode(str_replace($repls, $chars, $_GET['password'] ));
Updated by Dave Crane about 9 years ago
Nice shortening of the sed string!
I believe the str_replace in openvpn.auth-user.php isn't needed either.
I'm no expert in php, but according to: http://php.net/manual/en/reserved.variables.get.php, anything retrieved through $_GET is automatically urlDecoded.
It could use a comment or two for clarity though. :)
I also agree this should be committed.