core icon indicating copy to clipboard operation
core copied to clipboard

ipsec: connections VTI support for dynamic local/remote IP

Open pfoo opened this issue 1 year ago • 10 comments

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

  • [x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
  • [x] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Describe the bug

With legacy IPsec configuration it is possible to :

  • input FQDN in ipsec remote outter IP configuration.
  • use interface name for local outter IP configuration.

VTI interface was then created/modified according to FQDN resolution and/or interface IP change, this was usefull when having a dynamic local IP (assigned to the interface) or a dynamic remote IP.

Main issue : With the new IPsec Connection UI it is impossible to add FQDN or interface name in the Virtual Tunnel Interfaces tab, which breaks dynamic IP VTI configurations.

IPSec configuration itself should work as we can input FQDN in local_addrs/remote_addrs fields, however the help text state that "If FQDNs are assigned, they are resolved every time a configuration lookup is done." Secondary issue : An option to regularly check if the FQDN changed it's IP (or maybe allow to use alias) and reload ipsec and associated automatic-pass firewall rules might be useful as it will prevent unnecessary use of %any.

For now new IPsec connection UI seems broken for dynamic local/remote IP in routed/VTI setup.

To Reproduce

Steps to reproduce the behavior:

  1. Go to VPN -> IPsec -> Virtual Tunnel Interfaces
  2. Click on 'Add'
  3. Try to enter anything else than an IP address in 'Local address' or 'Remote address'
  4. See error : 'Please specify a valid address.'

Expected behavior

Allow IPsec routed/VTI configurations to works with local and remote dynamic IPs :

  • VTI tab -> For remote addresses : Add ability to enter FQDN and/or aliases and resolve them regularly and update VTI interface accordingly.
  • VTI tab -> For local addresses : Add ability to track an interface to update the VTI local IP accordingly. This would also means we need to specify if we want to track ipv4 or ipv6 of the interface.
  • IPSec Connection tab -> make sure FQDN are regularly resolved or allow to use aliases and reload ipsec when the IP changes.

Describe alternatives you considered

The only workaround would be to manually update the VTI interface configuration every time the local/remote IP changes.

Additional context

Some reports on the forum : https://forum.opnsense.org/index.php?topic=35140.0 https://forum.opnsense.org/index.php?topic=33117.0

Environment

OPNsense 23.7.2-amd64

pfoo avatar Aug 25 '23 18:08 pfoo

can't work reliable, see note in https://docs.opnsense.org/manual/vpnet.html#route-based-vti

AdSchellevis avatar Aug 25 '23 18:08 AdSchellevis

No it won't be as reliable as with a static IP. But I have a working VTI configuration with dynamic IP for months now on opnsense, years even on pfsense.

As soon as you have dynamic IP somewhere, it does not work reliably anymore, VTI or not, as you will have to tolerate some downtime.

The tunnel endpoints can be changed live with a single ifconfig command. The main issue is detecting when it needs to be changed.

Unfortunately a lot of us are still stuck to dynamic IP especially on non-professional home ISP

pfoo avatar Aug 25 '23 22:08 pfoo

if/when if_ipsec(4) supports the endpoints to be changed without recreating the interface, we might reconsider putting time in this, but until that time, this isn't a priority as it will break somewhere and lead to hard to explain support cases. While developing the new module we tried different options (binding to non existing endpoints, changing on updates), but it's just not reliable due to upstream reasons.

AdSchellevis avatar Aug 26 '23 08:08 AdSchellevis

No it won't be as reliable as with a static IP. But I have a working VTI configuration with dynamic IP for months now on opnsense, years even on pfsense.

I double that. I use IPsec VTI since a few years now (I believe starting with 21.1) and did not have any major problems so far. In my case the IPs do not change frequently, only once each few months, I'd say. Anyway, restarting the IPsec tunnel on the centre firewall is still easier than modifying the configuration (causing potential config errors) after each IP change.

8191 avatar Sep 01 '23 19:09 8191

You can keep using the legacy tunnels, maybe eventually we think of something that is sustainable or an upstream change will offer the possibility to change if_ipsec(4) addressing on the fly without breaking traffic at some point.

AdSchellevis avatar Sep 01 '23 19:09 AdSchellevis

+1 for this. I'm also using a Site-to-Site Tunnel with dynamic IPs on both sides for over a year now. With some monit rules to check the tunnel health and dead peer detection enabled, the downtime was ~5-10 minutes every few weeks.

cektom avatar Oct 29 '23 13:10 cektom

@cektom Hello, I might have rules to check tunnel health and dead peer detection enabled. I need to put the same configuration in place

bipbip365 avatar Nov 23 '23 21:11 bipbip365

This https://github.com/opnsense/core/commit/816ecae2c5be3c3e9efa22aa653be19d32afc419 should do the trick, but now I'm looking for people to try it out.

AdSchellevis avatar Feb 12 '24 17:02 AdSchellevis

I could successfully set this up with one dynamic IP and one static at least. (Leaving both addresses empty).

joni1993 avatar Feb 12 '24 21:02 joni1993

@joni1993 thanks for testing. I'll leave this here for a little while, maybe someone else want to try it as well. Merging should be relatively safe as setups with local/remote addresses set are not provisioned using the new hook.

AdSchellevis avatar Feb 13 '24 07:02 AdSchellevis

nobody else interested in this? that's surprising....

AdSchellevis avatar Feb 26 '24 16:02 AdSchellevis

still in use as a "legacy config" - but ever wondered why this did not work using the new way - how is this testable?

stefankubis avatar Mar 01 '24 15:03 stefankubis

opnsense-patch 816ecae ?

AdSchellevis avatar Mar 01 '24 15:03 AdSchellevis

Seems to be working ! I however have to figure a way to force my ISP to change my ipv4 to fully test this

pfoo avatar Mar 29 '24 08:03 pfoo

@AdSchellevis: I am not sure what exactly changed, but for me it is now not working anymore. Not sure if it was a coincidence it worked back then or something else changed (did not find any related code changes).

Enabling the old setup works - i noticed following changes comparing the old and new UI method:

ifconfig does not show the tunnel attribute when using the new UI - also it is not RUNNING. Manually executing ifconfig ipsec100 tunnel LOCAL_WAN_IP REMOTE_WAN_IP (the ips substituted ofcourse), brings the tunnel into RUNNING and makes the remote inner tunnel ip respond. Manually reapplying routes also brings connectivity over the tunnel to remote networks.

If i can help any further, just let me know.

EDIT: Above method worked once, now i can't get it working again. EDIT2: Needed to manually reconnect the tunnel.

joni1993 avatar Apr 22 '24 15:04 joni1993

@joni1993 when the old type works, it's unlikely related to this change as both are being setup quite similar, on my end a statically configure vti looks normal after an update.

https://github.com/opnsense/core/blob/bad7f2519b7747213c127826ebdb31929419b3d9/src/etc/inc/plugins.inc.d/ipsec.inc#L1717-L1825

The patch https://github.com/opnsense/core/commit/659ed117f474264f4f3d64039c6fc6dd0ae6ffa9 was released in 24.1.4 if I'm not mistaken.

AdSchellevis avatar Apr 22 '24 17:04 AdSchellevis

Are they really set up similar? I do not know where the generated files are coming from but looking at /usr/local/etc/swanctl/swanctl.conf, the old method generates local_addrs and remote_addrs configuration for the connection, while the new method does not generate such configuration entries.

On the other hand the new method generates the updown event handler in the children section (which the old one does not) - and manually executing the handler like /usr/local/opnsense/scripts/ipsec/updown_event.py --connection_child 782d9cb1-8af0-4890-a8bf-007eb1934394 --action up --reqid 100 --local MYIP --remote REMOTEIP throws:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/configparser.py", line 789, in get
    value = d[option]
  File "/usr/local/lib/python3.9/collections/__init__.py", line 941, in __getitem__
    return self.__missing__(key)            # support subclasses that define __missing__
  File "/usr/local/lib/python3.9/collections/__init__.py", line 933, in __missing__
    raise KeyError(key)
KeyError: 'connection_child'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/opnsense/scripts/ipsec/updown_event.py", line 64, in <module>
    or cnf.get(section, 'connection_child') == cmd_args.connection_child:
  File "/usr/local/lib/python3.9/configparser.py", line 792, in get
    raise NoOptionError(option, section)
configparser.NoOptionError: No option 'connection_child' in section: 'vti_72691b462a734a82963ade379b5fb4b5'

joni1993 avatar Apr 23 '24 09:04 joni1993

Changing the updown event script as follows:


diff --git a/src/opnsense/scripts/ipsec/updown_event.py b/src/opnsense/scripts/ipsec/updown_event.py
index dd3030460..6facc4b13 100755
--- a/src/opnsense/scripts/ipsec/updown_event.py
+++ b/src/opnsense/scripts/ipsec/updown_event.py
@@ -59,9 +59,8 @@ if __name__ == '__main__':
             spds = []
             vtis = []
             for section in cnf.sections():
-                if cnf.get(section, 'reqid') == cmd_args.reqid \
-                        or cnf.get(section, 'connection_child') == cmd_args.connection_child:
-                    if section.startswith('spd_'):
+                if cnf.get(section, 'reqid') == cmd_args.reqid:
+                    if cnf.has_option(section,"connection_child") and cnf.get(section, 'connection_child') == cmd_args.connection_child and section.startswith('spd_'):
                         spds.append({
                             'reqid': cmd_args.reqid,
                             'local' : cmd_args.local,

Adds the tunnel property to the interface correctly and bringing up the tunnel. Only one manual disconnect/connect was needed after changing the script - disabling and enabling the vti was not enough. (why? - when creating a vti one want's to fire the event to bring up the tunnel?)

I don't know if the patch is correct for the SPD part though.. but the vti part of src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf definetely never have the connection_child section

joni1993 avatar Apr 23 '24 10:04 joni1993

@joni1993 what does the reqid_events.conf looks like on your end? it's unlikely there are VTI sections in the configuration when properly configured with static addresses

https://github.com/opnsense/core/blob/b27881c4361a21959a66283789f875c813bbd8d1/src/opnsense/service/templates/OPNsense/IPsec/reqid_events.conf#L21

Without sections starting with vti_ the ifconfig commands are not being executed.

AdSchellevis avatar Apr 23 '24 17:04 AdSchellevis

I am not using static addresses - thats the whole point of this issue right? ;)

"Phase 1" (Connections): Local: Remote: <Remote IP>

"Local Authentication" (Connections): Round: 0 Authentication: PSK id: <DynDns Hostname> [A static CNAME pointing to a DynDns domain]

"Remote Authentication" (Connections): Round: 0 Authentication: PSK id: <Remote IP>

"Children" (Connections): Mode: Tunnel Start action: Trap+Start DPD action: Start Reqid: 100 Local: 0.0.0.0/0 Remote: 0.0.0.0/0

Pre-Shared-Keys: Local Identifier: <DynDNS Hostname> Remote Identifier: <Remote IP> PSK: <PW> Type: PSK

Virtual Tunnel Interface: Reqid: 100 Local address: (because it uses dynamic ips) Remote address: (because both need to be empty if one is) Tunnel local address: <Local VTI IP> [x.y.z.1] Tunnel remote address: <Remote VTI IP> [x.y.z.2]

reqid_events:

...
[vti_6066f6a169734e1089435d68c390ff17]
reqid = 100
description = VPN
uuid = 6066f6a1-6973-4e10-8943-5d68c390ff17
...

joni1993 avatar Apr 23 '24 18:04 joni1993

@joni1993 you mentioned your setup worked before this change, dynamic addresses weren't supported before 21.1.4 for the new connections.

AdSchellevis avatar Apr 23 '24 18:04 AdSchellevis

It worked when i was on 21.1.1 + manually applying the patch.

But i suspect it maybe worked because back then i did just disable the old config and enabled the new config at the same time and when applying some "old state"/IKE/Phase2 got reused for the new config - which made it seem working.

But yesterday after updating to 24.1.6, it did not work anymore, so i suspect it initially only worked because of some quirks/coincidence/leftovers-states.

joni1993 avatar Apr 23 '24 18:04 joni1993

if the log contains the event being processed in : https://github.com/opnsense/core/blob/b27881c4361a21959a66283789f875c813bbd8d1/src/opnsense/scripts/ipsec/updown_event.py#L55

we can add some additional logging to help local debugging, in that case just open a new ticket to request more logging in this event.

AdSchellevis avatar Apr 23 '24 18:04 AdSchellevis

Yes it does include the line - but the script probably fails as written in my above comment - because the tunnel property is never applied.

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/configparser.py", line 789, in get
    value = d[option]
  File "/usr/local/lib/python3.9/collections/__init__.py", line 941, in __getitem__
    return self.__missing__(key)            # support subclasses that define __missing__
  File "/usr/local/lib/python3.9/collections/__init__.py", line 933, in __missing__
    raise KeyError(key)
KeyError: 'connection_child'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/opnsense/scripts/ipsec/updown_event.py", line 64, in <module>
    or cnf.get(section, 'connection_child') == cmd_args.connection_child:
  File "/usr/local/lib/python3.9/configparser.py", line 792, in get
    raise NoOptionError(option, section)
configparser.NoOptionError: No option 'connection_child' in section: 'vti_72691b462a734a82963ade379b5fb4b5'

joni1993 avatar Apr 23 '24 21:04 joni1993

@joni1993 ah, got it, missed that part in your initial comment, I'll take a look

AdSchellevis avatar Apr 24 '24 06:04 AdSchellevis

@joni1993 can you try https://github.com/opnsense/core/commit/b0bf317640c17874fa781846a81a39e76517fc05 ?

AdSchellevis avatar Apr 24 '24 06:04 AdSchellevis

Yes - this works and brings up the tunnel. Interestingly after disconnecting the tunnel and connecting again after tearing down and bringing up the VTI, the static routes for the gateway did not get applied.

joni1993 avatar Apr 24 '24 08:04 joni1993

the event shouldn't try to enforce routing, but if we only change the outer addressing, the inner shouldn't change anyway

AdSchellevis avatar Apr 24 '24 08:04 AdSchellevis

I agree. Somehow the Gateway Interface magically changed to some arbitrary interface...

Anyway I'd call this case closed :)

joni1993 avatar Apr 24 '24 08:04 joni1993