From 33bdec184f08d955979484d63bb60fba3942adca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 20 Dec 2022 10:20:35 +0100 Subject: [PATCH 1/2] ukify: catch error when loading foreign pe file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autodetection code is supposed to throw ValueError when it cannot figure out the version so that we fall back to the next method. With the patch: Kernel version not specified, starting autodetection šŸ˜–. Real-Mode Kernel Header magic not found + readelf --notes vmlinuz/arm64/vmlinuz-6.0.9-300.fc37.aarch64 readelf: vmlinuz/arm64/vmlinuz-6.0.9-300.fc37.aarch64: Error: Not an ELF file - it has the wrong magic bytes at the start Found uname version: 6.0.9-300.fc37.aarch64 --- src/ukify/ukify.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index e9e5d13d13..cd27f92a97 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -161,7 +161,10 @@ class Uname: ] print('+', shell_join(cmd)) - notes = subprocess.check_output(cmd, text=True) + try: + notes = subprocess.check_output(cmd, stderr=subprocess.PIPE, text=True) + except subprocess.CalledProcessError as e: + raise ValueError(e.stderr.strip()) from e if not (m := re.search(cls.NOTES_PATTERN, notes, re.MULTILINE)): raise ValueError('Cannot find Linux version note') From 095ff238d064f7dfbf75c9cd13834ed892fe5fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 20 Dec 2022 10:38:01 +0100 Subject: [PATCH 2/2] ukify: check early if inputs exist and are readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's much nicer for the user if we fail early instead of doing partial processing if we cannot read some input. We can't do those checks immediately from argparse.Parser.parse_args(), because we want to fully process the commandline first. In particular, even with invalid args, if --help is specified somewhere, we want to handle that. Thus, we need to delay the checks after argparse.Parser.parse_args() returns. Ukify didn't have type annotations on functions, but it probably should. Jƶrg's suggested correction included them and we might just as well start here. --- src/ukify/ukify.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index cd27f92a97..0c4461c9a9 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -64,6 +64,15 @@ def shell_join(cmd): return ' '.join(shlex.quote(str(x)) for x in cmd) +def path_is_readable(s: str | None) -> pathlib.Path | None: + """Convert a filename string to a Path and verify access.""" + if s is None: + return None + p = pathlib.Path(s) + p.open().close() + return p + + def pe_executable_size(filename): import pefile @@ -675,26 +684,36 @@ usage: ukify [optionsā€¦] linux initrdā€¦ opts = p.parse_args(args) + path_is_readable(opts.linux) + for initrd in opts.initrd or (): + path_is_readable(initrd) + path_is_readable(opts.devicetree) + path_is_readable(opts.pcrpkey) + for key in opts.pcr_private_keys or (): + path_is_readable(key) + for key in opts.pcr_public_keys or (): + path_is_readable(key) + if opts.cmdline and opts.cmdline.startswith('@'): - opts.cmdline = pathlib.Path(opts.cmdline[1:]) + opts.cmdline = path_is_readable(opts.cmdline[1:]) if opts.os_release is not None and opts.os_release.startswith('@'): - opts.os_release = pathlib.Path(opts.os_release[1:]) + opts.os_release = path_is_readable(opts.os_release[1:]) elif opts.os_release is None: p = pathlib.Path('/etc/os-release') if not p.exists(): - p = pathlib.Path('/usr/lib/os-release') + p = path_is_readable('/usr/lib/os-release') opts.os_release = p if opts.efi_arch is None: opts.efi_arch = guess_efi_arch() if opts.stub is None: - opts.stub = f'/usr/lib/systemd/boot/efi/linux{opts.efi_arch}.efi.stub' + opts.stub = path_is_readable(f'/usr/lib/systemd/boot/efi/linux{opts.efi_arch}.efi.stub') if opts.signing_engine is None: - opts.sb_key = pathlib.Path(opts.sb_key) if opts.sb_key else None - opts.sb_cert = pathlib.Path(opts.sb_cert) if opts.sb_cert else None + opts.sb_key = path_is_readable(opts.sb_key) if opts.sb_key else None + opts.sb_cert = path_is_readable(opts.sb_cert) if opts.sb_cert else None if bool(opts.sb_key) ^ bool(opts.sb_cert): raise ValueError('--secureboot-private-key= and --secureboot-certificate= must be specified together')