Project

General

Profile

Bug #4177

Bug in OpenVPN user/pass auth

Added by Meteotest Informatik over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
High
Assignee:
Ermal Luçi
Category:
OpenVPN
Target version:
Start date:
01/05/2015
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.2
Affected Architecture:

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).

Associated revisions

Revision e821f30e (diff)
Added by Ermal Luçi over 4 years ago

Fixes #4177 convert password to base64 to be submitted to avoid issues with special chars in shell and HTTP GET parameter passing. Probably should add POST support to fcgicli.

Revision 30656f66 (diff)
Added by Ermal Luçi over 4 years ago

Fixes #4177 convert password to base64 to be submitted to avoid issues with special chars in shell and HTTP GET parameter passing. Probably should add POST support to fcgicli.

Revision 21165e64 (diff)
Added by Ermal Luçi over 4 years ago

Prevent echo to insert a newline(\n) at the secret string. Fixes #4177

Revision 5cd24cf1 (diff)
Added by Ermal Luçi over 4 years ago

Prevent echo to insert a newline(\n) at the secret string. Fixes #4177

Revision 6a752ca2 (diff)
Added by Ermal Luçi over 4 years ago

Put the value of password under double quotes(") to avoid issues with special characters in passwords. Ticket #4177

Revision 907cc718 (diff)
Added by Ermal Luçi over 4 years ago

Put the value of password under double quotes(") to avoid issues with special characters in passwords. Ticket #4177

History

#1 Updated by Phillip Davis over 4 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.

#2 Updated by Meteotest Informatik over 4 years ago

For instance if the password ends with a +

#3 Updated by Jim Thompson over 4 years ago

  • Assignee set to Ermal Luçi

#4 Updated by Jim Thompson over 4 years ago

  • Target version set to 2.2

#5 Updated by Chris Buechler over 4 years ago

  • Status changed from New to Confirmed

#6 Updated by Chris Buechler over 4 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.

#7 Updated by Ermal Luçi over 4 years ago

  • Status changed from Confirmed to Feedback

base64 is a better solution, implemented now.

#8 Updated by Ermal Luçi over 4 years ago

  • % Done changed from 0 to 100

#9 Updated by Phillip Davis over 4 years ago

@Ermal - that fix only got applied to master. It needs to be in RELENG_2_2 also.

#10 Updated by Ermal Luçi over 4 years ago

#11 Updated by Chris Buechler over 4 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.

#12 Updated by Ermal Luçi over 4 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.

#13 Updated by Ermal Luçi over 4 years ago

#14 Updated by Ermal Luçi over 4 years ago

#15 Updated by Chris Buechler over 4 years ago

  • Status changed from Feedback to Resolved

works now

#16 Updated by Maciej Blachnio over 4 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][]/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][]/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']));

#17 Updated by Maciej Blachnio over 4 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']));

#18 Updated by Dave Crane over 4 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')

#19 Updated by Maciej Blachnio over 4 years ago

True. Why did I not think of that :)
Anyway, I believe this should be commited somewhere in a new release.
Regards
Smirk

#20 Updated by Maciej Blachnio over 4 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'] ));

#21 Updated by Dave Crane over 4 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.

Also available in: Atom PDF