Project

General

Profile

Actions

Bug #4177

closed

Bug in OpenVPN user/pass auth

Added by Anonymous over 7 years ago. Updated over 7 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:
Plus Target Version:
Release Notes:
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).

Actions #1

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

Actions #2

Updated by Anonymous over 7 years ago

For instance if the password ends with a +

Actions #3

Updated by Jim Thompson over 7 years ago

  • Assignee set to Ermal Luçi
Actions #4

Updated by Jim Thompson over 7 years ago

  • Target version set to 2.2
Actions #5

Updated by Chris Buechler over 7 years ago

  • Status changed from New to Confirmed
Actions #6

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

Actions #7

Updated by Ermal Luçi over 7 years ago

  • Status changed from Confirmed to Feedback

base64 is a better solution, implemented now.

Actions #8

Updated by Ermal Luçi over 7 years ago

  • % Done changed from 0 to 100
Actions #9

Updated by Phillip Davis over 7 years ago

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

Actions #10

Updated by Ermal Luçi over 7 years ago

Actions #11

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

Actions #12

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

Actions #13

Updated by Ermal Luçi over 7 years ago

Actions #14

Updated by Ermal Luçi over 7 years ago

Actions #15

Updated by Chris Buechler over 7 years ago

  • Status changed from Feedback to Resolved

works now

Actions #16

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

Actions #17

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

Actions #18

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

Actions #19

Updated by Maciej Blachnio over 7 years ago

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

Actions #20

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

Actions #21

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

Actions

Also available in: Atom PDF