diff --git a/smoketest/scripts/cli/test_interfaces_tunnel.py b/smoketest/scripts/cli/test_interfaces_tunnel.py index 8eb32b7cad5..65c7d71c05d 100755 --- a/smoketest/scripts/cli/test_interfaces_tunnel.py +++ b/smoketest/scripts/cli/test_interfaces_tunnel.py @@ -26,6 +26,7 @@ remote_ip4 = '192.0.2.100' remote_ip6 = '2001:db8::ffff' source_if = 'dum2222' +source_if2 = 'dum2223' mtu = 1476 class TunnelInterfaceTest(BasicInterfaceTest.TestCase): @@ -45,10 +46,12 @@ def setUpClass(cls): # 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']) + cls.cli_set(cls, ['interfaces', 'dummy', source_if2]) @classmethod def tearDownClass(cls): cls.cli_delete(cls, ['interfaces', 'dummy', source_if]) + cls.cli_delete(cls, ['interfaces', 'dummy', source_if2]) super().tearDownClass() def test_ipv4_encapsulations(self): @@ -321,10 +324,25 @@ def test_tunnel_src_any_gre_key(self): # GRE key must be supplied with a 0.0.0.0 source address with self.assertRaises(ConfigSessionError): self.cli_commit() + + # A zero key is no key at all for such a tunnel - with an any + # source-address and no remote it catches every packet, exactly as a + # keyless tunnel would + self.cli_set(self._base_path + [interface, 'parameters', 'ip', 'key', '0']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_set(self._base_path + [interface, 'parameters', 'ip', 'key', key]) self.cli_commit() + # A remote address identifies the tunnel on its own, so a zero key is + # no longer a problem + self.cli_set(self._base_path + [interface, 'parameters', 'ip', 'key', '0']) + self.cli_set(self._base_path + [interface, 'remote', remote_ip4]) + + self.cli_commit() + def test_multiple_gre_tunnel_same_remote(self): tunnels = { 'tun10' : { @@ -394,6 +412,213 @@ def test_multiple_gre_tunnel_different_remote(self): self.assertEqual(tunnel_config['encapsulation'], conf['linkinfo']['info_kind']) self.assertEqual(tunnel_config['remote'], conf['linkinfo']['info_data']['remote']) + def test_multiple_gre_tunnel_same_key_different_remote(self): + # The Kernel identifies a tunnel by local address, remote address, + # source-interface and key - a differing remote address is enough to + # make both tunnels unique, even if they share one GRE key + ip_key = '10' + tunnels = { + 'tun10': '1.2.3.4', + 'tun20': '1.2.3.5', + } + + for tunnel, remote in tunnels.items(): + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-address', self.local_v4]) + self.cli_set(self._base_path + [tunnel, 'remote', remote]) + self.cli_set(self._base_path + [tunnel, 'parameters', 'ip', 'key', ip_key]) + + self.cli_commit() + + for tunnel, remote in tunnels.items(): + conf = get_interface_config(tunnel) + + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(self.local_v4, conf['linkinfo']['info_data']['local']) + self.assertEqual(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_same_key_different_source_interface(self): + # Tunnels bound to different source-interfaces are distinct for the + # Kernel, thus they are free to share one GRE key + ip_key = '20' + tunnels = { + 'tun10': source_if, + 'tun20': source_if2, + } + + for tunnel, interface in tunnels.items(): + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-interface', interface]) + self.cli_set(self._base_path + [tunnel, 'parameters', 'ip', 'key', ip_key]) + + self.cli_commit() + + for tunnel, interface in tunnels.items(): + conf = get_interface_config(tunnel) + + self.assertEqual(interface, conf['link']) + self.assertEqual('gre', conf['linkinfo']['info_kind']) + 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_any_remote(self): + # The Kernel stores an unset remote address as the any address, thus + # "remote 0.0.0.0" and an unset remote must be treated alike - creating + # both would fail with "add tunnel "gre0" failed: File exists" + ip_key = '30' + + for tunnel in ['tun10', 'tun20']: + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-address', self.local_v4]) + self.cli_set(self._base_path + [tunnel, 'parameters', 'ip', 'key', ip_key]) + self.cli_set(self._base_path + ['tun10', 'remote', '0.0.0.0']) + + # Both tunnels resolve to the same Kernel tunnel - this must be rejected + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # A differing key makes them unique again + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', '31']) + self.cli_commit() + + def test_multiple_gre_tunnel_keyless_and_keyed(self): + # A keyless tunnel is distinct from a keyed one even when both share + # the same local and remote address - the Kernel only matches a tunnel + # carrying no key against another tunnel carrying no key + ip_key = '40' + + for tunnel in ['tun10', 'tun20']: + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-address', self.local_v4]) + self.cli_set(self._base_path + ['tun20', 'remote', '0.0.0.0']) + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', ip_key]) + + self.cli_commit() + + # Re-verifying the keyless tunnel must keep succeeding - if it does not, + # the next commit touching it fails and it is lost on the next boot + self.cli_set(self._base_path + ['tun10', 'description', 'foo']) + self.cli_commit() + + conf = get_interface_config('tun10') + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(self.local_v4, conf['linkinfo']['info_data']['local']) + self.assertNotIn('ikey', conf['linkinfo']['info_data']) + + conf = get_interface_config('tun20') + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(self.local_v4, conf['linkinfo']['info_data']['local']) + 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_keyless_different_source_interface(self): + # Tunnels bound to different source-interfaces stay distinct for the + # Kernel even when they share one local and remote address - "dev" is + # compared as the tunnel link index - so no GRE key is needed to tell + # them apart + remote = '1.2.3.4' + tunnels = { + 'tun10': source_if, + 'tun20': source_if2, + } + + for tunnel, interface in tunnels.items(): + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-address', self.local_v4]) + self.cli_set(self._base_path + [tunnel, 'source-interface', interface]) + self.cli_set(self._base_path + [tunnel, 'remote', remote]) + + self.cli_commit() + + for tunnel, interface in tunnels.items(): + conf = get_interface_config(tunnel) + + self.assertEqual(interface, conf['link']) + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(self.local_v4, conf['linkinfo']['info_data']['local']) + self.assertEqual(remote, conf['linkinfo']['info_data']['remote']) + self.assertNotIn('ikey', conf['linkinfo']['info_data']) + + def test_multiple_gre_tunnel_zero_key(self): + # Carrying neither a source-address nor a remote, these tunnels are only + # told apart by their key - and a zero key does not do that, it cannot be + # told from an unset one on receive. A non-zero key can + for tunnel in ['tun10', 'tun20']: + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-interface', source_if]) + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', '0']) + + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', '50']) + self.cli_commit() + + conf = get_interface_config('tun20') + self.assertEqual('0.0.0.50', conf['linkinfo']['info_data']['ikey']) + self.assertEqual('0.0.0.50', conf['linkinfo']['info_data']['okey']) + + # Dropping back to a zero key must be rejected just the same. Only the + # tunnel which changed is verified again here, so this is the case a + # check trusting its own zero key - but not the neighbours - lets + # through: both tunnels commit, and every later commit touching the + # keyless one is refused from then on + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', '0']) + + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + def test_multiple_gre_tunnel_zero_key_with_endpoints(self): + # A zero key does tell a tunnel apart from a keyless one as soon as a + # local or a remote address is set - the Kernel then matches through + # ip_tunnel_key_match(), which tests the flag saying that a key is set + # before it compares the value + for tunnel in ['tun10', 'tun20']: + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-address', self.local_v4]) + self.cli_set(self._base_path + ['tun20', 'parameters', 'ip', 'key', '0']) + + # The same holds when it is the remote address which identifies them + for tunnel in ['tun30', 'tun40']: + self.cli_set(self._base_path + [tunnel, 'encapsulation', 'gre']) + self.cli_set(self._base_path + [tunnel, 'source-interface', source_if]) + self.cli_set(self._base_path + [tunnel, 'remote', remote_ip4]) + self.cli_set(self._base_path + ['tun40', 'parameters', 'ip', 'key', '0']) + + self.cli_commit() + + # Re-verifying the keyless tunnels must keep succeeding - if it does + # not, the next commit touching them fails and they are lost on the + # next boot + for tunnel in ['tun10', 'tun30']: + self.cli_set(self._base_path + [tunnel, 'description', 'foo']) + + self.cli_commit() + + for tunnel in ['tun10', 'tun20']: + conf = get_interface_config(tunnel) + + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(self.local_v4, conf['linkinfo']['info_data']['local']) + + for tunnel in ['tun30', 'tun40']: + conf = get_interface_config(tunnel) + + self.assertEqual(source_if, conf['link']) + self.assertEqual('gre', conf['linkinfo']['info_kind']) + self.assertEqual(remote_ip4, conf['linkinfo']['info_data']['remote']) + + # The keyless tunnels must have stayed keyless. That the commit went + # through at all is what proves the zero key reached the Kernel with its + # flag set - dropped, it would have left two keyless tunnels sharing one + # endpoint and the second of them could not have been created + for tunnel in ['tun10', 'tun30']: + conf = get_interface_config(tunnel) + + self.assertNotIn('ikey', conf['linkinfo']['info_data']) + def test_tunnel_invalid_source_interface(self): encapsulation = 'gre' remote = '192.0.2.1' diff --git a/src/conf_mode/interfaces_tunnel.py b/src/conf_mode/interfaces_tunnel.py index a86a2fa8418..1797b709ea0 100755 --- a/src/conf_mode/interfaces_tunnel.py +++ b/src/conf_mode/interfaces_tunnel.py @@ -38,6 +38,29 @@ from vyos import airbag airbag.enable() + +def get_tunnel_endpoint(address): + """ + The Kernel stores an unconfigured tunnel endpoint as the any address, thus + "0.0.0.0" and "::" must compare equal to an endpoint which is not set at all + """ + if address in ['0.0.0.0', '::']: + return None + return address + + +def get_tunnel_key(key, no_endpoints): + """ + A zero GRE key only fails to make a tunnel unique when neither a local nor a + remote address is set - ip_tunnel_lookup() then ends in a loop which compares + the key without testing the flag saying that a key is set at all. A tunnel + which carries an address is matched before that, by ip_tunnel_key_match(), + where a zero key and an unset key differ + """ + if no_endpoints and key == '0': + return None + return key + def get_config(config=None): """ Retrieve CLI config as dictionary. Dictionary can never be empty, as at least @@ -122,13 +145,25 @@ def verify(tunnel): if 'direction' not in tunnel['parameters']['erspan']: raise ConfigError('ERSPAN version 2 requires direction to be set!') - # If tunnel source is any and gre key is not set + # If tunnel source is any and the gre key does not identify the tunnel 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(f'"parameters ip key" must be set for {interface} when '\ - 'encapsulation is GRE!') + if ( + tunnel['encapsulation'] in ['gre'] + and dict_search('source_address', tunnel) == '0.0.0.0' + ): + # The source-address being the any address, a tunnel without a remote + # carries no endpoint at all - a zero key does not tell such a tunnel + # apart from a keyless one either, see get_tunnel_key() + no_endpoints = get_tunnel_endpoint(dict_search('remote', tunnel)) is None + key = dict_search('parameters.ip.key', tunnel) + if get_tunnel_key(key, no_endpoints) is None: + # Only a tunnel which has no remote either is asked for a + # non-zero key, a zero one does identify the rest + tmp = 'set to a non-zero value' if no_endpoints else 'set' + raise ConfigError( + f'"parameters ip key" must be {tmp} for {interface} when ' + 'encapsulation is GRE!' + ) gre_encapsulations = ['gre', 'gretap'] if tunnel['encapsulation'] in gre_encapsulations and 'other_tunnels' in tunnel: @@ -142,36 +177,88 @@ def verify(tunnel): not in gre_encapsulations: continue - our_address = dict_search('source_address', tunnel) + our_address = get_tunnel_endpoint(dict_search('source_address', tunnel)) our_key = dict_search('parameters.ip.key', tunnel) - their_address = dict_search('source_address', o_tunnel_conf) + our_source_if = dict_search('source_interface', tunnel) + our_remote = get_tunnel_endpoint(dict_search('remote', tunnel)) + their_address = get_tunnel_endpoint( + 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 "{o_tunnel}"!') + their_source_if = dict_search('source_interface', o_tunnel_conf) + their_remote = get_tunnel_endpoint(dict_search('remote', o_tunnel_conf)) + + # A zero key only fails to make a tunnel unique when neither of the + # two carries a local or a remote address, see get_tunnel_key(). Both + # sides must classify it the same way, else the very same pair of + # tunnels is accepted or rejected depending on which of the two is + # being verified. Remember whether one was configured, the error + # message differs from the one for a tunnel carrying no key at all. + no_endpoints = not any( + [our_address, our_remote, their_address, their_remote] + ) + zero_key = '0' in [our_key, their_key] + our_key = get_tunnel_key(our_key, no_endpoints) + their_key = get_tunnel_key(their_key, no_endpoints) + + # The Kernel identifies a tunnel by the tuple of local address, remote + # address, source-interface and - if configured - the GRE key, see + # ip_tunnel_find() in net/ipv4/ip_tunnel.c. A differing remote address + # alone already makes both tunnels unique, whether a key is used or not. + if our_remote != their_remote: + continue + + if our_key is not None: + # Prevent the same key for 2 tunnels sharing both endpoints. T2920 + if ( + their_address == our_address + and their_source_if == our_source_if + and their_key == our_key + ): + # Report the source as configured and not as normalised, + # else an "any" source-address would render as "None". + # One of both is always present, see verify_tunnel() + tmp = dict_search('source_address', tunnel) or our_source_if + raise ConfigError( + f'Key "{our_key}" for source "{tmp}" is already used ' + f'for tunnel "{o_tunnel}"!' + ) else: - 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 cannot have more then one GRE tunnel + # A keyless tunnel never collides with a keyed one, as the Kernel + # only matches a tunnel carrying no key against another tunnel + # carrying no key, see ip_tunnel_key_match() in + # include/net/ip_tunnels.h + if their_key is not None: + continue + + # If no IP GRE key is defined we cannot have more than 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 our_remote == their_remote: - if our_address is not None and their_address == our_address: - # If set to the same values, this is always a fail - raise ConfigError(f'Missing required "ip key" parameter when '\ - 'running more then one GRE based tunnel on the '\ - 'same source-address') - - if their_source_if == our_source_if and their_address == our_address: - # Note that lack of None check on these is deliberate. - # source-if and source-ip matching while unset (all None) is a fail - # source-ifs set and matching with unset source-ips is a fail - raise ConfigError(f'Missing required "ip key" parameter when '\ - 'running more then one GRE based tunnel on the '\ - 'same source-interface') + if their_address == our_address and their_source_if == our_source_if: + # A differing source-interface alone already keeps both apart, + # it is passed as "dev" and compared by the Kernel as the + # tunnel link index, see ip_tunnel_find(). + # Note that lack of a None check here is deliberate. + # source-if and source-ip matching while unset (all None) is a fail + # source-ifs set and matching with unset source-ips is a fail + + # Name what the two tunnels really have in common. An "any" + # source-address normalises to None and cannot pick the noun, + # and a tunnel carrying no source-interface always has a + # source-address, see verify_tunnel() + tmp = 'source-address' + if our_source_if is not None: + tmp = 'source-interface' + if zero_key: + raise ConfigError( + 'A zero "ip key" parameter cannot be told apart from ' + 'an unset one - use a non-zero key to run more than ' + f'one GRE based tunnel on the same {tmp} as "{o_tunnel}"' + ) + raise ConfigError( + 'Missing required "ip key" parameter when running more ' + f'than one GRE based tunnel on the same {tmp}' + ) # Keys are not allowed with ipip and sit tunnels if tunnel['encapsulation'] in ['ipip', 'sit']: