tunnel: T4267: "parameters ip key" on GRE not required for different remotes

2 files changed, 139 insertions, 41 deletions

diff --git a/smoketest/scripts/cli/test_interfaces_tunnel.py b/smoketest/scripts/cli/test_interfaces_tunnel.py
index fc2e254d6..b2c045b56 100755
--- a/smoketest/scripts/cli/test_interfaces_tunnel.py
+++ b/smoketest/scripts/cli/test_interfaces_tunnel.py
@@ -44,14 +44,14 @@ class TunnelInterfaceTest(BasicInterfaceTest.TestCase):
         # call base-classes classmethod
         super(cls, cls).setUpClass()
 
-    def setUp(self):
-        super().setUp()
-        self.cli_set(['interfaces', 'dummy', source_if, 'address', self.local_v4 + '/32'])
-        self.cli_set(['interfaces', 'dummy', source_if, 'address', self.local_v6 + '/128'])
+        # create some test interfaces
+        cls.cli_set(cls, ['interfaces', 'dummy', source_if, 'address', cls.local_v4 + '/32'])
+        cls.cli_set(cls, ['interfaces', 'dummy', source_if, 'address', cls.local_v6 + '/128'])
 
-    def tearDown(self):
-        self.cli_delete(['interfaces', 'dummy', source_if])
-        super().tearDown()
+    @classmethod
+    def tearDownClass(cls):
+        cls.cli_delete(cls, ['interfaces', 'dummy', source_if])
+        super().tearDownClass()
 
     def test_ipv4_encapsulations(self):
         # When running tests ensure that for certain encapsulation types the
@@ -312,5 +312,89 @@ class TunnelInterfaceTest(BasicInterfaceTest.TestCase):
         conf = get_interface_config(interface)
         self.assertEqual(new_remote,    conf['linkinfo']['info_data']['remote'])
 
+    def test_tunnel_src_any_gre_key(self):
+        interface = f'tun1280'
+        encapsulation = 'gre'
+        src_addr = '0.0.0.0'
+        key = '127'
+
+        self.cli_set(self._base_path + [interface, 'encapsulation', encapsulation])
+        self.cli_set(self._base_path + [interface, 'source-address', src_addr])
+        # GRE key must be supplied with a 0.0.0.0 source address
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_set(self._base_path + [interface, 'parameters', 'ip', 'key', key])
+
+        self.cli_commit()
+
+    def test_multiple_gre_tunnel_same_remote(self):
+        tunnels = {
+            'tun10' : {
+                'encapsulation' : 'gre',
+                'source_interface' : source_if,
+                'remote' : '1.2.3.4',
+            },
+            'tun20' : {
+                'encapsulation' : 'gre',
+                'source_interface' : source_if,
+                'remote' : '1.2.3.4',
+            },
+        }
+
+        for tunnel, tunnel_config in tunnels.items():
+            self.cli_set(self._base_path + [tunnel, 'encapsulation', tunnel_config['encapsulation']])
+            if 'source_interface' in tunnel_config:
+                self.cli_set(self._base_path + [tunnel, 'source-interface', tunnel_config['source_interface']])
+            if 'remote' in tunnel_config:
+                self.cli_set(self._base_path + [tunnel, 'remote', tunnel_config['remote']])
+
+        # GRE key must be supplied when two or more tunnels are formed to the same desitnation
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        for tunnel, tunnel_config in tunnels.items():
+            self.cli_set(self._base_path + [tunnel, 'parameters', 'ip', 'key', tunnel.lstrip('tun')])
+
+        self.cli_commit()
+
+        for tunnel, tunnel_config in tunnels.items():
+            conf = get_interface_config(tunnel)
+            ip_key = tunnel.lstrip('tun')
+
+            self.assertEqual(tunnel_config['source_interface'], conf['link'])
+            self.assertEqual(tunnel_config['encapsulation'],    conf['linkinfo']['info_kind'])
+            self.assertEqual(tunnel_config['remote'],           conf['linkinfo']['info_data']['remote'])
+            self.assertEqual(f'0.0.0.{ip_key}',                 conf['linkinfo']['info_data']['ikey'])
+            self.assertEqual(f'0.0.0.{ip_key}',                 conf['linkinfo']['info_data']['okey'])
+
+    def test_multiple_gre_tunnel_different_remote(self):
+        tunnels = {
+            'tun10' : {
+                'encapsulation' : 'gre',
+                'source_interface' : source_if,
+                'remote' : '1.2.3.4',
+            },
+            'tun20' : {
+                'encapsulation' : 'gre',
+                'source_interface' : source_if,
+                'remote' : '1.2.3.5',
+            },
+        }
+
+        for tunnel, tunnel_config in tunnels.items():
+            self.cli_set(self._base_path + [tunnel, 'encapsulation', tunnel_config['encapsulation']])
+            if 'source_interface' in tunnel_config:
+                self.cli_set(self._base_path + [tunnel, 'source-interface', tunnel_config['source_interface']])
+            if 'remote' in tunnel_config:
+                self.cli_set(self._base_path + [tunnel, 'remote', tunnel_config['remote']])
+
+        self.cli_commit()
+
+        for tunnel, tunnel_config in tunnels.items():
+            conf = get_interface_config(tunnel)
+
+            self.assertEqual(tunnel_config['source_interface'], conf['link'])
+            self.assertEqual(tunnel_config['encapsulation'],    conf['linkinfo']['info_kind'])
+            self.assertEqual(tunnel_config['remote'],           conf['linkinfo']['info_data']['remote'])
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py
index 4c1204b4e..433764b8a 100755
--- a/src/conf_mode/interfaces-tunnel.py
+++ b/src/conf_mode/interfaces-tunnel.py
@@ -18,7 +18,6 @@ import os
 
 from sys import exit
 from netifaces import interfaces
