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

[Su, Bao Cheng]

On Mon, 2022-06-27 at 06:55 +0200, Heinrich Schuchardt wrote:
> On 6/27/22 05:43, Su, Bao Cheng wrote:
> > 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:
> 
> You could try 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fxypron%2Fefi_analyzer&data=05%7C01%7Cbaocheng.su%40ad011.siemens.com%7Cc732b1c183ff4cf56ca708da57f943f2%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637919025335317245%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W1gO6h2%2BiFSUzdNTFi551gyoxlybMOEwQgBnf%2Bswnjw%3D&reserved=0
> .
> 
> > > 
> > > 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
> 
> "Overlap" means that bytes of the image belong to two sections.
> 
> An example of overlap would be:
> 
> section 1: 0x1000 - 0x2000
> section 2: 0x1800 - 0x2800
> 
> "Gap" means that bytes between two sections don't belong to any
> section:
> 
> section1: 0x1000 - 0x2000
> section2: 0x2800 - 0x3800
> 

Gap between sections (in our case between section 2 and 3) produces the
overlapped data while hashing, due to MicroSoft's algorithm.

Baocheng

> > stays consistent among these tools, although the last part is
> > hashed
> > twice indeed.
> 
> We will have to update U-Boot's unit tests to contain an example with
> a
> gap. How do you create these files with a gap?
> 
> Best regards
> 
> Heinrich
> 
> > 
> > Baocheng
> > 
> > > HTH,
> > > Jan
> > > 
> > > [1]
> > >  
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsiemens%2Fefibootguard%2Fblob%2Fmaster%2Fdocs%2FUNIFIED-KERNEL.md&data=05%7C01%7Cbaocheng.su%40ad011.siemens.com%7Cc732b1c183ff4cf56ca708da57f943f2%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637919025335317245%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=xkpfQ9j5gnNMqj%2BbUMBnhp2D1%2BKR0bZlsmJY50lJW9s%3D&reserved=0
> > > 
> > 
>