From 2af582984c47ebe0a8d3bbc2733e0767d04cda0f Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Mon, 10 Jul 2023 12:43:58 -0600 Subject: [PATCH] subiquity.network: cloud-init networking when netplan root-readonly When cloudinit.features.NETPLAN_CONFIG_ROOT_READ_ONLY is True, cloud-init will write /etc/netplan/50-cloud-init.yaml as read-only root. This added security allows for subiquity to use cloud-init's network renderer directly allowing both datasource and network configuration passed in one place. Read cloud-init features from /run/cloud-init/combined-cloud-config.json when present. Any netplan wifi configuration can be specified in a single root-read-only network config file /etc/cloud/cloud.cfg.d/90-installer-network.cfg instead of having a separate config file for wifi, which could contain credentials. This simplifies golden image creation from images installed using subiquity because image builders will not need to track down and purge separate /etc/netplan/00-installer-config.yaml and /etc/netplan/subiquity-disable-cloudinit-networking.cfg when preparing a golden image. Eventually, netplan config validation and cloudinit will support separation of sensitive configuration by cloud-init without needing to pre-categorize sensitive information. This will allow cloud-init to grow to ability to write separate world-readable configuration from config which is security sensitive with no change needed in subiquity. --- subiquity/cloudinit.py | 12 ++++ subiquity/models/network.py | 71 +++++++++++++++------- subiquity/models/tests/test_subiquity.py | 75 +++++++++++++++++------- 3 files changed, 116 insertions(+), 42 deletions(-) create mode 100644 subiquity/cloudinit.py diff --git a/subiquity/cloudinit.py b/subiquity/cloudinit.py new file mode 100644 index 00000000..0893ecd4 --- /dev/null +++ b/subiquity/cloudinit.py @@ -0,0 +1,12 @@ +"""Shared cloudinit utility functions""" + +import json + + +def get_host_combined_cloud_config() -> dict: + """Return the host system /run/cloud-init/combined-cloud-config.json""" + try: + with open("/run/cloud-init/combined-cloud-config.json") as fp: + return json.load(fp) + except (IOError, OSError, AttributeError, json.decoder.JSONDecodeError): + return {} diff --git a/subiquity/models/network.py b/subiquity/models/network.py index c217e860..eadbe9b2 100644 --- a/subiquity/models/network.py +++ b/subiquity/models/network.py @@ -16,6 +16,7 @@ import logging import subprocess +from subiquity import cloudinit from subiquitycore.models.network import NetworkModel as CoreNetworkModel from subiquitycore.utils import arun_command @@ -41,31 +42,57 @@ class NetworkModel(CoreNetworkModel): # perfect solution because in principle there could be wired 802.1x # stuff that has secrets too but the subiquity UI does not support any # of that yet so this will do for now. - wifis = netplan['network'].pop('wifis', None) - r = { - 'write_files': { - 'etc_netplan_installer': { - 'path': 'etc/netplan/00-installer-config.yaml', - 'content': self.stringify_config(netplan), + + # If host cloud-init version has no readable combined-cloud-config, + # default to False. + cloud_cfg = cloudinit.get_host_combined_cloud_config() + use_cloudinit_net = cloud_cfg.get('features', {}).get( + 'NETPLAN_CONFIG_ROOT_READ_ONLY', False + ) + + if use_cloudinit_net: + r = { + 'write_files': { + 'etc_netplan_installer': { + 'path': ( + 'etc/cloud/cloud.cfg.d/90-installer-network.cfg' + ), + 'content': self.stringify_config(netplan), + 'permissions': '0600', }, - 'nonet': { - 'path': ('etc/cloud/cloud.cfg.d/' - 'subiquity-disable-cloudinit-networking.cfg'), - 'content': 'network: {config: disabled}\n', + } + } + else: + # Separate sensitive wifi config from world-readable config + wifis = netplan['network'].pop('wifis', None) + r = { + 'write_files': { + # Disable cloud-init networking + 'no_cloudinit_net': { + 'path': ( + 'etc/cloud/cloud.cfg.d/' + 'subiquity-disable-cloudinit-networking.cfg' + ), + 'content': 'network: {config: disabled}\n', + }, + # World-readable netplan without sensitive wifi config + 'etc_netplan_installer': { + 'path': 'etc/netplan/00-installer-config.yaml', + 'content': self.stringify_config(netplan), }, }, } - if wifis is not None: - netplan_wifi = { - 'network': { - 'version': 2, - 'wifis': wifis, + if wifis is not None: + netplan_wifi = { + 'network': { + 'version': 2, + 'wifis': wifis, }, } - r['write_files']['etc_netplan_installer_wifi'] = { - 'path': 'etc/netplan/00-installer-config-wifi.yaml', - 'content': self.stringify_config(netplan_wifi), - 'permissions': '0600', + r['write_files']['etc_netplan_installer_wifi'] = { + 'path': 'etc/netplan/00-installer-config-wifi.yaml', + 'content': self.stringify_config(netplan_wifi), + 'permissions': '0600', } return r @@ -79,8 +106,10 @@ class NetworkModel(CoreNetworkModel): try: cp = await arun_command(("nmcli", "networking"), check=True) except subprocess.CalledProcessError as exc: - log.warning("failed to run nmcli networking," - " considering NetworkManager disabled.") + log.warning( + "failed to run nmcli networking," + " considering NetworkManager disabled." + ) log.debug("stderr: %s", exc.stderr) return False except FileNotFoundError: diff --git a/subiquity/models/tests/test_subiquity.py b/subiquity/models/tests/test_subiquity.py index fb4b8a9d..abcedbce 100644 --- a/subiquity/models/tests/test_subiquity.py +++ b/subiquity/models/tests/test_subiquity.py @@ -76,6 +76,7 @@ class TestModelNames(unittest.TestCase): class TestSubiquityModel(unittest.IsolatedAsyncioTestCase): + maxDiff = None def writtenFiles(self, config): for k, v in config.get('write_files', {}).items(): @@ -267,8 +268,6 @@ class TestSubiquityModel(unittest.IsolatedAsyncioTestCase): model.identity.add_user(main_user) model.userdata = {} expected_files = { - 'etc/cloud/cloud.cfg.d/subiquity-disable-cloudinit-networking.cfg': - 'network: {config: disabled}\n', 'etc/cloud/cloud.cfg.d/99-installer.cfg': re.compile('datasource:\n None:\n metadata:\n instance-id: .*\n userdata_raw: "#cloud-config\\\\ngrowpart:\\\\n mode: \\\'off\\\'\\\\npreserve_hostname: true\\\\n\\\\\n'), # noqa 'etc/hostname': 'somehost\n', 'etc/cloud/ds-identify.cfg': 'policy: enabled\n', @@ -280,39 +279,73 @@ class TestSubiquityModel(unittest.IsolatedAsyncioTestCase): "/" + key for key in expected_files.keys() if "host" not in key ] cfg_files.append( - # Obtained from NetworkModel.render - "/etc/netplan/00-installer-config.yaml", + # Obtained from NetworkModel.render when cloud-init features + # NETPLAN_CONFIG_ROOT_READ_ONLY is True + "/etc/cloud/cloud.cfg.d/90-installer-network.cfg" ) header = "# Autogenerated by Subiquity: 2004-03-05 ... UTC\n" - with self.subTest('Stable releases Jammy do not disable cloud-init'): + with self.subTest( + 'Stable releases Jammy do not disable cloud-init.' + ' NETPLAN_ROOT_READ_ONLY=True uses cloud-init networking' + ): lsb_release.return_value = {"release": "22.04"} expected_files['etc/cloud/clean.d/99-installer'] = ( CLOUDINIT_CLEAN_FILE_TMPL.format( header=header, cfg_files=json.dumps(sorted(cfg_files)) ) ) - for (cpath, content, perms) in model._cloud_init_files(): - if isinstance(expected_files[cpath], re.Pattern): - self.assertIsNotNone(expected_files[cpath].match(content)) - else: - self.assertEqual(expected_files[cpath], content) + with unittest.mock.patch( + "subiquity.cloudinit.open", + mock.mock_open( + read_data=json.dumps( + {"features": {"NETPLAN_CONFIG_ROOT_READ_ONLY": True}} + ) + ), + ): + for (cpath, content, perms) in model._cloud_init_files(): + if isinstance(expected_files[cpath], re.Pattern): + self.assertIsNotNone( + expected_files[cpath].match(content) + ) + else: + self.assertEqual(expected_files[cpath], content) - with self.subTest('Kinetic and later disable cloud-init post install'): + with self.subTest( + 'Kinetic++ disables cloud-init post install.' + ' NETPLAN_ROOT_READ_ONLY=False avoids cloud-init networking' + ): lsb_release.return_value = {"release": "22.10"} cfg_files.append( # Added by _cloud_init_files for 22.10 and later releases - "/etc/cloud/cloud-init.disabled", + '/etc/cloud/cloud-init.disabled', ) - expected_files['etc/cloud/clean.d/99-installer'] = ( - CLOUDINIT_CLEAN_FILE_TMPL.format( - header=header, cfg_files=json.dumps(sorted(cfg_files)) - ) + # Obtained from NetworkModel.render + cfg_files.remove('/etc/cloud/cloud.cfg.d/90-installer-network.cfg') + cfg_files.append('/etc/netplan/00-installer-config.yaml') + cfg_files.append( + '/etc/cloud/cloud.cfg.d/' + 'subiquity-disable-cloudinit-networking.cfg' ) - for (cpath, content, perms) in model._cloud_init_files(): - if isinstance(expected_files[cpath], re.Pattern): - self.assertIsNotNone(expected_files[cpath].match(content)) - else: - self.assertEqual(expected_files[cpath], content) + expected_files[ + 'etc/cloud/clean.d/99-installer' + ] = CLOUDINIT_CLEAN_FILE_TMPL.format( + header=header, cfg_files=json.dumps(sorted(cfg_files)) + ) + with unittest.mock.patch( + 'subiquity.cloudinit.open', + mock.mock_open( + read_data=json.dumps( + {'features': {'NETPLAN_CONFIG_ROOT_READ_ONLY': False}} + ) + ), + ): + for (cpath, content, perms) in model._cloud_init_files(): + if isinstance(expected_files[cpath], re.Pattern): + self.assertIsNotNone( + expected_files[cpath].match(content) + ) + else: + self.assertEqual(expected_files[cpath], content) def test_validatecloudconfig_schema(self): model = self.make_model()