ipsec: vti: T3831: avoid usinf xfrm if_id 0 - implement shift by one

The key defaults to 0 and will match any policies which similarly do not have
a lookup key configuration. This means that a vti0 named interface will pull in
all traffic and others will stop working. Thus we simply shift the key by one
to also support a vti0 interface.

3 files changed, 31 insertions, 10 deletions

diff --git a/data/templates/ipsec/swanctl/peer.tmpl b/data/templates/ipsec/swanctl/peer.tmpl
index 98c09436c..e039e98aa 100644
--- a/data/templates/ipsec/swanctl/peer.tmpl
+++ b/data/templates/ipsec/swanctl/peer.tmpl
@@ -61,8 +61,11 @@
                 local_ts = 0.0.0.0/0,::/0
                 remote_ts = 0.0.0.0/0,::/0
                 updown = "/etc/ipsec.d/vti-up-down {{ peer_conf.vti.bind }} {{ peer_conf.dhcp_interface if peer_conf.dhcp_interface is defined else 'no' }}"
-                if_id_in = {{ peer_conf.vti.bind | replace('vti', '') }}
-                if_id_out = {{ peer_conf.vti.bind | replace('vti', '') }}
+                {# The key defaults to 0 and will match any policies which similarly do not have a lookup key configuration. #}
+                {# Thus we simply shift the key by one to also support a vti0 interface #}
+{%              set if_id = peer_conf.vti.bind | replace('vti', '') | int +1 %}
+                if_id_in = {{ if_id }}
+                if_id_out = {{ if_id }}
                 ipcomp = {{ 'yes' if vti_esp.compression is defined and vti_esp.compression == 'enable' else 'no' }}
                 mode = {{ vti_esp.mode }}
 {%     if peer[0:1] == '@' %}
@@ -117,8 +120,11 @@
 {%       endif %}
 {%       if peer_conf.vti is defined and peer_conf.vti.bind is defined %}
                 updown = "/etc/ipsec.d/vti-up-down {{ peer_conf.vti.bind }} {{ peer_conf.dhcp_interface if peer_conf.dhcp_interface is defined else 'no' }}"
-                if_id_in = {{ peer_conf.vti.bind | replace('vti', '') }}
-                if_id_out = {{ peer_conf.vti.bind | replace('vti', '') }}
+                {# The key defaults to 0 and will match any policies which similarly do not have a lookup key configuration. #}
+                {# Thus we simply shift the key by one to also support a vti0 interface #}
+{%              set if_id = peer_conf.vti.bind | replace('vti', '') | int +1 %}
+                if_id_in = {{ if_id }}
+                if_id_out = {{ if_id }}
 {%       endif %}
             }
 {%       if tunnel_conf.passthrough is defined and tunnel_conf.passthrough %}
diff --git a/python/vyos/ifconfig/vti.py b/python/vyos/ifconfig/vti.py
index 470ebbff3..c50cd5ce9 100644
--- a/python/vyos/ifconfig/vti.py
+++ b/python/vyos/ifconfig/vti.py
@@ -35,8 +35,11 @@ class VTIIf(Interface):
         mapping = {
             'source_interface' : 'dev',
         }
-
         if_id = self.ifname.lstrip('vti')
+        # The key defaults to 0 and will match any policies which similarly do
+        # not have a lookup key configuration - thus we shift the key by one
+        # to also support a vti0 interface
+        if_id = str(int(if_id) +1)
         cmd = f'ip link add {self.ifname} type xfrm if_id {if_id}'
         for vyos_key, iproute2_key in mapping.items():
             # dict_search will return an empty dict "{}" for valueless nodes like
diff --git a/smoketest/scripts/cli/test_vpn_ipsec.py b/smoketest/scripts/cli/test_vpn_ipsec.py
index f33268083..71a9d5137 100755
--- a/smoketest/scripts/cli/test_vpn_ipsec.py
+++ b/smoketest/scripts/cli/test_vpn_ipsec.py
@@ -128,7 +128,6 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
         self.cli_delete(base_path)
         self.cli_delete(nhrp_path)
         self.cli_delete(tunnel_path)
-        self.cli_delete(vti_path)
         self.cli_delete(ethernet_path)
         self.cli_commit()
 
@@ -228,6 +227,11 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
         self.cli_commit()
 
         swanctl_conf = read_file(swanctl_file)
+        if_id = vti.lstrip('vti')
+        # The key defaults to 0 and will match any policies which similarly do
+        # not have a lookup key configuration - thus we shift the key by one
+        # to also support a vti0 interface
+        if_id = str(int(if_id) +1)
         swanctl_conf_lines = [
             f'version = 2',
             f'auth = psk',
@@ -238,8 +242,8 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
             f'mode = tunnel',
             f'local_ts = 172.16.10.0/24,172.16.11.0/24',
             f'remote_ts = 172.17.10.0/24,172.17.11.0/24',
-            f'if_id_in = {vti.lstrip("vti")}', # will be 10 for vti10
-            f'if_id_out = {vti.lstrip("vti")}',
+            f'if_id_in = {if_id}', # will be 11 for vti10 - shifted by one
+            f'if_id_out = {if_id}',
             f'updown = "/etc/ipsec.d/vti-up-down {vti} no"'
         ]
         for line in swanctl_conf_lines:
@@ -346,6 +350,11 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
 
         swanctl_conf = read_file(swanctl_file)
         tmp = peer_ip.replace('.', '-')
+        if_id = vti.lstrip('vti')
+        # The key defaults to 0 and will match any policies which similarly do
+        # not have a lookup key configuration - thus we shift the key by one
+        # to also support a vti0 interface
+        if_id = str(int(if_id) +1)
         swanctl_lines = [
             f'peer_{tmp}',
             f'version = 0', # key-exchange not set - defaulting to 0 for ikev1 and ikev2
@@ -362,8 +371,8 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
             f'local_ts = 0.0.0.0/0,::/0',
             f'remote_ts = 0.0.0.0/0,::/0',
             f'updown = "/etc/ipsec.d/vti-up-down {vti} no"',
-            f'if_id_in = {vti.lstrip("vti")}', # will be 10 for vti10
-            f'if_id_out = {vti.lstrip("vti")}',
+            f'if_id_in = {if_id}', # will be 11 for vti10
+            f'if_id_out = {if_id}',
             f'ipcomp = no',
             f'mode = tunnel',
             f'start_action = start',
@@ -378,5 +387,8 @@ class TestVPNIPsec(VyOSUnitTestSHIM.TestCase):
         for line in swanctl_secrets_lines:
             self.assertIn(line, swanctl_conf)
 
+        # There is only one VTI test so no need to delete this globally in tearDown()
+        self.cli_delete(vti_path)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)