From 7d991ecb9f87f863e1e78ce7e2d06c4d2f9568db Mon Sep 17 00:00:00 2001 From: Anton Hvornum Date: Wed, 17 Nov 2021 18:02:20 +0000 Subject: Fixing broken encryption support in GRUB (#724) * Added multiple `partprobe` calls and added a `.partprobe()` function on partitions, filesystem and blockdevice. * Adding retry attempts to all UUID related operations tied to the boot process * Tweaked logging for mounting and disk related operations * Removed potential SysCall exception disruptor causing exceptions to go by unnoticed * Increased the start position from 1MiB to 5MiB of /boot partition * Optimized the GRUB installation & config code * Improved Partition().uuid to never return None. Instead it will raise an exception if it can't get a PARTUUID within X retries with Y delay per attempt. * Increased sleep timer for partition uuid retrieval, because even with a 3 second sleep it wasn't long enough even on fast devices. * Make GRUB install to /dev/sda instead of /dev/sda1. * Added 10 retries for retreiving PARTUUID with a one second sleep. Instead of increasing the sleep simply add more retries until we find a good balance on slower disks. --- archinstall/lib/disk/filesystem.py | 17 ++++++++++++----- archinstall/lib/disk/partition.py | 26 ++++++++++++++++++-------- archinstall/lib/disk/user_guides.py | 8 ++++---- archinstall/lib/general.py | 16 ++++++---------- archinstall/lib/installer.py | 24 ++++++++++++++---------- 5 files changed, 54 insertions(+), 37 deletions(-) (limited to 'archinstall/lib') diff --git a/archinstall/lib/disk/filesystem.py b/archinstall/lib/disk/filesystem.py index 2eb1864d..fe7be498 100644 --- a/archinstall/lib/disk/filesystem.py +++ b/archinstall/lib/disk/filesystem.py @@ -33,12 +33,18 @@ class Filesystem: return True def partuuid_to_index(self, uuid): - output = json.loads(SysCommand(f"lsblk --json -o+PARTUUID {self.blockdevice.device}").decode('UTF-8')) + for i in range(10): + self.partprobe() + output = json.loads(SysCommand(f"lsblk --json -o+PARTUUID {self.blockdevice.device}").decode('UTF-8')) + + for device in output['blockdevices']: + for index, partition in enumerate(device['children']): + if (partuuid := partition.get('partuuid', None)) and partuuid.lower() == uuid: + return index + + time.sleep(1) - for device in output['blockdevices']: - for index, partition in enumerate(device['children']): - if partition['partuuid'].lower() == uuid: - return index + raise DiskError(f"Failed to convert PARTUUID {uuid} to a partition index number on blockdevice {self.blockdevice.device}") def load_layout(self, layout :dict): from ..luks import luks2 @@ -105,6 +111,7 @@ class Filesystem: partition['device_instance'].format(partition['filesystem']['format'], options=partition.get('options', [])) if partition.get('boot', False): + log(f"Marking partition {partition['device_instance']} as bootable.") self.set(self.partuuid_to_index(partition['device_instance'].uuid), 'boot on') def find_partition(self, mountpoint): diff --git a/archinstall/lib/disk/partition.py b/archinstall/lib/disk/partition.py index 3630a6f4..faa0838f 100644 --- a/archinstall/lib/disk/partition.py +++ b/archinstall/lib/disk/partition.py @@ -147,14 +147,17 @@ class Partition: This is more reliable than relying on /dev/disk/by-partuuid as it doesn't seam to be able to detect md raid partitions. """ + for i in range(10): + self.partprobe() - partuuid_struct = SysCommand(f'lsblk -J -o+PARTUUID {self.path}') - if not partuuid_struct.exit_code == 0: - raise DiskError(f"Could not get PARTUUID for {self.path}: {partuuid_struct}") + partuuid_struct = SysCommand(f'lsblk -J -o+PARTUUID {self.path}') + if partuuid_struct.exit_code == 0: + if partition_information := next(iter(json.loads(partuuid_struct.decode('UTF-8'))['blockdevices']), None): + return partition_information.get('partuuid', None) - for partition in json.loads(partuuid_struct.decode('UTF-8'))['blockdevices']: - return partition.get('partuuid', None) - return None + time.sleep(1) + + raise DiskError(f"Could not get PARTUUID for {self.path} using 'lsblk -J -o+PARTUUID {self.path}'") @property def encrypted(self): @@ -177,6 +180,9 @@ class Partition: # raise DiskError(f'Could not find appropriate parent for encrypted partition {self}') return self.path + def partprobe(self): + SysCommand(f'bash -c "partprobe"') + def detect_inner_filesystem(self, password): log(f'Trying to detect inner filesystem format on {self} (This might take a while)', level=logging.INFO) from ..luks import luks2 @@ -315,9 +321,13 @@ class Partition: try: if options: - SysCommand(f"/usr/bin/mount -o {options} {self.path} {target}") + mnt_handle = SysCommand(f"/usr/bin/mount -o {options} {self.path} {target}") else: - SysCommand(f"/usr/bin/mount {self.path} {target}") + mnt_handle = SysCommand(f"/usr/bin/mount {self.path} {target}") + + # TODO: Should be redundant to check for exit_code + if mnt_handle.exit_code != 0: + raise DiskError(f"Could not mount {self.path} to {target} using options {options}") except SysCallError as err: raise err diff --git a/archinstall/lib/disk/user_guides.py b/archinstall/lib/disk/user_guides.py index a70a82db..e9332b7b 100644 --- a/archinstall/lib/disk/user_guides.py +++ b/archinstall/lib/disk/user_guides.py @@ -23,7 +23,7 @@ def suggest_single_disk_layout(block_device, default_filesystem=None): layout[block_device.path]['partitions'].append({ # Boot "type" : "primary", - "start" : "1MiB", + "start" : "5MiB", "size" : "513MiB", "boot" : True, "encrypted" : False, @@ -36,7 +36,7 @@ def suggest_single_disk_layout(block_device, default_filesystem=None): layout[block_device.path]['partitions'].append({ # Root "type" : "primary", - "start" : "513MiB", + "start" : "518MiB", "encrypted" : False, "format" : True, "size" : "100%" if (using_subvolumes or block_device.size < MIN_SIZE_TO_ALLOW_HOME_PART) else f"{min(block_device.size, 20)*1024}MiB", @@ -115,7 +115,7 @@ def suggest_multi_disk_layout(block_devices, default_filesystem=None): layout[root_device.path]['partitions'].append({ # Boot "type" : "primary", - "start" : "1MiB", + "start" : "5MiB", "size" : "513MiB", "boot" : True, "encrypted" : False, @@ -128,7 +128,7 @@ def suggest_multi_disk_layout(block_devices, default_filesystem=None): layout[root_device.path]['partitions'].append({ # Root "type" : "primary", - "start" : "513MiB", + "start" : "518MiB", "encrypted" : False, "format" : True, "size" : "100%", diff --git a/archinstall/lib/general.py b/archinstall/lib/general.py index 21683425..f3773755 100644 --- a/archinstall/lib/general.py +++ b/archinstall/lib/general.py @@ -382,18 +382,14 @@ class SysCommand: if self.session: return True - try: - self.session = SysCommandWorker(self.cmd, callbacks=self._callbacks, peak_output=self.peak_output, environment_vars=self.environment_vars) + self.session = SysCommandWorker(self.cmd, callbacks=self._callbacks, peak_output=self.peak_output, environment_vars=self.environment_vars) - while self.session.ended is None: - self.session.poll() + while self.session.ended is None: + self.session.poll() - if self.peak_output: - sys.stdout.write('\n') - sys.stdout.flush() - - except SysCallError: - return False + if self.peak_output: + sys.stdout.write('\n') + sys.stdout.flush() return True diff --git a/archinstall/lib/installer.py b/archinstall/lib/installer.py index 7ac80047..fff62201 100644 --- a/archinstall/lib/installer.py +++ b/archinstall/lib/installer.py @@ -174,16 +174,17 @@ class Installer: mountpoints[partition['mountpoint']] = partition for mountpoint in sorted(mountpoints.keys()): - log(f"Mounting {mountpoint} to {self.target}{mountpoint}", level=logging.INFO) if mountpoints[mountpoint].get('encrypted', False): loopdev = storage.get('ENC_IDENTIFIER', 'ai') + 'loop' if not (password := mountpoints[mountpoint].get('!password', None)): raise RequirementError(f"Missing mountpoint {mountpoint} encryption password in layout: {mountpoints[mountpoint]}") with luks2(mountpoints[mountpoint]['device_instance'], loopdev, password, auto_unmount=False) as unlocked_device: + log(f"Mounting {mountpoint} to {self.target}{mountpoint} using {unlocked_device}", level=logging.INFO) unlocked_device.mount(f"{self.target}{mountpoint}") else: + log(f"Mounting {mountpoint} to {self.target}{mountpoint} using {mountpoints[mountpoint]['device_instance']}", level=logging.INFO) mountpoints[mountpoint]['device_instance'].mount(f"{self.target}{mountpoint}") time.sleep(1) @@ -607,25 +608,28 @@ class Installer: self.pacstrap('grub') # no need? if real_device := self.detect_encryption(root_partition): - _file = "/etc/default/grub" root_uuid = SysCommand(f"blkid -s UUID -o value {real_device.path}").decode().rstrip() + _file = "/etc/default/grub" add_to_CMDLINE_LINUX = f"sed -i 's/GRUB_CMDLINE_LINUX=\"\"/GRUB_CMDLINE_LINUX=\"cryptdevice=UUID={root_uuid}:cryptlvm\"/'" enable_CRYPTODISK = "sed -i 's/#GRUB_ENABLE_CRYPTODISK=y/GRUB_ENABLE_CRYPTODISK=y/'" + log(f"Using UUID {root_uuid} of {real_device} as encrypted root identifier.", level=logging.INFO) SysCommand(f"/usr/bin/arch-chroot {self.target} {add_to_CMDLINE_LINUX} {_file}") SysCommand(f"/usr/bin/arch-chroot {self.target} {enable_CRYPTODISK} {_file}") + log(f"GRUB uses {boot_partition.path} as the boot partition.", level=logging.INFO) if has_uefi(): - self.pacstrap('efibootmgr') # TODO: Do we need? - SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB') - SysCommand(f'/usr/bin/arch-chroot {self.target} grub-mkconfig -o /boot/grub/grub.cfg') - self.helper_flags['bootloader'] = True - return True + self.pacstrap('efibootmgr') # TODO: Do we need? Yes, but remove from minimal_installation() instead? + if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=x86_64-efi --efi-directory=/boot --bootloader-id=GRUB')).exit_code == 0: + raise DiskError(f"Could not install GRUB to {self.target}/boot: {handle}") else: - SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=i386-pc --recheck {boot_partition.path}') - SysCommand(f'/usr/bin/arch-chroot {self.target} grub-mkconfig -o /boot/grub/grub.cfg') - self.helper_flags['bootloader'] = True + if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-install --target=i386-pc --recheck {boot_partition.parent}')).exit_code == 0: + raise DiskError(f"Could not install GRUB to {boot_partition.path}: {handle}") + + if not (handle := SysCommand(f'/usr/bin/arch-chroot {self.target} grub-mkconfig -o /boot/grub/grub.cfg')).exit_code == 0: + raise DiskError(f"Could not configure GRUB: {handle}") + self.helper_flags['bootloader'] = True elif bootloader == 'efistub': self.pacstrap('efibootmgr') -- cgit v1.2.3-54-g00ecf