diff --git a/ironic_python_agent/efi_utils.py b/ironic_python_agent/efi_utils.py index da5e97b69..d866fb6bf 100644 --- a/ironic_python_agent/efi_utils.py +++ b/ironic_python_agent/efi_utils.py @@ -275,8 +275,19 @@ def get_boot_records(): :return: an iterator yielding pairs (boot number, boot record). """ - efi_output = utils.execute('efibootmgr', '-v') - for line in efi_output[0].split('\n'): + # Invokes binary=True so we get a python3 bytestream back. + efi_output = utils.execute('efibootmgr', '-v', binary=True) + if hasattr(efi_output[0], 'decode'): + # Bytes must be decoded before regex can be run and + # matching to work as intended. + # Also ignore errors on decoding, as we can basically get + # garbage out of the nvram record, this way we don't fail + # hard on unrelated records. + cmd_output = efi_output[0].decode('utf-16', errors='ignore') + else: + # This is a fallback. + cmd_output = str(efi_output[0]) + for line in cmd_output.split('\n'): match = _ENTRY_LABEL.match(line) if match is not None: yield (match[1], match[2]) @@ -293,7 +304,7 @@ def add_boot_record(device, efi_partition, loader, label): # https://linux.die.net/man/8/efibootmgr utils.execute('efibootmgr', '-v', '-c', '-d', device, '-p', str(efi_partition), '-w', '-L', label, - '-l', loader) + '-l', loader, binary=True) def remove_boot_record(boot_num): @@ -301,7 +312,7 @@ def remove_boot_record(boot_num): :param boot_num: the number of the boot record """ - utils.execute('efibootmgr', '-b', boot_num, '-B') + utils.execute('efibootmgr', '-b', boot_num, '-B', binary=True) def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 46be48c2a..949ff494d 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -575,7 +575,7 @@ def _efi_boot_setup(device, efi_system_part_uuid=None, target_boot_mode=None): if boot.current_boot_mode == 'uefi': try: - utils.execute('efibootmgr', '--version') + utils.execute('efibootmgr', '--version', binary=True) except FileNotFoundError: LOG.warning("efibootmgr is not available in the ramdisk") else: diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 46afe0ef8..3423c9bb1 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -234,17 +234,17 @@ class TestImageExtension(base.IronicAgentTest): ('', ''), ('', ''), ('', ''), ('', '')]) - expected = [mock.call('efibootmgr', '--version'), + expected = [mock.call('efibootmgr', '--version', binary=True), mock.call('partx', '-av', '/dev/fake', attempts=3, delay_on_retry=True), mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -282,17 +282,17 @@ class TestImageExtension(base.IronicAgentTest): ('', ''), ('', ''), ('', ''), ('', '')]) - expected = [mock.call('efibootmgr', '--version'), + expected = [mock.call('efibootmgr', '--version', binary=True), mock.call('partx', '-av', '/dev/fake', attempts=3, delay_on_retry=True), mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -339,18 +339,18 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) ('', ''), ('', ''), ('', ''), ('', '')]) - expected = [mock.call('efibootmgr', '--version'), + expected = [mock.call('efibootmgr', '--version', binary=True), mock.call('partx', '-av', '/dev/fake', attempts=3, delay_on_retry=True), mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0000', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -401,19 +401,19 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 (stdout_msg, ''), ('', ''), ('', ''), ('', ''), ('', ''), ('', '')]) - expected = [mock.call('efibootmgr', '--version'), + expected = [mock.call('efibootmgr', '--version', binary=True), mock.call('partx', '-av', '/dev/fake', attempts=3, delay_on_retry=True), mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), - mock.call('efibootmgr', '-b', '0004', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0000', '-B', binary=True), + mock.call('efibootmgr', '-b', '0004', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -454,21 +454,21 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 ('', ''), ('', ''), ('', '')]) - expected = [mock.call('efibootmgr', '--version'), + expected = [mock.call('efibootmgr', '--version', binary=True), mock.call('partx', '-av', '/dev/fake', attempts=3, delay_on_retry=True), mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic2', '-l', - '\\WINDOWS\\system32\\winload.efi'), + '\\WINDOWS\\system32\\winload.efi', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -517,7 +517,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 root_uuid=self.fake_root_uuid, efi_system_part_uuid=None).join() self.assertIsNotNone(result.command_error) - expected = [mock.call('efibootmgr', '--version')] + expected = [mock.call('efibootmgr', '--version', binary=True)] mock_execute.assert_has_calls(expected) @mock.patch.object(os.path, 'exists', lambda *_: True) diff --git a/ironic_python_agent/tests/unit/test_efi_utils.py b/ironic_python_agent/tests/unit/test_efi_utils.py index 64de61bce..4752f7421 100644 --- a/ironic_python_agent/tests/unit/test_efi_utils.py +++ b/ironic_python_agent/tests/unit/test_efi_utils.py @@ -120,11 +120,11 @@ class TestRunEfiBootmgr(base.IronicAgentTest): self.fake_dev, self.fake_efi_system_part, self.fake_dir) - expected = [mock.call('efibootmgr', '-v'), + expected = [mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', self.fake_efi_system_part, '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI')] + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True)] self.assertIsNone(result) mock_execute.assert_has_calls(expected) @@ -184,11 +184,11 @@ class TestManageUefi(base.IronicAgentTest): expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -219,30 +219,34 @@ class TestManageUefi(base.IronicAgentTest): # at the start of the file, where as Red Hat *does* csv_file_data = u'shimx64.efi,Vendor String,,Grub2MadeUSDoThis\n' # This test also handles deleting a pre-existing matching vendor - # string in advance. + # string in advance. This string also includes a UTF16 character + # *on* purpose, to force proper decoding to be tested and garbage + # characters which can be found in OVMF test VM NVRAM records. dupe_entry = """ BootCurrent: 0001 Timeout: 0 seconds BootOrder: 0000,00001 -Boot0000* Vendor String HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) -Boot0001 Vendor String HD(2,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) -Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) +Boot0000 UTF16ΓΏ HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0001* Vendor String HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0002 Vendor String HD(2,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0003: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)N.....YM....R,Y. """ # noqa This is a giant literal string for testing. - mock_execute.side_effect = iter([('', ''), (dupe_entry, ''), + mock_execute.side_effect = iter([('', ''), + (dupe_entry.encode('utf-16'), ''), ('', ''), ('', ''), ('', ''), ('', ''), ('', '')]) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), - mock.call('efibootmgr', '-b', '0001', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0001', '-B', binary=True), + mock.call('efibootmgr', '-b', '0002', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'Vendor String', '-l', - '\\EFI\\vendor\\shimx64.efi'), + '\\EFI\\vendor\\shimx64.efi', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -274,11 +278,11 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) expected = [mock.call('mount', '/dev/fakenvme0p1', self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/fakenvme0', '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -309,11 +313,11 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -360,14 +364,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) attempts=3, delay_on_retry=True), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sda3', '-p', '3', '-w', '-L', 'ironic1 (RAID, part0)', - '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), - mock.call('efibootmgr', '-v'), + '-l', '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sdb3', '-p', '3', '-w', '-L', 'ironic1 (RAID, part1)', - '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), + '-l', '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -425,7 +429,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -462,7 +466,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True)]