From 9068e1a98e8d74484bc6cadc7b12944515e12191 Mon Sep 17 00:00:00 2001 From: Ruben Herold Date: Sat, 31 Oct 2026 08:21:49 +0100 Subject: ipsec: T9543: start a peer again on reset when connection-type is unset A site-to-site peer without connection-type is started at commit, because the router treats an unset value as initiate. The reset command only started a peer again when initiate was set explicitly in the configuration. After a reset such a peer was terminated and stayed down, including its VTI interface. Make initiate the default value of connection-type in the CLI definition, so every consumer sees the same value, and read the peer configuration with defaults in the reset command. A peer that is disabled, whose name starts with an at sign or whose remote address is any is not started at commit, so reset keeps skipping it instead of reporting a failed initiation. --- smoketest/scripts/cli/test_vpn_ipsec.py | 90 +++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_vpn_ipsec.py b/smoketest/scripts/cli/test_vpn_ipsec.py index 744486022..93cb085f9 100755 --- a/smoketest/scripts/cli/test_vpn_ipsec.py +++ b/smoketest/scripts/cli/test_vpn_ipsec.py @@ -24,6 +24,7 @@ from vyos.configsession import ConfigSessionError from vyos.ifconfig import Interface from vyos.utils.convert import encode_to_base64 from vyos.utils.process import process_named_running +from vyos.utils.process import rc_cmd from vyos.utils.file import read_file from vyos.xml_ref import default_value @@ -884,6 +885,95 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase): swanctl_conf = read_file(swanctl_file) self.assertIn(f'unique = never', swanctl_conf) + def test_site_to_site_reset_without_connection_type(self): + # T9543: a peer without 'connection-type' is started at commit, so + # 'reset vpn ipsec site-to-site peer' has to start it again as well. + local_address = '192.0.2.10' + + self.assertEqual( + default_value( + base_path + ['site-to-site', 'peer', connection_name, 'connection-type'] + ), + 'initiate', + ) + + psk_base_path = base_path + ['authentication', 'psk', connection_name] + self.cli_set(psk_base_path + ['id', local_address]) + self.cli_set(psk_base_path + ['id', peer_ip]) + self.cli_set(psk_base_path + ['secret', secret]) + + peer_base_path = base_path + ['site-to-site', 'peer', connection_name] + self.cli_set(peer_base_path + ['authentication', 'mode', 'pre-shared-secret']) + self.cli_set(peer_base_path + ['authentication', 'local-id', local_address]) + self.cli_set(peer_base_path + ['authentication', 'remote-id', peer_ip]) + self.cli_set(peer_base_path + ['ike-group', ike_group]) + self.cli_set(peer_base_path + ['default-esp-group', esp_group]) + self.cli_set(peer_base_path + ['local-address', local_address]) + self.cli_set(peer_base_path + ['remote-address', peer_ip]) + + tunnel_base_path = peer_base_path + ['tunnel', '1'] + self.cli_set(tunnel_base_path + ['local', 'prefix', '172.16.10.0/24']) + self.cli_set(tunnel_base_path + ['remote', 'prefix', '172.17.10.0/24']) + self.cli_commit() + + # the peer is not reachable, the start at commit leaves an IKE SA + # that is still connecting + def ike_sa_id(): + _, out = rc_cmd('sudo swanctl --list-sas') + found = re.search(rf'^{connection_name}: #(\d+),', out, re.MULTILINE) + return found.group(1) if found else None + + up, first_id = self.wait_for_result(ike_sa_id, lambda x: x is not None) + self.assertTrue(up, 'peer without connection-type was not started') + + out = self.op_mode( + ['reset', 'vpn', 'ipsec', 'site-to-site', 'peer', connection_name] + ) + self.assertIn('initiate result: success', out) + + up, second_id = self.wait_for_result( + ike_sa_id, lambda x: x is not None and x != first_id + ) + self.assertTrue( + up, 'peer without connection-type was not started again by reset' + ) + + # a peer that is disabled, named '@...' or without a remote address is + # not started at commit, reset must not try to start it either + not_started_peers = { + '@dynamic': ('any', False), + 'responder': ('any', False), + 'disabled': (peer_ip, True), + } + for index, (name, (remote, disabled)) in enumerate(not_started_peers.items()): + not_started_path = base_path + ['site-to-site', 'peer', name] + self.cli_set( + not_started_path + ['authentication', 'mode', 'pre-shared-secret'] + ) + self.cli_set( + not_started_path + ['authentication', 'local-id', local_address] + ) + self.cli_set(not_started_path + ['ike-group', ike_group]) + self.cli_set(not_started_path + ['default-esp-group', esp_group]) + self.cli_set(not_started_path + ['local-address', local_address]) + self.cli_set(not_started_path + ['remote-address', remote]) + self.cli_set( + not_started_path + + ['tunnel', '1', 'local', 'prefix', f'172.16.{20 + index}.0/24'] + ) + self.cli_set( + not_started_path + + ['tunnel', '1', 'remote', 'prefix', f'172.17.{20 + index}.0/24'] + ) + if disabled: + self.cli_set(not_started_path + ['disable']) + self.cli_commit() + + for name in not_started_peers: + out = self.op_mode(['reset', 'vpn', 'ipsec', 'site-to-site', 'peer', name]) + self.assertNotIn('Initiation was failed', out) + self.assertNotIn('Failed to initiate', out) + def test_dmvpn(self): ike_lifetime = '3600' esp_lifetime = '1800' -- cgit v1.2.3 From a55cf4670f02a14a8582669abacb4fdeb01c7a62 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 31 Oct 2026 12:00:00 +0100 Subject: ipsec: T9543: drop checks for an unset connection-type Since connection-type defaults to initiate, it is always present in the peer configuration. The swanctl template and the reset command still handled an unset value as a separate case, which can no longer happen. Compare against the value directly instead. The reset smoketest only checked for error messages that cannot appear, because a failed reset already aborts the test. It now verifies that reset does not start a peer that is not started at commit. --- data/templates/ipsec/swanctl/peer.j2 | 6 +++--- smoketest/scripts/cli/test_vpn_ipsec.py | 3 +-- src/op_mode/ipsec.py | 7 +++---- 3 files changed, 7 insertions(+), 9 deletions(-) (limited to 'smoketest/scripts/cli') diff --git a/data/templates/ipsec/swanctl/peer.j2 b/data/templates/ipsec/swanctl/peer.j2 index d0a51d81a..f38291866 100644 --- a/data/templates/ipsec/swanctl/peer.j2 +++ b/data/templates/ipsec/swanctl/peer.j2 @@ -41,7 +41,7 @@ {% if peer[0:1] == '@' %} keyingtries = 0 reauth_time = 0 -{% elif peer_conf.connection_type is not vyos_defined or peer_conf.connection_type is vyos_defined('initiate') %} +{% elif peer_conf.connection_type is vyos_defined('initiate') %} keyingtries = 0 {% if reauth and ike.key_exchange is vyos_defined('ikev2') %} reauth_time = {{ ike.lifetime }}s @@ -115,7 +115,7 @@ mode = {{ vti_esp.mode }} {% if peer[0:1] == '@' %} start_action = none -{% elif peer_conf.connection_type is not vyos_defined or peer_conf.connection_type is vyos_defined('initiate') %} +{% elif peer_conf.connection_type is vyos_defined('initiate') %} start_action = start {% elif peer_conf.connection_type is vyos_defined('trap') %} start_action = trap @@ -179,7 +179,7 @@ mode = {{ tunnel_esp.mode }} {% if peer[0:1] == '@' %} start_action = none -{% elif peer_conf.connection_type is not vyos_defined or peer_conf.connection_type is vyos_defined('initiate') %} +{% elif peer_conf.connection_type is vyos_defined('initiate') %} start_action = start {% elif peer_conf.connection_type is vyos_defined('trap') %} start_action = trap diff --git a/smoketest/scripts/cli/test_vpn_ipsec.py b/smoketest/scripts/cli/test_vpn_ipsec.py index 93cb085f9..80d9c25c9 100755 --- a/smoketest/scripts/cli/test_vpn_ipsec.py +++ b/smoketest/scripts/cli/test_vpn_ipsec.py @@ -971,8 +971,7 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase): for name in not_started_peers: out = self.op_mode(['reset', 'vpn', 'ipsec', 'site-to-site', 'peer', name]) - self.assertNotIn('Initiation was failed', out) - self.assertNotIn('Failed to initiate', out) + self.assertNotIn('initiate result', out) def test_dmvpn(self): ike_lifetime = '3600' diff --git a/src/op_mode/ipsec.py b/src/op_mode/ipsec.py index 65bfce1bf..43f3e25d8 100755 --- a/src/op_mode/ipsec.py +++ b/src/op_mode/ipsec.py @@ -668,13 +668,12 @@ def reset_peer(peer: str, tunnel: typing.Optional[str] = None) -> None: peer_config = _get_sitetosite_peer_config(peer) # initiate SAs only if 'connection-type=initiate', as at commit a disabled # peer, a peer named '@...' or without a remote address is not started - remote_address = peer_config.get('remote_address', []) if ( - peer_config.get('connection_type') == 'initiate' + peer_config['connection_type'] == 'initiate' and 'disable' not in peer_config and peer[0:1] != '@' - and remote_address - and 'any' not in remote_address + and 'remote_address' in peer_config + and 'any' not in peer_config['remote_address'] ): _initiate_peer_with_childsas(peer, tunnel) -- cgit v1.2.3 From 5d352d7902019469d2f87aa8fc398af2de213dee Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 31 Oct 2026 14:44:57 +0100 Subject: smoketest: T9543: assign the local address in the ipsec reset test The reset test configured its peers with a local address that was not present on any interface, so starting the peer at commit relied on how charon handles a source address the router does not own. Assign the address to the interface IPsec is bound to and remove it again at the end of the test. --- smoketest/scripts/cli/test_vpn_ipsec.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'smoketest/scripts/cli') diff --git a/smoketest/scripts/cli/test_vpn_ipsec.py b/smoketest/scripts/cli/test_vpn_ipsec.py index 80d9c25c9..6672690b3 100755 --- a/smoketest/scripts/cli/test_vpn_ipsec.py +++ b/smoketest/scripts/cli/test_vpn_ipsec.py @@ -897,6 +897,11 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase): 'initiate', ) + # IPsec is bound to this interface in setUpClass() + self.cli_set( + ethernet_path + [interface, 'vif', vif, 'address', f'{local_address}/24'] + ) + psk_base_path = base_path + ['authentication', 'psk', connection_name] self.cli_set(psk_base_path + ['id', local_address]) self.cli_set(psk_base_path + ['id', peer_ip]) @@ -973,6 +978,8 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase): out = self.op_mode(['reset', 'vpn', 'ipsec', 'site-to-site', 'peer', name]) self.assertNotIn('initiate result', out) + self.cli_delete(ethernet_path + [interface, 'vif', vif, 'address']) + def test_dmvpn(self): ike_lifetime = '3600' esp_lifetime = '1800' -- cgit v1.2.3