From d3533162d1926f04560357ad588cc7757422fc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Apr 2023 17:50:07 +0200 Subject: [PATCH 01/20] 90-loaderentry: make sure that variables are set We unconditionally use the variables later on, so let's make sure that they were passed as expected. --- src/kernel-install/90-loaderentry.install.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kernel-install/90-loaderentry.install.in b/src/kernel-install/90-loaderentry.install.in index e8e8cf37c3..f4ba4708ba 100755 --- a/src/kernel-install/90-loaderentry.install.in +++ b/src/kernel-install/90-loaderentry.install.in @@ -28,9 +28,9 @@ INITRD_OPTIONS_SHIFT=4 [ "$KERNEL_INSTALL_LAYOUT" = "bls" ] || exit 0 -MACHINE_ID="$KERNEL_INSTALL_MACHINE_ID" -ENTRY_TOKEN="$KERNEL_INSTALL_ENTRY_TOKEN" -BOOT_ROOT="$KERNEL_INSTALL_BOOT_ROOT" +MACHINE_ID="${KERNEL_INSTALL_MACHINE_ID:?}" +ENTRY_TOKEN="${KERNEL_INSTALL_ENTRY_TOKEN:?}" +BOOT_ROOT="${KERNEL_INSTALL_BOOT_ROOT:?}" [ -n "$BOOT_MNT" ] || BOOT_MNT="$(stat -c %m "$BOOT_ROOT")" if [ "$BOOT_MNT" = '/' ]; then From 50f4add445a19e7bb40b88e473ede3489b4f2a6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Apr 2023 17:53:18 +0200 Subject: [PATCH 02/20] ukify: use UPPERCASE for parameter names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We generally nowadays use UPPERCASE for parameters in variuos help text. Let's be consistent here too, and also drop duplicated 'usage:': $ ukify -h usage: ukify [options…] LINUX INITRD… ukify -h | --help Build and sign Unified Kernel Images positional arguments: LINUX vmlinuz file [.linux section] INITRD… initrd files [.initrd section] ... --- src/ukify/ukify.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 3fbeb2b215..fc9c4767b1 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -4,7 +4,7 @@ # pylint: disable=missing-docstring,invalid-name,import-outside-toplevel # pylint: disable=consider-using-with,unspecified-encoding,line-too-long # pylint: disable=too-many-locals,too-many-statements,too-many-return-statements -# pylint: disable=too-many-branches +# pylint: disable=too-many-branches,fixme import argparse import collections @@ -658,7 +658,7 @@ def parse_args(args=None): description='Build and sign Unified Kernel Images', allow_abbrev=False, usage='''\ -usage: ukify [options…] [linux [initrd…]] +ukify [options…] [LINUX INITRD…] ukify -h | --help ''') @@ -666,10 +666,12 @@ usage: ukify [options…] [linux [initrd…]] p.error = lambda message: p.exit(2, f'{p.prog}: error: {message}\n') p.add_argument('linux', + metavar='LINUX', type=pathlib.Path, nargs="?", help='vmlinuz file [.linux section]') p.add_argument('initrd', + metavar='INITRD…', type=pathlib.Path, nargs='*', help='initrd files [.initrd section]') From d9c8f075af486791319fde78b5d7248195b6e5e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Apr 2023 17:58:52 +0200 Subject: [PATCH 03/20] ukify: add missing header This file is installed, so it should have the long header. --- src/ukify/ukify.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index fc9c4767b1..599f872bc1 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -1,5 +1,20 @@ #!/usr/bin/env python3 # SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# systemd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with systemd; If not, see . # pylint: disable=missing-docstring,invalid-name,import-outside-toplevel # pylint: disable=consider-using-with,unspecified-encoding,line-too-long From 9a01fe3906682eeb0fe29fb8ef9cadd1dff353e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 Apr 2023 18:10:58 +0200 Subject: [PATCH 04/20] meson: avoid building executables that won't be installed When executable() or custom_target() has install: that is conditional as is false (i.e. not install:true), it won't be built by default. (build_by_default: defaults to install:). But if that program is added to public_programs, it will be build by default because it is pulled in by the test, effectively defeating the disablement. While at it, make 'ukify' follow the same pattern as 'kernel-install'. They will be used later together. --- meson.build | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index c6dfc7709c..2b0f89f83d 100644 --- a/meson.build +++ b/meson.build @@ -2507,7 +2507,9 @@ exe = executable( versiondep], install_rpath : rootpkglibdir, install : conf.get('ENABLE_ANALYZE') == 1) -public_programs += exe +if conf.get('ENABLE_ANALYZE') == 1 + public_programs += exe +endif if want_tests != 'false' test('test-compare-versions', @@ -4353,7 +4355,9 @@ exe = custom_target( install : want_kernel_install, install_mode : 'rwxr-xr-x', install_dir : bindir) -public_programs += exe +if want_kernel_install + public_programs += exe +endif if want_tests != 'false' and want_kernel_install test('test-kernel-install', @@ -4362,15 +4366,15 @@ if want_tests != 'false' and want_kernel_install args : [exe.full_path(), loaderentry_install]) endif -if want_ukify - exe = custom_target( +exe = custom_target( 'ukify', input : 'src/ukify/ukify.py', output : 'ukify', command : [jinja2_cmdline, '@INPUT@', '@OUTPUT@'], - install : true, + install : want_ukify, install_mode : 'rwxr-xr-x', install_dir : rootlibexecdir) +if want_ukify public_programs += exe endif From b62ee354dd68349812f0526622c4e164b4a89f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 Apr 2023 18:19:48 +0200 Subject: [PATCH 05/20] meson: allow building .standalone on demand We can always build the standalone version whenever we build the normal version (the dependencies are the same). In most builds standalone binaries would be disabled. But it is occasionally useful to have them for testing, so move the conditional to install:, so the binaries can be build by giving the explicit target name. The default of 'build_by_default' for executable() is sadly true (since meson 0.38.0), so need to specify build_by_default: too. Also add systemd-shutdown.standalone to public_programs for additional testing. --- meson.build | 125 +++++++++++++++++++++++++++------------------------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/meson.build b/meson.build index 2b0f89f83d..88ef5679c5 100644 --- a/meson.build +++ b/meson.build @@ -4008,20 +4008,21 @@ if enable_sysusers args : exe.full_path()) endif + exe = executable( + 'systemd-sysusers.standalone', + 'src/sysusers/sysusers.c', + include_directories : includes, + c_args : '-DSTANDALONE', + link_with : [libshared_static, + libbasic, + libbasic_gcrypt, + libsystemd_static], + dependencies : [userspace, + versiondep], + build_by_default: have_standalone_binaries, + install : have_standalone_binaries, + install_dir : rootbindir) if have_standalone_binaries - exe = executable( - 'systemd-sysusers.standalone', - 'src/sysusers/sysusers.c', - include_directories : includes, - c_args : '-DSTANDALONE', - link_with : [libshared_static, - libbasic, - libbasic_gcrypt, - libsystemd_static], - dependencies : [userspace, - versiondep], - install : true, - install_dir : rootbindir) public_programs += exe if want_tests != 'false' @@ -4054,21 +4055,22 @@ if conf.get('ENABLE_TMPFILES') == 1 args : exe.full_path()) endif + exe = executable( + 'systemd-tmpfiles.standalone', + systemd_tmpfiles_sources, + include_directories : includes, + c_args : '-DSTANDALONE', + link_with : [libshared_static, + libbasic, + libbasic_gcrypt, + libsystemd_static], + dependencies : [libacl, + userspace, + versiondep], + build_by_default: have_standalone_binaries, + install : have_standalone_binaries, + install_dir : rootbindir) if have_standalone_binaries - exe = executable( - 'systemd-tmpfiles.standalone', - systemd_tmpfiles_sources, - include_directories : includes, - c_args : '-DSTANDALONE', - link_with : [libshared_static, - libbasic, - libbasic_gcrypt, - libsystemd_static], - dependencies : [libacl, - userspace, - versiondep], - install : true, - install_dir : rootbindir) public_programs += exe if want_tests != 'false' @@ -4168,26 +4170,27 @@ if conf.get('ENABLE_REPART') == 1 install_dir : rootbindir) public_programs += exe + exe = executable( + 'systemd-repart.standalone', + systemd_repart_sources, + include_directories : includes, + c_args : '-DSTANDALONE', + link_with : [libshared_static, + libbasic, + libbasic_gcrypt, + libsystemd_static, + libshared_fdisk], + dependencies : [libblkid, + libfdisk, + libopenssl, + threads, + userspace, + versiondep], + build_by_default: have_standalone_binaries, + install_rpath : rootpkglibdir, + install : have_standalone_binaries, + install_dir : rootbindir) if have_standalone_binaries - exe = executable( - 'systemd-repart.standalone', - systemd_repart_sources, - include_directories : includes, - c_args : '-DSTANDALONE', - link_with : [libshared_static, - libbasic, - libbasic_gcrypt, - libsystemd_static, - libshared_fdisk], - dependencies : [libblkid, - libfdisk, - libopenssl, - threads, - userspace, - versiondep], - install_rpath : rootpkglibdir, - install : true, - install_dir : rootbindir) public_programs += exe endif endif @@ -4204,21 +4207,23 @@ executable( install : true, install_dir : rootlibexecdir) +executable( + 'systemd-shutdown.standalone', + systemd_shutdown_sources, + include_directories : includes, + c_args : '-DSTANDALONE', + link_with : [libshared_static, + libbasic, + libsystemd_static], + dependencies : [libmount, + userspace, + versiondep], + build_by_default: have_standalone_binaries, + install_rpath : rootpkglibdir, + install : have_standalone_binaries, + install_dir : rootlibexecdir) if have_standalone_binaries - executable( - 'systemd-shutdown.standalone', - systemd_shutdown_sources, - include_directories : includes, - c_args : '-DSTANDALONE', - link_with : [libshared_static, - libbasic, - libsystemd_static], - dependencies : [libmount, - userspace, - versiondep], - install_rpath : rootpkglibdir, - install : true, - install_dir : rootlibexecdir) + public_programs += exe endif executable( From cb3b451e11e5dc32f12560c63e36bacb8df164f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Apr 2023 08:27:21 +0200 Subject: [PATCH 06/20] test_ukify: fix loop iteration We'd try to access 'linux' or 'initrd' after failing to set it. --- src/ukify/test/test_ukify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 1b58f05d4c..1f468906d3 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -148,7 +148,7 @@ def kernel_initrd(): linux = f"{item['root']}{item['linux']}" initrd = f"{item['root']}{item['initrd'][0]}" except (KeyError, IndexError): - pass + continue return [linux, initrd] else: return None From 3f7e77fae1d3915c654d847a8b6c229767fe71e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Apr 2023 08:32:09 +0200 Subject: [PATCH 07/20] test_ukify: fix two failing tests Fixup for 22ad038ac6e4fe5e4a68555f0e70bd0a16fb5616 and 3fc5eed47091363247012454df458e1a3303bf12. It seems that the tests are not executed properly in CI. Nevertheless, test-ukify appears in logs: rpm-build:fedora-rawhide-x86_64: 409/1191 systemd / test-ukify OK 0.16s This is strange. --- src/ukify/test/test_ukify.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 1f468906d3..7b9f855ff3 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -91,7 +91,7 @@ def test_parse_args_many(): assert opts.sb_key == 'SBKEY' assert opts.sb_cert == 'SBCERT' assert opts.sign_kernel is False - assert opts.tools == pathlib.Path('TOOLZ/') + assert opts.tools == [pathlib.Path('TOOLZ/')] assert opts.output == pathlib.Path('OUTPUT') assert opts.measure is False @@ -109,13 +109,11 @@ def test_parse_sections(): assert opts.sections[0].name == 'test' assert isinstance(opts.sections[0].content, pathlib.Path) assert opts.sections[0].tmpfile - assert opts.sections[0].offset is None assert opts.sections[0].measure is False assert opts.sections[1].name == 'test2' assert opts.sections[1].content == pathlib.Path('FILE') assert opts.sections[1].tmpfile is None - assert opts.sections[1].offset is None assert opts.sections[1].measure is False def test_help(capsys): @@ -242,7 +240,6 @@ def test_uname_scraping(kernel_initrd): uname = ukify.Uname.scrape(kernel_initrd[0]) assert re.match(r'\d+\.\d+\.\d+', uname) - def test_efi_signing(kernel_initrd, tmpdir): if kernel_initrd is None: pytest.skip('linux+initrd not found') From 5143a47a8165b6d7b7f634025901804ab1c0e2c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Apr 2023 20:22:25 +0200 Subject: [PATCH 08/20] ukify: rework option parsing to support a config file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some ways this is similar to mkosi: we have a argparse.ArgumentParser() with a bunch of options, and a configparser.ConfigParser() with an overlapping set of options. Many options are settable in both places, but not all. In mkosi, we define this in three places (a dataclass, and a function for argparse, and a function for configparser). Here, we have one huge list of ConfigItem instances. Each instance specifies the full metadata for both parsers. Argparse generates a --help string for all the options, and we also append a config file sample to --help based on the ConfigItem data: $ python src/ukify/ukify.py --help|tail -n 25 config file: [UKI] Linux = LINUX Initrd = INITRD… Cmdline = TEXT|@PATH OSRelease = TEXT|@PATH DeviceTree = PATH Splash = BMP PCRPKey = KEY Uname = VERSION EFIArch = ia32|x64|arm|aa64|riscv64 Stub = STUB PCRBanks = BANK… SigningEngine = ENGINE SecureBootPrivateKey = SB_KEY SecureBootCertificate = SB_CERT SignKernel = SIGN_KERNEL [PCRSignature:NAME] PCRPrivateKey = PATH PCRPublicKey = PATH Phases = PHASE-PATH… While writing this I needed to check the argument parsing, so I added a --summary switch. It just pretty-prints the resulting option dictionary: $ python src/ukify/ukify.py /efi//3a9d668b4db749398a4a5e78a03bffa5/6.2.11-300.fc38.x86_64/linux /efi//3a9d668b4db749398a4a5e78a03bffa5/6.2.11-300.fc38.x86_64/initrd --pcr-private-key=PRIV.key --pcr-public-key=PUB.key --config=man/ukify-example.conf --summary Host arch 'x86_64', EFI arch 'x64' {'_groups': [0, 'initrd', 'system'], 'cmdline': 'A1 B2 C3', 'config': 'man/ukify-example.conf', 'devicetree': None, 'efi_arch': 'x64', 'initrd': [PosixPath('initrd1'), PosixPath('initrd2'), PosixPath('initrd3'), PosixPath('/efi/3a9d668b4db749398a4a5e78a03bffa5/6.2.11-300.fc38.x86_64/initrd')], 'linux': PosixPath('/efi/3a9d668b4db749398a4a5e78a03bffa5/6.2.11-300.fc38.x86_64/linux'), 'measure': None, 'os_release': PosixPath('/etc/os-release'), 'output': 'linux.efi', 'pcr_banks': ['sha1', 'sha384'], 'pcr_private_keys': [PosixPath('PRIV.key'), PosixPath('pcr-private-initrd-key.pem'), PosixPath('pcr-private-system-key.pem')], 'pcr_public_keys': [PosixPath('PUB.key'), PosixPath('pcr-public-initrd-key.pem'), PosixPath('pcr-public-system-key.pem')], 'pcrpkey': None, 'phase_path_groups': [None, ['enter-initrd'], ['enter-initrd:leave-initrd', 'enter-initrd:leave-initrd:sysinit', 'enter-initrd:leave-initrd:sysinit:ready']], 'sb_cert': PosixPath('mkosi.secure-boot.crt'), 'sb_key': PosixPath('mkosi.secure-boot.key'), 'sections': [], 'sign_kernel': None, 'signing_engine': None, 'splash': None, 'stub': PosixPath('/usr/lib/systemd/boot/efi/linuxx64.efi.stub'), 'summary': True, 'tools': None, 'uname': None} With --summary, existence of input paths is not checked. I think we'll want to show them, instead of throwing an error, but in red, similarly to 'bootctl list'. This also fixes tests which were failing with e.g. E FileNotFoundError: [Errno 2] No such file or directory: '/ARG1' =========================== short test summary info ============================ FAILED ../src/ukify/test/test_ukify.py::test_parse_args_minimal - FileNotFoun... FAILED ../src/ukify/test/test_ukify.py::test_parse_args_many - FileNotFoundEr... FAILED ../src/ukify/test/test_ukify.py::test_parse_sections - FileNotFoundErr... =================== 3 failed, 10 passed, 3 skipped in 1.51s ==================== --- src/ukify/ukify.py | 570 +++++++++++++++++++++++++++++++++------------ 1 file changed, 417 insertions(+), 153 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 599f872bc1..033acba3d7 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -22,6 +22,7 @@ # pylint: disable=too-many-branches,fixme import argparse +import configparser import collections import dataclasses import fnmatch @@ -29,10 +30,12 @@ import itertools import json import os import pathlib +import pprint import re import shlex import shutil import subprocess +import sys import tempfile import typing @@ -84,18 +87,6 @@ def shell_join(cmd): return ' '.join(shlex.quote(str(x)) for x in cmd) -def path_is_readable(s: typing.Optional[str]) -> typing.Optional[pathlib.Path]: - """Convert a filename string to a Path and verify access.""" - if s is None: - return None - p = pathlib.Path(s) - try: - p.open().close() - except IsADirectoryError: - pass - return p - - def round_up(x, blocksize=4096): return (x + blocksize - 1) // blocksize * blocksize @@ -337,11 +328,13 @@ def check_inputs(opts): if name in {'output', 'tools'}: continue - if not isinstance(value, pathlib.Path): - continue - - # Open file to check that we can read it, or generate an exception - value.open().close() + if isinstance(value, pathlib.Path): + # Open file to check that we can read it, or generate an exception + value.open().close() + elif isinstance(value, list): + for item in value: + if isinstance(item, pathlib.Path): + item.open().close() check_splash(opts.splash) @@ -668,157 +661,412 @@ def make_uki(opts): print(f"Wrote {'signed' if opts.sb_key else 'unsigned'} {opts.output}") -def parse_args(args=None): +@dataclasses.dataclass(frozen=True) +class ConfigItem: + @staticmethod + def config_list_prepend(namespace, group, dest, value): + "Prepend value to namespace." + + assert not group + + old = getattr(namespace, dest, []) + setattr(namespace, dest, value + old) + + @staticmethod + def config_set_if_unset(namespace, group, dest, value): + "Set namespace. to value only if it was None" + + assert not group + + if getattr(namespace, dest) is None: + setattr(namespace, dest, value) + + @staticmethod + def config_set_group(namespace, group, dest, value): + "Set namespace.[idx] to value, with idx derived from group" + + if group not in namespace._groups: + namespace._groups += [group] + idx = namespace._groups.index(group) + + old = getattr(namespace, dest, None) + if old is None: + old = [] + setattr(namespace, dest, + old + ([None] * (idx - len(old))) + [value]) + + @staticmethod + def parse_boolean(s: str) -> bool: + "Parse 1/true/yes/y/t/on as true and 0/false/no/n/f/off/None as false" + s_l = s.lower() + if s_l in {'1', 'true', 'yes', 'y', 't', 'on'}: + return True + if s_l in {'0', 'false', 'no', 'n', 'f', 'off'}: + return False + raise ValueError('f"Invalid boolean literal: {s!r}') + + # arguments for argparse.ArgumentParser.add_argument() + name: typing.Union[str, typing.List[str]] + dest: str = None + metavar: str = None + type: typing.Callable = None + nargs: str = None + action: typing.Callable = None + default: typing.Any = None + version: str = None + choices: typing.List[str] = None + help: str = None + + # metadata for config file parsing + config_key: str = None + config_push: typing.Callable[..., ...] = config_set_if_unset + + def _names(self) -> typing.Tuple[str]: + return self.name if isinstance(self.name, tuple) else (self.name,) + + def argparse_dest(self) -> str: + # It'd be nice if argparse exported this, but I don't see that in the API + if self.dest: + return self.dest + return self._names()[0].lstrip('-').replace('-', '_') + + def add_to(self, parser: argparse.ArgumentParser): + kwargs = { key:val + for key in dataclasses.asdict(self) + if (key not in ('name', 'config_key', 'config_push') and + (val := getattr(self, key)) is not None) } + args = self._names() + parser.add_argument(*args, **kwargs) + + def apply_config(self, namespace, section, group, key, value) -> None: + assert f'{section}/{key}' == self.config_key + dest = self.argparse_dest() + + if self.action == argparse.BooleanOptionalAction: + # We need to handle this case separately: the options are called + # --foo and --no-foo, and no argument is parsed. But in the config + # file, we have Foo=yes or Foo=no. + conv = self.parse_boolean + elif self.type: + conv = self.type + else: + conv = lambda s:s + + if self.nargs == '*': + value = [conv(v) for v in value.split()] + else: + value = conv(value) + + self.config_push(namespace, group, dest, value) + + def config_example(self) -> typing.Tuple[typing.Optional[str]]: + if not self.config_key: + return None, None, None + section_name, key = self.config_key.split('/', 1) + if section_name.endswith(':'): + section_name += 'NAME' + if self.choices: + value = '|'.join(self.choices) + else: + value = self.metavar or self.argparse_dest().upper() + return (section_name, key, value) + + +CONFIG_ITEMS = [ + ConfigItem( + '--version', + action = 'version', + version = f'ukify {__version__}', + ), + + ConfigItem( + '--summary', + help = 'print parsed config and exit', + action = 'store_true', + ), + + ConfigItem( + 'linux', + metavar = 'LINUX', + type = pathlib.Path, + nargs = '?', + help = 'vmlinuz file [.linux section]', + config_key = 'UKI/Linux', + ), + + ConfigItem( + 'initrd', + metavar = 'INITRD…', + type = pathlib.Path, + nargs = '*', + help = 'initrd files [.initrd section]', + config_key = 'UKI/Initrd', + config_push = ConfigItem.config_list_prepend, + ), + + ConfigItem( + ('--config', '-c'), + metavar = 'PATH', + help = 'configuration file', + ), + + ConfigItem( + '--cmdline', + metavar = 'TEXT|@PATH', + help = 'kernel command line [.cmdline section]', + config_key = 'UKI/Cmdline', + ), + + ConfigItem( + '--os-release', + metavar = 'TEXT|@PATH', + help = 'path to os-release file [.osrel section]', + config_key = 'UKI/OSRelease', + ), + + ConfigItem( + '--devicetree', + metavar = 'PATH', + type = pathlib.Path, + help = 'Device Tree file [.dtb section]', + config_key = 'UKI/DeviceTree', + ), + ConfigItem( + '--splash', + metavar = 'BMP', + type = pathlib.Path, + help = 'splash image bitmap file [.splash section]', + config_key = 'UKI/Splash', + ), + ConfigItem( + '--pcrpkey', + metavar = 'KEY', + type = pathlib.Path, + help = 'embedded public key to seal secrets to [.pcrpkey section]', + config_key = 'UKI/PCRPKey', + ), + ConfigItem( + '--uname', + metavar='VERSION', + help='"uname -r" information [.uname section]', + config_key = 'UKI/Uname', + ), + + ConfigItem( + '--efi-arch', + metavar = 'ARCH', + choices = ('ia32', 'x64', 'arm', 'aa64', 'riscv64'), + help = 'target EFI architecture', + config_key = 'UKI/EFIArch', + ), + + ConfigItem( + '--stub', + type = pathlib.Path, + help = 'path to the sd-stub file [.text,.data,… sections]', + config_key = 'UKI/Stub', + ), + + ConfigItem( + '--section', + dest = 'sections', + metavar = 'NAME:TEXT|@PATH', + type = Section.parse_arg, + action = 'append', + default = [], + help = 'additional section as name and contents [NAME section]', + ), + + ConfigItem( + '--pcr-banks', + metavar = 'BANK…', + type = parse_banks, + config_key = 'UKI/PCRBanks', + ), + + ConfigItem( + '--signing-engine', + metavar = 'ENGINE', + help = 'OpenSSL engine to use for signing', + config_key = 'UKI/SigningEngine', + ), + ConfigItem( + '--secureboot-private-key', + dest = 'sb_key', + help = 'path to key file or engine-specific designation for SB signing', + config_key = 'UKI/SecureBootPrivateKey', + ), + ConfigItem( + '--secureboot-certificate', + dest = 'sb_cert', + help = 'path to certificate file or engine-specific designation for SB signing', + config_key = 'UKI/SecureBootCertificate', + ), + + ConfigItem( + '--sign-kernel', + action = argparse.BooleanOptionalAction, + help = 'Sign the embedded kernel', + config_key = 'UKI/SignKernel', + ), + + ConfigItem( + '--pcr-private-key', + dest = 'pcr_private_keys', + metavar = 'PATH', + type = pathlib.Path, + action = 'append', + help = 'private part of the keypair for signing PCR signatures', + config_key = 'PCRSignature:/PCRPrivateKey', + config_push = ConfigItem.config_set_group, + ), + ConfigItem( + '--pcr-public-key', + dest = 'pcr_public_keys', + metavar = 'PATH', + type = pathlib.Path, + action = 'append', + help = 'public part of the keypair for signing PCR signatures', + config_key = 'PCRSignature:/PCRPublicKey', + config_push = ConfigItem.config_set_group, + ), + ConfigItem( + '--phases', + dest = 'phase_path_groups', + metavar = 'PHASE-PATH…', + type = parse_phase_paths, + action = 'append', + help = 'phase-paths to create signatures for', + config_key = 'PCRSignature:/Phases', + config_push = ConfigItem.config_set_group, + ), + + ConfigItem( + '--tools', + type = pathlib.Path, + action = 'append', + help = 'Directories to search for tools (systemd-measure, …)', + ), + + ConfigItem( + ('--output', '-o'), + type = pathlib.Path, + help = 'output file path', + ), + + ConfigItem( + '--measure', + action = argparse.BooleanOptionalAction, + help = 'print systemd-measure output for the UKI', + ), +] + +CONFIGFILE_ITEMS = { item.config_key:item + for item in CONFIG_ITEMS + if item.config_key } + + +def apply_config(namespace, filename=None): + if filename is None: + filename = namespace.config + if filename is None: + return + + # Fill in ._groups based on --pcr-public-key=, --pcr-private-key=, and --phases=. + assert '_groups' not in namespace + n_pcr_priv = len(namespace.pcr_private_keys or ()) + namespace._groups = list(range(n_pcr_priv)) + + cp = configparser.ConfigParser( + comment_prefixes='#', + inline_comment_prefixes='#', + delimiters='=', + empty_lines_in_values=False, + interpolation=None, + strict=False) + # Do not make keys lowercase + cp.optionxform = lambda option: option + + cp.read(filename) + + for section_name, section in cp.items(): + idx = section_name.find(':') + if idx >= 0: + section_name, group = section_name[:idx+1], section_name[idx+1:] + if not section_name or not group: + raise ValueError('Section name components cannot be empty') + if ':' in group: + raise ValueError('Section name cannot contain more than one ":"') + else: + group = None + for key, value in section.items(): + if item := CONFIGFILE_ITEMS.get(f'{section_name}/{key}'): + item.apply_config(namespace, section_name, group, key, value) + else: + print(f'Unknown config setting [{section_name}] {key}=') + + +def config_example(): + prev_section = None + for item in CONFIG_ITEMS: + section, key, value = item.config_example() + if section: + if prev_section != section: + if prev_section: + yield '' + yield f'[{section}]' + prev_section = section + yield f'{key} = {value}' + + +def create_parser(): p = argparse.ArgumentParser( description='Build and sign Unified Kernel Images', allow_abbrev=False, usage='''\ ukify [options…] [LINUX INITRD…] - ukify -h | --help -''') +''', + epilog='\n '.join(('config file:', *config_example())), + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + + for item in CONFIG_ITEMS: + item.add_to(p) # Suppress printing of usage synopsis on errors p.error = lambda message: p.exit(2, f'{p.prog}: error: {message}\n') - p.add_argument('linux', - metavar='LINUX', - type=pathlib.Path, - nargs="?", - help='vmlinuz file [.linux section]') - p.add_argument('initrd', - metavar='INITRD…', - type=pathlib.Path, - nargs='*', - help='initrd files [.initrd section]') + return p - p.add_argument('--cmdline', - metavar='TEXT|@PATH', - help='kernel command line [.cmdline section]') - - p.add_argument('--os-release', - metavar='TEXT|@PATH', - help='path to os-release file [.osrel section]') - - p.add_argument('--devicetree', - metavar='PATH', - type=pathlib.Path, - help='Device Tree file [.dtb section]') - p.add_argument('--splash', - metavar='BMP', - type=pathlib.Path, - help='splash image bitmap file [.splash section]') - p.add_argument('--pcrpkey', - metavar='KEY', - type=pathlib.Path, - help='embedded public key to seal secrets to [.pcrpkey section]') - p.add_argument('--uname', - metavar='VERSION', - help='"uname -r" information [.uname section]') - - p.add_argument('--efi-arch', - metavar='ARCH', - choices=('ia32', 'x64', 'arm', 'aa64', 'riscv64'), - help='target EFI architecture') - - p.add_argument('--stub', - type=pathlib.Path, - help='path to the sd-stub file [.text,.data,… sections]') - - p.add_argument('--section', - dest='sections', - metavar='NAME:TEXT|@PATH', - type=Section.parse_arg, - action='append', - default=[], - help='additional section as name and contents [NAME section]') - - p.add_argument('--pcr-private-key', - dest='pcr_private_keys', - metavar='PATH', - type=pathlib.Path, - action='append', - help='private part of the keypair for signing PCR signatures') - p.add_argument('--pcr-public-key', - dest='pcr_public_keys', - metavar='PATH', - type=pathlib.Path, - action='append', - help='public part of the keypair for signing PCR signatures') - p.add_argument('--phases', - dest='phase_path_groups', - metavar='PHASE-PATH…', - type=parse_phase_paths, - action='append', - help='phase-paths to create signatures for') - - p.add_argument('--pcr-banks', - metavar='BANK…', - type=parse_banks) - - p.add_argument('--signing-engine', - metavar='ENGINE', - help='OpenSSL engine to use for signing') - p.add_argument('--secureboot-private-key', - dest='sb_key', - help='path to key file or engine-specific designation for SB signing') - p.add_argument('--secureboot-certificate', - dest='sb_cert', - help='path to certificate file or engine-specific designation for SB signing') - - p.add_argument('--sign-kernel', - action=argparse.BooleanOptionalAction, - help='Sign the embedded kernel') - - p.add_argument('--tools', - type=pathlib.Path, - action='append', - help='Directories to search for tools (systemd-measure, ...)') - - p.add_argument('--output', '-o', - type=pathlib.Path, - help='output file path') - - p.add_argument('--measure', - action=argparse.BooleanOptionalAction, - help='print systemd-measure output for the UKI') - - p.add_argument('--version', - action='version', - version=f'ukify {__version__}') - - opts = p.parse_args(args) - - if opts.linux is not None: - 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) +def finalize_options(opts): if opts.cmdline and opts.cmdline.startswith('@'): - opts.cmdline = path_is_readable(opts.cmdline[1:]) + opts.cmdline = pathlib.Path(opts.cmdline[1:]) + elif opts.cmdline: + # Drop whitespace from the commandline. If we're reading from a file, + # we copy the contents verbatim. But configuration specified on the commandline + # or in the config file may contain additional whitespace that has no meaning. + opts.cmdline = ' '.join(opts.cmdline.split()) - if opts.os_release is not None and opts.os_release.startswith('@'): - opts.os_release = path_is_readable(opts.os_release[1:]) - elif opts.os_release is None and opts.linux is not None: + if opts.os_release and opts.os_release.startswith('@'): + opts.os_release = pathlib.Path(opts.os_release[1:]) + elif not opts.os_release and opts.linux: p = pathlib.Path('/etc/os-release') if not p.exists(): - p = path_is_readable('/usr/lib/os-release') + p = pathlib.Path('/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 = path_is_readable(f'/usr/lib/systemd/boot/efi/linux{opts.efi_arch}.efi.stub') + opts.stub = pathlib.Path(f'/usr/lib/systemd/boot/efi/linux{opts.efi_arch}.efi.stub') if opts.signing_engine is 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 opts.sb_key: + opts.sb_key = pathlib.Path(opts.sb_key) + if opts.sb_cert: + opts.sb_cert = pathlib.Path(opts.sb_cert) if bool(opts.sb_key) ^ bool(opts.sb_cert): raise ValueError('--secureboot-private-key= and --secureboot-certificate= must be specified together') @@ -826,14 +1074,6 @@ ukify [options…] [LINUX INITRD…] if opts.sign_kernel and not opts.sb_key: raise ValueError('--sign-kernel requires --secureboot-private-key= and --secureboot-certificate= to be specified') - n_pcr_pub = None if opts.pcr_public_keys is None else len(opts.pcr_public_keys) - n_pcr_priv = None if opts.pcr_private_keys is None else len(opts.pcr_private_keys) - n_phase_path_groups = None if opts.phase_path_groups is None else len(opts.phase_path_groups) - if n_pcr_pub is not None and n_pcr_pub != n_pcr_priv: - raise ValueError('--pcr-public-key= specifications must match --pcr-private-key=') - if n_phase_path_groups is not None and n_phase_path_groups != n_pcr_priv: - raise ValueError('--phases= specifications must match --pcr-private-key=') - if opts.output is None: if opts.linux is None: raise ValueError('--output= must be specified when building a PE addon') @@ -843,6 +1083,30 @@ ukify [options…] [LINUX INITRD…] for section in opts.sections: section.check_name() + if opts.summary: + # TODO: replace pprint() with some fancy formatting. + pprint.pprint(vars(opts)) + sys.exit() + + +def parse_args(args=None): + p = create_parser() + opts = p.parse_args(args) + + # Check that --pcr-public-key=, --pcr-private-key=, and --phases= + # have either the same number of arguments are are not specified at all. + n_pcr_pub = None if opts.pcr_public_keys is None else len(opts.pcr_public_keys) + n_pcr_priv = None if opts.pcr_private_keys is None else len(opts.pcr_private_keys) + n_phase_path_groups = None if opts.phase_path_groups is None else len(opts.phase_path_groups) + if n_pcr_pub is not None and n_pcr_pub != n_pcr_priv: + raise ValueError('--pcr-public-key= specifications must match --pcr-private-key=') + if n_phase_path_groups is not None and n_phase_path_groups != n_pcr_priv: + raise ValueError('--phases= specifications must match --pcr-private-key=') + + apply_config(opts) + + finalize_options(opts) + return opts From 7081db294c817db37e6db472bd5e15c670bdcdd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Apr 2023 20:23:18 +0200 Subject: [PATCH 09/20] =?UTF-8?q?ukify:=20PeError=20=E2=86=92=20PEError?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't lowercase acronyms in systemd usually. Remove unnused f'' prefix to avoid a pylint warning. --- src/ukify/ukify.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 033acba3d7..abf34ed071 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -44,15 +44,15 @@ import pefile __version__ = '{{PROJECT_VERSION}} ({{GIT_VERSION}})' EFI_ARCH_MAP = { - # host_arch glob : [efi_arch, 32_bit_efi_arch if mixed mode is supported] - 'x86_64' : ['x64', 'ia32'], - 'i[3456]86' : ['ia32'], - 'aarch64' : ['aa64'], - 'arm[45678]*l' : ['arm'], - 'loongarch32' : ['loongarch32'], - 'loongarch64' : ['loongarch64'], - 'riscv32' : ['riscv32'], - 'riscv64' : ['riscv64'], + # host_arch glob : [efi_arch, 32_bit_efi_arch if mixed mode is supported] + 'x86_64' : ['x64', 'ia32'], + 'i[3456]86' : ['ia32'], + 'aarch64' : ['aa64'], + 'arm[45678]*l' : ['arm'], + 'loongarch32' : ['loongarch32'], + 'loongarch64' : ['loongarch64'], + 'riscv32' : ['riscv32'], + 'riscv64' : ['riscv64'], } EFI_ARCHES: list[str] = sum(EFI_ARCH_MAP.values(), []) @@ -451,7 +451,7 @@ def pairwise(iterable): return zip(a, b) -class PeError(Exception): +class PEError(Exception): pass @@ -493,12 +493,12 @@ def pe_add_sections(uki: UKI, output: str): warnings = pe.get_warnings() if warnings: - raise PeError(f'pefile warnings treated as errors: {warnings}') + raise PEError(f'pefile warnings treated as errors: {warnings}') security = pe.OPTIONAL_HEADER.DATA_DIRECTORY[pefile.DIRECTORY_ENTRY['IMAGE_DIRECTORY_ENTRY_SECURITY']] if security.VirtualAddress != 0: # We could strip the signatures, but why would anyone sign the stub? - raise PeError(f'Stub image is signed, refusing.') + raise PEError('Stub image is signed, refusing.') for section in uki.sections: new_section = pefile.SectionStructure(pe.__IMAGE_SECTION_HEADER_format__, pe=pe) @@ -506,7 +506,7 @@ def pe_add_sections(uki: UKI, output: str): offset = pe.sections[-1].get_file_offset() + new_section.sizeof() if offset + new_section.sizeof() > pe.OPTIONAL_HEADER.SizeOfHeaders: - raise PeError(f'Not enough header space to add section {section.name}.') + raise PEError(f'Not enough header space to add section {section.name}.') data = section.content.read_bytes() From 47a6df4da0a852daae9458b7cb10fb1e43322c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Apr 2023 16:06:53 +0200 Subject: [PATCH 10/20] test_ukify: add tests for the new functionality --- src/ukify/test/test_ukify.py | 88 ++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 7b9f855ff3..3ffa8fe843 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -14,6 +14,7 @@ import shutil import subprocess import sys import tempfile +import textwrap try: import pytest @@ -46,6 +47,93 @@ def test_round_up(): assert ukify.round_up(4096) == 4096 assert ukify.round_up(4097) == 8192 +def test_namespace_creation(): + ns = ukify.create_parser().parse_args(('A','B')) + assert ns.linux == pathlib.Path('A') + assert ns.initrd == [pathlib.Path('B')] + +def test_config_example(): + ex = ukify.config_example() + assert '[UKI]' in ex + assert 'Splash = BMP' in ex + +def test_apply_config(tmp_path): + config = tmp_path / 'config1.conf' + config.write_text(textwrap.dedent( + f''' + [UKI] + Linux = LINUX + Initrd = initrd1 initrd2 + initrd3 + Cmdline = 1 2 3 4 5 + 6 7 8 + OSRelease = @some/path1 + DeviceTree = some/path2 + Splash = some/path3 + Uname = 1.2.3 + EFIArch=arm + Stub = some/path4 + PCRBanks = sha512,sha1 + SigningEngine = engine1 + SecureBootPrivateKey = some/path5 + SecureBootCertificate = some/path6 + SignKernel = no + + [PCRSignature:NAME] + PCRPrivateKey = some/path7 + PCRPublicKey = some/path8 + Phases = {':'.join(ukify.KNOWN_PHASES)} + ''')) + + ns = ukify.create_parser().parse_args(('A','B')) + ns.linux = None + ns.initrd = [] + ukify.apply_config(ns, config) + + assert ns.linux == pathlib.Path('LINUX') + assert ns.initrd == [pathlib.Path('initrd1'), + pathlib.Path('initrd2'), + pathlib.Path('initrd3')] + assert ns.cmdline == '1 2 3 4 5\n6 7 8' + assert ns.os_release == '@some/path1' + assert ns.devicetree == pathlib.Path('some/path2') + assert ns.splash == pathlib.Path('some/path3') + assert ns.efi_arch == 'arm' + assert ns.stub == pathlib.Path('some/path4') + assert ns.pcr_banks == ['sha512', 'sha1'] + assert ns.signing_engine == 'engine1' + assert ns.sb_key == 'some/path5' + assert ns.sb_cert == 'some/path6' + assert ns.sign_kernel == False + + assert ns._groups == ['NAME'] + assert ns.pcr_private_keys == [pathlib.Path('some/path7')] + assert ns.pcr_public_keys == [pathlib.Path('some/path8')] + assert ns.phase_path_groups == [['enter-initrd:leave-initrd:sysinit:ready:shutdown:final']] + + ukify.finalize_options(ns) + + assert ns.linux == pathlib.Path('LINUX') + assert ns.initrd == [pathlib.Path('initrd1'), + pathlib.Path('initrd2'), + pathlib.Path('initrd3')] + assert ns.cmdline == '1 2 3 4 5 6 7 8' + assert ns.os_release == pathlib.Path('some/path1') + assert ns.devicetree == pathlib.Path('some/path2') + assert ns.splash == pathlib.Path('some/path3') + assert ns.efi_arch == 'arm' + assert ns.stub == pathlib.Path('some/path4') + assert ns.pcr_banks == ['sha512', 'sha1'] + assert ns.signing_engine == 'engine1' + assert ns.sb_key == 'some/path5' + assert ns.sb_cert == 'some/path6' + assert ns.sign_kernel == False + + assert ns._groups == ['NAME'] + assert ns.pcr_private_keys == [pathlib.Path('some/path7')] + assert ns.pcr_public_keys == [pathlib.Path('some/path8')] + assert ns.phase_path_groups == [['enter-initrd:leave-initrd:sysinit:ready:shutdown:final']] + def test_parse_args_minimal(): opts = ukify.parse_args('arg1 arg2'.split()) assert opts.linux == pathlib.Path('arg1') From ca1abaa5c4ffbd0f72f5bbbd98a70db925a82503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Apr 2023 18:07:22 +0200 Subject: [PATCH 11/20] 60-ukify: kernel-install plugin that calls ukify to create a UKI 60-ukify.install calls ukify with a config file, so singing and policies and splash will be done through the ukify config file, without 60-ukify.install knowing anything directly. In meson.py, the variable for loaderentry.install.in is used just once, let's drop it. (I guess this approach was copied from kernel_install_in, which is used in another file.) The general idea is based on cvlc12's #27119, but now in Python instead of bash. --- src/kernel-install/60-ukify.install.in | 224 +++++++++++++++++++++++++ src/kernel-install/meson.build | 12 +- 2 files changed, 234 insertions(+), 2 deletions(-) create mode 100755 src/kernel-install/60-ukify.install.in diff --git a/src/kernel-install/60-ukify.install.in b/src/kernel-install/60-ukify.install.in new file mode 100755 index 0000000000..7c29f7e8af --- /dev/null +++ b/src/kernel-install/60-ukify.install.in @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: LGPL-2.1-or-later +# -*- mode: python-mode -*- +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# systemd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with systemd; If not, see . + +# pylint: disable=missing-docstring,invalid-name,import-outside-toplevel +# pylint: disable=consider-using-with,unspecified-encoding,line-too-long +# pylint: disable=too-many-locals,too-many-statements,too-many-return-statements +# pylint: disable=too-many-branches,redefined-builtin,fixme + +import argparse +import os +import runpy +import shlex +from pathlib import Path +from typing import Optional + +__version__ = '{{PROJECT_VERSION}} ({{GIT_VERSION}})' + +try: + VERBOSE = int(os.environ['KERNEL_INSTALL_VERBOSE']) > 0 +except (KeyError, ValueError): + VERBOSE = False + +# Override location of ukify and the boot stub for testing and debugging. +UKIFY = os.getenv('KERNEL_INSTALL_UKIFY', '/usr/lib/systemd/ukify') +BOOT_STUB = os.getenv('KERNEL_INSTALL_BOOT_STUB') + + +def shell_join(cmd): + # TODO: drop in favour of shlex.join once shlex.join supports pathlib.Path. + return ' '.join(shlex.quote(str(x)) for x in cmd) + +def log(*args, **kwargs): + if VERBOSE: + print(*args, **kwargs) + +def path_is_readable(p: Path, dir=False) -> None: + """Verify access to a file or directory.""" + try: + p.open().close() + except IsADirectoryError: + if dir: + return + raise + +def mandatory_variable(name): + try: + return os.environ[name] + except KeyError as e: + raise KeyError(f'${name} must be set in the environment') from e + +def parse_args(args=None): + p = argparse.ArgumentParser( + description='kernel-install plugin to build a Unified Kernel Image', + allow_abbrev=False, + usage='60-ukify.install COMMAND KERNEL_VERSION ENTRY_DIR KERNEL_IMAGE INITRD…', + ) + + # Suppress printing of usage synopsis on errors + p.error = lambda message: p.exit(2, f'{p.prog}: error: {message}\n') + + p.add_argument('command', + metavar='COMMAND', + help="The action to perform. Only 'add' is supported.") + p.add_argument('kernel_version', + metavar='KERNEL_VERSION', + help='Kernel version string') + p.add_argument('entry_dir', + metavar='ENTRY_DIR', + type=Path, + nargs='?', + help='Type#1 entry directory (ignored)') + p.add_argument('kernel_image', + metavar='KERNEL_IMAGE', + type=Path, + nargs='?', + help='Kernel binary') + p.add_argument('initrd', + metavar='INITRD…', + type=Path, + nargs='*', + help='Initrd files') + p.add_argument('--version', + action='version', + version=f'systemd {__version__}') + + opts = p.parse_args(args) + + if opts.command == 'add': + opts.staging_area = Path(mandatory_variable('KERNEL_INSTALL_STAGING_AREA')) + path_is_readable(opts.staging_area, dir=True) + + opts.entry_token = mandatory_variable('KERNEL_INSTALL_ENTRY_TOKEN') + opts.machine_id = mandatory_variable('KERNEL_INSTALL_MACHINE_ID') + + return opts + +def we_are_wanted() -> bool: + KERNEL_INSTALL_LAYOUT = os.getenv('KERNEL_INSTALL_LAYOUT') + + if KERNEL_INSTALL_LAYOUT != 'uki': + log(f'{KERNEL_INSTALL_LAYOUT=}, quitting.') + return False + + KERNEL_INSTALL_UKI_GENERATOR = os.getenv('KERNEL_INSTALL_UKI_GENERATOR') + + if KERNEL_INSTALL_UKI_GENERATOR != 'ukify': + log(f'{KERNEL_INSTALL_UKI_GENERATOR=}, quitting.') + return False + + log('KERNEL_INSTALL_LAYOUT and KERNEL_INSTALL_UKI_GENERATOR are good') + return True + + +def config_file_location() -> Optional[Path]: + if root := os.getenv('KERNEL_INSTALL_CONF_ROOT'): + p = Path(root) / 'uki.conf' + else: + p = Path('/etc/kernel/uki.conf') + if p.exists(): + return p + return None + + +def kernel_cmdline_base() -> list[str]: + if root := os.getenv('KERNEL_INSTALL_CONF_ROOT'): + return Path(root).joinpath('cmdline').read_text().split() + + for cmdline in ('/etc/kernel/cmdline', + '/usr/lib/kernel/cmdline'): + try: + return Path(cmdline).read_text().split() + except FileNotFoundError: + continue + + options = Path('/proc/cmdline').read_text().split() + return [opt for opt in options + if not opt.startswith(('BOOT_IMAGE=', 'initrd='))] + + +def kernel_cmdline(opts) -> str: + options = kernel_cmdline_base() + + # If the boot entries are named after the machine ID, then suffix the kernel + # command line with the machine ID we use, so that the machine ID remains + # stable, even during factory reset, in the initrd (where the system's machine + # ID is not directly accessible yet), and if the root file system is volatile. + if (opts.entry_token == opts.machine_id and + not any(opt.startswith('systemd.machine_id=') for opt in options)): + options += [f'systemd.machine_id={opts.machine_id}'] + + # TODO: we unconditionally set the cmdline here, ignoring the setting in + # the config file. Should we not do that? + + # Prepend a space so that '@' does not get misinterpreted + return ' ' + ' '.join(options) + + +def call_ukify(opts): + # Punish me harder. + # We want this: + # ukify = importlib.machinery.SourceFileLoader('ukify', UKIFY).load_module() + # but it throws a DeprecationWarning. + # https://stackoverflow.com/questions/67631/how-can-i-import-a-module-dynamically-given-the-full-path + # https://github.com/python/cpython/issues/65635 + # offer "explanations", but to actually load a python file without a .py extension, + # the "solution" is 4+ incomprehensible lines. + # The solution with runpy gives a dictionary, which isn't great, but will do. + ukify = runpy.run_path(UKIFY, run_name='ukify') + + # Create "empty" namespace. We want to override just a few settings, + # so it doesn't make sense to duplicate all the fields. We use a hack + # to pre-populate the namespace like argparse would, all defaults. + # We need to specify the two mandatory arguments to not get an error. + opts2 = ukify['create_parser']().parse_args(('A','B')) + + opts2.config = config_file_location() + opts2.uname = opts.kernel_version + opts2.linux = opts.kernel_image + opts2.initrd = opts.initrd + # Note that 'uki.efi' is the name required by 90-uki-copy.install. + opts2.output = opts.staging_area / 'uki.efi' + + opts2.cmdline = kernel_cmdline(opts) + if BOOT_STUB: + opts2.stub = BOOT_STUB + + # opts2.summary = True + + ukify['apply_config'](opts2) + ukify['finalize_options'](opts2) + ukify['check_inputs'](opts2) + ukify['make_uki'](opts2) + + log(f'{opts2.output} has been created') + + +def main(): + opts = parse_args() + if opts.command != 'add': + return + if not we_are_wanted(): + return + + call_ukify(opts) + + +if __name__ == '__main__': + main() diff --git a/src/kernel-install/meson.build b/src/kernel-install/meson.build index f5db4432c9..95aa0d9497 100644 --- a/src/kernel-install/meson.build +++ b/src/kernel-install/meson.build @@ -1,11 +1,19 @@ # SPDX-License-Identifier: LGPL-2.1-or-later kernel_install_in = files('kernel-install.in') -loaderentry_install_in = files('90-loaderentry.install.in') + +ukify_install = custom_target( + '60-ukify.install', + input : '60-ukify.install.in', + output : '60-ukify.install', + command : [jinja2_cmdline, '@INPUT@', '@OUTPUT@'], + install : want_kernel_install and want_ukify, + install_mode : 'rwxr-xr-x', + install_dir : kernelinstalldir) loaderentry_install = custom_target( '90-loaderentry.install', - input : loaderentry_install_in, + input : '90-loaderentry.install.in', output : '90-loaderentry.install', command : [jinja2_cmdline, '@INPUT@', '@OUTPUT@'], install : want_kernel_install, From f9a6cb0e138ddeeebe767b2632a35fa933e53c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 13 Apr 2023 18:11:39 +0200 Subject: [PATCH 12/20] test-kernel-install: test 60-ukify.install and 90-uki-copy.install We install a kernel with layout=uki and uki_generator=ukify, and test that a UKI gets installed in the expected place. The two plugins cooperate, so it's easiest to test them together. --- meson.build | 25 +++++++++++-------- src/kernel-install/meson.build | 10 +++++--- src/kernel-install/test-kernel-install.sh | 30 ++++++++++++++++++++--- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/meson.build b/meson.build index 88ef5679c5..910b55d3c0 100644 --- a/meson.build +++ b/meson.build @@ -4352,7 +4352,7 @@ executable( install : true, install_dir : rootlibexecdir) -exe = custom_target( +kernel_install = custom_target( 'kernel-install', input : kernel_install_in, output : 'kernel-install', @@ -4364,14 +4364,7 @@ if want_kernel_install public_programs += exe endif -if want_tests != 'false' and want_kernel_install - test('test-kernel-install', - test_kernel_install_sh, - env : test_env, - args : [exe.full_path(), loaderentry_install]) -endif - -exe = custom_target( +ukify = custom_target( 'ukify', input : 'src/ukify/ukify.py', output : 'ukify', @@ -4380,7 +4373,19 @@ exe = custom_target( install_mode : 'rwxr-xr-x', install_dir : rootlibexecdir) if want_ukify - public_programs += exe + public_programs += ukify +endif + +if want_tests != 'false' and want_kernel_install + args = [kernel_install.full_path(), loaderentry_install, uki_copy_install] + if want_ukify + args += [ukify.full_path(), ukify_install] + endif + + test('test-kernel-install', + test_kernel_install_sh, + env : test_env, + args : args) endif ############################################################ diff --git a/src/kernel-install/meson.build b/src/kernel-install/meson.build index 95aa0d9497..744071b9e9 100644 --- a/src/kernel-install/meson.build +++ b/src/kernel-install/meson.build @@ -20,10 +20,12 @@ loaderentry_install = custom_target( install_mode : 'rwxr-xr-x', install_dir : kernelinstalldir) -kernel_install_files = files( - '50-depmod.install', - '90-uki-copy.install', -) +uki_copy_install = files('90-uki-copy.install') + +kernel_install_files = [ + files('50-depmod.install'), + uki_copy_install, +] if want_kernel_install install_data(kernel_install_files, diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh index 4cbf16f0df..c9d73ff8e3 100755 --- a/src/kernel-install/test-kernel-install.sh +++ b/src/kernel-install/test-kernel-install.sh @@ -7,7 +7,10 @@ set -o pipefail export SYSTEMD_LOG_LEVEL=debug kernel_install="${1:?}" -plugin="${2:?}" +loaderentry_install="${2:?}" +uki_copy_install="${3:?}" +ukify="${4:-}" +ukify_install="${5:-}" if [[ -d "${PROJECT_BUILD_ROOT:-}" ]]; then bootctl="${PROJECT_BUILD_ROOT}/bootctl" else @@ -36,11 +39,14 @@ MACHINE_ID=badbadbadbadbadbad6abadbadbadbad EOF export KERNEL_INSTALL_CONF_ROOT="$D/sources" -export KERNEL_INSTALL_PLUGINS="$plugin" +# We "install" multiple plugins, but control which ones will be active via install.conf. +export KERNEL_INSTALL_PLUGINS="${ukify_install} ${loaderentry_install} ${uki_copy_install}" export BOOT_ROOT="$D/boot" export BOOT_MNT="$D/boot" export MACHINE_ID='3e0484f3634a418b8e6a39e8828b03e3' +export KERNEL_INSTALL_UKIFY="$ukify" +# Test type#1 installation "$kernel_install" -v add 1.1.1 "$D/sources/linux" "$D/sources/initrd" entry="$BOOT_ROOT/loader/entries/the-token-1.1.1.conf" @@ -91,7 +97,25 @@ grep -qE '^initrd .*/the-token/1.1.1/initrd' "$entry" grep -qE 'image' "$BOOT_ROOT/the-token/1.1.1/linux" grep -qE 'initrd' "$BOOT_ROOT/the-token/1.1.1/initrd" -if test -x "$bootctl"; then +# Install UKI +if [ -f "$ukify" ]; then + cat >>"$D/sources/install.conf" < Date: Fri, 14 Apr 2023 18:45:24 +0200 Subject: [PATCH 13/20] test/60-ukify: override stub location in tests Without this, build would fail if the stub is not available in /usr/lib/. --- meson.build | 7 +++++-- src/boot/efi/meson.build | 6 +++++- src/kernel-install/test-kernel-install.sh | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 910b55d3c0..28086b16e6 100644 --- a/meson.build +++ b/meson.build @@ -2178,6 +2178,9 @@ public_programs = [] # D-Bus introspection XML export dbus_programs = [] +# A list of boot stubs. Required for testing of ukify. +boot_stubs = [] + basic_includes = include_directories( 'src/basic', 'src/fundamental', @@ -4378,8 +4381,8 @@ endif if want_tests != 'false' and want_kernel_install args = [kernel_install.full_path(), loaderentry_install, uki_copy_install] - if want_ukify - args += [ukify.full_path(), ukify_install] + if want_ukify and boot_stubs.length() > 0 + args += [ukify.full_path(), ukify_install, boot_stubs[0]] endif test('test-kernel-install', diff --git a/src/boot/efi/meson.build b/src/boot/efi/meson.build index 67ce00838d..9a560cf193 100644 --- a/src/boot/efi/meson.build +++ b/src/boot/efi/meson.build @@ -334,7 +334,7 @@ foreach efi_elf_binary : efi_elf_binaries # FIXME: Use build_tgt.name() with meson >= 0.54.0 name = fs.name(efi_elf_binary.full_path()).split('.')[0] name += name.startswith('linux') ? '.efi.stub' : '.efi' - boot_targets += custom_target( + exe = custom_target( name, output : name, input : efi_elf_binary, @@ -351,6 +351,10 @@ foreach efi_elf_binary : efi_elf_binaries '@INPUT@', '@OUTPUT@', ]) + boot_targets += exe + if name.startswith('linux') + boot_stubs += exe + endif endforeach alias_target('systemd-boot', boot_targets) diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh index c9d73ff8e3..4be8771359 100755 --- a/src/kernel-install/test-kernel-install.sh +++ b/src/kernel-install/test-kernel-install.sh @@ -11,6 +11,7 @@ loaderentry_install="${2:?}" uki_copy_install="${3:?}" ukify="${4:-}" ukify_install="${5:-}" +boot_stub="${6:-}" if [[ -d "${PROJECT_BUILD_ROOT:-}" ]]; then bootctl="${PROJECT_BUILD_ROOT}/bootctl" else @@ -45,6 +46,7 @@ export BOOT_ROOT="$D/boot" export BOOT_MNT="$D/boot" export MACHINE_ID='3e0484f3634a418b8e6a39e8828b03e3' export KERNEL_INSTALL_UKIFY="$ukify" +export KERNEL_INSTALL_BOOT_STUB="$boot_stub" # Test type#1 installation "$kernel_install" -v add 1.1.1 "$D/sources/linux" "$D/sources/initrd" From a4b329e6aa0bfbd422b0dfbf4dff7111d84692ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 Apr 2023 18:53:49 +0200 Subject: [PATCH 14/20] TODO: remove two entries 0ccfd3564b2532a4da6526a9e030362c4a142b77 implemented one of the items, and this pull requests handles the other one. --- TODO | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/TODO b/TODO index b7d5813f17..116d0ee0ae 100644 --- a/TODO +++ b/TODO @@ -840,9 +840,7 @@ Features: virtio-fs. * for vendor-built signed initrds: - - kernel-install should be able to install pre-built unified kernel images in - type #2 drop-in dir in the ESP. - - kernel-install should be able install encrypted creds automatically for + - kernel-install should be able to install encrypted creds automatically for machine id, root pw, rootfs uuid, resume partition uuid, and place next to EFI kernel, for sd-stub to pick them up. These creds should be locked to the TPM, and bind to the right PCR the kernel is measured to. @@ -1919,9 +1917,6 @@ Features: - teach it to prepare an ESP wholesale, i.e. with mkfs.vfat invocation - teach it to copy in unified kernel images and maybe type #1 boot loader spec entries from host -* kernel-install: - - optionally, support generating type #2 entries instead of type #1, including signing them - * logind: - logind: optionally, ignore idle-hint logic for autosuspend, block suspend as long as a session is around - logind: wakelock/opportunistic suspend support From a758f95c334927e8547ec01b620c8518b1d2212d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Apr 2023 18:43:50 +0200 Subject: [PATCH 15/20] ukify: appease mypy Note to self: PEP 585 introduced using collection types as types, and is available since 3.9. PEP 604 allows writing unions with "|", but is only available since 3.10, so not yet here because we maintain compat with 3.9. --- src/ukify/ukify.py | 63 ++++++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index abf34ed071..7e9c7cc9ae 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -37,9 +37,13 @@ import shutil import subprocess import sys import tempfile -import typing +from typing import (Any, + Callable, + IO, + Optional, + Union) -import pefile +import pefile # type: ignore __version__ = '{{PROJECT_VERSION}} ({{GIT_VERSION}})' @@ -228,7 +232,7 @@ class Uname: class Section: name: str content: pathlib.Path - tmpfile: typing.Optional[typing.IO] = None + tmpfile: Optional[IO] = None measure: bool = False @classmethod @@ -272,7 +276,7 @@ class Section: @dataclasses.dataclass class UKI: - executable: list[typing.Union[pathlib.Path, str]] + executable: list[Union[pathlib.Path, str]] sections: list[Section] = dataclasses.field(default_factory=list, init=False) def add_section(self, section): @@ -664,7 +668,12 @@ def make_uki(opts): @dataclasses.dataclass(frozen=True) class ConfigItem: @staticmethod - def config_list_prepend(namespace, group, dest, value): + def config_list_prepend( + namespace: argparse.Namespace, + group: Optional[str], + dest: str, + value: Any, + ) -> None: "Prepend value to namespace." assert not group @@ -673,7 +682,12 @@ class ConfigItem: setattr(namespace, dest, value + old) @staticmethod - def config_set_if_unset(namespace, group, dest, value): + def config_set_if_unset( + namespace: argparse.Namespace, + group: Optional[str], + dest: str, + value: Any, + ) -> None: "Set namespace. to value only if it was None" assert not group @@ -682,7 +696,12 @@ class ConfigItem: setattr(namespace, dest, value) @staticmethod - def config_set_group(namespace, group, dest, value): + def config_set_group( + namespace: argparse.Namespace, + group: Optional[str], + dest: str, + value: Any, + ) -> None: "Set namespace.[idx] to value, with idx derived from group" if group not in namespace._groups: @@ -706,22 +725,23 @@ class ConfigItem: raise ValueError('f"Invalid boolean literal: {s!r}') # arguments for argparse.ArgumentParser.add_argument() - name: typing.Union[str, typing.List[str]] - dest: str = None - metavar: str = None - type: typing.Callable = None - nargs: str = None - action: typing.Callable = None - default: typing.Any = None - version: str = None - choices: typing.List[str] = None - help: str = None + name: Union[str, tuple[str, str]] + dest: Optional[str] = None + metavar: Optional[str] = None + type: Optional[Callable] = None + nargs: Optional[str] = None + action: Optional[Union[str, Callable]] = None + default: Any = None + version: Optional[str] = None + choices: Optional[tuple[str, ...]] = None + help: Optional[str] = None # metadata for config file parsing - config_key: str = None - config_push: typing.Callable[..., ...] = config_set_if_unset + config_key: Optional[str] = None + config_push: Callable[[argparse.Namespace, Optional[str], str, Any], None] = \ + config_set_if_unset - def _names(self) -> typing.Tuple[str]: + def _names(self) -> tuple[str, ...]: return self.name if isinstance(self.name, tuple) else (self.name,) def argparse_dest(self) -> str: @@ -742,6 +762,7 @@ class ConfigItem: assert f'{section}/{key}' == self.config_key dest = self.argparse_dest() + conv: Callable[[str], Any] if self.action == argparse.BooleanOptionalAction: # We need to handle this case separately: the options are called # --foo and --no-foo, and no argument is parsed. But in the config @@ -759,7 +780,7 @@ class ConfigItem: self.config_push(namespace, group, dest, value) - def config_example(self) -> typing.Tuple[typing.Optional[str]]: + def config_example(self) -> tuple[Optional[str], Optional[str], Optional[str]]: if not self.config_key: return None, None, None section_name, key = self.config_key.split('/', 1) From 041f536f9a9dfbf8023db680f536f0080ad054c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 22 Apr 2023 11:20:11 +0200 Subject: [PATCH 16/20] test_ukify: propagate failure Oops. This explains why the tests were "passing" in CI even though a direct pytest invocation would fail. --- src/ukify/test/test_ukify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 3ffa8fe843..1013f649be 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -495,4 +495,4 @@ def test_pcr_signing2(kernel_initrd, tmpdir): assert len(sig['sha1']) == 6 # six items for six phases paths if __name__ == '__main__': - pytest.main([__file__, '-v']) + sys.exit(pytest.main([__file__, '-v'])) From 55be961f48cfb5116776bf43956eb3b9600a3f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 22 Apr 2023 13:10:28 +0200 Subject: [PATCH 17/20] test_ukify: rework how --flakes argument is appended The usual approach is to put 'addopts = --flakes' in setup.cfg. Unfortunately this fails badly when pytest-flakes is not installed: ERROR: usage: test_ukify.py [options] [file_or_dir] [file_or_dir] [...] test_ukify.py: error: unrecognized arguments: --flakes pytest-flakes is not packaged everywhere, and this test is not very important, so let's just do it only if pytest-flakes is available. We now detect if pytest-flakes is available and only add '--flakes' conditionally. This unfortunately means that when invoked via 'pytest' or directly as 'src/ukify/test/test_ukify.py', '--flakes' will not be appended automatically. But I don't see a nice way to achieve previous automatic behaviour. (I first considered making 'setup.cfg' templated. But then it is created in the build directory, but we would need it in the source directory for pytest to load it automatically. So to load the file, we'd need to give an argument to pytest anyway, so we don't gain anything with this more complex approach.) --- src/ukify/test/meson.build | 18 +++++++++++++++--- src/ukify/test/setup.cfg | 2 -- src/ukify/test/test_ukify.py | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) delete mode 100644 src/ukify/test/setup.cfg diff --git a/src/ukify/test/meson.build b/src/ukify/test/meson.build index e39178f892..e78e76c673 100644 --- a/src/ukify/test/meson.build +++ b/src/ukify/test/meson.build @@ -1,7 +1,19 @@ # SPDX-License-Identifier: LGPL-2.1-or-later if want_ukify and want_tests != 'false' - test('test-ukify', - files('test_ukify.py'), - env : test_env) + have_pytest_flakes = pymod.find_installation( + 'python3', + required : false, + modules : ['pytest_flakes'], + ).found() + + args = ['-v'] + if have_pytest_flakes + args += ['--flakes'] + endif + + test('test-ukify', + files('test_ukify.py'), + args: args, + env : test_env) endif diff --git a/src/ukify/test/setup.cfg b/src/ukify/test/setup.cfg deleted file mode 100644 index 1f655da834..0000000000 --- a/src/ukify/test/setup.cfg +++ /dev/null @@ -1,2 +0,0 @@ -[tool:pytest] -addopts = --flakes diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 1013f649be..3cd895fe36 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -495,4 +495,4 @@ def test_pcr_signing2(kernel_initrd, tmpdir): assert len(sig['sha1']) == 6 # six items for six phases paths if __name__ == '__main__': - sys.exit(pytest.main([__file__, '-v'])) + sys.exit(pytest.main(sys.argv)) From bac18826e950f5cc66a8e747135535c77afec234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 22 Apr 2023 13:17:32 +0200 Subject: [PATCH 18/20] ci: install pytest-flakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some web searches say that it's packaged for those distros and not the others… v2: - drop arch. https://aur.archlinux.org/packages/python-pytest-flakes exists, but installation fails in CI. --- mkosi.presets/00-base/mkosi.conf.d/10-fedora.conf | 3 +++ mkosi.presets/00-base/mkosi.conf.d/10-opensuse.conf | 1 + 2 files changed, 4 insertions(+) diff --git a/mkosi.presets/00-base/mkosi.conf.d/10-fedora.conf b/mkosi.presets/00-base/mkosi.conf.d/10-fedora.conf index 035715979c..eba6f040da 100644 --- a/mkosi.presets/00-base/mkosi.conf.d/10-fedora.conf +++ b/mkosi.presets/00-base/mkosi.conf.d/10-fedora.conf @@ -4,6 +4,9 @@ Distribution=fedora [Content] +Packages= + python3dist(pytest-flakes) + BuildPackages= pkgconfig(libgcrypt) pkgconfig(xencontrol) diff --git a/mkosi.presets/00-base/mkosi.conf.d/10-opensuse.conf b/mkosi.presets/00-base/mkosi.conf.d/10-opensuse.conf index b8bce7148e..4ed5f6ff7c 100644 --- a/mkosi.presets/00-base/mkosi.conf.d/10-opensuse.conf +++ b/mkosi.presets/00-base/mkosi.conf.d/10-opensuse.conf @@ -27,6 +27,7 @@ Packages= libxkbcommon0 libzstd1 pam + python3-pytest-flakes shadow tpm2-0-tss xz From 248be6ef37af773c3b7b73a996470f2c95fec7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 24 Apr 2023 12:40:08 +0200 Subject: [PATCH 19/20] man: describe all the changes to ukify As in mkosi(1), let's describe the config file and commandline options together. This is nice for us, because we don't need to duplicate descriptions and we're less likely to forget to update one place or the other. This is also nice for users, because they can easily figure out what can be configured where. The options are now ordered by config file section. --summary was not described before. More examples are added. --- man/ukify.xml | 439 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 285 insertions(+), 154 deletions(-) diff --git a/man/ukify.xml b/man/ukify.xml index c6bfbdc9d9..6aa136298d 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -44,212 +44,302 @@ Additional sections will be inserted into the UKI, either automatically or only if a specific option is provided. See the discussions of - , - , - , - , - , - , + Cmdline=/, + OSRelease=/, + DeviceTree=/, + Splash=/, + PCRPKey=/, + Uname=/, and below. ukify can also be used to assemble a PE binary that is not executable but contains auxiliary data, for example additional kernel command line entries. - If PCR signing keys are provided via the and - options, PCR values that will be seen after booting with the given - kernel, initrd, and other sections, will be calculated, signed, and embedded in the UKI. + If PCR signing keys are provided via the + PCRPrivateKey=/ and + PCRPublicKey=/ options, PCR values that will be seen + after booting with the given kernel, initrd, and other sections, will be calculated, signed, and embedded + in the UKI. systemd-measure1 is used to perform this calculation and signing. The calculation of PCR values is done for specific boot phase paths. Those can be specified with - the option. If not specified, the default provided by - systemd-measure is used. It is also possible to specify the - , , and - arguments more than once. Signatures will be then performed with each of the specified keys. When both - and are used, they must be specified the - same number of times, and then the n-th boot phase path set will be signed by the n-th key. This can be - used to build different trust policies for different phases of the boot. + the Phases=/ option. If not specified, the default provided + by systemd-measure is used. It is also possible to specify the + PCRPrivateKey=/, + PCRPublicKey=/, and + Phases=/ arguments more than once. Signatures will then be + performed with each of the specified keys. On the command line, when both and + are used, they must be specified the same number of times, and then + the n-th boot phase path set will be signed by the n-th key. This can be used to build different trust + policies for different phases of the boot. In the config file, PCRPrivateKey=, + PCRPublicKey=, and Phases= are grouped into separate sections, + describing separate boot phases. - If a SecureBoot signing key is provided via the option, - the resulting PE binary will be signed as a whole, allowing the resulting UKI to be trusted by - SecureBoot. Also see the discussion of automatic enrollment in + If a SecureBoot signing key is provided via the + SecureBootPrivateKey=/ option, the resulting + PE binary will be signed as a whole, allowing the resulting UKI to be trusted by SecureBoot. Also see the + discussion of automatic enrollment in systemd-boot7. - Options + Configuration settings - The LINUX and INITRD positional arguments are - optional. If more than one INITRD are specified, they will all be combined into - a single PE section. This is useful to for example prepend microcode before the actual initrd. + Settings can appear in configuration files (the syntax with SomeSetting=value) and on the command line (the syntax + with ). For some command + line parameters, a single-letter shortcut is also allowed. In the configuration files, the setting must + be in the appropriate section, so the descriptions are grouped by section below. When the same setting + appears in the configuration file and on the command line, generally the command line setting has higher + priority and overwrites the config file setting completely. If some setting behaves differently, this is + described below. - The following options are understood: + The LINUX and INITRD positional arguments, or + the equivalent Linux= and Initrd= settings, are optional. If more + than one initrd is specified, they will all be combined into a single PE section. This is useful to, for + example, prepend microcode before the actual initrd. - - - + The following options and settings are understood: - Specify the kernel command line (the .cmdline section). The - argument may be a literal string, or @ followed by a path name. If not specified, - no command line will be embedded. - + + Commandline-only options - - + + + - Specify the os-release description (the .osrel section). The - argument may be a literal string, or @ followed by a path name. If not specified, - the os-release5 - file will be picked up from the host system. - + Load configuration from the given config file. In general, settings specified in + the config file have lower precedence than the settings specified via options. In cases where the + commandline option does not fully override the config file setting are explicitly mentioned in the + descriptions of individual options. + - - + + + - Specify the devicetree description (the .dtb section). The - argument is a path to a compiled binary DeviceTree file. If not specified, the section will not be - present. - + Enable or disable a call to systemd-measure to print + pre-calculated PCR values. Defaults to false. + - - + + - Specify a picture to display during boot (the .splash section). - The argument is a path to a BMP file. If not specified, the section will not be present. - - + Specify an arbitrary additional section + NAME. Note that the name is used as-is, and if the + section name should start with a dot, it must be included in NAME. The + argument may be a literal string, or @ followed by a path name. This option may be + specified more than once. Any sections specified in this fashion will be inserted (in order) before + the .linux section which is always last. + - - + + - Specify a path to a public key to embed in the .pcrpkey section. - If not specified, and there's exactly one argument, that key will - be used. Otherwise, the section will not be present. - + Specify one or more directories with helper tools. ukify will + look for helper tools in those directories first, and if not found, try to load them from + $PATH in the usual fashion. + - - + + - Specify the kernel version (as in uname -r, the - .uname section). If not specified, an attempt will be made to extract the version - string from the kernel image. It is recommended to pass this explicitly if known, because the - extraction is based on heuristics and not very reliable. If not specified and extraction fails, the - section will not be present. - + The output filename. If not specified, the name of the + LINUX argument, with the suffix .unsigned.efi or + .signed.efi will be used, depending on whether signing for SecureBoot was + performed. + - - + + - Specify an arbitrary additional section - NAME. Note that the name is used as-is, and if the - section name should start with a dot, it must be included in NAME. The - argument may be a literal string, or @ followed by a path name. This option may be - specified more than once. Any sections specified in this fashion will be inserted (in order) before - the .linux section which is always last. - + Print a summary of loaded config and exit. This is useful to check how the options + form the configuration file and the commandline are combined. + - - + + + + - Specify a private key to use for signing PCR policies. This option may be specified - more than once, in which case multiple signatures will be made. - + + [UKI] section - - + + + Linux=LINUX + positional argument LINUX - Specify a public key to use for signing PCR policies. This option may be specified - more than once, similarly to the option. If not present, the - public keys will be extracted from the private keys. If present, the this option must be specified - the same number of times as the option. - + A path to the kernel binary. + - - + + Initrd=INITRD... + positional argument INITRD - A comma or space-separated list of colon-separated phase paths to sign a policy for. - If not present, the default of - systemd-measure1 - will be used. When this argument is present, it must appear the same number of times as the - option. Each set of boot phase paths will be signed with the - corresponding private key. - + Zero or more initrd paths. In the configuration file, items are separated by + whitespace. The initrds are combined in the order of specification, with the initrds specified in + the config file first. + - - + + Cmdline=TEXT|@PATH + - A comma or space-separated list of PCR banks to sign a policy for. If not present, - all known banks will be used (sha1, sha256, - sha384, sha512), which will fail if not supported by the - system. - + The kernel command line (the .cmdline section). The argument may + be a literal string, or @ followed by a path name. If not specified, no command + line will be embedded. + - - + + OSRelease=TEXT|@PATH + - A path to a private key to use for signing of the resulting binary. If the - option is used, this may also be an engine-specific - designation. - + The os-release description (the .osrel section). The argument + may be a literal string, or @ followed by a path name. If not specified, the + os-release5 file + will be picked up from the host system. + - - + + DeviceTree=PATH + - A path to a certificate to use for signing of the resulting binary. If the - option is used, this may also be an engine-specific - designation. - + The devicetree description (the .dtb section). The argument is a + path to a compiled binary DeviceTree file. If not specified, the section will not be present. + + - - + + Splash=PATH + - An "engine" to for signing of the resulting binary. This option is currently passed - verbatim to the option of - sbsign1. - - + A picture to display during boot (the .splash section). The + argument is a path to a BMP file. If not specified, the section will not be present. + + - - - + + PCRPKey=PATH + - Override the detection of whether to sign the Linux binary itself before it is - embedded in the combined image. If not specified, it will be signed if a SecureBoot signing key is - provided via the option and the binary has not already - been signed. If is specified, and the binary has already been signed, - the signature will be appended anyway. - + A path to a public key to embed in the .pcrpkey section. If not + specified, and there's exactly one + PCRPublicKey=/ argument, that key will be used. + Otherwise, the section will not be present. + - - + + Uname=VERSION + - Specify one or more directories with helper tools. ukify will look - for helper tools in those directories first, and if not found, try to load them from - $PATH in the usual fashion. - + Specify the kernel version (as in uname -r, the + .uname section). If not specified, an attempt will be made to extract the + version string from the kernel image. It is recommended to pass this explicitly if known, because + the extraction is based on heuristics and not very reliable. If not specified and extraction fails, + the section will not be present. + - - - + + PCRBanks=PATH + - Enable or disable a call to systemd-measure to print - pre-calculated PCR values. Defaults to false. - + A comma or space-separated list of PCR banks to sign a policy for. If not present, + all known banks will be used (sha1, sha256, + sha384, sha512), which will fail if not supported by the + system. + - - + + SecureBootPrivateKey=SB_KEY + - The output filename. If not specified, the name of the - LINUX argument, with the suffix .unsigned.efi or - .signed.efi will be used, depending on whether signing for SecureBoot was - performed. - + A path to a private key to use for signing of the resulting binary. If the + SigningEngine=/ option is used, this may also be + an engine-specific designation. + - - - + + SecureBootCertificate=SB_CERT + + + A path to a certificate to use for signing of the resulting binary. If the + SigningEngine=/ option is used, this may also + be an engine-specific designation. + + + + SigningEngine=ENGINE + + + An "engine" to for signing of the resulting binary. This option is currently passed + verbatim to the option of + sbsign1. + + + + + SignKernel=BOOL + + + + Override the detection of whether to sign the Linux binary itself before it is + embedded in the combined image. If not specified, it will be signed if a SecureBoot signing key is + provided via the + SecureBootPrivateKey=/ option and the + binary has not already been signed. If + SignKernel=/ is true, and the binary has already + been signed, the signature will be appended anyway. + + + + + + [PCRSignature:<replaceable>NAME</replaceable>] section + + In the config file, those options are grouped by section. On the commandline, they + must be specified in the same order. The sections specified in both sources are combined. + + + + + PCRPrivateKey=PATH + + + A private key to use for signing PCR policies. On the commandline, this option may + be specified more than once, in which case multiple signatures will be made. + + + + PCRPublicKey=PATH + + + A public key to use for signing PCR policies. + + On the commandline, this option may be specified more than once, similarly to the + option. If not present, the public keys will be extracted from + the private keys. On the commandline, if present, the this option must be specified the same number + of times as the option. + + + + Phases=LIST + + + A comma or space-separated list of colon-separated phase paths to sign a policy + for. Each set of boot phase paths will be signed with the corresponding private key. If not + present, the default of + systemd-measure1 + will be used. + + On the commandline, when this argument is present, it must appear the same number of times as + the option. + + + @@ -258,7 +348,7 @@ Minimal invocation - ukify \ + $ ukify \ /lib/modules/6.0.9-300.fc37.x86_64/vmlinuz \ /some/path/initramfs-6.0.9-300.fc37.x86_64.img \ --cmdline='quiet rw' @@ -270,7 +360,7 @@ All the bells and whistles - /usr/lib/systemd/ukify \ + # /usr/lib/systemd/ukify \ /lib/modules/6.0.9-300.fc37.x86_64/vmlinuz \ early_cpio \ /some/path/initramfs-6.0.9-300.fc37.x86_64.img \ @@ -299,6 +389,45 @@ combined image will be signed with the SecureBoot key sb.key. + + All the bells and whistles, via a config file + + This is the same as the previous example, but this time the configuration is stored in a + file: + + $ cat ukify.conf +[UKI] +Initrd=early_cpio +Cmdline=quiet rw rhgb + +SecureBootPrivateKey=sb.key +SecureBootCerificate=sb.cert +SignKernel=yes +PCRBanks=sha384,sha512 + +[PCRSignature:initrd] +PCRPrivateKey=pcr-private-initrd-key.pem +PCRPublicKey=pcr-public-initrd-key.pem +Phases=enter-initrd + +[PCRSignature:system] +PCRPrivateKey=pcr-private-system-key.pem +PCRPublicKey=pcr-public-system-key.pem +Phases=enter-initrd:leave-initrd + enter-initrd:leave-initrd:sysinit + enter-initrd:leave-initrd:sysinit:ready + +# /usr/lib/systemd/ukify -c ukify.conf \ + /lib/modules/6.0.9-300.fc37.x86_64/vmlinuz \ + /some/path/initramfs-6.0.9-300.fc37.x86_64.img + + + One "initrd" (early_cpio) is specified in the config file, and + the other initrd (initramfs-6.0.9-300.fc37.x86_64.img) is specified + on the commandline. This may be useful for example when the first initrd contains microcode for the CPU + and does not need to be updated when the kernel version changes, unlike the actual initrd. + + Kernel command line auxiliary PE @@ -309,7 +438,8 @@ --output=debug.cmdline.efi - This creates a signed PE binary that contains an additional kernel command line parameter. + This creates a signed PE binary that contains the additional kernel command line parameter + debug. @@ -319,6 +449,7 @@ systemd1, systemd-stub7, systemd-boot7, + systemd-measure1, systemd-pcrphase.service8 From 46886f130d505f483ee1305a51f04196a551e9a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 4 May 2023 15:17:27 +0200 Subject: [PATCH 20/20] test_ukify: add test for combining config and cmdline --- src/ukify/test/test_ukify.py | 80 ++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 3cd895fe36..d221825019 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -166,6 +166,7 @@ def test_parse_args_many(): ]) assert opts.linux == pathlib.Path('/ARG1') assert opts.initrd == [pathlib.Path('/ARG2'), pathlib.Path('/ARG3 WITH SPACE')] + assert opts.cmdline == 'a b c' assert opts.os_release == 'K1=V1\nK2=V2' assert opts.devicetree == pathlib.Path('DDDDTTTT') assert opts.splash == pathlib.Path('splash') @@ -204,6 +205,85 @@ def test_parse_sections(): assert opts.sections[1].tmpfile is None assert opts.sections[1].measure is False +def test_config_priority(tmp_path): + config = tmp_path / 'config1.conf' + config.write_text(textwrap.dedent( + f''' + [UKI] + Linux = LINUX + Initrd = initrd1 initrd2 + initrd3 + Cmdline = 1 2 3 4 5 + 6 7 8 + OSRelease = @some/path1 + DeviceTree = some/path2 + Splash = some/path3 + Uname = 1.2.3 + EFIArch=arm + Stub = some/path4 + PCRBanks = sha512,sha1 + SigningEngine = engine1 + SecureBootPrivateKey = some/path5 + SecureBootCertificate = some/path6 + SignKernel = no + + [PCRSignature:NAME] + PCRPrivateKey = some/path7 + PCRPublicKey = some/path8 + Phases = {':'.join(ukify.KNOWN_PHASES)} + ''')) + + opts = ukify.parse_args( + ['/ARG1', '///ARG2', '/ARG3 WITH SPACE', + '--cmdline= a b c ', + '--os-release=K1=V1\nK2=V2', + '--devicetree=DDDDTTTT', + '--splash=splash', + '--pcrpkey=PATH', + '--uname=1.2.3', + '--stub=STUBPATH', + '--pcr-private-key=PKEY1', + '--pcr-public-key=PKEY2', + '--pcr-banks=SHA1,SHA256', + '--signing-engine=ENGINE', + '--secureboot-private-key=SBKEY', + '--secureboot-certificate=SBCERT', + '--sign-kernel', + '--no-sign-kernel', + '--tools=TOOLZ///', + '--output=OUTPUT', + '--measure', + ]) + + ukify.apply_config(opts, config) + ukify.finalize_options(opts) + + assert opts.linux == pathlib.Path('/ARG1') + assert opts.initrd == [pathlib.Path('initrd1'), + pathlib.Path('initrd2'), + pathlib.Path('initrd3'), + pathlib.Path('/ARG2'), + pathlib.Path('/ARG3 WITH SPACE')] + assert opts.cmdline == 'a b c' + assert opts.os_release == 'K1=V1\nK2=V2' + assert opts.devicetree == pathlib.Path('DDDDTTTT') + assert opts.splash == pathlib.Path('splash') + assert opts.pcrpkey == pathlib.Path('PATH') + assert opts.uname == '1.2.3' + assert opts.stub == pathlib.Path('STUBPATH') + assert opts.pcr_private_keys == [pathlib.Path('PKEY1'), + pathlib.Path('some/path7')] + assert opts.pcr_public_keys == [pathlib.Path('PKEY2'), + pathlib.Path('some/path8')] + assert opts.pcr_banks == ['SHA1', 'SHA256'] + assert opts.signing_engine == 'ENGINE' + assert opts.sb_key == 'SBKEY' + assert opts.sb_cert == 'SBCERT' + assert opts.sign_kernel is False + assert opts.tools == [pathlib.Path('TOOLZ/')] + assert opts.output == pathlib.Path('OUTPUT') + assert opts.measure is True + def test_help(capsys): with pytest.raises(SystemExit): ukify.parse_args(['--help'])