From cabb8dda8e682b04f9fa25a2833dbd53b6f73cfe Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Tue, 1 Nov 2022 13:48:32 +0100 Subject: [PATCH 1/3] add the ability to only generate a subset of curtin fs actions DEVICES == everything up to and including partitioning FORMAT_MOUNT == formatting and mounting --- subiquity/models/filesystem.py | 51 +++++++++++++++++++--- subiquity/models/tests/test_filesystem.py | 50 +++++++++++++++++++++ subiquity/server/controllers/filesystem.py | 3 +- 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 785e9031..4ee50b74 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -17,6 +17,7 @@ from abc import ABC, abstractmethod import attr import collections import copy +import enum import fnmatch import itertools import logging @@ -1025,6 +1026,25 @@ class PartitionAlignmentData: ebr_space: int = 0 +class ActionRenderMode(enum.Enum): + # The default for FilesystemModel.render() is to render actions + # for devices that have changes, but not e.g. a hard drive that + # will be untouched by the installation process. + DEFAULT = enum.auto() + # ALL means render actions for all model objects. This is used to + # send information to the client. + ALL = enum.auto() + # DEVICES means to just render actions for setting up block + # devices, e.g. partitioning disks and assembling RAIDs but not + # any format or mount actions. + DEVICES = enum.auto() + # FORMAT_MOUNT means to just render actions to format and mount + # the block devices. References to block devices will be replaced + # by "type: device" actions that just refer to the block devices + # by path. + FORMAT_MOUNT = enum.auto() + + class FilesystemModel(object): target = None @@ -1371,7 +1391,8 @@ class FilesystemModel(object): return objs - def _render_actions(self, include_all=False): + def _render_actions(self, + mode: ActionRenderMode = ActionRenderMode.DEFAULT): # The curtin storage config has the constraint that an action must be # preceded by all the things that it depends on. We handle this by # repeatedly iterating over all actions and checking if we can emit @@ -1425,9 +1446,11 @@ class FilesystemModel(object): mountpoints = {m.path: m.id for m in self.all_mounts()} log.debug('mountpoints %s', mountpoints) - work = [ - a for a in self._actions - if not getattr(a, 'preserve', False) or include_all + if mode == ActionRenderMode.ALL: + work = list(self._actions) + else: + work = [ + a for a in self._actions if not getattr(a, 'preserve', False) ] while work: @@ -1444,13 +1467,29 @@ class FilesystemModel(object): raise Exception("\n".join(msg)) work = next_work + if mode == ActionRenderMode.DEVICES: + r = [act for act in r if act['type'] not in ('format', 'mount')] + if mode == ActionRenderMode.FORMAT_MOUNT: + r = [act for act in r if act['type'] in ('format', 'mount')] + devices = [] + for act in r: + if act['type'] == 'format': + device = { + 'type': 'device', + 'id': 'synth-device-{}'.format(len(devices)), + 'path': self._one(id=act['volume']).path, + } + devices.append(device) + act['volume'] = device['id'] + r = devices + r + return r - def render(self): + def render(self, mode: ActionRenderMode = ActionRenderMode.DEFAULT): config = { 'storage': { 'version': self.storage_version, - 'config': self._render_actions(), + 'config': self._render_actions(mode=mode), }, } if self.swap is not None: diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index b45b1800..6278ec5e 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -20,6 +20,7 @@ import attr from parameterized import parameterized from subiquity.models.filesystem import ( + ActionRenderMode, Bootloader, dehumanize_size, Disk, @@ -932,6 +933,55 @@ class TestAutoInstallConfig(unittest.TestCase): self.assertTrue(disk2.id not in rendered_ids) self.assertTrue(disk2p1.id not in rendered_ids) + def test_render_all_does_include_unreferenced(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk2 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + disk2p1 = make_partition(model, disk2, preserve=True) + fs = model.add_filesystem(disk1p1, 'ext4') + model.add_mount(fs, '/') + rendered_ids = { + action['id'] + for action in model._render_actions(ActionRenderMode.ALL) + } + self.assertTrue(disk1.id in rendered_ids) + self.assertTrue(disk1p1.id in rendered_ids) + self.assertTrue(disk2.id in rendered_ids) + self.assertTrue(disk2p1.id in rendered_ids) + + def test_render_devices_skips_format_mount(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + fs = model.add_filesystem(disk1p1, 'ext4') + mnt = model.add_mount(fs, '/') + rendered_ids = { + action['id'] + for action in model._render_actions(ActionRenderMode.DEVICES) + } + self.assertTrue(disk1.id in rendered_ids) + self.assertTrue(disk1p1.id in rendered_ids) + self.assertTrue(fs.id not in rendered_ids) + self.assertTrue(mnt.id not in rendered_ids) + + def test_render_format_mount(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + disk1p1.path = '/dev/vda1' + fs = model.add_filesystem(disk1p1, 'ext4') + mnt = model.add_mount(fs, '/') + actions = model._render_actions(ActionRenderMode.FORMAT_MOUNT) + rendered_by_id = {action['id']: action for action in actions} + self.assertTrue(disk1.id not in rendered_by_id) + self.assertTrue(disk1p1.id not in rendered_by_id) + self.assertTrue(fs.id in rendered_by_id) + self.assertTrue(mnt.id in rendered_by_id) + vol_id = rendered_by_id[fs.id]['volume'] + self.assertEqual(rendered_by_id[vol_id]['type'], 'device') + self.assertEqual(rendered_by_id[vol_id]['path'], '/dev/vda1') + def test_render_includes_all_partitions(self): model = make_model(Bootloader.NONE) disk1 = make_disk(model, preserve=True) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 4e2fbbdb..7e3b814f 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -73,6 +73,7 @@ from subiquity.common.types import ( StorageResponseV2, ) from subiquity.models.filesystem import ( + ActionRenderMode, align_up, align_down, _Device, @@ -370,7 +371,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): bootloader=self.model.bootloader, error_report=self.full_probe_error(), orig_config=self.model._orig_config, - config=self.model._render_actions(include_all=True), + config=self.model._render_actions(mode=ActionRenderMode.ALL), blockdev=self.model._probe_data['blockdev'], dasd=self.model._probe_data.get('dasd', {}), storage_version=self.model.storage_version) From 5dafdb916d08003d650367e1e6431027a41da282 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Thu, 3 Nov 2022 10:59:57 +0100 Subject: [PATCH 2/3] call into snapd to set up encryption when required --- examples/snaps/v2-changes-6/0000.json | 1 + examples/snaps/v2-changes-6/0001.json | 1 + examples/snaps/v2-changes-6/0002.json | 1 + examples/snaps/v2-changes-6/0003.json | 1 + examples/snaps/v2-changes-6/0004.json | 1 + examples/snaps/v2-changes-6/0005.json | 1 + scripts/runtests.sh | 9 +++++- scripts/validate-yaml.py | 8 ++++-- subiquity/models/filesystem.py | 13 +++++++++ subiquity/server/controllers/filesystem.py | 24 ++++++++++++++++ subiquity/server/controllers/install.py | 28 +++++++++++++++---- .../controllers/tests/test_filesystem.py | 16 +++++++++++ subiquitycore/snapd.py | 2 ++ 13 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 examples/snaps/v2-changes-6/0000.json create mode 100644 examples/snaps/v2-changes-6/0001.json create mode 100644 examples/snaps/v2-changes-6/0002.json create mode 100644 examples/snaps/v2-changes-6/0003.json create mode 100644 examples/snaps/v2-changes-6/0004.json create mode 100644 examples/snaps/v2-changes-6/0005.json diff --git a/examples/snaps/v2-changes-6/0000.json b/examples/snaps/v2-changes-6/0000.json new file mode 100644 index 00000000..35598074 --- /dev/null +++ b/examples/snaps/v2-changes-6/0000.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z"}],"ready":false,"spawn-time":"2022-10-28T10:04:23.859232232Z"}} diff --git a/examples/snaps/v2-changes-6/0001.json b/examples/snaps/v2-changes-6/0001.json new file mode 100644 index 00000000..35598074 --- /dev/null +++ b/examples/snaps/v2-changes-6/0001.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z"}],"ready":false,"spawn-time":"2022-10-28T10:04:23.859232232Z"}} diff --git a/examples/snaps/v2-changes-6/0002.json b/examples/snaps/v2-changes-6/0002.json new file mode 100644 index 00000000..35598074 --- /dev/null +++ b/examples/snaps/v2-changes-6/0002.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z"}],"ready":false,"spawn-time":"2022-10-28T10:04:23.859232232Z"}} diff --git a/examples/snaps/v2-changes-6/0003.json b/examples/snaps/v2-changes-6/0003.json new file mode 100644 index 00000000..35598074 --- /dev/null +++ b/examples/snaps/v2-changes-6/0003.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z"}],"ready":false,"spawn-time":"2022-10-28T10:04:23.859232232Z"}} diff --git a/examples/snaps/v2-changes-6/0004.json b/examples/snaps/v2-changes-6/0004.json new file mode 100644 index 00000000..35598074 --- /dev/null +++ b/examples/snaps/v2-changes-6/0004.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Doing","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z"}],"ready":false,"spawn-time":"2022-10-28T10:04:23.859232232Z"}} diff --git a/examples/snaps/v2-changes-6/0005.json b/examples/snaps/v2-changes-6/0005.json new file mode 100644 index 00000000..80d201a3 --- /dev/null +++ b/examples/snaps/v2-changes-6/0005.json @@ -0,0 +1 @@ +{"type":"sync","status-code":200,"status":"OK","result":{"id":"6","kind":"install-step-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Done","tasks":[{"id":"45","kind":"install-setup-storage-encryption","summary":"Setup storage encryption for installing system \"classic\"","status":"Done","progress":{"label":"","done":1,"total":1},"spawn-time":"2022-10-28T10:04:23.859233629Z","ready-time":"2022-10-28T10:04:29.092142293Z"}],"ready":true,"spawn-time":"2022-10-28T10:04:23.859232232Z","ready-time":"2022-10-28T10:04:29.092145478Z","data":{"encrypted-devices":{"system-data":"/dev/mapper/ubuntu-data","system-save":"/dev/mapper/ubuntu-save"}}}} diff --git a/scripts/runtests.sh b/scripts/runtests.sh index 13a2df5d..34f27ae3 100755 --- a/scripts/runtests.sh +++ b/scripts/runtests.sh @@ -25,7 +25,14 @@ validate () { fi if [ "${mode}" = "install" ]; then - python3 scripts/validate-yaml.py "$tmpdir"/var/log/installer/curtin-install/subiquity-partitioning.conf + cfgs= + for stage in partitioning formatting; do + cfg="$tmpdir"/var/log/installer/curtin-install/subiquity-$stage.conf + if [ -e $cfg ]; then + cfgs="$cfgs $cfg" + fi + done + python3 scripts/validate-yaml.py $cfgs if [ ! -e $tmpdir/subiquity-client-debug.log ] || [ ! -e $tmpdir/subiquity-server-debug.log ]; then echo "log file not created" exit 1 diff --git a/scripts/validate-yaml.py b/scripts/validate-yaml.py index 39dfd2a6..34df2199 100644 --- a/scripts/validate-yaml.py +++ b/scripts/validate-yaml.py @@ -75,13 +75,17 @@ class StorageChecker: assert '/' in self.path_to_mount -config = yaml.safe_load(open(sys.argv[1])) def main(): storage_checker = StorageChecker() - for action in config['storage']['config']: + actions = [] + for path in sys.argv[1:]: + config = yaml.safe_load(open(path)) + actions.extend(config['storage']['config']) + + for action in actions: try: storage_checker.check(action) except Exception: diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 4ee50b74..e4785510 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -959,6 +959,19 @@ class DM_Crypt: return self.volume.size - LUKS_OVERHEAD +@fsobj("device") +class ArbitraryDevice(_Device): + ptable = attr.ib(default=None) + path = attr.ib(default=None) + + @property + def size(self): + return 0 + + ok_for_raid = False + ok_for_lvm_vg = False + + @fsobj("format") class Filesystem: fstype = attr.ib() diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 7e3b814f..8ad6fa42 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -74,6 +74,7 @@ from subiquity.common.types import ( ) from subiquity.models.filesystem import ( ActionRenderMode, + ArbitraryDevice, align_up, align_down, _Device, @@ -142,6 +143,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): self._core_boot_classic_error: str = '' self._system_mounter: Optional[Mounter] = None self._role_to_device: Dict[snapdapi.Role: _Device] = {} + self.use_tpm: bool = False def load_autoinstall_data(self, data): log.debug("load_autoinstall_data %s", data) @@ -496,6 +498,27 @@ class FilesystemController(SubiquityController, FilesystemManipulator): on_volume_structure.device = self._role_to_device[role].path return {key: on_volume} + @with_context(description="configuring TPM-backed full disk encryption") + async def setup_encryption(self, context): + label = self.app.base_model.source.current.snapd_system_label + result = await snapdapi.post_and_wait( + self.app.snapdapi, + self.app.snapdapi.v2.systems[label].POST, + snapdapi.SystemActionRequest( + action=snapdapi.SystemAction.INSTALL, + step=snapdapi.SystemActionStep.SETUP_STORAGE_ENCRYPTION, + on_volumes=self._on_volumes())) + role_to_encrypted_device = result['encrypted-devices'] + for role, enc_path in role_to_encrypted_device.items(): + role = snapdapi.Role(role) + arb_device = ArbitraryDevice(m=self.model, path=enc_path) + self.model._actions.append(arb_device) + part = self._role_to_device[role] + for fs in self.model._all(type='format'): + if fs.volume == part: + fs.volume = arb_device + self._role_to_device[role] = arb_device + @with_context(description="making system bootable") async def finish_install(self, context): label = self.app.base_model.source.current.snapd_system_label @@ -510,6 +533,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): async def guided_POST(self, data: GuidedChoice) -> StorageResponse: log.debug(data) if self._system is not None: + self.use_tpm = data.use_tpm self.apply_system(data.disk_id) await self.configured() else: diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py index 106fa1a9..d20fa9bb 100644 --- a/subiquity/server/controllers/install.py +++ b/subiquity/server/controllers/install.py @@ -14,6 +14,7 @@ # along with this program. If not, see . import asyncio +import functools import json import logging import os @@ -39,6 +40,7 @@ from subiquity.common.types import ( from subiquity.journald import ( journald_listen, ) +from subiquity.models.filesystem import ActionRenderMode from subiquity.server.controller import ( SubiquityController, ) @@ -187,10 +189,12 @@ class InstallController(SubiquityController): } } - def acquire_filesystem_config(self, step: CurtinPartitioningStep, - ) -> Dict[str, Any]: + def acquire_filesystem_config( + self, step: CurtinPartitioningStep, + mode: ActionRenderMode = ActionRenderMode.DEFAULT + ) -> Dict[str, Any]: cfg = self.acquire_initial_config(step) - cfg.update(self.model.filesystem.render()) + cfg.update(self.model.filesystem.render(mode=mode)) cfg['storage']['device_map_path'] = str(step.device_map_path) return cfg @@ -255,10 +259,24 @@ class InstallController(SubiquityController): ] if self.model.source.current.snapd_system_label: fs_controller = self.app.controllers.Filesystem - steps.extend([ + steps.append( make_curtin_step( name="partitioning", stages=["partitioning"], - acquire_config=self.acquire_filesystem_config, + acquire_config=functools.partial( + self.acquire_filesystem_config, + mode=ActionRenderMode.DEVICES), + cls=CurtinPartitioningStep, + device_map_path=logs_dir / "device-map.json", + ).run, + ) + if fs_controller.use_tpm: + steps.append(fs_controller.setup_encryption) + steps.extend([ + make_curtin_step( + name="formatting", stages=["partitioning"], + acquire_config=functools.partial( + self.acquire_filesystem_config, + mode=ActionRenderMode.FORMAT_MOUNT), cls=CurtinPartitioningStep, device_map_path=logs_dir / "device-map.json", ).run, diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 213f96dc..4814b9c2 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -538,6 +538,22 @@ class TestCoreBootInstallMethods(IsolatedAsyncioTestCase): self.assertEqual(set(mounts.keys()), {'/', '/boot', '/boot/efi'}) device_map = {p.id: random_string() for p in disk.partitions()} self.fsc.update_devices(device_map) + + with mock.patch.object(snapdapi, "post_and_wait", + new_callable=mock.AsyncMock) as mocked: + mocked.return_value = { + 'encrypted-devices': { + snapdapi.Role.SYSTEM_DATA: 'enc-system-data', + }, + } + await self.fsc.setup_encryption(context=self.fsc.context) + + # setup_encryption mutates the filesystem model objects to + # reference the newly created encrypted objects so re-read the + # mount to device mapping. + mounts = {m.path: m.device.volume for m in model._all(type='mount')} + self.assertEqual(mounts['/'].path, 'enc-system-data') + with mock.patch.object(snapdapi, "post_and_wait", new_callable=mock.AsyncMock) as mocked: await self.fsc.finish_install(context=self.fsc.context) diff --git a/subiquitycore/snapd.py b/subiquitycore/snapd.py index 83efb0fb..05cbb156 100644 --- a/subiquitycore/snapd.py +++ b/subiquitycore/snapd.py @@ -152,6 +152,8 @@ class FakeSnapdConnection: change = "15" else: change = "5" + elif step == 'setup-storage-encryption': + change = "6" if change is not None: return _FakeMemoryResponse({ "type": "async", From 5971541bebb8815d132f5a9b6b486884a471361e Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Fri, 11 Nov 2022 14:43:54 +1300 Subject: [PATCH 3/3] use @ogayot's better bash Co-authored-by: Olivier Gayot --- scripts/runtests.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/runtests.sh b/scripts/runtests.sh index 34f27ae3..e4563de8 100755 --- a/scripts/runtests.sh +++ b/scripts/runtests.sh @@ -25,14 +25,14 @@ validate () { fi if [ "${mode}" = "install" ]; then - cfgs= + cfgs=() for stage in partitioning formatting; do cfg="$tmpdir"/var/log/installer/curtin-install/subiquity-$stage.conf - if [ -e $cfg ]; then - cfgs="$cfgs $cfg" + if [ -e "$cfg" ]; then + cfgs+=("$cfg") fi done - python3 scripts/validate-yaml.py $cfgs + python3 scripts/validate-yaml.py "${cfgs[@]}" if [ ! -e $tmpdir/subiquity-client-debug.log ] || [ ! -e $tmpdir/subiquity-server-debug.log ]; then echo "log file not created" exit 1