launchpad signing shimaa64.efi fails to validate

Bug #1921387 reported by Dimitri John Ledkov on 2021-03-25
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
lp-signing
Undecided
Unassigned
sbsigntool (Ubuntu)
Undecided
Unassigned
Bionic
Undecided
Unassigned

Bug Description

[Impact]

 * Calculating the hash of the binary is ill defined if there are gaps in sections, or sections are not aligned to ensure that signature table is aligned.

 * This results in sbsign/sbverify to calculate incorrect hash when there are gaps, such as in shimaa64.efi as built on focal with sbat.

 * This was fixed in eoan, but launchpad signing service uses sbsign from bionic.

 * Thus if binaries have gaps launchpad is producing signatures that are covering the wrong authenticode hash.

[Test Plan]

 * Signatures produced by sbsign in bionic, must be able to verify with sbverify from focal or later.

 * Old signatures generated by launchpad should fail validation

 * Enrolling certificate into db and booting secureboot arm VM must work

ie.

# install old sbsign

# Test old launchpad generated signature, ensure that it fails:

cd $(mktemp -d)
wget http://ppa.launchpad.net/xnox/nonvirt/ubuntu/dists/hirsute/main/signed/shim-arm64/15.3-0ubuntu1~ppa1/signed.tar.gz
tar xvf ./signed.tar.gz

sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed

Verification will pass with old sbsign, but it is wrong.

# Generate new key on bionic

update-secureboot-policy --new-key
openssl x509 -inform der -outform pem -in /var/lib/shim-signed/mok/MOK.der -out /var/lib/shim-signed/mok/MOK.pem

cp 15.3-0ubuntu1~ppa1/shimaa64.efi old-sbsign.efi
cp 15.3-0ubuntu1~ppa1/shimaa64.efi new-sbsign.efi

# self-sign the binary using the old sbsign
sbsign --key /var/lib/shim-signed/mok/MOK.priv --cert /var/lib/shim-signed/mok/MOK.pem old-sbsign.efi

# Detach the signature and print the message digest

sbattach --detach old-sbsign-signature.p7c old-sbsign.efi.signed

openssl pkcs7 -inform der -in old-sbsign-signature.p7c -print | grep -A5 messageDi

# upgrade to new sbsign

# check that verifcation now fails

sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed

should now fail.

# self-sign with new sbsign
sbsign --key /var/lib/shim-signed/mok/MOK.priv --cert /var/lib/shim-signed/mok/MOK.pem new-sbsign.efi

# Detach the signature and print the message digest
sbattach --detach new-sbsign-signature.p7c new-sbsign.efi.signed

openssl pkcs7 -inform der -in new-sbsign-signature.p7c -print | grep -A5 messageDi

# Also detach the launchpad signature and print digest

sbattach --detach lp-sbsign-signature.p7c 15.3-0ubuntu1~ppa1/shimaa64.efi.signed

openssl pkcs7 -inform der -in lp-sbsign-signature.p7c -print | grep -A5 messageDi

The correct digest is, which should be in the new-sbsign-signature.p7c:
            object: messageDigest (1.2.840.113549.1.9.4)
            set:
              OCTET STRING:
                0000 - 6a 83 1f 9e cb 7a 68 7f-17 c0 9d 81 c0 j....zh......
                000d - 6b 17 b2 c3 1c d7 ed b5-b3 89 49 a3 c1 k.........I..
                001a - 8d 75 59 d3 b3 11 .uY...

The wrong digest is, which is in lp & old sbsign signatures:
            object: messageDigest (1.2.840.113549.1.9.4)
            set:
              OCTET STRING:
                0000 - 2a c3 bb e6 20 27 6b b2-58 f8 8d 50 eb *... 'k.X..P.
                000d - 1e 88 68 a3 12 08 7a 1d-27 e5 42 e6 0e ..h...z.'.B..
                001a - e4 24 9a 5c 0a 92 .$.\..

 * Additionally to that, check that existing bionic x64 binaries still verify correctly. I.e. grub / kernel.

[Where problems could occur]

 * Existing edk2 OVMF machines in bionic possibly are calculating checksums unpadded, and thus this change will make the new signatures fail to validate in edk2 OVMF. However, the binaries on amd64 do not have gaps and thus have always had correct signatures. arm64 binaries with gaps do not exist in bionic.

[Other Info]

Original bug report:

launchpad signed shimaa64.efi fails to validate on focal

cd $(mktemp -d)

wget http://ppa.launchpad.net/xnox/nonvirt/ubuntu/dists/hirsute/main/signed/shim-arm64/15.3-0ubuntu1~ppa1/signed.tar.gz

tar xvf ./signed.tar.gz

sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed

Signature verification failed

And yet inside bionic-amd64 chroot I get:

# sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed
warning: gap in section table:
    .data : 0x0007f000 - 0x000b3800,
    .sbat : 0x000b4000 - 0x000b5000,
gaps in the section table may result in different checksums
warning: data remaining[740864 vs 800872]: gaps between PE/COFF sections?
Signature verification OK

However,

If in xenial-amd64 I perform

update-secureboot-policy new-key
openssl x509 -inform der -outform pem -in /var/lib/shim-signed/mok/MOK.der -out /var/lib/shim-signed/mok/MOK.pem

sbsign --key /var/lib/shim-signed/mok/MOK.priv --cert /var/lib/shim-signed/mok/MOK.pem 15.3-0ubuntu1~ppa1/shimaa64.efi

sbverify --cert /var/lib/shim-signed/mok/MOK.pem 15.3-0ubuntu1~ppa1/shimaa64.efi.signed
Signature verification OK

Looks like something is dodgy in sbverify in bionic; i.e. it calculates / signs / verifies wrong hash.

description: updated
description: updated
description: updated
Changed in sbsigntool (Ubuntu):
status: New → Fix Released
description: updated
description: updated
description: updated
description: updated
Dimitri John Ledkov (xnox) wrote :

Also good to check that existing bionic x64 binaries still verify correctly. I.e. grub / kernel.

Hello Dimitri, or anyone else affected,

Accepted sbsigntool into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/sbsigntool/0.9.2-2ubuntu1~18.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

description: updated
Changed in sbsigntool (Ubuntu Bionic):
status: New → Fix Committed
tags: added: verification-needed verification-needed-bionic
Dimitri John Ledkov (xnox) wrote :

# dpkg-query -W sbsigntool
sbsigntool 0.6-3.2ubuntu2

# sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed
warning: gap in section table:
    .data : 0x0007f000 - 0x000b3800,
    .sbat : 0x000b4000 - 0x000b5000,
gaps in the section table may result in different checksums
warning: data remaining[740864 vs 800872]: gaps between PE/COFF sections?
Signature verification OK

# openssl pkcs7 -inform der -in lp-sbsign-signature.p7c -print | grep -A5 messageDi
            object: messageDigest (1.2.840.113549.1.9.4)
            set:
              OCTET STRING:
                0000 - 2a c3 bb e6 20 27 6b b2-58 f8 8d 50 eb *... 'k.X..P.
                000d - 1e 88 68 a3 12 08 7a 1d-27 e5 42 e6 0e ..h...z.'.B..
                001a - e4 24 9a 5c 0a 92 .$.\..

# openssl pkcs7 -inform der -in old-sbsign-signature.p7c -print | grep -A5 messageDi
            object: messageDigest (1.2.840.113549.1.9.4)
            set:
              OCTET STRING:
                0000 - 2a c3 bb e6 20 27 6b b2-58 f8 8d 50 eb *... 'k.X..P.
                000d - 1e 88 68 a3 12 08 7a 1d-27 e5 42 e6 0e ..h...z.'.B..
                001a - e4 24 9a 5c 0a 92 .$.\..

Old sbsign is buggy.

description: updated
Dimitri John Ledkov (xnox) wrote :

# wget https://launchpad.net/ubuntu/+source/sbsigntool/0.9.2-2ubuntu1~18.04.1/+build/21207939/+files/sbsigntool_0.9.2-2ubuntu1~18.04.1_amd64.deb

# apt install ./sbsigntool_0.9.2-2ubuntu1~18.04.1_amd64.deb

# dpkg-query -W sbsigntool
sbsigntool 0.9.2-2ubuntu1~18.04.1

# sbverify --cert 15.3-0ubuntu1~ppa1/control/uefi.crt 15.3-0ubuntu1~ppa1/shimaa64.efi.signed
warning: gap in section table:
    .data : 0x0007f000 - 0x000b37a0,
    .sbat : 0x000b4000 - 0x000b5000,
gaps in the section table may result in different checksums
warning: data remaining[740768 vs 800872]: gaps between PE/COFF sections?
Hash doesn't match image
 got: dd7816e1c0158e8ac1cf546fd58c00e72f1f3d8c766420dff37702e522131933
 expecting: de00bed944a76d3b5569a1cd1ad7e68cd700ad88cbf8a14c6f67df277594f018
Image fails hash check
Signature verification failed

Old lp signature fails to verify now - good.

Resigning with new sbsign, the signature is generated with correct digest now

# openssl pkcs7 -inform der -in new-sbsign-signature.p7c -print | grep -A5 messageDi
            object: messageDigest (1.2.840.113549.1.9.4)
            set:
              OCTET STRING:
                0000 - 6a 83 1f 9e cb 7a 68 7f-17 c0 9d 81 c0 j....zh......
                000d - 6b 17 b2 c3 1c d7 ed b5-b3 89 49 a3 c1 k.........I..
                001a - 8d 75 59 d3 b3 11 .uY...

Dimitri John Ledkov (xnox) wrote :

Verifying existing binaries with new sbsigntool:

# wget http://archive.ubuntu.com/ubuntu/dists/bionic/main/uefi/fwupdate-amd64/current/fwupx64.efi.signed
# wget http://archive.ubuntu.com/ubuntu/dists/bionic/main/uefi/fwupdate-amd64/current/control/uefi.crt
# sbverify --cert ./uefi.crt ./fwupx64.efi.signed
warning: data remaining[63352 vs 71400]: gaps between PE/COFF sections?
Signature verification OK

# wget http://archive.ubuntu.com/ubuntu/dists/bionic/main/uefi/fwupdate-i386/current/fwupia32.efi.signed
# sbverify --cert ./uefi.crt ./fwupia32.efi.signed
warning: data remaining[54648 vs 63512]: gaps between PE/COFF sections?
Signature verification OK

# wget http://archive.ubuntu.com/ubuntu/dists/bionic/main/signed/linux-amd64/current/signed.tar.gz -O linux-signed.tar.gz
# tar xvf linux-signed.tar.gz
# sbverify --cert uefi.crt 4.15.0-20.21/vmlinuz-4.15.0-20-generic.efi.signed
warning: data remaining[8249064 vs 8249080]: gaps between PE/COFF sections?
Signature verification OK
# sbverify --cert uefi.crt 4.15.0-20.21/vmlinuz-4.15.0-20-lowlatency.efi.signed
warning: data remaining[8298216 vs 8298232]: gaps between PE/COFF sections?
Signature verification OK

# wget http://archive.ubuntu.com/ubuntu/dists/bionic/main/uefi/grub2-amd64/current/grubx64.efi.signed
# sbverify --cert uefi.crt grubx64.efi.signed
Signature verification OK

# wget http://ports.ubuntu.com/dists/bionic/main/uefi/grub2-arm64/current/grubaa64.efi.signed
# sbverify --cert uefi.crt ./grubaa64.efi.signed
Signature verification OK

All existing bionic signatures validate correctly. Thus the problem is really induced by gaps/ordering of the .sbat & .data sections, on arm64 with the very new sbat-capable binaries.

tags: added: verification-done verification-done-bionic
removed: verification-needed verification-needed-bionic
Colin Watson (cjwatson) wrote :

Do we need to do anything in Launchpad other than making sure that the relevant machines are upgraded to the latest sbsigntool?

affects: launchpad → lp-signing
Dimitri John Ledkov (xnox) wrote :

So shim upstream has changed how sections are ordered now, such that in shim 15.4 can now be signed by either old or new sbsigntool, and it verifies correctly in either case.

However, I still think it is a good idea to upgrade to the better sbsigntool to correctly sign even the odd looking binaries. As we will probably hit similar issues when we start adding UEFI secureboot support on new arches - i.e. riscv64.

So yes, upgrading lp-signing machines once this SRU is published will be needed. But it's not an urgent request anymore in terms of building or releasing shim 15.4.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package sbsigntool - 0.9.2-2ubuntu1~18.04.1

---------------
sbsigntool (0.9.2-2ubuntu1~18.04.1) bionic; urgency=medium

  * No-change backport to bionic:
    - fix alignment of binaries and thus correct hash calculation LP:
    #1921387

sbsigntool (0.9.2-2ubuntu1) eoan; urgency=low

  * Merge from Debian unstable. Remaining changes:
    - d/p/ubuntu-kernel-module-signing.patch and
      d/p/ubuntu-kernel-module-signing-fixes.patch: add the kernel module
      signing tool to the package.
    - d/p/ubuntu-clear-image-before-use.patch: avoid use of uninitialised
      data causing a startup crash.
  * Dropped changes, included upstream:
    - d/p/ubuntu-handle-odd-buffer-lengths-in-checksum.patch: correctly
      handle odd byte length buffers.
  * Dropped changes, obsoleted upstream:
    - d/p/ubuntu-tests-disable-pie.patch: disable PIE

sbsigntool (0.9.2-2) unstable; urgency=medium

  * Change Maintainer to be the EFI team, with Pierre and me as Uploaders
  * Remove the old alignment patch, looks to be un-needed now
  * Fix PE/COFF checksum calculation - only count the cert_table
    struct once when performing the calculation and counting buffer
    sizes.

sbsigntool (0.9.2-1) unstable; urgency=medium

  * Add watch file
  * New upstream version 0.9.2 (Closes: #920013, #828696)
  * Remove test file after clean
  * Refreshed quilt patches, and removed all that were merged
  * Use priority optional

 -- Dimitri John Ledkov <email address hidden> Thu, 25 Mar 2021 13:52:52 +0000

Changed in sbsigntool (Ubuntu Bionic):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for sbsigntool has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers