Merge pull request #3235 from vyos/mergify/bp/sagitta/pr-3229

T6192: allow binding SSH to multiple VRF instances (backport #3229)

debian/vyos-1x.postinst

@@ -192,3 +192,10 @@ systemctl enable vyos-config-cloud-init.service
 
 # Update XML cache
 python3 /usr/lib/python3/dist-packages/vyos/xml_ref/update_cache.py
+
+# Generate hardlinks for systemd units for multi VRF support
+# as softlinks will fail in systemd:
+# symlink target name type "ssh.service" does not match source, rejecting.
+if [ ! -f /lib/systemd/system/ssh@.service ]; then
+    ln /lib/systemd/system/ssh.service /lib/systemd/system/ssh@.service
+fi

interface-definitions/include/constraint/vrf.xml.i

@@ -0,0 +1,6 @@
+<!-- include start from constraint/vrf.xml.i -->
+<constraint>
+  <validator name="vrf-name"/>
+</constraint>
+<constraintErrorMessage>VRF instance name must be 15 characters or less and can not\nbe named as regular network interfaces.\nA name must starts from a letter.\n</constraintErrorMessage>
+<!-- include end -->

interface-definitions/include/interface/vrf.xml.i

@@ -9,6 +9,7 @@
     <completionHelp>
       <path>vrf name</path>
     </completionHelp>
+    #include <include/constraint/vrf.xml.i>
   </properties>
 </leafNode>
 <!-- include end -->

interface-definitions/include/vrf-multi.xml.i

@@ -0,0 +1,22 @@
+<!-- include start from interface/vrf.xml.i -->
+<leafNode name="vrf">
+  <properties>
+    <help>VRF instance name</help>
+    <completionHelp>
+      <path>vrf name</path>
+      <list>default</list>
+    </completionHelp>
+    <valueHelp>
+      <format>default</format>
+      <description>Explicitly start in default VRF</description>
+    </valueHelp>
+    <valueHelp>
+      <format>txt</format>
+      <description>VRF instance name</description>
+    </valueHelp>
+    #include <include/constraint/vrf.xml.i>
+    <multi/>
+  </properties>
+  <defaultValue>default</defaultValue>
+</leafNode>
+<!-- include end -->

interface-definitions/service_ssh.xml.in

@@ -262,7 +262,7 @@
               </constraint>
             </properties>
           </leafNode>
-          #include <include/interface/vrf.xml.i>
+          #include <include/vrf-multi.xml.i>
         </children>
       </node>
     </children>

interface-definitions/vrf.xml.in

@@ -16,10 +16,7 @@
       <tagNode name="name">
         <properties>
           <help>Virtual Routing and Forwarding instance</help>
-          <constraint>
-            <validator name="vrf-name"/>
-          </constraint>
-          <constraintErrorMessage>VRF instance name must be 15 characters or less and can not\nbe named as regular network interfaces.\nA name must starts from a letter.\n</constraintErrorMessage>
+          #include <include/constraint/vrf.xml.i>
           <valueHelp>
             <format>txt</format>
             <description>VRF instance name</description>

python/vyos/configverify.py

@@ -99,10 +99,17 @@ def verify_vrf(config):
     Common helper function used by interface implementations to perform
     recurring validation of VRF configuration.
     """
-    from netifaces import interfaces
-    if 'vrf' in config and config['vrf'] != 'default':
-        if config['vrf'] not in interfaces():
-            raise ConfigError('VRF "{vrf}" does not exist'.format(**config))
+    from vyos.utils.network import interface_exists
+    if 'vrf' in config:
+        vrfs = config['vrf']
+        if isinstance(vrfs, str):
+            vrfs = [vrfs]
+
+        for vrf in vrfs:
+            if vrf == 'default':
+                continue
+            if not interface_exists(vrf):
+                raise ConfigError(f'VRF "{vrf}" does not exist!')
 
         if 'is_bridge_member' in config:
             raise ConfigError(
@@ -169,13 +176,13 @@ def verify_mirror_redirect(config):
 
     It makes no sense to mirror traffic back at yourself!
     """
-    import os
+    from vyos.utils.network import interface_exists
     if {'mirror', 'redirect'} <= set(config):
         raise ConfigError('Mirror and redirect can not be enabled at the same time!')
 
     if 'mirror' in config:
         for direction, mirror_interface in config['mirror'].items():
-            if not os.path.exists(f'/sys/class/net/{mirror_interface}'):
+            if not interface_exists(mirror_interface):
                 raise ConfigError(f'Requested mirror interface "{mirror_interface}" '\
                                    'does not exist!')
 
@@ -185,7 +192,7 @@ def verify_mirror_redirect(config):
 
     if 'redirect' in config:
         redirect_ifname = config['redirect']
-        if not os.path.exists(f'/sys/class/net/{redirect_ifname}'):
+        if not interface_exists(redirect_ifname):
             raise ConfigError(f'Requested redirect interface "{redirect_ifname}" '\
                                'does not exist!')
 
@@ -243,6 +250,7 @@ def verify_interface_exists(ifname, warning_only=False):
     from vyos.base import Warning
     from vyos.configquery import ConfigTreeQuery
     from vyos.utils.dict import dict_search_recursive
+    from vyos.utils.network import interface_exists
 
     # Check if interface is present in CLI config
     config = ConfigTreeQuery()
@@ -251,7 +259,7 @@ def verify_interface_exists(ifname, warning_only=False):
         return True
 
     # Interface not found on CLI, try Linux Kernel
-    if os.path.exists(f'/sys/class/net/{ifname}'):
+    if interface_exists(ifname):
         return True
 
     message = f'Interface "{ifname}" does not exist!'

python/vyos/template.py

@@ -294,7 +294,8 @@ def network_from_ipv4(address):
 @register_filter('is_interface')
 def is_interface(interface):
     """ Check if parameter is a valid local interface name """
-    return os.path.exists(f'/sys/class/net/{interface}')
+    from vyos.utils.network import interface_exists
+    return interface_exists(interface)
 
 @register_filter('is_ip')
 def is_ip(addr):

smoketest/scripts/cli/test_service_ssh.py

@@ -32,7 +32,6 @@
 PROCESS_NAME = 'sshd'
 SSHD_CONF = '/run/sshd/sshd_config'
 base_path = ['service', 'ssh']
-vrf = 'mgmt'
 
 key_rsa = '/etc/ssh/ssh_host_rsa_key'
 key_dsa = '/etc/ssh/ssh_host_dsa_key'
@@ -51,13 +50,15 @@ def setUpClass(cls):
         # ensure we can also run this test on a live system - so lets clean
         # out the current configuration :)
         cls.cli_delete(cls, base_path)
+        cls.cli_delete(cls, ['vrf'])
 
     def tearDown(self):
         # Check for running process
         self.assertTrue(process_named_running(PROCESS_NAME))
 
         # delete testing SSH config
         self.cli_delete(base_path)
+        self.cli_delete(['vrf'])
         self.cli_commit()
 
         self.assertTrue(os.path.isfile(key_rsa))
@@ -79,7 +80,7 @@ def test_ssh_default(self):
 
         # Check configured port
         port = get_config_value('Port')[0]
-        self.assertEqual('22', port)
+        self.assertEqual('22', port) # default value
 
     def test_ssh_single_listen_address(self):
         # Check if SSH service can be configured and runs
@@ -141,10 +142,9 @@ def test_ssh_multiple_listen_addresses(self):
         for address in addresses:
             self.assertIn(address, tmp)
 
-    def test_ssh_vrf(self):
+    def test_ssh_vrf_single(self):
+        vrf = 'mgmt'
         # Check if SSH service can be bound to given VRF
-        port = '22'
-        self.cli_set(base_path + ['port', port])
         self.cli_set(base_path + ['vrf', vrf])
 
         # VRF does yet not exist - an error must be thrown
@@ -156,16 +156,32 @@ def test_ssh_vrf(self):
         # commit changes
         self.cli_commit()
 
-        # Check configured port
-        tmp = get_config_value('Port')
-        self.assertIn(port, tmp)
-
         # Check for process in VRF
         tmp = cmd(f'ip vrf pids {vrf}')
         self.assertIn(PROCESS_NAME, tmp)
 
-        # delete VRF
-        self.cli_delete(['vrf', 'name', vrf])
+    def test_ssh_vrf_multi(self):
+        # Check if SSH service can be bound to multiple VRFs
+        vrfs = ['red', 'blue', 'green']
+        for vrf in vrfs:
+            self.cli_set(base_path + ['vrf', vrf])
+
+        # VRF does yet not exist - an error must be thrown
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
+        table = 12345
+        for vrf in vrfs:
+            self.cli_set(['vrf', 'name', vrf, 'table', str(table)])
+            table += 1
+
+        # commit changes
+        self.cli_commit()
+
+        # Check for process in VRF
+        for vrf in vrfs:
+            tmp = cmd(f'ip vrf pids {vrf}')
+            self.assertIn(PROCESS_NAME, tmp)
 
     def test_ssh_login(self):
         # Perform SSH login and command execution with a predefined user. The

src/conf_mode/container.py

@@ -32,6 +32,7 @@
 from vyos.utils.process import call
 from vyos.utils.process import cmd
 from vyos.utils.process import run
+from vyos.utils.network import interface_exists
 from vyos.template import bracketize_ipv6
 from vyos.template import inc_ip
 from vyos.template import is_ipv4
@@ -471,7 +472,7 @@ def apply(container):
             # T5147: Networks are started only as soon as there is a consumer.
             # If only a network is created in the first place, no need to assign
             # it to a VRF as there's no consumer, yet.
-            if os.path.exists(f'/sys/class/net/{network_name}'):
+            if interface_exists(network_name):
                 tmp = Interface(network_name)
                 tmp.add_ipv6_eui64_address('fe80::/64')
                 tmp.set_vrf(network_config.get('vrf', ''))

src/conf_mode/qos.py

@@ -36,8 +36,9 @@
 from vyos.qos import RoundRobin
 from vyos.qos import TrafficShaper
 from vyos.qos import TrafficShaperHFSC
-from vyos.utils.process import run
 from vyos.utils.dict import dict_search_recursive
+from vyos.utils.network import interface_exists
+from vyos.utils.process import run
 from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
@@ -214,7 +215,7 @@ def apply(qos):
         return None
 
     for interface, interface_config in qos['interface'].items():
-        if not os.path.exists(f'/sys/class/net/{interface}'):
+        if not interface_exists(interface):
             # When shaper is bound to a dialup (e.g. PPPoE) interface it is
             # possible that it is yet not availbale when to QoS code runs.
             # Skip the configuration and inform the user

src/conf_mode/service_ssh.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2018-2022 VyOS maintainers and contributors
+# Copyright (C) 2018-2024 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
@@ -30,7 +30,6 @@
 airbag.enable()
 
 config_file = r'/run/sshd/sshd_config'
-systemd_override = r'/run/systemd/system/ssh.service.d/override.conf'
 
 sshguard_config_file = '/etc/sshguard/sshguard.conf'
 sshguard_whitelist = '/etc/sshguard/whitelist'
@@ -81,8 +80,6 @@ def generate(ssh):
     if not ssh:
         if os.path.isfile(config_file):
             os.unlink(config_file)
-        if os.path.isfile(systemd_override):
-            os.unlink(systemd_override)
 
         return None
 
@@ -99,13 +96,10 @@ def generate(ssh):
         call(f'ssh-keygen -q -N "" -t ed25519 -f {key_ed25519}')
 
     render(config_file, 'ssh/sshd_config.j2', ssh)
-    render(systemd_override, 'ssh/override.conf.j2', ssh)
 
     if 'dynamic_protection' in ssh:
         render(sshguard_config_file, 'ssh/sshguard_config.j2', ssh)
         render(sshguard_whitelist, 'ssh/sshguard_whitelist.j2', ssh)
-    # Reload systemd manager configuration
-    call('systemctl daemon-reload')
 
     return None
 
@@ -114,7 +108,7 @@ def apply(ssh):
     systemd_service_sshguard = 'sshguard.service'
     if not ssh:
         # SSH access is removed in the commit
-        call(f'systemctl stop {systemd_service_ssh}')
+        call(f'systemctl stop ssh@*.service')
         call(f'systemctl stop {systemd_service_sshguard}')
         return None
 
@@ -126,9 +120,13 @@ def apply(ssh):
     # we need to restart the service if e.g. the VRF name changed
     systemd_action = 'reload-or-restart'
     if 'restart_required' in ssh:
+        # this is only true if something for the VRFs changed, thus we
+        # stop all VRF services and only restart then new ones
+        call(f'systemctl stop ssh@*.service')
         systemd_action = 'restart'
 
-    call(f'systemctl {systemd_action} {systemd_service_ssh}')
+    for vrf in ssh['vrf']:
+        call(f'systemctl {systemd_action} ssh@{vrf}.service')
     return None
 
 if __name__ == '__main__':

src/etc/systemd/system/ssh@.service.d/vrf-override.conf

@@ -0,0 +1,13 @@
+[Unit]
+StartLimitIntervalSec=0
+After=vyos-router.service
+ConditionPathExists=/run/sshd/sshd_config
+
+[Service]
+EnvironmentFile=
+ExecStart=
+ExecStart=ip vrf exec %i /usr/sbin/sshd -f /run/sshd/sshd_config
+Restart=always
+RestartPreventExitStatus=
+RestartSec=10
+RuntimeDirectoryPreserve=yes

src/tests/test_template.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2020-2023 VyOS maintainers and contributors
+# Copyright (C) 2020-2024 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
@@ -14,9 +14,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
 import vyos.template
 
+from vyos.utils.network import interface_exists
 from ipaddress import ip_network
 from unittest import TestCase
 
@@ -26,7 +26,7 @@ def setUp(self):
 
     def test_is_interface(self):
         for interface in ['lo', 'eth0']:
-            if os.path.exists(f'/sys/class/net/{interface}'):
+            if interface_exists(interface):
                 self.assertTrue(vyos.template.is_interface(interface))
             else:
                 self.assertFalse(vyos.template.is_interface(interface))