-from ipaddress import IPv4Address
 
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
@@ -50,8 +49,24 @@ def get_config(config=None):
     base = ['interfaces', 'tunnel']
     tunnel = get_interface_dict(conf, base)
 
-    tmp = leaf_node_changed(conf, ['encapsulation'])
-    if tmp: tunnel.update({'encapsulation_changed': {}})
+    if 'deleted' not in tunnel:
+        tmp = leaf_node_changed(conf, ['encapsulation'])
+        if tmp: tunnel.update({'encapsulation_changed': {}})
+
+        # We also need to inspect other configured tunnels as there are Kernel
+        # restrictions where we need to comply. E.g. GRE tunnel key can't be used
+        # twice, or with multiple GRE tunnels to the same location we must specify
+        # a GRE key
+        conf.set_level(base)
+        tunnel['other_tunnels'] = conf.get_config_dict([], key_mangling=('-', '_'),
+                                                      get_first_key=True,
+                                                      no_tag_node_value_mangle=True)
+        # delete our own instance from this dict
+        ifname = tunnel['ifname']
+        del tunnel['other_tunnels'][ifname]
+        # if only one tunnel is present on the system, no need to keep this key
+        if len(tunnel['other_tunnels']) == 0:
+            del tunnel['other_tunnels']
 
     # We must check if our interface is configured to be a DMVPN member
     nhrp_base = ['protocols', 'nhrp', 'tunnel']
@@ -92,48 +107,47 @@ def verify(tunnel):
             if 'direction' not in tunnel['parameters']['erspan']:
                 raise ConfigError('ERSPAN version 2 requires direction to be set!')
 
-    # If tunnel source address any and key not set
+    # If tunnel source is any and gre key is not set
+    interface = tunnel['ifname']
     if tunnel['encapsulation'] in ['gre'] and \
        dict_search('source_address', tunnel) == '0.0.0.0' and \
        dict_search('parameters.ip.key', tunnel) == None:
-        raise ConfigError('Tunnel parameters ip key must be set!')
+        raise ConfigError(f'"parameters ip key" must be set for {interface} when '\
+                           'encapsulation is GRE!')
 
-    if tunnel['encapsulation'] in ['gre', 'gretap']:
+    gre_encapsulations = ['gre', 'gretap']
+    if tunnel['encapsulation'] in gre_encapsulations and 'other_tunnels' in tunnel:
         # Check pairs tunnel source-address/encapsulation/key with exists tunnels.
         # Prevent the same key for 2 tunnels with same source-address/encap. T2920
-        for tunnel_if in Section.interfaces('tunnel'):
-            # It makes no sense to run the test against our own interface we
-            # are currently configuring
-            if tunnel['ifname'] == tunnel_if:
-                continue
-
-            tunnel_cfg = get_interface_config(tunnel_if)
+        for o_tunnel, o_tunnel_conf in tunnel['other_tunnels'].items():
             # no match on encapsulation - bail out
-            if dict_search('linkinfo.info_kind', tunnel_cfg) != tunnel['encapsulation']:
+            our_encapsulation = tunnel['encapsulation']
+            their_encapsulation = o_tunnel_conf['encapsulation']
+            if our_encapsulation in gre_encapsulations and their_encapsulation \
+                not in gre_encapsulations:
                 continue
 
-            new_source_address = dict_search('source_address', tunnel)
-            new_source_interface = dict_search('source_interface', tunnel)
-            if dict_search('parameters.ip.key', tunnel) != None:
-                # Convert tunnel key to ip key, format "ip -j link show"
-                # 1 => 0.0.0.1, 999 => 0.0.3.231
-                orig_new_key = dict_search('parameters.ip.key', tunnel)
-                new_key = IPv4Address(int(orig_new_key))
-                new_key = str(new_key)
-                if dict_search('address', tunnel_cfg) == new_source_address and \
-                   dict_search('linkinfo.info_data.ikey', tunnel_cfg) == new_key:
-                    raise ConfigError(f'Key "{orig_new_key}" for source-address "{new_source_address}" ' \
+            our_address = dict_search('source_address', tunnel)
+            our_key = dict_search('parameters.ip.key', tunnel)
+            their_address = dict_search('source_address', o_tunnel_conf)
+            their_key = dict_search('parameters.ip.key', o_tunnel_conf)
+            if our_key != None:
+                if their_address == our_address and their_key == our_key:
+                    raise ConfigError(f'Key "{our_key}" for source-address "{our_address}" ' \
                                       f'is already used for tunnel "{tunnel_if}"!')
             else:
-                # If no IP GRE key is used we can not have more then one GRE tunnel
-                # bound to any one interface/IP address. This will result in a OS
-                # PermissionError: add tunnel "gre0" failed: File exists
-                if (dict_search('address', tunnel_cfg) == new_source_address or
-                   (dict_search('address', tunnel_cfg) == '0.0.0.0' and
-                    dict_search('link', tunnel_cfg) == new_source_interface)):
-                    raise ConfigError(f'Missing required "ip key" parameter when \
-                                      running more then one GRE based tunnel on the \
-                                      same source-interface/source-address')
+                our_source_if = dict_search('source_interface', tunnel)
+                their_source_if = dict_search('source_interface', o_tunnel_conf)
+                our_remote = dict_search('remote', tunnel)
+                their_remote = dict_search('remote', o_tunnel_conf)
+                # If no IP GRE key is defined we can not have more then one GRE tunnel
+                # bound to any one interface/IP address and the same remote. This will
+                # result in a OS  PermissionError: add tunnel "gre0" failed: File exists
+                if (their_address == our_address or our_source_if == their_source_if) and \
+                    our_remote == their_remote:
+                    raise ConfigError(f'Missing required "ip key" parameter when '\
+                                       'running more then one GRE based tunnel on the '\
+                                       'same source-interface/source-address')
 
     # Keys are not allowed with ipip and sit tunnels
     if tunnel['encapsulation'] in ['ipip', 'sit']: