[PATCH] bg_gen_unified_kernel: Drop symbol table from image

6 views
Skip to first unread message

Jan Kiszka

unread,
Jun 27, 2022, 7:27:27 AM6/27/22
to efibootguard-dev, Su, Bao Cheng (RC-CN DF FA R&D), Christian Storm
From: Jan Kiszka <jan.k...@siemens.com>

Keeping it between the last stub section data and the first section we
append create an problematic gap between section data in the file. This
is not explicitly allowed by the Authenticode specification for PE
files. That spec rather assumes that there is only extra data at the end
of the file which it demands to be hashed as well. To formula provided
in spec to calculate the start and size of that extra data fails if
there a gaps between sections. While signing tools and EDK2 seem to be
fine with that, we are in a gray zone here with the generated image.

Avoid this by simply ripping out the symbol table before appending our
extra sections. We do that by tracking the final end of all section data
per PEHeaders, even across section additions in order to stay
consistent with this new (internal) API.

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

diff --git a/tools/bg_gen_unified_kernel b/tools/bg_gen_unified_kernel
index 551bda6..f794309 100755
--- a/tools/bg_gen_unified_kernel
+++ b/tools/bg_gen_unified_kernel
@@ -102,6 +102,7 @@ class PEHeaders:
exit(1)

self.first_data = len(blob)
+ self.end_of_sections = 0

self.sections = []
for n in range(num_sections):
@@ -115,6 +116,10 @@ class PEHeaders:
if section.data_size and section.data_offs < self.first_data:
self.first_data = section.data_offs

+ end_of_section = section.data_offs + section.data_size
+ if end_of_section > self.end_of_sections:
+ self.end_of_sections = end_of_section
+
self.sections.append(section)

section_offs += 0x28
@@ -200,6 +205,10 @@ class PEHeaders:
if sect.data_size > 0:
sect.data_offs += file_relocation

+ end_of_section = section.data_offs + section.data_size
+ if end_of_section > self.end_of_sections:
+ self.end_of_sections = end_of_section
+

def main():
parser = argparse.ArgumentParser(
@@ -235,10 +244,11 @@ def main():

pe_headers = PEHeaders('stub image', stub)
stub_first_data = pe_headers.first_data
+ stub_end_of_sections = pe_headers.end_of_sections
file_align = pe_headers.get_file_alignment()

# Add extra section headers
- current_offs = align(len(stub), file_align)
+ current_offs = align(stub_end_of_sections, file_align)
sect_size = align(len(cmdline), file_align)
cmdline_section = Section(b'.cmdline', sect_size, 0x30000,
sect_size, current_offs,
@@ -315,7 +325,7 @@ def main():
image += bytearray(pe_headers.first_data - len(image))

# Write remaining stub
- image += stub[stub_first_data:]
+ image += stub[stub_first_data:stub_end_of_sections]

# Write data of extra sections
image += bytearray(cmdline_section.data_offs - len(image))
--
2.35.3

Christian Storm

unread,
Jul 1, 2022, 11:13:17 AM7/1/22
to efibootguard-dev
Hi,

> From: Jan Kiszka <jan.k...@siemens.com>
>
> Keeping it between the last stub section data and the first section we
> append create an problematic gap between section data in the file. This

Shouldn't it read 'create*s* a [problematic]'?

> is not explicitly allowed by the Authenticode specification for PE
> files. That spec rather assumes that there is only extra data at the end
> of the file which it demands to be hashed as well. To formula provided

Shouldn't it read 'The [formula]'?

> in

[...] the [...]

> spec to calculate the start and size of that extra data fails if
> there a

Shouldn't it read 'are'?
Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED SES-DE
Otto-Hahn-Ring 6, 81739 München, Germany

Jan Kiszka

unread,
Jul 6, 2022, 8:23:06 AM7/6/22
to efibootguard-dev, Christian Storm
From: Jan Kiszka <jan.k...@siemens.com>

Keeping it between the last stub section data and the first section we
append creates a problematic gap between section data in the file. This
is not explicitly allowed by the Authenticode specification for PE
files. That spec rather assumes that there is only extra data at the end
of the file which it demands to be hashed as well. The formula provided
in the spec to calculate the start and size of that extra data fails if
there are gaps between sections. While signing tools and EDK2 seem to be
fine with that, we are in a gray zone here with the generated image.

Avoid this by simply ripping out the symbol table before appending our
extra sections. We do that by tracking the final end of all section data
per PEHeaders, even across section additions in order to stay
consistent with this new (internal) API.

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

Changes in v2:
- typo fixes in commit message, no code changes

tools/bg_gen_unified_kernel | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/bg_gen_unified_kernel b/tools/bg_gen_unified_kernel
index dc5328c..afda8ce 100755
@@ -314,7 +324,7 @@ def main():
Reply all
Reply to author
Forward
0 new messages