From e786c018b2da54c89ad007036e52781118e86a39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Quentin=20Th=C3=A9bault?= <quentin.thebault@defenso.fr>
Date: Tue, 26 Nov 2024 09:59:44 +0100
Subject: [PATCH] ioc_start.py: allow 'none' bridge in interfaces (#47)

Iocage currently expects interfaces to be specified in the nic:bridge
format, where bridge cannot be none. This results in iocage always
creating a bridge to which VNET jail epair interfaces are added as
members.

In a scenario where the user wants jails to be isolated on the data-link
layer (OSI layer 2 / Ethernet) and use the host as a router, this bridge
is unnecessery. It can also result in illegitimate cross-jail traffic
being allowed, since pf filtering on bridge interfaces is disabled by
default on FreeBSD systems (net.link.bridge.pfil_bridge=0).

Closes #44
---
 iocage_lib/ioc_start.py                   |  8 ++--
 tests/data_classes.py                     |  7 +--
 tests/functional_tests/0004_start_test.py | 55 +++++++++++++++++++++++
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/iocage_lib/ioc_start.py b/iocage_lib/ioc_start.py
index c53f9bb2..f11d986c 100644
--- a/iocage_lib/ioc_start.py
+++ b/iocage_lib/ioc_start.py
@@ -1151,7 +1151,7 @@ def start_network_interface_vnet(
         """
         Start VNET on interface
 
-        :param nic_defs: comma separated interface definitions (nic, bridge)
+        :param nic_defs: comma separated interface definitions (nic:bridge, nic:bridge...)
         :param net_configs: Tuple of IP address and router pairs
         :param jid: The jails ID
         """
@@ -1167,7 +1167,7 @@ def start_network_interface_vnet(
             try:
                 if self.get(f"{nic}_mtu") != 'auto':
                     membermtu = self.get(f"{nic}_mtu")
-                elif not nat_addr:
+                elif not nat_addr and bridge != 'none':
                     membermtu = self.find_bridge_mtu(bridge)
                 else:
                     membermtu = self.get('vnet_default_mtu')
@@ -1301,7 +1301,7 @@ def start_network_vnet_iface(self, nic, bridge, mtu, jid, nat_addr=0):
                 stderr=su.STDOUT
             )
 
-            if not nat_addr:
+            if bridge != 'none' and not nat_addr:
                 try:
                     # Host interface as supplied by user also needs to be on
                     # the bridge
@@ -1319,7 +1319,7 @@ def start_network_vnet_iface(self, nic, bridge, mtu, jid, nat_addr=0):
                     ['ifconfig', bridge, 'addm', f'{nic}.{jid}', 'up'],
                     stderr=su.STDOUT
                 )
-            else:
+            elif nat_addr:
                 iocage_lib.ioc_common.checkoutput(
                     ['ifconfig', f'{nic}.{jid}', 'inet', f'{nat_addr}/30'],
                     stderr=su.STDOUT
diff --git a/tests/data_classes.py b/tests/data_classes.py
index 6285a400..b1b7f836 100644
--- a/tests/data_classes.py
+++ b/tests/data_classes.py
@@ -669,10 +669,11 @@ def ips(self):
                 ip for ip in ips if ip not in skip_ips
             ]
 
-    def run_command(self, command):
+    def run_command(self, command, jailed=True):
         # Returns a tuple - stdout, stderr
-        assert self.running is True
-        command = ['jexec', f'ioc-{self.name}'] + command
+        if jailed:
+            assert self.running is True
+            command = ['jexec', f'ioc-{self.name}'] + command
         try:
             stdout, stderr = subprocess.Popen(
                 command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
diff --git a/tests/functional_tests/0004_start_test.py b/tests/functional_tests/0004_start_test.py
index 66da418a..73f64e17 100644
--- a/tests/functional_tests/0004_start_test.py
+++ b/tests/functional_tests/0004_start_test.py
@@ -23,6 +23,10 @@
 # POSSIBILITY OF SUCH DAMAGE.
 
 import pytest
+import inspect
+import tempfile
+import os
+import re
 
 
 require_root = pytest.mark.require_root
@@ -54,3 +58,54 @@ def test_02_start_rc_jail(invoke_cli, resource_selector):
         assert jail.running is True, f'{jail.name} not running'
 
 # TODO: Let's also start jails in a single command to test that out
+
+@require_root
+@require_zpool
+def test_03_create_and_start_nobridge_vnet_jail(release, jail, invoke_cli):
+    jail = jail('nobridge_jail')
+
+    fd, path = tempfile.mkstemp()
+
+    try:
+        with os.fdopen(fd, 'w') as tmp:
+            tmp.write(inspect.cleandoc(f"""
+                #!/bin/sh
+                jailname=ioc-{jail.name}
+                jid=$(jls -j $jailname jid)
+                iface=vnet0.$jid
+                ifconfig $iface inet6 fe80::1/64
+            """))
+        os.chmod(path, 0o755)
+
+        invoke_cli([
+            'create', '-r', release, '-n', jail.name,
+            'boot=on', 'vnet=on',
+            'interfaces=vnet0:none', 'vnet_default_interface=none',
+            f'ip4_addr=none', 'ip6_addr=vnet0|fe80::2/64',
+            'defaultrouter6=none', 'defaultrouter=none',
+            f'exec_poststart={path}'
+        ])
+
+        assert jail.exists is True
+        assert jail.running is True
+
+        stdout, stderr = jail.run_command(['ifconfig'])
+        assert bool(stderr) is False, f'Ifconfig returned an error: {stderr}'
+        assert 'fe80::2%epair0b' in stdout
+
+        stdout, stderr = jail.run_command(['ifconfig'], jailed=False)
+
+        assert bool(stderr) is False, f'Ifconfig returned an error: {stderr}'
+        assert re.search(r'bridge[0-9]', stdout) is None, 'Unexpected bridge was created.'
+        assert f'fe80::1%vnet0.{jail.jid}' in stdout
+        assert f'description: associated with jail: {jail.name} as nic: epair0b'
+
+        stdout, stderr = jail.run_command(['ping', '-c', '1', f'fe80::2%vnet0.{jail.jid}'], jailed=False)
+        assert bool(stderr) is False, f'Ping returned an error: {stderr}'
+
+        invoke_cli([
+            'destroy', jail.name, '-f'
+        ])
+
+    finally:
+        os.remove(path)