Merge pull request #2732 from vyos/mergify/bp/sagitta/pr-2728

T5880: verify_source_interface() should not allow dynamic interfaces like ppp, l2tp, ipoe or sstpc client interfaces (backport #2728)

4 files changed, 31 insertions, 10 deletions

diff --git a/interface-definitions/include/constraint/interface-name.xml.i b/interface-definitions/include/constraint/interface-name.xml.i
index 1b14eabf5..3e7c4e667 100644
--- a/interface-definitions/include/constraint/interface-name.xml.i
+++ b/interface-definitions/include/constraint/interface-name.xml.i
@@ -1,4 +1,4 @@
 <!-- include start from constraint/interface-name.xml.i -->
-<regex>(bond|br|dum|en|ersp|eth|gnv|ifb|lan|l2tp|l2tpeth|macsec|peth|ppp|pppoe|pptp|sstp|tun|veth|vti|vtun|vxlan|wg|wlan|wwan)[0-9]+(.\d+)?|lo</regex>
+<regex>(bond|br|dum|en|ersp|eth|gnv|ifb|ipoe|lan|l2tp|l2tpeth|macsec|peth|ppp|pppoe|pptp|sstp|sstpc|tun|veth|vti|vtun|vxlan|wg|wlan|wwan)[0-9]+(.\d+)?|lo</regex>
 <validator name="file-path --lookup-path /sys/class/net --directory"/>
 <!-- include end -->
diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index 27055c863..85423142d 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -281,16 +281,22 @@ def verify_source_interface(config):
     perform recurring validation of the existence of a source-interface
     required by e.g. peth/MACvlan, MACsec ...
     """
+    import re
     from netifaces import interfaces
-    if 'source_interface' not in config:
-        raise ConfigError('Physical source-interface required for '
-                          'interface "{ifname}"'.format(**config))
 
-    if config['source_interface'] not in interfaces():
-        raise ConfigError('Specified source-interface {source_interface} does '
-                          'not exist'.format(**config))
+    ifname = config['ifname']
+    if 'source_interface' not in config:
+        raise ConfigError(f'Physical source-interface required for "{ifname}"!')
 
     src_ifname = config['source_interface']
+    # We do not allow sourcing other interfaces (e.g. tunnel) from dynamic interfaces
+    tmp = re.compile(r'(ppp|pppoe|sstpc|l2tp|ipoe)[0-9]+')
+    if tmp.match(src_ifname):
+        raise ConfigError(f'Can not source "{ifname}" from dynamic interface "{src_ifname}"!')
+
+    if src_ifname not in interfaces():
+        raise ConfigError(f'Specified source-interface {src_ifname} does not exist')
+
     if 'source_interface_is_bridge_member' in config:
         bridge_name = next(iter(config['source_interface_is_bridge_member']))
         raise ConfigError(f'Invalid source-interface "{src_ifname}". Interface '
@@ -303,7 +309,6 @@ def verify_source_interface(config):
 
     if 'is_source_interface' in config:
         tmp = config['is_source_interface']
-        src_ifname = config['source_interface']
         raise ConfigError(f'Can not use source-interface "{src_ifname}", it already ' \
                           f'belongs to interface "{tmp}"!')
 
diff --git a/smoketest/scripts/cli/test_interfaces_tunnel.py b/smoketest/scripts/cli/test_interfaces_tunnel.py
index 2a7a519fd..dd9f1d2d1 100755
--- a/smoketest/scripts/cli/test_interfaces_tunnel.py
+++ b/smoketest/scripts/cli/test_interfaces_tunnel.py
@@ -393,5 +393,21 @@ class TunnelInterfaceTest(BasicInterfaceTest.TestCase):
             self.assertEqual(tunnel_config['encapsulation'],    conf['linkinfo']['info_kind'])
             self.assertEqual(tunnel_config['remote'],           conf['linkinfo']['info_data']['remote'])
 
+    def test_tunnel_invalid_source_interface(self):
+        encapsulation = 'gre'
+        remote = '192.0.2.1'
+        interface = 'tun7543'
+
+        self.cli_set(self._base_path + [interface, 'encapsulation', encapsulation])
+        self.cli_set(self._base_path + [interface, 'remote', remote])
+
+        for dynamic_interface in ['l2tp0', 'ppp4220', 'sstpc0', 'ipoe654']:
+            self.cli_set(self._base_path + [interface, 'source-interface', dynamic_interface])
+            # verify() - we can not source from dynamic interfaces
+            with self.assertRaises(ConfigSessionError):
+                self.cli_commit()
+        self.cli_set(self._base_path + [interface, 'source-interface', 'eth0'])
+        self.cli_commit()
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/interfaces_tunnel.py b/src/conf_mode/interfaces_tunnel.py
index 91aed9cc3..efa5ebc64 100755
--- a/src/conf_mode/interfaces_tunnel.py
+++ b/src/conf_mode/interfaces_tunnel.py
@@ -24,7 +24,7 @@ from vyos.configdict import get_interface_dict
 from vyos.configdict import is_node_changed
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
-from vyos.configverify import verify_interface_exists
+from vyos.configverify import verify_source_interface
 from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_mirror_redirect
 from vyos.configverify import verify_vrf
@@ -166,7 +166,7 @@ def verify(tunnel):
     verify_mirror_redirect(tunnel)
 
     if 'source_interface' in tunnel:
-        verify_interface_exists(tunnel['source_interface'])
+        verify_source_interface(tunnel)
 
     # TTL != 0 and nopmtudisc are incompatible, parameters and ip use default
     # values, thus the keys are always present.