Bug#928491: efisiglist produces broken signature lists
Package: pesign
Version: 0.112-4
Severity: important
Tags: upstream patch
I'm wanting to use efisiglist to generate a dbx list in our shim
package at build time. Unfortunately, I've just found (after a lot of
debugging) that efisiglist gets things wrong and produces malformed
output. I've fixed it - see attached patch.
About to push this upstream too, but adding a bug here to help track
things.
-- System Information:
Debian Release: buster/sid
APT prefers testing
APT policy: (500, 'testing'), (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 4.19.0-4-amd64 (SMP w/4 CPU cores)
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
Versions of packages pesign depends on:
ii coolkey 1.1.0-13+b1
ii libc6 2.28-8
ii libefivar1 37-2
ii libnspr4 2:4.20-1
ii libnss3 2:3.42.1-1
ii libnss3-tools 2:3.42.1-1
ii libpopt0 1.16-12
ii libuuid1 2.33.1-0.1
ii opensc 0.19.0-1
pesign recommends no packages.
pesign suggests no packages.
-- no debconf information
Description: Fix bugs in efisiglist
* Fix handling of structure sizes in a couple of places, so that memory
copies etc. work properly.
* In signature_list_realize(), don't simply copy the *sl struct over
the *esl struct - they're *not* exactly the same! Copy fields by
hand.
Author: Steve McIntyre <93sam@debian.org>
---
--- pesign-0.112.orig/src/siglist.c
+++ pesign-0.112/src/siglist.c
@@ -77,12 +77,14 @@ static struct sig_type sig_types[] = {
};
static int num_sig_types = sizeof (sig_types) / sizeof (struct sig_type);
+/* How much space does a signature list entry take? Count the space
+ * for the GUID, then the signature/cert itself. */
static int32_t
get_sig_type_size(const efi_guid_t *sig_type)
{
for (int i = 0; i < num_sig_types; i++) {
if (!memcmp(sig_type, sig_types[i].type, sizeof (*sig_type)))
- return sig_types[i].size;
+ return sig_types[i].size + sizeof(efi_guid_t);
}
return -1;
}
@@ -99,7 +101,7 @@ signature_list_new(const efi_guid_t *Sig
return NULL;
sl->SignatureType = SignatureType;
- sl->SignatureSize = size + sizeof (efi_guid_t);
+ sl->SignatureSize = size;
sl->SignatureListSize = sizeof (struct efi_signature_list);
return sl;
@@ -137,11 +139,21 @@ signature_list_add_sig(signature_list *s
sl->realized = NULL;
}
+ /* The sigsize passed in by the caller is just enough for
+ * their object (hash or cert). But sl->SignatureSize includes
+ * the size of the GUID before the hash/cert itself, so
+ * account for that too here. */
+ sigsize += sizeof (efi_guid_t);
+
+ /* If we're adding an x509 cert onto the end of a list of
+ * hashes, we will need to resize the list entries to cope */
if (!efi_guid_cmp(sl->SignatureType, &efi_guid_x509_cert)) {
if (sigsize > sl->SignatureSize)
- resize_entries(sl, sigsize + sizeof (efi_guid_t));
+ resize_entries(sl, sigsize);
} else if (sigsize !=
(unsigned long long)get_sig_type_size(sl->SignatureType)) {
+
+ /* size mismatch - error out */
char *guidname = NULL;
int rc = efi_guid_to_id_guid(sl->SignatureType, &guidname);
if (rc < 0) {
@@ -217,7 +229,10 @@ signature_list_realize(signature_list *s
return -1;
esl = ret;
- memcpy(esl, sl, sizeof (*esl));
+ memcpy(&esl->SignatureType, sl->SignatureType, sizeof *sl->SignatureType);
+ esl->SignatureListSize = sl->SignatureListSize;
+ esl->SignatureHeaderSize = 0;
+ esl->SignatureSize = sl->SignatureSize;
uint8_t *pos = ret + sizeof (*esl);
for (int i = 0; i < count; i++) {
Reply to: