From 5c52078162f0c83cdbb5e0bdade9a4677982f394 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Tue, 26 Sep 2023 12:04:01 -0400 Subject: [PATCH 1/2] ukify: automatically infer --signtool from the parameters given --signtool is actually useless: it can be inferred depending on if --secureboot-certificate-name (pesign) is given, or --secureboot-private-key and --secureboot-certificate (sbsign) is given. Leave the option just for backwards compatibility. --- src/ukify/ukify.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 432dc87988d..40550e8c541 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -1277,8 +1277,9 @@ CONFIG_ITEMS = [ '--signtool', choices = ('sbsign', 'pesign'), dest = 'signtool', - default = 'sbsign', - help = 'whether to use sbsign or pesign. Default is sbsign.', + help = 'whether to use sbsign or pesign. It will also be inferred by the other \ + parameters given: when using --secureboot-{private-key/certificate}, sbsign \ + will be used, otherwise pesign will be used', config_key = 'UKI/SecureBootSigningTool', ), ConfigItem( @@ -1571,12 +1572,19 @@ def finalize_options(opts): if opts.sb_cert: opts.sb_cert = pathlib.Path(opts.sb_cert) - if opts.signtool == 'sbsign': - if bool(opts.sb_key) ^ bool(opts.sb_cert): - raise ValueError('--secureboot-private-key= and --secureboot-certificate= must be specified together when using --signtool=sbsign') - else: - if not bool(opts.sb_cert_name): - raise ValueError('--secureboot-certificate-name must be specified when using --signtool=pesign') + if bool(opts.sb_key) ^ bool(opts.sb_cert): + # one param only given, sbsign need boths + raise ValueError('--secureboot-private-key= and --secureboot-certificate= must be specified together') + elif bool(opts.sb_key) and bool(opts.sb_cert): + # both param given, infer sbsign and in case it was given, ensure signtool=sbsign + if opts.signtool and opts.signtool != 'sbsign': + raise ValueError(f'Cannot provide --signtool={opts.signtool} with --secureboot-private-key= and --secureboot-certificate=') + opts.signtool = 'sbsign' + elif bool(opts.sb_cert_name): + # sb_cert_name given, infer pesign and in case it was given, ensure signtool=pesign + if opts.signtool and opts.signtool != 'pesign': + raise ValueError(f'Cannot provide --signtool={opts.signtool} with --secureboot-certificate-name=') + opts.signtool = 'pesign' 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') From 1277575408493eb67e04be6f3f8187d67933e7ff Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Wed, 27 Sep 2023 04:28:45 -0400 Subject: [PATCH 2/2] ukify: override default option value with config file If an option like SecureBootCertificateDir is given, it should override the default '/etc/pki/pesign'. Until now the config file option were always ignored if they had a default. So from now on, every ConfigItem with a config_key and default field should also give config_push = ConfigItem.config_set. --- src/ukify/test/test_ukify.py | 22 ++++++++++------------ src/ukify/ukify.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 7c25aace81d..b12c09d4bf9 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -266,6 +266,7 @@ def test_parse_sections(): def test_config_priority(tmp_path): config = tmp_path / 'config1.conf' + # config: use pesign and give certdir + certname config.write_text(textwrap.dedent( f''' [UKI] @@ -282,10 +283,8 @@ def test_config_priority(tmp_path): Stub = some/path4 PCRBanks = sha512,sha1 SigningEngine = engine1 - SignTool = pesign - SecureBootPrivateKey = some/path5 - SecureBootCertificate = some/path6 - SecureBootCertificateDir = some/path7 + SecureBootSigningTool = pesign + SecureBootCertificateDir = some/path5 SecureBootCertificateName = some/name1 SignKernel = no @@ -295,6 +294,7 @@ def test_config_priority(tmp_path): Phases = {':'.join(ukify.KNOWN_PHASES)} ''')) + # args: use sbsign and give key + cert, should override pesign opts = ukify.parse_args( ['build', '--linux=/ARG1', @@ -311,11 +311,9 @@ def test_config_priority(tmp_path): '--pcr-public-key=PKEY2', '--pcr-banks=SHA1,SHA256', '--signing-engine=ENGINE', - '--signtool=pesign', + '--signtool=sbsign', '--secureboot-private-key=SBKEY', '--secureboot-certificate=SBCERT', - '--secureboot-certificate-dir=SBPATH', - '--secureboot-certificate-name=SBNAME', '--sign-kernel', '--no-sign-kernel', '--tools=TOOLZ///', @@ -345,11 +343,11 @@ def test_config_priority(tmp_path): pathlib.Path('some/path8')] assert opts.pcr_banks == ['SHA1', 'SHA256'] assert opts.signing_engine == 'ENGINE' - assert opts.signtool == 'pesign' - assert opts.sb_key == 'SBKEY' - assert opts.sb_cert == 'SBCERT' - assert opts.sb_certdir == 'SBPATH' - assert opts.sb_cert_name == 'SBNAME' + assert opts.signtool == 'sbsign' # from args + assert opts.sb_key == 'SBKEY' # from args + assert opts.sb_cert == 'SBCERT' # from args + assert opts.sb_certdir == 'some/path5' # from config + assert opts.sb_cert_name == 'some/name1' # from config assert opts.sign_kernel is False assert opts.tools == [pathlib.Path('TOOLZ/')] assert opts.output == pathlib.Path('OUTPUT') diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 40550e8c541..1ed8aadccad 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -1036,6 +1036,19 @@ class ConfigItem: if getattr(namespace, dest) is None: setattr(namespace, dest, value) + @staticmethod + def config_set( + namespace: argparse.Namespace, + group: Optional[str], + dest: str, + value: Any, + ) -> None: + "Set namespace. to value only if it was None" + + assert not group + + setattr(namespace, dest, value) + @staticmethod def config_set_group( namespace: argparse.Namespace, @@ -1300,6 +1313,7 @@ CONFIG_ITEMS = [ default = '/etc/pki/pesign', help = 'required by --signtool=pesign. Path to nss certificate database directory for PE signing. Default is /etc/pki/pesign', config_key = 'UKI/SecureBootCertificateDir', + config_push = ConfigItem.config_set ), ConfigItem( '--secureboot-certificate-name', @@ -1314,6 +1328,7 @@ CONFIG_ITEMS = [ default = 365 * 10, help = "period of validity (in days) for a certificate created by 'genkey'", config_key = 'UKI/SecureBootCertificateValidity', + config_push = ConfigItem.config_set ), ConfigItem(