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

Fix OpenVPN dynamic gateway detection #4433

Closed
wants to merge 1 commit into from

Conversation

hippi-viking
Copy link

This is a quick hack to fix OpenVPN dynamic gateway detection for 99% of the cases. It is highly unlikely someone has a VPN gateway on their own local tunnel IP / loopback. The gateway usually is the .1 address on the same subnet as the client, this fix will help in that case. If this is not the case, a proper parser should be written (which could be overkill for simple use cases).

This is a quick hack to fix OpenVPN dynamic gateway detection for 99% of the cases. It is highly unlikely someone has a VPN gateway on their own local tunnel IP / loopback. The gateway usually is the .1 address on the same subnet as the client, this fix will help in that case. If this is not the case, a proper parser should be written (which could be overkill for simple use cases).
@AdSchellevis
Copy link
Member

this likely won't get merged, different discussions have been going on this subject (like #4134 (comment), #4268 (comment)), but in order help this further one will have to prepare test setups for different scenario's and explain what the expected outcome should be when using different options.

@hippi-viking
Copy link
Author

Thanks, this will be a local patch only then as well for the time being. How can I help with the test setups? I have multiple commercial VPN tunnel clients set up, they all work as per the proposed patch. I cannot imagine a scenario where the local tunnel address might be the gateway (not even for an OpenVPN server), so the proposed patch should not break anything in any case.

@AdSchellevis
Copy link
Member

If the issues only involve cases where either route_vpn_gateway and ifconfig_remote are empty, we could probably focus on the ifconfig_local setting for now. For policy based routing we need to know the other endpoint (otherwise we could probably just send traffic to the device), but question is how it's defined (why is it always x.x.x.1 for example) and what happens for ipv6. A couple of test scenarios would probably help a lot.

@Ulrar
Copy link

Ulrar commented Oct 27, 2020

If a "real" fix is going to take time and effort, could you maybe just add a tickbox "use device as gateway" in the interface settings instead ? That wouldn't break any existing setup and would allow people affected by this to finally be able to use their VPN.
I actually expected This interface does not require an intermediate system to act as a gateway to do that but it doesn't seem to make any difference when ticked.

I know in my specific case using the first IP in the block works but only because it's on the correct device, my VPN's admin very specifically told me that I shouldn't have an IP at all as the gateway, and I suspect that's true for a lot of people. That's likely why it comes up empty in those variables.

@AdSchellevis
Copy link
Member

For normal destination based routing you probably shouldn't need an address, people trying to use policy based routing should likely add a manual gateway with the correct endpoint. This is the same discussion as #4268 (comment) , when the gateway can't be predicted reliably the script should probably just remove the router file in this section

elif [ -n "${ifconfig_local}" ]; then
/bin/echo ${ifconfig_local} > /tmp/$1_router

@Ulrar
Copy link

Ulrar commented Oct 27, 2020 via email

@AdSchellevis
Copy link
Member

