From c2807ad315d145a40477e3aace476874f0c379bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Th=C3=A9bault?= Date: Sun, 17 Nov 2024 00:41:47 +0900 Subject: [PATCH] ioc_start.py: allow 'none' bridge in interfaces 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 --- .cirrus.yml | 6 ++- iocage_lib/ioc_create.py | 2 +- iocage_lib/ioc_start.py | 8 ++-- iocage_lib/ioc_upgrade.py | 2 +- iocage_lib/iocage.py | 2 +- pytest.ini | 1 + tests/conftest.py | 14 ++++++ tests/data_classes.py | 7 +-- tests/functional_tests/0004_start_test.py | 57 +++++++++++++++++++++++ 9 files changed, 88 insertions(+), 11 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 0301ecc0..9b3d59f1 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -18,6 +18,8 @@ iocage_tests_task: - pkg install -Uy git python3 py311-sqlite3 configure_python_script: - python3 -m ensurepip + enable_routing_script: + - sysctl net.inet.ip.forwarding=1 matrix: - name: Build python packages install_rust_script: @@ -45,8 +47,10 @@ iocage_tests_task: - pkg install_iocage_script: - python3 setup.py install - test_script: pytest --zpool=pool tests/functional_tests --junit-xml=reports/pytest-report.xml -rA --image + ifconfig_script: ifconfig + test_script: pytest --zpool=pool tests/functional_tests --junit-xml=reports/pytest-report.xml -rA --image --nobridge_jail_ip=192.168.2.2 always: + ifconfig_after_script: ifconfig pytest_results_artifacts: path: reports/*.xml format: junit diff --git a/iocage_lib/ioc_create.py b/iocage_lib/ioc_create.py index 0a312abf..18c61462 100644 --- a/iocage_lib/ioc_create.py +++ b/iocage_lib/ioc_create.py @@ -864,7 +864,7 @@ def create_install_packages(self, jail_uuid, location, # To avoid a user being prompted about pkg. pkg_retry = 1 while True: - pkg_install = su.run(["pkg-static", "-j", jid, "install", "-q", + pkg_install = su.run(["/usr/local/sbin/pkg-static", "-j", jid, "install", "-q", "-y", "pkg"], stdout=su.PIPE, stderr=su.STDOUT) 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/iocage_lib/ioc_upgrade.py b/iocage_lib/ioc_upgrade.py index 3c2272cf..5d23bc18 100644 --- a/iocage_lib/ioc_upgrade.py +++ b/iocage_lib/ioc_upgrade.py @@ -188,7 +188,7 @@ def upgrade_jail(self): if f_rel.startswith('12'): # https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=239498 cp = su.Popen( - ['pkg-static', '-j', self.jid, 'install', '-q', '-f', '-y', 'pkg'], + ['/usr/local/sbin/pkg-static', '-j', self.jid, 'install', '-q', '-f', '-y', 'pkg'], stdout=su.PIPE, stderr=su.PIPE, env=self.upgrade_env ) _, stderr = cp.communicate() diff --git a/iocage_lib/iocage.py b/iocage_lib/iocage.py index e1a60a11..f9fb1a5a 100644 --- a/iocage_lib/iocage.py +++ b/iocage_lib/iocage.py @@ -1962,7 +1962,7 @@ def update(self, pkgs=False, server=None, verify=True): 'message': 'Updating pkgs...' }) pkg_update = su.run( - ['pkg-static', '-j', jid, 'update', '-q', '-f'], + ['/usr/local/sbin/pkg-static', '-j', jid, 'update', '-q', '-f'], stdout=su.PIPE, stderr=su.STDOUT ) if pkg_update.returncode: diff --git a/pytest.ini b/pytest.ini index 9919cb60..7b696ba9 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,6 +7,7 @@ markers = require_root require_dhcp require_jail_ip + require_nobridge_jail_ip require_networking require_nat require_upgrade diff --git a/tests/conftest.py b/tests/conftest.py index be54a33f..e185bc85 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -79,6 +79,10 @@ def pytest_addoption(parser): '--jail_ip', action='store', default=None, help='Static IP to use creating jails' ) + parser.addoption( + '--nobridge_jail_ip', action='store', default=None, + help='Static IP to use creating nobridge jails' + ) parser.addoption( '--dhcp', action='store_true', default=False, help='Use DHCP for creating jails' @@ -124,6 +128,12 @@ def pytest_runtest_setup(item): and not item.config.getvalue('jail_ip') ): pytest.skip('Need --jail_ip option to run') + + if ( + 'require_nobridge_jail_ip' in item.keywords + and not item.config.getvalue('nobridge_jail_ip') + ): + pytest.skip('Need --nobridge_jail_ip option to run') if ( 'require_networking' in item.keywords @@ -153,6 +163,10 @@ def jail_ip(request): """Specify a jail ip to use.""" return request.config.getoption('--jail_ip') +@pytest.fixture +def nobridge_jail_ip(request): + """Specify a jail ip to use.""" + return request.config.getoption('--nobridge_jail_ip') @pytest.fixture def dhcp(request): 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..d9ff6000 100644 --- a/tests/functional_tests/0004_start_test.py +++ b/tests/functional_tests/0004_start_test.py @@ -23,10 +23,15 @@ # POSSIBILITY OF SUCH DAMAGE. import pytest +import inspect +import tempfile +import os +import re require_root = pytest.mark.require_root require_zpool = pytest.mark.require_zpool +require_nobridge_jail_ip = pytest.mark.require_nobridge_jail_ip @require_root @@ -54,3 +59,55 @@ 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 +@require_nobridge_jail_ip +def test_03_create_and_start_nobridge_vnet_jail(release, jail, invoke_cli, nobridge_jail_ip): + 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)