From 6fa7913830d401a51ac35b35713fe5edf2063a64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 17:32:47 +0200 Subject: [PATCH 01/11] ukify: use pager for --help The output is now too long to fit on one page, let's use a pager automatically like in other places. The implementation is copied from mkosi, but adjusted to follow what other systemd tools do. --- src/ukify/ukify.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index a9c21601df9..1320c46a718 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -33,6 +33,7 @@ import json import os import pathlib import pprint +import pydoc import re import shlex import shutil @@ -43,6 +44,7 @@ from typing import (Any, Callable, IO, Optional, + Sequence, Union) import pefile # type: ignore @@ -88,6 +90,15 @@ def guess_efi_arch(): return efi_arch +def page(text: str, enabled: Optional[bool]) -> None: + if enabled: + # Initialize less options from $SYSTEMD_LESS or provide a suitable fallback. + os.environ['LESS'] = os.getenv('SYSTEMD_LESS', 'FRSXMK') + pydoc.pager(text) + else: + print(text) + + 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) @@ -1128,10 +1139,23 @@ def config_example(): yield f'{key} = {value}' +class PagerHelpAction(argparse._HelpAction): # pylint: disable=protected-access + def __call__( + self, + parser: argparse.ArgumentParser, + namespace: argparse.Namespace, + values: Union[str, Sequence[Any], None] = None, + option_string: Optional[str] = None + ) -> None: + page(parser.format_help(), True) + parser.exit() + + def create_parser(): p = argparse.ArgumentParser( description='Build and sign Unified Kernel Images', allow_abbrev=False, + add_help=False, usage='''\ ukify [options…] [LINUX INITRD…] ''', @@ -1145,6 +1169,13 @@ ukify [options…] [LINUX INITRD…] # Suppress printing of usage synopsis on errors p.error = lambda message: p.exit(2, f'{p.prog}: error: {message}\n') + # Make --help paged + p.add_argument( + '-h', '--help', + action=PagerHelpAction, + help='show this help message and exit', + ) + return p From 794385f5df66683ea5c163722e4f2f4574745287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 20:27:48 +0200 Subject: [PATCH 02/11] ukify: fix synopsis in --help Missed in a3b227d28a823d59e3281109f5369d0f63b40f62. --- src/ukify/ukify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 1320c46a718..e7b3db65254 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -1157,7 +1157,7 @@ def create_parser(): allow_abbrev=False, add_help=False, usage='''\ -ukify [options…] [LINUX INITRD…] +ukify [options…] VERB ''', epilog='\n '.join(('config file:', *config_example())), formatter_class=argparse.RawDescriptionHelpFormatter, From 24f4ff8a09d8212642bf01f859bb01272ae75845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 20:32:33 +0200 Subject: [PATCH 03/11] man,test: root or hardware is not needed for ukify --- man/ukify.xml | 4 ++-- src/ukify/test/test_ukify.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/man/ukify.xml b/man/ukify.xml index 098dacfb99f..b2e7f82d8fe 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -415,7 +415,7 @@ All the bells and whistles - # /usr/lib/systemd/ukify build \ + $ /usr/lib/systemd/ukify build \ --linux=/lib/modules/6.0.9-300.fc37.x86_64/vmlinuz \ --initrd=early_cpio \ --initrd=/some/path/initramfs-6.0.9-300.fc37.x86_64.img \ @@ -472,7 +472,7 @@ Phases=enter-initrd:leave-initrd enter-initrd:leave-initrd:sysinit enter-initrd:leave-initrd:sysinit:ready -# /usr/lib/systemd/ukify -c ukify.conf build \ +$ /usr/lib/systemd/ukify -c ukify.conf build \ --linux=/lib/modules/6.0.9-300.fc37.x86_64/vmlinuz \ --initrd=/some/path/initramfs-6.0.9-300.fc37.x86_64.img diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index eae82c7f88f..5829bae6985 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -588,7 +588,7 @@ def test_pcr_signing(kernel_initrd, tmpdir): '--uname=1.2.3', '--cmdline=ARG1 ARG2 ARG3', '--os-release=ID=foobar\n', - '--pcr-banks=sha1', # use sha1 as that is most likely to be supported + '--pcr-banks=sha1', # use sha1 because it doesn't really matter f'--pcrpkey={pub.name}', f'--pcr-public-key={pub.name}', f'--pcr-private-key={priv.name}', @@ -655,7 +655,7 @@ def test_pcr_signing2(kernel_initrd, tmpdir): '--uname=1.2.3', '--cmdline=ARG1 ARG2 ARG3', '--os-release=ID=foobar\n', - '--pcr-banks=sha1', # use sha1 as that is most likely to be supported + '--pcr-banks=sha1', f'--pcrpkey={pub2.name}', f'--pcr-public-key={pub.name}', f'--pcr-private-key={priv.name}', From 9a1cb2038e93213b177ed0bd85b709a7530c9d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 21:03:10 +0200 Subject: [PATCH 04/11] ukify: split out iteration over phase path groups and keys In preparation for future changes. --- src/ukify/ukify.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index e7b3db65254..ee8a9029bc3 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -381,6 +381,19 @@ def combine_signatures(pcrsigs): return json.dumps(combined) +def key_path_groups(opts): + if not opts.pcr_private_keys: + return + + n_priv = len(opts.pcr_private_keys or ()) + pub_keys = opts.pcr_public_keys or [None] * n_priv + pp_groups = opts.phase_path_groups or [None] * n_priv + + yield from zip(opts.pcr_private_keys, + pub_keys, + pp_groups) + + def call_systemd_measure(uki, linux, opts): measure_tool = find_tool('systemd-measure', '/usr/lib/systemd/systemd-measure', @@ -414,10 +427,6 @@ def call_systemd_measure(uki, linux, opts): # PCR signing if opts.pcr_private_keys: - n_priv = len(opts.pcr_private_keys or ()) - pp_groups = opts.phase_path_groups or [None] * n_priv - pub_keys = opts.pcr_public_keys or [None] * n_priv - pcrsigs = [] cmd = [ @@ -431,9 +440,7 @@ def call_systemd_measure(uki, linux, opts): for bank in banks), ] - for priv_key, pub_key, group in zip(opts.pcr_private_keys, - pub_keys, - pp_groups): + for priv_key, pub_key, group in key_path_groups(opts): extra = [f'--private-key={priv_key}'] if pub_key: extra += [f'--public-key={pub_key}'] From b09a5315f5263b1e344e6612514e3ea08600f1d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Jun 2023 09:10:49 +0200 Subject: [PATCH 05/11] ukify: move verb mangling to finalize_options() This simplifies the logic: finalize_options() is the step that does the checks and the mangling. The checks for consistency are done in more places, so we need to pass a verb (we only have 'build', but once we add other verbs, any would do). --- src/kernel-install/60-ukify.install.in | 2 +- src/ukify/test/test_ukify.py | 2 +- src/ukify/ukify.py | 55 ++++++++++++-------------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/kernel-install/60-ukify.install.in b/src/kernel-install/60-ukify.install.in index 0927bd7a2e8..96ca2482b06 100755 --- a/src/kernel-install/60-ukify.install.in +++ b/src/kernel-install/60-ukify.install.in @@ -186,7 +186,7 @@ def call_ukify(opts): # Create "empty" namespace. We want to override just a few settings, so it # doesn't make sense to configure everything. We pretend to parse an empty # argument set to prepopulate the namespace with the defaults. - opts2 = ukify['create_parser']().parse_args(()) + opts2 = ukify['create_parser']().parse_args(['build']) opts2.config = config_file_location() opts2.uname = opts.kernel_version diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 5829bae6985..3ca9b531c24 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -87,7 +87,7 @@ def test_apply_config(tmp_path): Phases = {':'.join(ukify.KNOWN_PHASES)} ''')) - ns = ukify.create_parser().parse_args(()) + ns = ukify.create_parser().parse_args(['build']) ns.linux = None ns.initrd = [] ukify.apply_config(ns, config) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index ee8a9029bc3..9abaefec9ae 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -1187,6 +1187,31 @@ ukify [options…] VERB def finalize_options(opts): + # Figure out which syntax is being used, one of: + # ukify verb --arg --arg --arg + # ukify linux initrd… + if len(opts.positional) == 1 and opts.positional[0] in VERBS: + opts.verb = opts.positional[0] + elif opts.linux or opts.initrd: + raise ValueError('--linux/--initrd options cannot be used with positional arguments') + else: + print("Assuming obsolete commandline syntax with no verb. Please use 'build'.") + if opts.positional: + opts.linux = pathlib.Path(opts.positional[0]) + # If we have initrds from parsing config files, append our positional args at the end + opts.initrd = (opts.initrd or []) + [pathlib.Path(arg) for arg in opts.positional[1:]] + opts.verb = 'build' + + # 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=') + if opts.cmdline and opts.cmdline.startswith('@'): opts.cmdline = pathlib.Path(opts.cmdline[1:]) elif opts.cmdline: @@ -1244,37 +1269,9 @@ def finalize_options(opts): def parse_args(args=None): - p = create_parser() - opts = p.parse_args(args) - - # Figure out which syntax is being used, one of: - # ukify verb --arg --arg --arg - # ukify linux initrd… - if len(opts.positional) == 1 and opts.positional[0] in VERBS: - opts.verb = opts.positional[0] - elif opts.linux or opts.initrd: - raise ValueError('--linux/--initrd options cannot be used with positional arguments') - else: - print("Assuming obsolete commandline syntax with no verb. Please use 'build'.") - if opts.positional: - opts.linux = pathlib.Path(opts.positional[0]) - opts.initrd = [pathlib.Path(arg) for arg in opts.positional[1:]] - opts.verb = 'build' - - # 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=') - + opts = create_parser().parse_args(args) apply_config(opts) - finalize_options(opts) - return opts From a1c80efddc057b4d1dcddc51dbb1244e8df51752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 21:06:20 +0200 Subject: [PATCH 06/11] ukify: add 'genkey' verb The idea is to make it easy to generate all the signing key and certs that can be used for local signing. The verb is the modeled after 'mkosi genkey', but there are some important differences: we generate the keys to the paths where they will be read from, both pcr signing keys and the SecureBoot certificate+key. If any of the outputs exist, operation is refused. Maybe we could add a --force option in the future, but this operation should be rare, so I think it's better to refuse to overwrite anything initially. I'm only doing a token man page change here. https://github.com/systemd/systemd/pull/27621 reworks the man page, and the changes done here would conflict heavily with that work. I'll submit a follow-up patch later. --- man/ukify.xml | 1 + src/ukify/test/test_ukify.py | 52 +++++++++++++ src/ukify/ukify.py | 141 +++++++++++++++++++++++++++++++++-- 3 files changed, 188 insertions(+), 6 deletions(-) diff --git a/man/ukify.xml b/man/ukify.xml index b2e7f82d8fe..283d58b3b05 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -25,6 +25,7 @@ /usr/lib/systemd/ukify OPTIONS build + genkey diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 3ca9b531c24..ac39a719402 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -698,5 +698,57 @@ def test_pcr_signing2(kernel_initrd, tmpdir): assert list(sig.keys()) == ['sha1'] assert len(sig['sha1']) == 6 # six items for six phases paths +def test_key_cert_generation(tmpdir): + opts = ukify.parse_args([ + 'genkey', + f"--pcr-public-key={tmpdir / 'pcr1.pub.pem'}", + f"--pcr-private-key={tmpdir / 'pcr1.priv.pem'}", + '--phases=enter-initrd enter-initrd:leave-initrd', + f"--pcr-public-key={tmpdir / 'pcr2.pub.pem'}", + f"--pcr-private-key={tmpdir / 'pcr2.priv.pem'}", + '--phases=sysinit ready', + f"--secureboot-private-key={tmpdir / 'sb.priv.pem'}", + f"--secureboot-certificate={tmpdir / 'sb.cert.pem'}", + ]) + assert opts.verb == 'genkey' + ukify.check_cert_and_keys_nonexistent(opts) + ukify.generate_keys(opts) + + if not shutil.which('openssl'): + return + + for key in (tmpdir / 'pcr1.priv.pem', + tmpdir / 'pcr2.priv.pem', + tmpdir / 'sb.priv.pem'): + out = subprocess.check_output([ + 'openssl', 'rsa', + '-in', key, + '-text', + '-noout', + ], text = True) + assert 'Private-Key' in out + assert '2048 bit' in out + + for pub in (tmpdir / 'pcr1.pub.pem', + tmpdir / 'pcr2.pub.pem'): + out = subprocess.check_output([ + 'openssl', 'rsa', + '-pubin', + '-in', pub, + '-text', + '-noout', + ], text = True) + assert 'Public-Key' in out + assert '2048 bit' in out + + out = subprocess.check_output([ + 'openssl', 'x509', + '-in', tmpdir / 'sb.cert.pem', + '-text', + '-noout', + ], text = True) + assert 'Certificate' in out + assert 'Issuer: CN = SecureBoot signing key on host' in out + if __name__ == '__main__': sys.exit(pytest.main(sys.argv)) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 9abaefec9ae..4fc3ce2e192 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -25,8 +25,10 @@ import argparse import configparser +import contextlib import collections import dataclasses +import datetime import fnmatch import itertools import json @@ -37,6 +39,7 @@ import pydoc import re import shlex import shutil +import socket import subprocess import sys import tempfile @@ -356,6 +359,17 @@ def check_inputs(opts): check_splash(opts.splash) +def check_cert_and_keys_nonexistent(opts): + # Raise if any of the keys and certs are found on disk + paths = itertools.chain( + (opts.sb_key, opts.sb_cert), + *((priv_key, pub_key) + for priv_key, pub_key, _ in key_path_groups(opts))) + for path in paths: + if path and path.exists(): + raise ValueError(f'{path} is present') + + def find_tool(name, fallback=None, opts=None): if opts and opts.tools: for d in opts.tools: @@ -385,7 +399,7 @@ def key_path_groups(opts): if not opts.pcr_private_keys: return - n_priv = len(opts.pcr_private_keys or ()) + n_priv = len(opts.pcr_private_keys) pub_keys = opts.pcr_public_keys or [None] * n_priv pp_groups = opts.phase_path_groups or [None] * n_priv @@ -729,6 +743,116 @@ def make_uki(opts): print(f"Wrote {'signed' if sign_args_present else 'unsigned'} {opts.output}") +ONE_DAY = datetime.timedelta(1, 0, 0) + + +@contextlib.contextmanager +def temporary_umask(mask: int): + # Drop bits from umask + old = os.umask(0) + os.umask(old | mask) + try: + yield + finally: + os.umask(old) + + +def generate_key_cert_pair( + common_name: str, + keylength: int = 2048, + valid_days: int = 365 * 10, # TODO: can we drop the expiration date? +) -> tuple[bytes]: + + from cryptography import x509 + import cryptography.hazmat.primitives as hp + + # We use a keylength of 2048 bits. That is what Microsoft documents as + # supported/expected: + # https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/windows-secure-boot-key-creation-and-management-guidance?view=windows-11#12-public-key-cryptography + + now = datetime.datetime.utcnow() + + key = hp.asymmetric.rsa.generate_private_key( + public_exponent=65537, + key_size=keylength, + ) + cert = x509.CertificateBuilder( + ).subject_name( + x509.Name([x509.NameAttribute(x509.oid.NameOID.COMMON_NAME, common_name)]) + ).issuer_name( + x509.Name([x509.NameAttribute(x509.oid.NameOID.COMMON_NAME, common_name)]) + ).not_valid_before( + now, + ).not_valid_after( + now + ONE_DAY * valid_days + ).serial_number( + x509.random_serial_number() + ).public_key( + key.public_key() + ).add_extension( + x509.BasicConstraints(ca=False, path_length=None), + critical=True, + ).sign( + private_key=key, + algorithm=hp.hashes.SHA256(), + ) + + cert_pem = cert.public_bytes( + encoding=hp.serialization.Encoding.PEM, + ) + key_pem = key.private_bytes( + encoding=hp.serialization.Encoding.PEM, + format=hp.serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=hp.serialization.NoEncryption(), + ) + + return key_pem, cert_pem + + +def generate_priv_pub_key_pair(keylength : int = 2048) -> tuple[bytes]: + import cryptography.hazmat.primitives as hp + + key = hp.asymmetric.rsa.generate_private_key( + public_exponent=65537, + key_size=keylength, + ) + priv_key_pem = key.private_bytes( + encoding=hp.serialization.Encoding.PEM, + format=hp.serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=hp.serialization.NoEncryption(), + ) + pub_key_pem = key.public_key().public_bytes( + encoding=hp.serialization.Encoding.PEM, + format=hp.serialization.PublicFormat.SubjectPublicKeyInfo, + ) + + return priv_key_pem, pub_key_pem + + +def generate_keys(opts): + # This will generate keys and certificates and write them to the paths that + # are specified as input paths. + if opts.sb_key or opts.sb_cert: + fqdn = socket.getfqdn() + cn = f'SecureBoot signing key on host {fqdn}' + key_pem, cert_pem = generate_key_cert_pair(common_name=cn) + print(f'Writing SecureBoot private key to {opts.sb_key}') + with temporary_umask(0o077): + opts.sb_key.write_bytes(key_pem) + print(f'Writing SecureBoot certicate to {opts.sb_cert}') + opts.sb_cert.write_bytes(cert_pem) + + for priv_key, pub_key, _ in key_path_groups(opts): + priv_key_pem, pub_key_pem = generate_priv_pub_key_pair() + + print(f'Writing private key for PCR signing to {priv_key}') + with temporary_umask(0o077): + priv_key.write_bytes(priv_key_pem) + if pub_key: + print(f'Writing public key for PCR signing to {pub_key}') + pub_key.write_bytes(pub_key_pem) + + @dataclasses.dataclass(frozen=True) class ConfigItem: @staticmethod @@ -861,7 +985,7 @@ class ConfigItem: return (section_name, key, value) -VERBS = ('build',) +VERBS = ('build', 'genkey') CONFIG_ITEMS = [ ConfigItem( @@ -1253,7 +1377,7 @@ def finalize_options(opts): if opts.sign_kernel and not opts.sb_key and not opts.sb_cert_name: raise ValueError('--sign-kernel requires either --secureboot-private-key= and --secureboot-certificate= (for sbsign) or --secureboot-certificate-name= (for pesign) to be specified') - if opts.output is None: + if opts.verb == 'build' and opts.output is None: if opts.linux is None: raise ValueError('--output= must be specified when building a PE addon') suffix = '.efi' if opts.sb_key or opts.sb_cert_name else '.unsigned.efi' @@ -1277,9 +1401,14 @@ def parse_args(args=None): def main(): opts = parse_args() - check_inputs(opts) - assert opts.verb == 'build' - make_uki(opts) + if opts.verb == 'build': + check_inputs(opts) + make_uki(opts) + elif opts.verb == 'genkey': + check_cert_and_keys_nonexistent(opts) + generate_keys(opts) + else: + assert False if __name__ == '__main__': From ff7580e280dc7b817ec38b5aa3c97293ff4b2bb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 21:07:58 +0200 Subject: [PATCH 07/11] test_ukify: cleanups suggested by pylint --- src/ukify/test/test_ukify.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index ac39a719402..2027d65ecd3 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -4,6 +4,7 @@ # pylint: disable=missing-docstring,redefined-outer-name,invalid-name # pylint: disable=unused-import,import-outside-toplevel,useless-else-on-loop # pylint: disable=consider-using-with,wrong-import-position,unspecified-encoding +# pylint: disable=protected-access import base64 import json @@ -106,7 +107,7 @@ def test_apply_config(tmp_path): assert ns.signing_engine == 'engine1' assert ns.sb_key == 'some/path5' assert ns.sb_cert == 'some/path6' - assert ns.sign_kernel == False + assert ns.sign_kernel is False assert ns._groups == ['NAME'] assert ns.pcr_private_keys == [pathlib.Path('some/path7')] @@ -129,7 +130,7 @@ def test_apply_config(tmp_path): assert ns.signing_engine == 'engine1' assert ns.sb_key == 'some/path5' assert ns.sb_cert == 'some/path6' - assert ns.sign_kernel == False + assert ns.sign_kernel is False assert ns._groups == ['NAME'] assert ns.pcr_private_keys == [pathlib.Path('some/path7')] @@ -447,7 +448,7 @@ def test_sections(kernel_initrd, tmpdir): for sect in 'text osrel cmdline linux initrd uname test'.split(): assert re.search(fr'^\s*\d+\s+.{sect}\s+0', dump, re.MULTILINE) -def test_addon(kernel_initrd, tmpdir): +def test_addon(tmpdir): output = f'{tmpdir}/addon.efi' args = [ 'build', @@ -459,7 +460,7 @@ def test_addon(kernel_initrd, tmpdir): args += [f'--stub={stub}'] expected_exceptions = () else: - expected_exceptions = FileNotFoundError, + expected_exceptions = (FileNotFoundError,) opts = ukify.parse_args(args) try: From 27140fc7d18bf249c01d51771d8e4857100e3431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Jun 2023 21:31:17 +0200 Subject: [PATCH 08/11] man: add example how to configure automatic signing Fixes #978. --- man/uki.conf.example | 14 ++++++++++++++ man/ukify.xml | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 man/uki.conf.example diff --git a/man/uki.conf.example b/man/uki.conf.example new file mode 100644 index 00000000000..84a9f77b8d7 --- /dev/null +++ b/man/uki.conf.example @@ -0,0 +1,14 @@ +[UKI] +SecureBootPrivateKey=/etc/kernel/secure-boot.key.pem +SecureBootCertificate=/etc/kernel/secure-boot.cert.pem + +[PCRSignature:initrd] +Phases=enter-initrd +PCRPrivateKey=/etc/kernel/pcr-initrd.key.pem +PCRPublicKey=/etc/kernel/pcr-initrd.pub.pem + +[PCRSignature:system] +Phases=enter-initrd:leave-initrd enter-initrd:leave-initrd:sysinit + enter-initrd:leave-initrd:sysinit:ready +PCRPrivateKey=/etc/kernel/pcr-system.key.pem +PCRPublicKey=/etc/kernel/pcr-system.pub.pem diff --git a/man/ukify.xml b/man/ukify.xml index 283d58b3b05..6895301d016 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -499,6 +499,36 @@ $ /usr/lib/systemd/ukify -c ukify.conf build \ This creates a signed PE binary that contains the additional kernel command line parameter debug with SBAT metadata referring to the owner of the addon. + + + Decide signing policy and create certificate and keys + + First, let's create an config file that specifies what signatures shall be made: + + # cat >/etc/kernel/uki.conf <<EOF +EOF + + Next, we can generate the certificate and keys: + # /usr/lib/systemd/ukify genkey --config=/etc/kernel/uki.conf +Writing SecureBoot private key to /etc/kernel/secure-boot.key.pem +Writing SecureBoot certicate to /etc/kernel/secure-boot.cert.pem +Writing private key for PCR signing to /etc/kernel/pcr-initrd.key.pem +Writing public key for PCR signing to /etc/kernel/pcr-initrd.pub.pem +Writing private key for PCR signing to /etc/kernel/pcr-system.key.pem +Writing public key for PCR signing to /etc/kernel/pcr-system.pub.pem + + + (Both operations need to be done as root to allow write access + to /etc/kernel/.) + + Subsequent invocations of using the config file + (/usr/lib/systemd/ukify build --config=/etc/kernel/uki.conf) + will use this certificate and key files. Note that the + kernel-install8 + plugin 60-ukify.install uses /etc/kernel/uki.conf + by default, so after this file has been created, installations of kernels that create a UKI on the + local machine using kernel-install would perform signing using this config. + From 13a29b0efb1a614f3d211ec083981b69bdd7edf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Jun 2023 10:05:02 +0200 Subject: [PATCH 09/11] test_ukify: skip test requiring cryptography I'll add the dep in Fedora spec so it does get tested at least in some cases. --- src/ukify/test/test_ukify.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 2027d65ecd3..a6778bb694d 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -713,6 +713,9 @@ def test_key_cert_generation(tmpdir): ]) assert opts.verb == 'genkey' ukify.check_cert_and_keys_nonexistent(opts) + + pytest.importorskip('cryptography') + ukify.generate_keys(opts) if not shutil.which('openssl'): From a3f758b3104ee1161d2dbf5a8c1be653340b1672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 12 Jun 2023 12:22:32 +0200 Subject: [PATCH 10/11] man/ukify: add structure to describe verbs, document genkey This just adds two paragraphs of text, but moves a lot of the text one level down into a subsection. --- man/ukify.xml | 111 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 67 insertions(+), 44 deletions(-) diff --git a/man/ukify.xml b/man/ukify.xml index 6895301d016..33d9d26d2de 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -35,60 +35,83 @@ Note: this command is experimental for now. While it is intended to become a regular component of systemd, it might still change in behaviour and interface. - ukify is a tool that combines components (usually a kernel, an initrd, and a - UEFI boot stub) to create a + ukify is a tool whose primary purpose is to combine components (usually a + kernel, an initrd, and a UEFI boot stub) to create a Unified Kernel Image (UKI) — a PE binary that can be executed by the firmware to start the embedded linux kernel. See systemd-stub7 for details about the stub. + - The two primary options that should be specified for the build verb are - Linux=/, and - Initrd=/. Initrd= accepts multiple - whitespace-separated paths and can be specified multiple times. + + Commands - 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=/, - SBAT=/, - and - below. + The following commands are understood: - 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. + + <command>build</command> - 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. + This command creates a Unified Kernel Image. The two primary options that should be specified for + the build verb are Linux=/, and + Initrd=/. Initrd= accepts multiple + whitespace-separated paths and can be specified multiple times. - The calculation of PCR values is done for specific boot phase paths. Those can be specified with - 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. + 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=/, + SBAT=/, + and + below. - 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. - + 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 + 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 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 + 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. + + + + + <command>genkey</command> + + This command creates the keys for PCR signing and the key and certificate used for SecureBoot + signing. The same configuration options that determine what keys and in which paths will be needed for + signing when build is used, here determine which keys will be created. See the + discussion of PCRPrivateKey=/, + PCRPublicKey=/, and + SecureBootPrivateKey=/ below. + + The output files must not exist. + From 814e4d7a671f3318485e6e272e5c6b34e71d3d3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Jun 2023 12:38:37 +0200 Subject: [PATCH 11/11] ukify: make the certficate validity configurable Requested in https://github.com/systemd/systemd/pull/27946/commits/4cc743319ae378d458050d6e0cfdeaa03addd379#r1228592001 --- man/ukify.xml | 8 ++++++++ src/ukify/ukify.py | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/man/ukify.xml b/man/ukify.xml index 33d9d26d2de..06ae550530d 100644 --- a/man/ukify.xml +++ b/man/ukify.xml @@ -329,6 +329,14 @@ This option is required by SecureBootSigningTool=pesign/. + + SecureBootCertificateValidity=DAYS + + + Period of validity (in days) for a certificate created by + genkey. Defaults to 3650, i.e. 10 years. + + SigningEngine=ENGINE diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 4fc3ce2e192..3db2bac384a 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -759,8 +759,8 @@ def temporary_umask(mask: int): def generate_key_cert_pair( common_name: str, + valid_days: int, keylength: int = 2048, - valid_days: int = 365 * 10, # TODO: can we drop the expiration date? ) -> tuple[bytes]: from cryptography import x509 @@ -835,7 +835,10 @@ def generate_keys(opts): if opts.sb_key or opts.sb_cert: fqdn = socket.getfqdn() cn = f'SecureBoot signing key on host {fqdn}' - key_pem, cert_pem = generate_key_cert_pair(common_name=cn) + key_pem, cert_pem = generate_key_cert_pair( + common_name=cn, + valid_days=opts.sb_cert_validity, + ) print(f'Writing SecureBoot private key to {opts.sb_key}') with temporary_umask(0o077): opts.sb_key.write_bytes(key_pem) @@ -1153,6 +1156,14 @@ uki.addon,1,UKI Addon,uki.addon,1,https://www.freedesktop.org/software/systemd/m help = 'required by --signtool=pesign. pesign needs a certificate nickname of nss certificate database entry to use for PE signing', config_key = 'UKI/SecureBootCertificateName', ), + ConfigItem( + '--secureboot-certificate-validity', + metavar = 'DAYS', + dest = 'sb_cert_validity', + default = 365 * 10, + help = "period of validity (in days) for a certificate created by 'genkey'", + config_key = 'UKI/SecureBootCertificateValidity', + ), ConfigItem( '--sign-kernel',