ipsec: T7594: Rename respond connection-type in IPSec peer settings to trap

[alexandr-san4ez]

Change summary

This config item is a continuous source of confusion and misconfigurations, and it is understandable. While CLI says this:

vyos@vyos# set vpn ipsec site-to-site peer PEER_NAME connection-type 
Possible completions:
   initiate             Bring the connection up immediately
   respond              Wait for the peer to initiate the connection
   none                 Load the connection only

The respond actually does not do what is noted in the description:

vyos-1x/data/templates/ipsec/swanctl/peer.j2

Lines 96 to 97 in 22c6a81

	{% elif peer_conf.connection_type is vyos_defined('respond') %}
	start_action = trap

In a pair of IPSec peers, to avoid SA duplication, one must keep silent and the other attempt to connect. With the initiate / respond pair, one peer will actively try to initiate a connection, and another will be silent, but only if there is no traffic that matches a traffic selector for a peer. If such traffic occurs, it will try to connect as well.

The risk is relatively low, but if this happens, such peers may loop into an endless renegotiation process.

To avoid confusion, @zdc suggests:
1. Rename respond to trap and fix the description.
2. Migrate all respond to none, so the system does what is configured in the CLI.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://vyos.dev/T7594

Related PR(s)

• ipsec: T7594: Change connection-type to trap for all peers and configurations vyos-documentation#1719

How to test / Smoketest result

Step 1: configure site-to-site peer with connection type 'respond'

conf
set interfaces vti vti1
set protocols static route 172.168.202.0/24 interface vti1
set vpn ipsec site-to-site peer B vti bind 'vti1'
set vpn ipsec authentication psk psk1 id 'A'
set vpn ipsec authentication psk psk1 id 'B'
set vpn ipsec authentication psk psk1 secret 'AB'
set vpn ipsec esp-group esp1 mode 'tunnel'
set vpn ipsec esp-group esp1 pfs 'disable'
set vpn ipsec esp-group esp1 proposal 10 encryption 'aes256'
set vpn ipsec esp-group esp1 proposal 10 hash 'sha256'
set vpn ipsec ike-group ike1 close-action 'none'
set vpn ipsec ike-group ike1 dead-peer-detection action 'clear'
set vpn ipsec ike-group ike1 proposal 10 encryption 'camellia256ccm96'
set vpn ipsec ike-group ike1 proposal 10 hash 'sha256'
set vpn ipsec interface 'eth0'
set vpn ipsec site-to-site peer B authentication local-id 'A'
set vpn ipsec site-to-site peer B authentication mode 'pre-shared-secret'
set vpn ipsec site-to-site peer B authentication remote-id 'B'
set vpn ipsec site-to-site peer B connection-type 'respond'                 # <<<
set vpn ipsec site-to-site peer B default-esp-group 'esp1'
set vpn ipsec site-to-site peer B ike-group 'ike1'
set vpn ipsec site-to-site peer B local-address '172.168.99.2'
set vpn ipsec site-to-site peer B remote-address '172.168.99.3'
set vpn ipsec site-to-site peer B vti bind 'vti1'
commit

Step 2: upgrade the router on new version

vyos@vyos:~$ add system image /path/to/vyos-image.iso
...
vyos@vyos:~$ sudo reboot now

Step 3: verify configuration after reboot the router

vyos@vyos:~$ sh conf commands | match connection-type
set vpn ipsec site-to-site peer B connection-type 'none'

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[github-actions]

👍
No issues in PR Title / Commit Title

[sever-sever]

@zdc I'm confused
We rename respond => trap,
But migration commands will replace respond => None ?
Yes, it will do what it is expected to do, but how many configs can this migration affect?
Wouldn't it be better to migrate the old syntax so as not to affect the current IPsec peer logic?

[sarthurdev]

Change makes sense, passes smoketests. Not tested locally.

Just the one minor suggestion.

Original config file for smoketests were returned

[aapostoliuk]

I agree with @sever-sever. Changing behavior can lead to customers' problems. I think migration should be from respond to trap

[dmbaturin]

This is a complicated situation. According to @zdc and the support team, this will solve problems for more customers than it will break configs for, and there's no solution that will fix what's broken and not touch what's working. If that's the case, I think we have to go with that.

[dmbaturin]

@nicolas-fort is for making trap the new default, and I trust his judgement on that as the head of support.

[dmbaturin]

Let's got with @nicolas-fort's judgement and migrate to trap. We should discuss what the new default for fresh installations should be (@zdc has concerns about making trap the default), but that's a different story.

[github-actions]

CI integration 👍 passed!

Details

CI logs

• CLI Smoketests (no interfaces) 👍 passed
• CLI Smoketests VPP 👍 passed
• CLI Smoketests (interfaces only) 👍 passed
• Config tests 👍 passed
• Config tests VPP 👍 passed
• RAID1 tests 👍 passed
• TPM tests 👍 passed