From e786c018b2da54c89ad007036e52781118e86a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Th=C3=A9bault?= Date: Tue, 26 Nov 2024 09:59:44 +0100 Subject: [PATCH 1/2] 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) From 8f580fa3e6724220106969bf0cdcd14cb4d8ec4a Mon Sep 17 00:00:00 2001 From: grembo Date: Tue, 26 Nov 2024 10:01:39 +0100 Subject: [PATCH 2/2] Add hardening measures on untar (#49) This adds hardening measures while untaring archives fetched over the network (including FreeBSD tarballs and iocage plugins), as implemented by TrueNAS, see: https://github.com/truenas/iocage/pull/358 This reduces the impact of intentionally malicious or accidentally broken archives. Please note that users are still advised to only fetch from trusted sources and make use of TLS to prevent MITM attacks. --- iocage_lib/ioc_fetch.py | 5 ++++- iocage_lib/ioc_plugin.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/iocage_lib/ioc_fetch.py b/iocage_lib/ioc_fetch.py index 585824fb..9a978f7d 100644 --- a/iocage_lib/ioc_fetch.py +++ b/iocage_lib/ioc_fetch.py @@ -47,6 +47,9 @@ from iocage_lib.pools import Pool from iocage_lib.dataset import Dataset +# deliberately crash if tarfile doesn't have required filter +tarfile.tar_filter + class IOCFetch: @@ -817,7 +820,7 @@ def fetch_extract(self, f): # removing them first. member = self.__fetch_extract_remove__(f) member = self.__fetch_check_members__(member) - f.extractall(dest, members=member) + f.extractall(dest, members=member, filter='tar') def fetch_update(self, cli=False, uuid=None): """This calls 'freebsd-update' to update the fetched RELEASE.""" diff --git a/iocage_lib/ioc_plugin.py b/iocage_lib/ioc_plugin.py index 9ea6bad1..4a80253f 100644 --- a/iocage_lib/ioc_plugin.py +++ b/iocage_lib/ioc_plugin.py @@ -61,6 +61,9 @@ GIT_LOCK = threading.Lock() RE_PLUGIN_VERSION = re.compile(r'"path":"([/\.\+,\d\w-]*)\.txz"') +# deliberately crash if tarfile doesn't have required filter +tarfile.tar_filter + class IOCPlugin(object): @@ -157,7 +160,7 @@ def download_parse_packagesite(packagesite_url): shutil.copyfileobj(r.raw, f) with tarfile.open(packagesite_txz_path) as p_file: - p_file.extractall(path=tmpdir) + p_file.extractall(path=tmpdir, filter='data') packagesite_path = os.path.join(tmpdir, 'packagesite.yaml') if not os.path.exists(packagesite_path):