Merge pull request #27539 from esposem/ukify_pesign

ukify: support pesign as alternative to sbsign 

3 files changed, 196 insertions, 53 deletions

diff --git a/man/ukify.xml b/man/ukify.xml
index 6aa136298d..f5a2fcc3e8 100644
--- a/man/ukify.xml
+++ b/man/ukify.xml
@@ -254,12 +254,22 @@
         </varlistentry>
 
         <varlistentry>
+          <term><varname>SecureBootSigningTool=<replaceable>SIGNER</replaceable></varname></term>
+          <term><option>--signtool=<replaceable>SIGNER</replaceable></option></term>
+
+          <listitem><para>Whether to use <literal>sbsign</literal> or <literal>pesign</literal>.
+          Depending on this choice, different parameters are required in order to sign an image.
+          Defaults to <literal>sbsign</literal>.</para></listitem>
+        </varlistentry>
+
+        <varlistentry>
           <term><varname>SecureBootPrivateKey=<replaceable>SB_KEY</replaceable></varname></term>
           <term><option>--secureboot-private-key=<replaceable>SB_KEY</replaceable></option></term>
 
           <listitem><para>A path to a private key to use for signing of the resulting binary. If the
           <varname>SigningEngine=</varname>/<option>--signing-engine=</option> option is used, this may also be
-          an engine-specific designation.</para></listitem>
+          an engine-specific designation. This option is required by
+          <varname>SecureBootSigningTool=sbsign</varname>/<option>--signtool=sbsign</option>. </para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -268,7 +278,25 @@
 
           <listitem><para>A path to a certificate to use for signing of the resulting binary. If the
           <varname>SigningEngine=</varname>/<option>--signing-engine=</option> option is used, this may also
-          be an engine-specific designation.</para></listitem>
+          be an engine-specific designation. This option is required by
+          <varname>SecureBootSigningTool=sbsign</varname>/<option>--signtool=sbsign</option>. </para></listitem>
+        </varlistentry>
+
+        <varlistentry>
+          <term><varname>SecureBootCertificateDir=<replaceable>SB_PATH</replaceable></varname></term>
+          <term><option>--secureboot-certificate-dir=<replaceable>SB_PATH</replaceable></option></term>
+
+          <listitem><para>A path to a nss certificate database directory to use for signing of the resulting binary.
+          Takes effect when <varname>SecureBootSigningTool=pesign</varname>/<option>--signtool=pesign</option> is used.
+          Defaults to <filename>/etc/pki/pesign</filename>.</para></listitem>
+        </varlistentry>
+
+        <varlistentry>
+          <term><varname>SecureBootCertificateName=<replaceable>SB_CERTNAME</replaceable></varname></term>
+          <term><option>--secureboot-certificate-name=<replaceable>SB_CERTNAME</replaceable></option></term>
+
+          <listitem><para>The name of the nss certificate database entry to use for signing of the resulting binary.
+          This option is required by <varname>SecureBootSigningTool=pesign</varname>/<option>--signtool=pesign</option>.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -435,7 +463,7 @@ Phases=enter-initrd:leave-initrd
       --secureboot-private-key=sb.key \
       --secureboot-certificate=sb.cert \
       --cmdline='debug' \
-      --output=debug.cmdline.efi
+      --output=debug.cmdline
       </programlisting>
 
       <para>This creates a signed PE binary that contains the additional kernel command line parameter
diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py
index 6853205958..692b7a384b 100755
--- a/src/ukify/test/test_ukify.py
+++ b/src/ukify/test/test_ukify.py
@@ -221,12 +221,15 @@ def test_config_priority(tmp_path):
         DeviceTree = some/path2
         Splash = some/path3
         Uname = 1.2.3
-        EFIArch=arm
+        EFIArch = arm
         Stub = some/path4
         PCRBanks = sha512,sha1
         SigningEngine = engine1
+        SignTool = pesign
         SecureBootPrivateKey = some/path5
         SecureBootCertificate = some/path6
+        SecureBootCertificateDir = some/path7
+        SecureBootCertificateName = some/name1
         SignKernel = no
 
         [PCRSignature:NAME]
@@ -248,8 +251,11 @@ def test_config_priority(tmp_path):
          '--pcr-public-key=PKEY2',
          '--pcr-banks=SHA1,SHA256',
          '--signing-engine=ENGINE',
+         '--signtool=pesign',
          '--secureboot-private-key=SBKEY',
          '--secureboot-certificate=SBCERT',
+         '--secureboot-certificate-dir=SBPATH',
+         '--secureboot-certificate-name=SBNAME',
          '--sign-kernel',
          '--no-sign-kernel',
          '--tools=TOOLZ///',
@@ -279,8 +285,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.sign_kernel is False
     assert opts.tools == [pathlib.Path('TOOLZ/')]
     assert opts.output == pathlib.Path('OUTPUT')
@@ -314,7 +323,7 @@ def kernel_initrd():
     for item in items:
         try:
             linux = f"{item['root']}{item['linux']}"
-            initrd = f"{item['root']}{item['initrd'][0]}"
+            initrd = f"{item['root']}{item['initrd'][0].split(' ')[0]}"
         except (KeyError, IndexError):
             continue
         return [linux, initrd]
@@ -410,7 +419,7 @@ 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):
+def test_efi_signing_sbsign(kernel_initrd, tmpdir):
     if kernel_initrd is None:
         pytest.skip('linux+initrd not found')
     if not shutil.which('sbsign'):
@@ -447,6 +456,48 @@ def test_efi_signing(kernel_initrd, tmpdir):
 
         assert 'Signature verification OK' in dump
 
+def test_efi_signing_pesign(kernel_initrd, tmpdir):
+    if kernel_initrd is None:
+        pytest.skip('linux+initrd not found')
+    if not shutil.which('pesign'):
+        pytest.skip('pesign not found')
+
+    nss_db = f'{tmpdir}/nss_db'
+    name = 'Test_Secureboot'
+    author = 'systemd'
+
+    subprocess.check_call(['mkdir', '-p', nss_db])
+    cmd = f'certutil -N --empty-password -d {nss_db}'.split(' ')
+    subprocess.check_call(cmd)
+    cmd = f'efikeygen -d {nss_db} -S -k -c CN={author} -n {name}'.split(' ')
+    subprocess.check_call(cmd)
+
+    output = f'{tmpdir}/signed.efi'
+    opts = ukify.parse_args([
+        *kernel_initrd,
+        f'--output={output}',
+        '--uname=1.2.3',
+        '--signtool=pesign',
+        '--cmdline=ARG1 ARG2 ARG3',
+        f'--secureboot-certificate-name={name}',
+        f'--secureboot-certificate-dir={nss_db}',
+    ])
+
+    try:
+        ukify.check_inputs(opts)
+    except OSError as e:
+        pytest.skip(str(e))
+
+    ukify.make_uki(opts)
+
+    # let's check that sbverify likes the resulting file
+    dump = subprocess.check_output([
+        'pesign', '-S',
+        '-i', output,
+    ], text=True)
+
+    assert f"The signer's common name is {author}" in dump
+
 def test_pcr_signing(kernel_initrd, tmpdir):
     if kernel_initrd is None:
         pytest.skip('linux+initrd not found')
diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py
index 7e9c7cc9ae..d87670eb24 100755
--- a/src/ukify/ukify.py
+++ b/src/ukify/ukify.py
@@ -353,8 +353,10 @@ def find_tool(name, fallback=None, opts=None):
     if shutil.which(name) is not None:
         return name
 
-    return fallback
+    if fallback is None:
+        print(f"Tool {name} not installed!")
 
+    return fallback
 
 def combine_signatures(pcrsigs):
     combined = collections.defaultdict(list)
@@ -548,50 +550,93 @@ def pe_add_sections(uki: UKI, output: str):
 
     pe.write(output)
 
+def signer_sign(cmd):
+    print('+', shell_join(cmd))
+    subprocess.check_call(cmd)
 
-def make_uki(opts):
-    # kernel payload signing
+def find_sbsign(opts=None):
+    return find_tool('sbsign', opts=opts)
 
-    sbsign_tool = find_tool('sbsign', opts=opts)
-    sbsign_invocation = [
+def sbsign_sign(sbsign_tool, input_f, output_f, opts=None):
+    sign_invocation = [
         sbsign_tool,
         '--key', opts.sb_key,
         '--cert', opts.sb_cert,
+        input_f,
+        '--output', output_f,
     ]
-
     if opts.signing_engine is not None:
-        sbsign_invocation += ['--engine', opts.signing_engine]
+        sign_invocation += ['--engine', opts.signing_engine]
+    signer_sign(sign_invocation)
+
+def find_pesign(opts=None):
+    return find_tool('pesign', opts=opts)
+
+def pesign_sign(pesign_tool, input_f, output_f, opts=None):
+    sign_invocation = [
+        pesign_tool, '-s', '--force',
+        '-n', opts.sb_certdir,
+        '-c', opts.sb_cert_name,
+        '-i', input_f,
+        '-o', output_f,
+    ]
+    signer_sign(sign_invocation)
 
-    sign_kernel = opts.sign_kernel
-    if sign_kernel is None and opts.linux is not None and opts.sb_key:
-        # figure out if we should sign the kernel
-        sbverify_tool = find_tool('sbverify', opts=opts)
+SBVERIFY = {
+    'name': 'sbverify',
+    'option': '--list',
+    'output': 'No signature table present',
+}
 
-        cmd = [
-            sbverify_tool,
-            '--list',
-            opts.linux,
-        ]
+PESIGCHECK = {
+    'name': 'pesign',
+    'option': '-i',
+    'output': 'No signatures found.',
+    'flags': '-S'
+}
 
-        print('+', shell_join(cmd))
-        info = subprocess.check_output(cmd, text=True)
+def verify(tool, opts):
+    verify_tool = find_tool(tool['name'], opts=opts)
+    cmd = [
+        verify_tool,
+        tool['option'],
+        opts.linux,
+    ]
+    if 'flags' in tool:
+        cmd.append(tool['flags'])
+
+    print('+', shell_join(cmd))
+    info = subprocess.check_output(cmd, text=True)
+
+    return tool['output'] in info
+
+def make_uki(opts):
+    # kernel payload signing
+
+    sign_tool = None
+    if opts.signtool == 'sbsign':
+        sign_tool = find_sbsign(opts=opts)
+        sign = sbsign_sign
+        verify_tool = SBVERIFY
+    else:
+        sign_tool = find_pesign(opts=opts)
+        sign = pesign_sign
+        verify_tool = PESIGCHECK
 
-        # sbverify has wonderful API
-        if 'No signature table present' in info:
-            sign_kernel = True
+    sign_args_present = opts.sb_key or opts.sb_cert_name
+
+    if sign_tool is None and sign_args_present:
+        raise ValueError(f'{opts.signtool}, required for signing, is not installed')
+
+    sign_kernel = opts.sign_kernel
+    if sign_kernel is None and opts.linux is not None and sign_args_present:
+        # figure out if we should sign the kernel
+        sign_kernel = verify(verify_tool, opts)
 
     if sign_kernel:
         linux_signed = tempfile.NamedTemporaryFile(prefix='linux-signed')
         linux = linux_signed.name
-
-        cmd = [
-            *sbsign_invocation,
-            opts.linux,
-            '--output', linux,
-        ]
-
-        print('+', shell_join(cmd))
-        subprocess.check_call(cmd)
+        sign(sign_tool, opts.linux, linux, opts=opts)
     else:
         linux = opts.linux
 
@@ -639,7 +684,7 @@ def make_uki(opts):
     if linux is not None:
         uki.add_section(Section.create('.linux', linux, measure=True))
 
-    if opts.sb_key:
+    if sign_args_present:
         unsigned = tempfile.NamedTemporaryFile(prefix='uki')
         output = unsigned.name
     else:
@@ -649,20 +694,14 @@ def make_uki(opts):
 
     # UKI signing
 
-    if opts.sb_key:
-        cmd = [
-            *sbsign_invocation,
-            unsigned.name,
-            '--output', opts.output,
-        ]
-        print('+', shell_join(cmd))
-        subprocess.check_call(cmd)
+    if sign_args_present:
+        sign(sign_tool, unsigned.name, opts.output, opts=opts)
 
         # We end up with no executable bits, let's reapply them
         os.umask(umask := os.umask(0))
         os.chmod(opts.output, 0o777 & ~umask)
 
-    print(f"Wrote {'signed' if opts.sb_key else 'unsigned'} {opts.output}")
+    print(f"Wrote {'signed' if sign_args_present else 'unsigned'} {opts.output}")
 
 
 @dataclasses.dataclass(frozen=True)
@@ -912,17 +951,38 @@ CONFIG_ITEMS = [
         config_key = 'UKI/SigningEngine',
     ),
     ConfigItem(
+        '--signtool',
+        choices = ('sbsign', 'pesign'),
+        dest = 'signtool',
+        default = 'sbsign',
+        help = 'whether to use sbsign or pesign. Default is sbsign.',
+        config_key = 'UKI/SecureBootSigningTool',
+    ),
+    ConfigItem(
         '--secureboot-private-key',
         dest = 'sb_key',
-        help = 'path to key file or engine-specific designation for SB signing',
+        help = 'required by --signtool=sbsign. 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',
+        help = 'required by --signtool=sbsign. sbsign needs a path to certificate file or engine-specific designation for SB signing',
         config_key = 'UKI/SecureBootCertificate',
     ),
+    ConfigItem(
+        '--secureboot-certificate-dir',
+        dest = 'sb_certdir',
+        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',
+    ),
+    ConfigItem(
+        '--secureboot-certificate-name',
+        dest = 'sb_cert_name',
+        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(
         '--sign-kernel',
@@ -1089,16 +1149,20 @@ def finalize_options(opts):
         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')
+    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('--certificate-name must be specified when using --signtool=pesign')
 
-    if opts.sign_kernel and not opts.sb_key:
-        raise ValueError('--sign-kernel requires --secureboot-private-key= and --secureboot-certificate= to be specified')
+    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.linux is None:
             raise ValueError('--output= must be specified when building a PE addon')
-        suffix = '.efi' if opts.sb_key else '.unsigned.efi'
+        suffix = '.efi' if opts.sb_key or opts.sb_cert_name else '.unsigned.efi'
         opts.output = opts.linux.name + suffix
 
     for section in opts.sections: