Summary: | Allow setting IPSec parameter "Auto" | ||
---|---|---|---|
Product: | IPFire | Reporter: | Tom Rymes <tomvend> |
Component: | --- | Assignee: | Michael Tremer <michael.tremer> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | Balancing | ||
Priority: | - Unknown - | CC: | peter.mueller |
Version: | 2 | ||
Hardware: | all | ||
OS: | Linux | ||
Bug Depends on: | |||
Bug Blocks: | 11618 | ||
Attachments: |
Modified vpnmain.cgi (allows selection of auto parameter)
Proposed patch Fixed Proposed Patch Second Fixed Proposed Patch |
Description
Tom Rymes
2015-01-28 15:13:57 UTC
A quick note to mention that initial testing indicates that, when auto=route, tunnels that have dropped will be properly reconnected when traffic bound for that tunnel is sent out. This is a major improvement for us, manifested as a perceived SIGNIFICANT improvement in IPSec reliability. To add more fuel to this fire, I have the following recommendation from Noel Kuntze on the StrongSwan mailing list regarding the use of auto= and dpdaction: "Use auto=route and dpdaction=clear between sites with static IPs. For connection between sites with mixed static and dynamic IPs, use auto=add and dpdaction=clear on the side with the static IP and auto=route and dpdaction=restart, or auto=route and dpdaction=clear on the side with the dynamic IP." https://lists.strongswan.org/pipermail/users/2015-July/008552.html This brings two things to mind: 1.) The current WUI does not allow me to do this. 2.) The current default setting is not recommended. Tom Michael: You and I spoke about this briefly today. Just a reminder that this would be handy. Perhaps a descriptive name for each setting would help avoid scaring people off with additional IPSec settings: Tunnel boot options: - Start tunnel immediately (auto=start). - Wait for remote system to initiate tunnel(auto=add). - Wait for traffic to start tunnel (auto=route). I would suggest that the "auto=" part of the description be left in so more advanced users aren't required to guess what the WUI is doing behind the scenes. Tom, do we actually need this any more? I think so. Based on the recommendations I received from Noel, using route or add is often preferred, and I think it helps avoid multiple Child SAs when both sides try to bring the tunnel back up at the same time. Another link to the discussion on the Strongswan Users list: http://comments.gmane.org/gmane.network.vpn.strongswan.user/10315 Also, as I mentioned above, the reasons I think that this should be implemented (for net to net, at least) are: 1.) The current WUI does not allow me to use recommended settings. 2.) The current default setting is not recommended. Neither of these are a Good Thing™. The main downside I can envision is that an added connection will not show as connected until you route some traffic over it and you change the setting from something other than "start". FWIW, I think that auto=route is generally how Cisco devices handle tunnels, if memory serves. You are right about Cisco. I am willing to implement this then. But probably not this year any more. Please remind me next year if you don't hear anything from me. I poked about a bit in the vpnmain.cgi file, and it seems to me that the following is needed to move this forward: 1.) Adding a drop-down menu to the "advanced" page. I think this will work, but could use proper translation/i18n formatting. This is near line 2581: <tr> <td width="15%">Auto:</td> <td> <select name='AUTO_EQUALS'> <option value='ignore' $selected{'AUTO_EQUALS'}{'ignore'}>ignore</option> <option value='add' $selected{'AUTO_EQUALS'}{'add'}>add</option> <option value='route' $selected{'AUTO_EQUALS'}{'route'}>route</option> <option value='start' $selected{'AUTO_EQUALS'}{'start'}>start</option> </select> </td> </tr> 2.) Adding a line similar to this in the block near line 1949: <input type='hidden' name='AUTO_EQUALS' value='$cgiparams{'AUTO_EQUALS'}' /> 3.) Adding confighash and cgiparams lines similar to these ones used for DPD at lines 1271, 1806, 2255, and 2282 (this is well beyond my skills): $cgiparams{'DPD_ACTION'} = $confighash{$cgiparams{'KEY'}}[27]; $confighash{$key}[27] = $cgiparams{'DPD_ACTION'}; 4.) Adding a block similar to this in the vicinity of line 2384: $selected{'AUTO_EQUALS'}{'ignore'} = ''; $selected{'AUTO_EQUALS'}{'add'} = ''; $selected{'AUTO_EQUALS'}{'route'} = ''; $selected{'AUTO_EQUALS'}{'start'} = ''; $selected{'AUTO_EQUALS'}{$cgiparams{'AUTO_EQUALS'}} = "selected='selected'"; I have no idea if this is helpful to anyone, but I thought I would try to pitch in in whatever small way I can. Please let me know if there is a better way to present this. I will endeavor to implement what I can and test over the holiday, but any pointers to help me out would be appreciated. I forgot one part: This section: # Automatically start only if a net-to-net connection if ($lconfighash{$key}[3] eq 'host') { print CONF "\tauto=add\n"; print CONF "\trightsourceip=$lvpnsettings{'RW_NET'}\n"; } else { print CONF "\tauto=route\n"; } Needs to be changed to this: # Automatically start only if a net-to-net connection if ($lconfighash{$key}[3] eq 'host') { print CONF "\tauto=add\n"; print CONF "\trightsourceip=$lvpnsettings{'RW_NET'}\n"; } else { my $autoequals = $lconfighash{$key}[33]; print CONF "\tauto=$autoequals\n"; } Also, I am guessing that these lines would work for #3, above (MOBIKE is the most recently added feature, and it is #32, so I used #33)? $cgiparams{'AUTO_EQUALS'} = $confighash{$cgiparams{'KEY'}}[33]; $confighash{$key}[33] = $cgiparams{'AUTO_EQUALS'}; OK, so my apologies for "thinking out loud" on the comments here, but I think it was helpful. I have mocked up a version of vpnmain.cgi that included the needed changes, as far as I can see. The one thing I noticed that was a problem was that existing tunnels have their configuration written out with no value for "auto=". If you edit the existing tunnel, its configuration will be properly updated. I thought that the entry on line 111 assigning a default value of 'start' to AUTO_EQUALS would avoid this problem, but apparently not. I am also utterly unaware of how these changes will affect a RoadWarrior tunnel, as I have never used one. I'm pretty sure that the config file will be unchanged, but it may be that the new control is visible on the Advanced page for a host-to-net tunnel. If that is the case, no matter what you select using the drop-down, the config will be written out with "auto-add". Hopefully this saves someone some work. Created attachment 391 [details]
Modified vpnmain.cgi (allows selection of auto parameter)
Created attachment 392 [details]
Proposed patch
In case this is preferred, I created a patch using the command:
" diff -Naur /srv/web/ipfire/cgi-bin/vpnmain.cgi.orig /srv/web/ipfire/cgi-bin/vpnmain.cgi > autoequalspatch.txt"
If this is not the correct way to do this, please let me know.
Created attachment 393 [details]
Fixed Proposed Patch
This is a fixed patch. I had accidentally created the original patch as compared to a Core 94 version of vpnmain.cgi by accident. Due to the many changes to vpnmain.cgi in Core 95, this was far from ideal.
I'm going to go home and stop polluting the bug tracker now.
Created attachment 394 [details]
Second Fixed Proposed Patch
Last Update, I promise. First Patch was run against the Core 94 file, the second one was run against a partially modified file. This version is properly run against the stock Core 95 file. I do not know if any changes were made to vpnmain.cgi for Core 96.
Are there any updates on this? My proposed patch needs a lot of polish (it defaults to "ignore" when you edit a tunnel originally set to some other setting, etc), but I think it gets most of the way there. I'd bring it all of the way, but I'm over my head when it comes to PHP. If anyone can lend a hand, I'd appreciate it. Still looking for an update on this. Using auto=route really does seem to improve stability for us, and a recent move to a new router meant we were running with the normal "auto=start", which meant that we were seeing severely reduced reliability and also multiple SAs for each connection only minutes after establishing a connection. I presume that this is due to both endpoints trying to establish a tunnel at the same time, but I cannot be certain. Either way, the current default is not recommended by the Strongswan team, and it really should be changed, especially as it results in a major reduction in stability for net-to-net tunnels. Allowing the user to choose his/her preferred setting on the advanced page would also be welcome. I know I made a mess of this comments thread above, and that my attempted patch is rank amateurism at its best, but anything that could be done to move this forward would be GREATLY appreciated, and I think a major improvement to the project!!!! Looking into this again, as I noticed today that the StrongSWAN team now considers auto=route to be the preferred setting for site-to-site tunnels for security reasons: "It is strongly advised to use auto=route in site-to-site setups to make sure that the kernel tells strongSwan to establish a CHILD_SA when there's no SA for a security policy. It is advisable to take a look at the strongswan.conf setting charon.ignore_acquire_ts when doing this." (from https://wiki.strongswan.org/projects/strongswan/wiki/SecurityRecommendations) Hey Tom, thank you for keeping the ball rolling - although I was so slow :) I took your patch and integrated it with a few modifications: http://git.ipfire.org/?p=ipfire-2.x.git;a=commitdiff;h=dcb406cc675c42f9add4a41c8a1e07eea7c3ab08 I tried to give the options better names for those people who don't know about the auto= parameter. I think it would have been nice to default to auto=route but I decided to be consistent and set the default to "Always On", i.e. auto=start. Regarding the security concerns: We have firewall rules in place which don't allow for those packets to be sent out the RED interface when they are not going through the VPN. We will keep those just to be sure. This is, indeed, still working well, and I'd say that this bug can be closed? Having said that, I still think that auto=route ought to be the default for new tunnels (let's not go changing existing tunnels!), as it is preferred by the StrongSwan team and results in much better reliability. AFAIK this has been fixed by now. |