From 712c1520f8ebfecf3bc35d9aca21d73d7188f0c2 Mon Sep 17 00:00:00 2001 From: rockfish-vyos <321823801+rockfish-vyos@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:57:40 +0000 Subject: ipsec: T9254: do not emit ESN transforms in IKE proposals get_esp_ike_cipher() renders both IKE_SA proposals and CHILD_SA proposals. It unconditionally appended the -esn / -noesn variants, which are only meaningful for ESP: ESN is Transform Type 5 per RFC 7296 section 3.3.2 and is defined for CHILD_SAs only. Emitting ESN in an IKE_SA proposal breaks interoperability with implementations that reject the malformed payload without replying at all. Observed against a Cisco FTD: the IKE_SA_INIT is silently discarded, no NO_PROPOSAL_CHOSEN and no INVALID_KE_PAYLOAD, so the tunnel never comes up and nothing on the initiator side explains it. Add an explicit esn parameter defaulting to True so ESP behaviour is unchanged, and pass esn=False at the four IKE call sites: peer.j2, profile.j2, remote_access.j2 and l2tp.j2. The five esp_proposals call sites keep ESN. ike_group could have served as the discriminator, since IKE call sites do not pass it, but that coupling is incidental: the argument only exists to resolve PFS. Smoke test assertions on the proposals lines are updated accordingly; the esp_proposals assertions are left untouched. --- python/vyos/template.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'python') diff --git a/python/vyos/template.py b/python/vyos/template.py index dbbc4df84..1233710f2 100755 --- a/python/vyos/template.py +++ b/python/vyos/template.py @@ -483,7 +483,16 @@ def get_first_ike_dh_group(ike_group): return 'dh-group2' # Fallback on dh-group2 @register_filter('get_esp_ike_cipher') -def get_esp_ike_cipher(group_config, ike_group=None): +def get_esp_ike_cipher(group_config, ike_group=None, esn=True): + """Render strongSwan proposal strings. + + esn=True : ESP/CHILD_SA proposals, where ESN transforms are meaningful + esn=False : IKE_SA proposals. ESN is a CHILD_SA transform (RFC 7296 + section 3.3.2, Transform Type 5) and has no meaning in an + IKE_SA proposal. Emitting it there breaks interoperability + with implementations that reject the malformed payload + without replying at all (observed with Cisco FTD, T9254). + """ pfs_lut = { 'dh-group1' : 'modp768', 'dh-group2' : 'modp1024', @@ -530,10 +539,12 @@ def get_esp_ike_cipher(group_config, ike_group=None): group = get_first_ike_dh_group(ike_group) tmp += '-' + pfs_lut[group] - # For 'optional' and 'disabled' we need two values as - # proposal without '-esn'/'-noesn' is incompatible with - # proposals with any of them. - if 'esn' in proposal: + # ESP/CHILD_SA only. For 'optional' and 'disabled' we need two + # values as a proposal without '-esn'/'-noesn' is incompatible + # with proposals carrying any of them. This pairing is meaningless + # for an IKE_SA, which has no ESN transform at all - see the esn + # parameter above. + if esn and 'esn' in proposal: if proposal['esn'] == 'required': tmp += '-esn' elif proposal['esn'] == 'optional': -- cgit v1.2.3 From 0db7521a96f3d314ec4914c72932cfbcced2dc98 Mon Sep 17 00:00:00 2001 From: rockfish-vyos <321823801+rockfish-vyos@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:17:41 +0000 Subject: ipsec: T9254: split get_esp_ike_cipher() into get_esp_cipher() / get_ike_cipher() get_esp_ike_cipher() took an esn flag defaulting to True (ESP/CHILD_SA semantics), and every IKE_SA call site had to remember to pass esn=False. Nothing enforced that, so a future call site could reintroduce the ESN-in-IKE bug this task fixed. Following review feedback from hedrok, split it into two public filters: get_esp_cipher(group_config, ike_group=None) ESP/CHILD_SA, esn=True get_ike_cipher(group_config) IKE_SA, esn=False always The shared implementation moves to _get_esp_ike_cipher() (no register_filter), so a template can no longer call it directly and skip picking one of the two entry points. Updated call sites: peer.j2, profile.j2, remote_access.j2, l2tp.j2 (proposals -> get_ike_cipher, esp_proposals -> get_esp_cipher), and test_template.py. --- data/templates/ipsec/swanctl/l2tp.j2 | 4 ++-- data/templates/ipsec/swanctl/peer.j2 | 6 +++--- data/templates/ipsec/swanctl/profile.j2 | 4 ++-- data/templates/ipsec/swanctl/remote_access.j2 | 4 ++-- python/vyos/template.py | 24 ++++++++++++++++++++++-- src/tests/test_template.py | 4 ++-- 6 files changed, 33 insertions(+), 13 deletions(-) (limited to 'python') diff --git a/data/templates/ipsec/swanctl/l2tp.j2 b/data/templates/ipsec/swanctl/l2tp.j2 index da3ff40d5..6f7145ca2 100644 --- a/data/templates/ipsec/swanctl/l2tp.j2 +++ b/data/templates/ipsec/swanctl/l2tp.j2 @@ -2,7 +2,7 @@ {% set l2tp_ike = ike_group[l2tp.ike_group] if l2tp.ike_group is vyos_defined else None %} {% set l2tp_esp = esp_group[l2tp.esp_group] if l2tp.esp_group is vyos_defined else None %} l2tp_remote_access { - proposals = {{ l2tp_ike | get_esp_ike_cipher(esn=False) | join(',') if l2tp_ike else l2tp_ike_default }} + proposals = {{ l2tp_ike | get_ike_cipher | join(',') if l2tp_ike else l2tp_ike_default }} local_addrs = {{ l2tp_outside_address }} dpd_delay = 15s dpd_timeout = 45s @@ -23,7 +23,7 @@ children { l2tp_remote_access_esp { mode = transport - esp_proposals = {{ l2tp_esp | get_esp_ike_cipher(l2tp_ike) | join(',') if l2tp_esp else l2tp_esp_default }} + esp_proposals = {{ l2tp_esp | get_esp_cipher(l2tp_ike) | join(',') if l2tp_esp else l2tp_esp_default }} life_time = {{ l2tp_esp.lifetime if l2tp_esp else l2tp.lifetime }}s local_ts = dynamic[/1701] remote_ts = dynamic diff --git a/data/templates/ipsec/swanctl/peer.j2 b/data/templates/ipsec/swanctl/peer.j2 index ee2efcc50..7ce2111c4 100644 --- a/data/templates/ipsec/swanctl/peer.j2 +++ b/data/templates/ipsec/swanctl/peer.j2 @@ -19,7 +19,7 @@ {% if peer_conf.childless is vyos_defined %} childless = {{ peer_conf.childless }} {% endif %} - proposals = {{ ike | get_esp_ike_cipher(esn=False) | join(',') }} + proposals = {{ ike | get_ike_cipher | join(',') }} version = {{ ike.key_exchange[4:] if ike.key_exchange is vyos_defined else "0" }} {% if peer_conf.virtual_address is vyos_defined %} vips = {{ peer_conf.virtual_address | join(', ') }} @@ -77,7 +77,7 @@ {% if peer_conf.vti.bind is vyos_defined and peer_conf.tunnel is not vyos_defined %} {% set vti_esp = esp_group[ peer_conf.vti.esp_group ] if peer_conf.vti.esp_group is vyos_defined else esp_group[ peer_conf.default_esp_group ] %} {{ name }}-vti { - esp_proposals = {{ vti_esp | get_esp_ike_cipher(ike) | join(',') }} + esp_proposals = {{ vti_esp | get_esp_cipher(ike) | join(',') }} {% if vti_esp.life_bytes is vyos_defined %} life_bytes = {{ vti_esp.life_bytes }} {% endif %} @@ -144,7 +144,7 @@ {% set remote_port = tunnel_conf.remote.port if tunnel_conf.remote.port is vyos_defined else '' %} {% set remote_suffix = '[{0}/{1}]'.format(proto, remote_port) if proto or remote_port else '' %} {{ name }}-tunnel-{{ tunnel_id }} { - esp_proposals = {{ tunnel_esp | get_esp_ike_cipher(ike) | join(',') }} + esp_proposals = {{ tunnel_esp | get_esp_cipher(ike) | join(',') }} {% if tunnel_esp.life_bytes is vyos_defined %} life_bytes = {{ tunnel_esp.life_bytes }} {% endif %} diff --git a/data/templates/ipsec/swanctl/profile.j2 b/data/templates/ipsec/swanctl/profile.j2 index 1aec635fd..187429a26 100644 --- a/data/templates/ipsec/swanctl/profile.j2 +++ b/data/templates/ipsec/swanctl/profile.j2 @@ -5,7 +5,7 @@ {% if profile_conf.bind.tunnel is vyos_defined %} {% for interface in profile_conf.bind.tunnel %} dmvpn-{{ name }}-{{ interface }} { - proposals = {{ ike_group[profile_conf.ike_group] | get_esp_ike_cipher(esn=False) | join(',') }} + proposals = {{ ike_group[profile_conf.ike_group] | get_ike_cipher | join(',') }} version = {{ ike.key_exchange[4:] if ike.key_exchange is vyos_defined else "0" }} rekey_time = {{ ike.lifetime }}s keyingtries = 0 @@ -26,7 +26,7 @@ {% endif %} children { dmvpn-{{ name }}-{{ interface }}-child { - esp_proposals = {{ esp | get_esp_ike_cipher(ike) | join(',') }} + esp_proposals = {{ esp | get_esp_cipher(ike) | join(',') }} rekey_time = {{ esp.lifetime }}s rand_time = 540s local_ts = dynamic[gre] diff --git a/data/templates/ipsec/swanctl/remote_access.j2 b/data/templates/ipsec/swanctl/remote_access.j2 index 7d89ced8b..b39885bf1 100644 --- a/data/templates/ipsec/swanctl/remote_access.j2 +++ b/data/templates/ipsec/swanctl/remote_access.j2 @@ -14,7 +14,7 @@ {% endif %} remote_addrs = %any local_addrs = {{ rw_conf.local_address if rw_conf.local_address is not vyos_defined('any') else '%any' }} # dhcp:{{ rw_conf.dhcp_interface if rw_conf.dhcp_interface is vyos_defined else 'no' }} - proposals = {{ ike_group[rw_conf.ike_group] | get_esp_ike_cipher(esn=False) | join(',') }} + proposals = {{ ike_group[rw_conf.ike_group] | get_ike_cipher | join(',') }} version = {{ ike.key_exchange[4:] if ike.key_exchange is vyos_defined else "0" }} send_certreq = no {% if rw_conf.authentication.always_send_cert is vyos_defined %} @@ -59,7 +59,7 @@ } children { {{ name }}-client { - esp_proposals = {{ esp | get_esp_ike_cipher(ike) | join(',') }} + esp_proposals = {{ esp | get_esp_cipher(ike) | join(',') }} {% if esp.life_bytes is vyos_defined %} life_bytes = {{ esp.life_bytes }} {% endif %} diff --git a/python/vyos/template.py b/python/vyos/template.py index 1233710f2..8d861096b 100755 --- a/python/vyos/template.py +++ b/python/vyos/template.py @@ -482,8 +482,7 @@ def get_first_ike_dh_group(ike_group): return 'dh-group' + proposal['dh_group'] return 'dh-group2' # Fallback on dh-group2 -@register_filter('get_esp_ike_cipher') -def get_esp_ike_cipher(group_config, ike_group=None, esn=True): +def _get_esp_ike_cipher(group_config, ike_group=None, esn=True): """Render strongSwan proposal strings. esn=True : ESP/CHILD_SA proposals, where ESN transforms are meaningful @@ -492,6 +491,10 @@ def get_esp_ike_cipher(group_config, ike_group=None, esn=True): IKE_SA proposal. Emitting it there breaks interoperability with implementations that reject the malformed payload without replying at all (observed with Cisco FTD, T9254). + + Not registered as a filter directly: callers must go through + get_esp_cipher() or get_ike_cipher() so esn can't be left at its + default where an IKE cipher is needed. """ pfs_lut = { 'dh-group1' : 'modp768', @@ -555,6 +558,23 @@ def get_esp_ike_cipher(group_config, ike_group=None, esn=True): ciphers.append(tmp) return ciphers + +@register_filter('get_esp_cipher') +def get_esp_cipher(group_config, ike_group=None): + """ESP/CHILD_SA proposals, where ESN transforms are meaningful.""" + return _get_esp_ike_cipher(group_config, ike_group=ike_group, esn=True) + + +@register_filter('get_ike_cipher') +def get_ike_cipher(group_config): + """IKE_SA proposals. ESN is a CHILD_SA transform (RFC 7296 section + 3.3.2, Transform Type 5) and has no meaning in an IKE_SA proposal. + Emitting it there breaks interoperability with implementations that + reject the malformed payload without replying at all (observed with + Cisco FTD, T9254). + """ + return _get_esp_ike_cipher(group_config, esn=False) + @register_filter('get_uuid') def get_uuid(seed): """ Get interface IP addresses""" diff --git a/src/tests/test_template.py b/src/tests/test_template.py index e2548602d..7ac396b68 100644 --- a/src/tests/test_template.py +++ b/src/tests/test_template.py @@ -184,11 +184,11 @@ class TestVyOSTemplate(TestCase): } for group_name, group_config in data['esp_group'].items(): - ciphers = vyos.template.get_esp_ike_cipher(group_config) + ciphers = vyos.template.get_esp_cipher(group_config) self.assertIn(ESP_DEFAULT, ','.join(ciphers)) for group_name, group_config in data['ike_group'].items(): - ciphers = vyos.template.get_esp_ike_cipher(group_config) + ciphers = vyos.template.get_ike_cipher(group_config) self.assertIn(IKEv2_DEFAULT, ','.join(ciphers)) def test_get_default_port(self): -- cgit v1.2.3