I don't mind fixing something in this area, the question remains what should we fix. My initial question about preparing different scenarios and their expected outcome is all aimed at collecting the relevant data points..... if the ${ifconfig_local} never works (which I assume, but haven't seen proof this covers all scenarios), we can aim at a solution when ${route_vpn_gateway} and ${ifconfig_remote} are not provided (same question applies for ipv6). Personally I do like to avoid hot fixing this without a testbed and a clear description of the issue. If someone wants to help preparing the test setups and detailing the issue, that would be great, if not, it will likely remain for at least a couple of months (I currently can't spare the time to test this properly).

@hippi-viking
Copy link
Author

Sadly I don't have IPv6 connection (not even my ISP supports it), so I cannot test it easily.
This patch is intended as a last resort fix for cases where ${ifconfig_remote} and ${route_vpn_gateway} are not defined, as ${ifconfig_local} will not work for sure. I would not mind setting up the gateway manually (I have been doing that for months), it is just annoying when you connect to a load-balanced VPN with different IP-ranges (so you have to change the gateway every time it reconnects).
x.x.x.1 seems to be the gateway on most commercial VPNs I have tried, which is of course not a standard but seems like a fairly common setting for it to work out of the box almost every time.

@Ulrar
Copy link

Ulrar commented Oct 28, 2020

So I finally got around to testing the patch in this PR and it works fine, except in my case the first IP of the bloc is .129. IMHO if we just added a line to calculate the first IP from the ${ifconfig_local} + subnet it would work fine for most people, which is likely more than the current behavior.
I'll keep applying it by hand for now I guess, thanks for the fix in the meantime

AdSchellevis added a commit that referenced this pull request Oct 28, 2020
@AdSchellevis
Copy link
Member

@Ulrar your probably right, although it would still be practical if more people test different configurations, this 0ad3ec4 should probably help for most setups (assuming ifconfig_netmask is being supplied in all cases. (#4134 (comment) seems to suggest this as well)

@hippi-viking
Copy link
Author

Thanks Ad.
For my cases netmask is indeed seems to be supplied:

(excerpts from the OpenVPN log:)
vpn1: /sbin/ifconfig ovpnc1 10.18.0.x 10.18.0.1 mtu 1500 netmask 255.255.0.0 up
vpn1: /usr/local/etc/inc/plugins.inc.d/openvpn/ovpn-linkup ovpnc1 1500 1585 10.18.0.x 255.255.0.0 init
vpn2: /sbin/ifconfig ovpnc2 172.27.0.x 172.27.0.1 mtu 1500 netmask 255.255.252.0 up
vpn2: /usr/local/etc/inc/plugins.inc.d/openvpn/ovpn-linkup ovpnc2 1500 1552 172.27.0.x 255.255.252.0 init

I was thinking either calculating the first IP to be the gateway(?) based on the local IP and netmask OR parsing the routing table (OpenVPN seems to set it up correctly based on the log) OR parsing the openvpn.log file itself to get the gateway IP address.

@AdSchellevis
Copy link
Member

so, the question is, does the calculation )in 0ad3ec4) work for all use-cases known so-far. I rather prevent parsing route tables or ifconfig statements to be honest.

@vlcty
Copy link

vlcty commented Nov 7, 2020

I also stumbled over this problem. I created issue #4268 .
With IPv6 everything is fine, because ifconfig_ipv6_remote is always passed via ENV which the ovpn-linkup script parses.

Fo IPv4 the problem only occures with topology subnet instead of net30. It seems that the first address is always assigned to the server. You COULD change it, however you normally don't do it. At least I have never seen it anywhere, but who am I.
ifconfig_netmask and ifconfig_local is also always passed as far as I can see so using it for calculations is In my eyes a good solution.

@AdSchellevis
Copy link
Member

If 0ad3ec4 seems to work for everybody in this thread, I propose to close this PR and probably #4419 as well.

@Ulrar
Copy link

Ulrar commented Nov 25, 2020

0ad3ec4 does work perfectly, any chance to get that merged at some point ? It'd be nice not to have to go edit the file after each update.

@AdSchellevis
Copy link
Member

@Ulrar ok, thanks for letting us know, I'll close #4419 and schedule this for the next release or the one after.

@kkoshelev
Copy link

Not fixed? Would anyone look at my data and help with settings?

I'm having similar problems in my setup. The openvpn gateway doesn't get correct settings. ovpn-linkup didn't get called with valid gateway address. I suspect the problem is with PUSH_REPLY sent by vpn server. I have two cases with exactly the same OPNsense settings, one of them works another doesn't.

PIA VPN - doesn't work
PiaVPN ovpnc2 gateway overview
IPv4 address | 10.7.112.24 / 24
Gateway IPv4 | 10.7.112.24 <- wrong ip address, should be 10.7.112.1

routing table
ipv4 | 10.7.112.0/24 | 10.7.112.1 | UGS | 0 | 1500 | ovpnc2 | PiaVPN |   |  
ipv4 | 10.7.112.1 | link#13 | UH | 0 | 1500 | ovpnc2 | PiaVPN |  
ipv4 | 10.7.112.24 | link#13 | UHS | 1098 | 16384 | lo0 | Loopback

logs
2020-12-01T18:37:00 | openvpn[6741] | /usr/local/etc/inc/plugins.inc.d/openvpn/ovpn-linkup ovpnc2 1500 1553 10.7.112.24 255.255.255.0 init
2020-12-01T18:37:00 | openvpn[6741] | /sbin/route add -net 10.7.112.0 10.7.112.1 255.255.255.0
2020-12-01T18:37:00 | openvpn[6741] | PUSH: Received control message: 'PUSH_REPLY,comp-lzo no,redirect-gateway def1,route-ipv6 2000::/3,dhcp-option DNS 10.0.0.243,route-gateway 10.7.112.1,topology subnet,ping 10,ping-restart 60,ifconfig 10.7.112.24 255.255.255.0,peer-id 2,cipher AES-128-GCM'

ExpressVPN - works
ovpnc1 gateway
IPv4 address | 10.158.0.70 / 32
Gateway IPv4 | 10.158.0.69

routing table
ipv4 | 10.158.0.69 | link#12 | UH | 69705 | 1500 | ovpnc1 | ExpressVPN |   |  
ipv4 | 10.158.0.70 | link#12 | UHS | 0 | 16384 | lo0 | Loopback |  

logs
2020-12-01T17:44:02 openvpn[70454] /usr/local/etc/inc/plugins.inc.d/openvpn/ovpn-linkup ovpnc1 1500 1557 10.158.0.70 10.158.0.69 init
2020-12-01T17:44:02 openvpn[70454] /sbin/ifconfig ovpnc1 10.158.0.70 10.158.0.69 mtu 1500 netmask 255.255.255.255 up
2020-12-01T17:44:02 openvpn[70454] PUSH: Received control message: 'PUSH_REPLY,redirect-gateway def1,dhcp-option DNS 10.158.0.1,comp-lzo no,route 10.158.0.1,topology net30,ping 10,ping-restart 60,ifconfig 10.158.0.70 10.158.0.69,peer-id 13,cipher AES-256-GCM'

fichtner pushed a commit that referenced this pull request Dec 7, 2020
…first network address as gateway address. for #4433

(cherry picked from commit 0ad3ec4)
@hippi-viking hippi-viking deleted the patch-2 branch December 10, 2020 08:43
oshogbo pushed a commit to DynFi/opnsense-core that referenced this pull request Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants