[PATCH 0/3] bg_gen_unified_kernel: Handle prematurely signed kernel stub gracefully

[Jan Kiszka]

This rejects signed kernel stubs as input because they neither make
sense nor lead to correct output. Furthermore, switch the script to
Python format strings.

Jan

Jan Kiszka (3):
bg_gen_unified_kernel: Prepare for processing type-dependent fields
bg_gen_unified_kernel: Detect and reject signed stub images
bg_gen_unified_kernel: Switch to format strings

tools/bg_gen_unified_kernel | 59 ++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 20 deletions(-)

--
2.43.0

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

Depending on the type of the image (32-bit PE32 vs 64-bit PE32+), some
fields in the optional header have different offsets. Prepare for
parsing some of those by using an offset array of 2 elements, one for
each type. Existing constants were not affected by this so far, thus
those will gain identical values for both types.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
tools/bg_gen_unified_kernel | 38 ++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/tools/bg_gen_unified_kernel b/tools/bg_gen_unified_kernel
index 0c4dd9f..8fc9b05 100755
--- a/tools/bg_gen_unified_kernel
+++ b/tools/bg_gen_unified_kernel
@@ -48,13 +48,13 @@ class Section:


class PEHeaders:
- OPT_OFFS_SIZE_OF_INIT_DATA = 0x8
- OPT_OFFS_ADDRESS_OF_ENTRY_POINT = 0x10
- OPT_OFFS_BASE_OF_CODE = 0x14
- OPT_OFFS_SECTION_ALIGNMENT = 0x20
- OPT_OFFS_FILE_ALIGNMENT = 0x24
- OPT_OFFS_SIZE_OF_IMAGE = 0x38
- OPT_OFFS_SIZE_OF_HEADERS = 0x3C
+ OPT_OFFS_SIZE_OF_INIT_DATA = [0x8, 0x8]
+ OPT_OFFS_ADDRESS_OF_ENTRY_POINT = [0x10, 0x10]
+ OPT_OFFS_BASE_OF_CODE = [0x14, 0x14]
+ OPT_OFFS_SECTION_ALIGNMENT = [0x20, 0x20]
+ OPT_OFFS_FILE_ALIGNMENT = [0x24, 0x24]
+ OPT_OFFS_SIZE_OF_IMAGE = [0x38, 0x38]
+ OPT_OFFS_SIZE_OF_HEADERS = [0x3C, 0x3C]

def __init__(self, name, blob):
# Parse headers: DOS, COFF, optional header
@@ -93,6 +93,16 @@ class PEHeaders:

self.opt_header = blob[coff_offs:self.header_size]

+ magic = struct.unpack_from('<H', self.opt_header)[0]
+ if magic == 0x10b:
+ self.is_pe_plus = False
+ elif magic == 0x20b:
+ self.is_pe_plus = True
+ else:
+ print("Invalid %s, unknown optional header magic" % name,
+ file=sys.stderr)
+ exit(1)
+
section_offs = self.header_size

self.header_size += num_sections * 0x28
@@ -124,14 +134,16 @@ class PEHeaders:

section_offs += 0x28

- def get_opt_header_field(self, offset):
- format = '<%dxI' % offset
+ def get_opt_header_field(self, offsets):
+ offs = offsets[1 if self.is_pe_plus else 0]
+ format = '<%dxI' % offs
return struct.unpack_from(format, self.opt_header)[0]

- def set_opt_header_field(self, offset, val):
- format = '<%dsI%ds' % (offset, len(self.opt_header) - offset - 4)
- self.opt_header = struct.pack(format, self.opt_header[:offset], val,
- self.opt_header[offset+4:])
+ def set_opt_header_field(self, offsets, val):
+ offs = offsets[1 if self.is_pe_plus else 0]
+ format = '<%dsI%ds' % (offs, len(self.opt_header) - offs - 4)
+ self.opt_header = struct.pack(format, self.opt_header[:offs], val,
+ self.opt_header[offs+4:])

def get_size_of_init_data(self):
return self.get_opt_header_field(PEHeaders.OPT_OFFS_SIZE_OF_INIT_DATA)
--
2.43.0

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

No functional change, just nicer to read.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

tools/bg_gen_unified_kernel | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/bg_gen_unified_kernel b/tools/bg_gen_unified_kernel
index 0051bf4..bf45abe 100755
--- a/tools/bg_gen_unified_kernel
+++ b/tools/bg_gen_unified_kernel
@@ -60,20 +60,20 @@ class PEHeaders:

def __init__(self, name, blob):
# Parse headers: DOS, COFF, optional header

if len(blob) < 0x40:
- print("Invalid %s, image too small" % name, file=sys.stderr)
+ print(f'Invalid {name}, image too small', file=sys.stderr)
exit(1)

(magic, pe_offs) = struct.unpack_from('<H58xI', blob)

if magic != 0x5a4d:
- print("Invalid %s, bad DOS header magic" % name, file=sys.stderr)
+ print(f'Invalid {name}, bad DOS header magic', file=sys.stderr)
exit(1)

self.dos_header = blob[:pe_offs]

self.header_size = pe_offs + 0x18
if self.header_size > len(blob):
- print("Invalid %s, incomplete COFF header" % name, file=sys.stderr)
+ print(f'Invalid {name}, incomplete COFF header', file=sys.stderr)
exit(1)

self.coff_header = blob[pe_offs:self.header_size]
@@ -81,14 +81,14 @@ class PEHeaders:
(magic, self.machine, num_sections, opt_header_size) = \
struct.unpack_from('<IHH12xH2x', self.coff_header)
if magic != 0x4550:
- print("Invalid %s, bad PE header magic" % name, file=sys.stderr)
+ print(f'Invalid {name}, bad PE header magic', file=sys.stderr)
exit(1)

coff_offs = self.header_size

self.header_size += opt_header_size
if self.header_size > len(blob):
- print("Invalid %s, incomplete optional header" % name,
+ print(f'Invalid {name}, incomplete optional header',
file=sys.stderr)
exit(1)

@@ -100,7 +100,7 @@ class PEHeaders:
elif magic == 0x20b:
self.is_pe_plus = True
else:
- print("Invalid %s, unknown optional header magic" % name,
+ print(f'Invalid {name}, unknown optional header magic',
file=sys.stderr)
exit(1)

@@ -108,7 +108,7 @@ class PEHeaders:

self.header_size += num_sections * 0x28

if self.header_size > len(blob):
- print("Invalid %s, incomplete section headers" % name,
+ print(f'Invalid {name}, incomplete section headers',
file=sys.stderr)
exit(1)

@@ -120,7 +120,7 @@ class PEHeaders:
section = Section.from_struct(
blob[section_offs:section_offs+0x28])
if section.data_offs + section.data_size > len(blob):
- print("Invalid %s, section data missing" % name,
+ print(f'Invalid {name}, section data missing',
file=sys.stderr)
exit(1)

@@ -137,12 +137,12 @@ class PEHeaders:

def get_opt_header_field(self, offsets):

offs = offsets[1 if self.is_pe_plus else 0]

- format = '<%dxI' % offs

+ format = f'<{offs}xI'
return struct.unpack_from(format, self.opt_header)[0]

def set_opt_header_field(self, offsets, val):

offs = offsets[1 if self.is_pe_plus else 0]

- format = '<%dsI%ds' % (offs, len(self.opt_header) - offs - 4)
+ format = f'<{offs}sI{len(self.opt_header) - offs - 4}s'

self.opt_header = struct.pack(format, self.opt_header[:offs], val,

self.opt_header[offs+4:])

--
2.43.0

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

Someone decided to sign the kernel stub. The generator does not expect
that and delivers in inconsistent UKI as output. We could address that
by removing the stub signature, but it is simply not worth it, given
that signing the stub never makes sense. Instead, detect the unexpected
input and simply reject it.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

tools/bg_gen_unified_kernel | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/tools/bg_gen_unified_kernel b/tools/bg_gen_unified_kernel
index 8fc9b05..0051bf4 100755
--- a/tools/bg_gen_unified_kernel
+++ b/tools/bg_gen_unified_kernel
@@ -55,6 +55,7 @@ class PEHeaders:
OPT_OFFS_FILE_ALIGNMENT = [0x24, 0x24]
OPT_OFFS_SIZE_OF_IMAGE = [0x38, 0x38]
OPT_OFFS_SIZE_OF_HEADERS = [0x3C, 0x3C]
+ OPT_OFFS_CERT_TABLE_SIZE = [0x84, 0x94]

def __init__(self, name, blob):
# Parse headers: DOS, COFF, optional header

@@ -255,6 +256,12 @@ def main():
stub = args.stub.read()

pe_headers = PEHeaders('stub image', stub)
+
+ if pe_headers.get_opt_header_field(PEHeaders.OPT_OFFS_CERT_TABLE_SIZE) > 0:
+ print("Signed stub image detected which is neither supported nor "
+ "makes any sense", file=sys.stderr)
+ exit(1)
+
stub_first_data = pe_headers.first_data
stub_end_of_sections = pe_headers.end_of_sections
file_align = pe_headers.get_file_alignment()
--
2.43.0