Bug#927464: unblock: sbsigntool/0.9.2-2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
Please unblock package sbsigntool
I've fixed a bug that I *would* have filed as RC, if I hadn't just
fixed it. :-) Patch sent upstream for a subtle bug breaking checksum
calculation in the PE/COFF binaries that are used for UEFI Secure
Boot.
This package is needed as part of the build process for our
shim-signed packages.
debdiff:
diff -Nru sbsigntool-0.9.2/debian/changelog sbsigntool-0.9.2/debian/changelog
--- sbsigntool-0.9.2/debian/changelog 2019-01-21 20:20:40.000000000 +0000
+++ sbsigntool-0.9.2/debian/changelog 2019-04-19 22:41:15.000000000 +0100
@@ -1,3 +1,13 @@
+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.
+
+ -- Steve McIntyre <93sam@debian.org> Fri, 19 Apr 2019 23:41:15 +0200
+
sbsigntool (0.9.2-1) unstable; urgency=medium
* Add watch file
diff -Nru sbsigntool-0.9.2/debian/control sbsigntool-0.9.2/debian/control
--- sbsigntool-0.9.2/debian/control 2019-01-21 20:16:08.000000000 +0000
+++ sbsigntool-0.9.2/debian/control 2019-04-19 15:54:34.000000000 +0100
@@ -1,7 +1,9 @@
Source: sbsigntool
Section: utils
Priority: optional
-Maintainer: Pierre Chifflier <pollux@debian.org>
+Maintainer: Debian EFI Team <debian-efi@lists.debian.org>
+Uploaders: Pierre Chifflier <pollux@debian.org>,
+ Steve McIntyre <93sam@debian.org>
Build-Depends: debhelper (>= 9.0.0),
dh-autoreconf,
gcc-multilib [amd64 i386 kfreebsd-amd64],
diff -Nru sbsigntool-0.9.2/debian/patches/Align-signature-data-to-8-bytes.patch sbsigntool-0.9.2/debian/patches/Align-signature-data-to-8-bytes.patch
--- sbsigntool-0.9.2/debian/patches/Align-signature-data-to-8-bytes.patch 2019-01-21 19:59:31.000000000 +0000
+++ sbsigntool-0.9.2/debian/patches/Align-signature-data-to-8-bytes.patch 1970-01-01 01:00:00.000000000 +0100
@@ -1,26 +0,0 @@
-From 8b6b7a9904881757254b92a928b95dfb8634605b Mon Sep 17 00:00:00 2001
-From: Steve Langasek <steve.langasek@canonical.com>
-Date: Fri, 12 Oct 2012 16:27:13 -0700
-Subject: [PATCH] Align signature data to 8 bytes
-
-Before appending the signature data to our binary, pad the file out to
-8-byte alignment. This matches the Microsoft signing implementation, which
-enables us to use sbattach to verify the integrity of the binaries returned
-by the SysDev signing service.
----
- src/image.c | 2 ++
- 1 file changed, 2 insertions(+)
-
-Index: sbsigntool/src/image.c
-===================================================================
---- sbsigntool.orig/src/image.c
-+++ sbsigntool/src/image.c
-@@ -495,6 +495,8 @@ reparse:
- * we've calculated during the pecoff parsing, so we need to redo that
- * too.
- */
-+ image->data_size = align_up(image->data_size, 8);
-+
- if (image->data_size > image->size) {
- image->buf = talloc_realloc(image, image->buf, uint8_t,
- image->data_size);
diff -Nru sbsigntool-0.9.2/debian/patches/fix_checksum_calc.patch sbsigntool-0.9.2/debian/patches/fix_checksum_calc.patch
--- sbsigntool-0.9.2/debian/patches/fix_checksum_calc.patch 1970-01-01 01:00:00.000000000 +0100
+++ sbsigntool-0.9.2/debian/patches/fix_checksum_calc.patch 2019-04-19 22:41:15.000000000 +0100
@@ -0,0 +1,37 @@
+Author: Steve McIntyre <93sam@debian.org>
+Date: Fri, 19 Jun 2019 23:35:44 +0200
+Description: Fix PE/COFF checksum calculation
+ Upstream reconfigured the code to allow for multiple checksums to be
+ included on a binary, but mis-handled the change to buffer and
+ checksum handling such that the cert_table header was now being
+ counted twice. Fix that.
+diff --git a/src/image.c b/src/image.c
+index 745191f..3ada37b 100644
+--- a/src/image.c
++++ b/src/image.c
+@@ -162,7 +162,6 @@ static void image_pecoff_update_checksum(struct image *image)
+ {
+ bool is_signed = image->sigsize && image->sigbuf;
+ uint32_t checksum;
+- struct cert_table_header *cert_table = image->cert_table;
+
+ /* We carefully only include the signature data in the checksum (and
+ * in the file length) if we're outputting the signature. Otherwise,
+@@ -180,16 +179,13 @@ static void image_pecoff_update_checksum(struct image *image)
+ (void *)(image->checksum + 1));
+
+ if (is_signed) {
+- checksum = csum_bytes(checksum,
+- cert_table, sizeof(*cert_table));
+-
+ checksum = csum_bytes(checksum, image->sigbuf, image->sigsize);
+ }
+
+ checksum += image->data_size;
+
+ if (is_signed)
+- checksum += sizeof(*cert_table) + image->sigsize;
++ checksum += image->sigsize;
+
+ *(image->checksum) = cpu_to_le32(checksum);
+ }
diff -Nru sbsigntool-0.9.2/debian/patches/series sbsigntool-0.9.2/debian/patches/series
--- sbsigntool-0.9.2/debian/patches/series 2019-01-21 20:04:50.000000000 +0000
+++ sbsigntool-0.9.2/debian/patches/series 2019-04-19 22:34:17.000000000 +0100
@@ -1,3 +1,3 @@
-Align-signature-data-to-8-bytes.patch
sbsign_check_write_return.patch
fix-efi-arch-detection.patch
+fix_checksum_calc.patch
unblock sbsigntool/0.9.2-2
-- System Information:
Debian Release: buster/sid
APT prefers testing
APT policy: (500, 'testing'), (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 4.19.0-2-amd64 (SMP w/4 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE=en_GB:en (charmap=UTF-8)
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
Reply to: