[PATCH] efi_loader: Allow overlapped extra data for PE hashing

[Su, Bao Cheng]

On Fri, 2022-06-24 at 11:44 +0200, Jan Kiszka wrote:
> On 24.06.22 10:53, Heinrich Schuchardt wrote:
> > On 6/24/22 07:32, Su, Bao Cheng wrote:
> > > During PE hashing, when holes exists between sections, the extra
> > > data
> > > calculated could be a dupulicated region of the last section.
> > > 
> > > Such PE image with holes existing between sections may contain
> > > the
> > > symbol table for the kernel, for example.
> > > 
> > > The Authenticode_PE spec does not rule how to deal with such
> > > scenario,
> > > however, other tools such as pesign and sbsign both have the
> > > overlapped
> > 
> > Thanks for analyzing differences in hashing.
> > 
> > Above you mention holes between sections. Here you talk about
> > overlapping sections. These two cases are obviously distinct.
> > 
> > Please, provide an accurate description.
> 
> Yeah, I also gave that feedback internally already as it left me a
> bit
> confused.
> 
> > 
> > Examples (in text form) would be helpful.
> 
> There is apparently no good PE dump tooling available, so I try to
> describe our scenario verbally:
> 
> We are generating a unified kernel image, similar to what systemd
> does,
> for ARM and ARM64 [1]. The stub has .text and .data sections, and
> then
> follows the symbol table (some versions of binutils allow to suppress
> it, other not, sigh). When appending the actual payload to that
> (kernel
> image, command line, initrd, dtbs), those sections are added right
> after
> the symbol table, creating an unhashed gap between the last stub
> section
> and the first appended one. That unified linux.efi is then signed and
> should be verifiable and bootable (as it is with EDK2).
> 

I will try to give a more straightforward description, considering
below PE image:

## PE Header:
  @0x00000000
   ...
### Section Header 1:
  ...
  @0x00000108 : 0x00008000             - SizeOfRawData
  @0x0000010C : 0x00001000             - PointerToRawData
  ...
### Section Header 2:
  ...
  @0x00000130 : 0x00001C00             - SizeOfRawData
  @0x00000134 : 0x00009000             - PointerToRawData
  ...
### Section Header 3:
  ...
  @0x00000158 : 0x00001200             - SizeOfRawData
  @0x0000015C : 0x0000B200             - PointerToRawData
  ...

From the section headers, the end offset of section 2 is 0x1C00 +
0x9000 = 0xAC00, however, the start offset of the section 3 is 0xB200,
there is a `hole` here of size 0x600 bytes. In our case Jan has
explained this is the symbol table.

According to PE hasing spec, when finished the parsing of sections, the
bytes_hashed should be calculated and compared to the (total PE size -
auth size), and if the bytes_hashed is lesser, it means there are extra
data need be hashed as well. 

According to spec, the offset of the extra data is set to bytes_hashed,
this does not cause overlapping for a normal PE image without holes
between sections, because the bytes_hashed is equal to the tail of the
last section. However, for our case the extra data is the overlapped
with the last section or sections, because the bytes_hashed is lesser
than the tail of the last section due to the `hole`.

U-Boot currently considers this part of data as overlapped and excludes
them from the hashing, however other tools or BLs such as
pesign/sbsign/EDK2 do not rule out the overlapped data, the hash result
stays consistent among these tools, although the last part is hashed
twice indeed.

Baocheng

> HTH,
> Jan
> 
> [1]
> https://github.com/siemens/efibootguard/blob/master/docs/UNIFIED-KERNEL.md
>