[PATCH] efi_loader: Allow overlapped extra data for PE hashing
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 regions hashed. And EDK2 hash the overlapped area as well.
Signed-off-by: Baocheng Su baocheng.su@siemens.com --- lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 9611398885..d85fb6ba08 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -481,7 +481,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, EFI_PRINT("extra data for hash: %zu\n", len - (bytes_hashed + authsz)); efi_image_region_add(regs, efi + bytes_hashed, - efi + len - authsz, 0); + efi + len - authsz, 1); }
/* Return Certificates Table */
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.
Examples (in text form) would be helpful.
Best regards
Heinrich
regions hashed. And EDK2 hash the overlapped area as well.
Signed-off-by: Baocheng Su baocheng.su@siemens.com
lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 9611398885..d85fb6ba08 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -481,7 +481,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, EFI_PRINT("extra data for hash: %zu\n", len - (bytes_hashed + authsz)); efi_image_region_add(regs, efi + bytes_hashed,
efi + len - authsz, 0);
efi + len - authsz, 1);}
/* Return Certificates Table */
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).
HTH, Jan
[1] https://github.com/siemens/efibootguard/blob/master/docs/UNIFIED-KERNEL.md
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
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://github.com/xypron/efi_analyzer.
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
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://github.com/siemens/efibootguard/blob/master/docs/UNIFIED-KERNEL.md
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... .
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...
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 regions hashed. And EDK2 hash the overlapped area as well.
Signed-off-by: Baocheng Su baocheng.su@siemens.com
lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 9611398885..d85fb6ba08 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -481,7 +481,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, EFI_PRINT("extra data for hash: %zu\n", len - (bytes_hashed + authsz)); efi_image_region_add(regs, efi + bytes_hashed,
efi + len - authsz, 0);
efi + len - authsz, 1);}
/* Return Certificates Table */
Let us consider the case that the sum of gaps between sections is greater than the size of the last section N.
start[N] > efi + bytes_hashed end[N] < efi + len - authsz
Sbsigntool and EDK II sort regions by start address before adding the extra data region and will accept this situation.
U-Boot's efi_image_region_add(nocheck = 1) will throw an error "%s: new region already part of another\n".
It seems that this patch is not a complete solution.
Best regards
Heinrich
On Mon, 2022-06-27 at 16:32 +0200, 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 regions hashed. And EDK2 hash the overlapped area as well.
Signed-off-by: Baocheng Su baocheng.su@siemens.com
lib/efi_loader/efi_image_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 9611398885..d85fb6ba08 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -481,7 +481,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, EFI_PRINT("extra data for hash: %zu\n", len - (bytes_hashed + authsz)); efi_image_region_add(regs, efi + bytes_hashed, - efi + len - authsz, 0); + efi + len - authsz, 1); }
/* Return Certificates Table */
Let us consider the case that the sum of gaps between sections is greater than the size of the last section N.
start[N] > efi + bytes_hashed end[N] < efi + len - authsz
Sbsigntool and EDK II sort regions by start address before adding the extra data region and will accept this situation.
U-Boot's efi_image_region_add(nocheck = 1) will throw an error "%s: new region already part of another\n".
This is the original code of efi_image_region_add:
``` for (i = 0; i < regs->num; i++) { reg = ®s->reg[i]; if (nocheck) continue;
/* new data after registered region */ if (start >= reg->data + reg->size) continue;
/* new data preceding registered region */ if (end <= reg->data) { for (j = regs->num - 1; j >= i; j--) memcpy(®s->reg[j + 1], ®s-
reg[j],
sizeof(*reg)); break; }
/* new data overlapping registered region */ EFI_PRINT("%s: new region already part of another\n", __func__); return EFI_INVALID_PARAMETER; } ```
Notice the `if (nocheck) continue;`, I would not say the `new region already part of another` be executed.
- Baocheng
It seems that this patch is not a complete solution.
Best regards
Heinrich
participants (3)
-
Heinrich Schuchardt -
Jan Kiszka -
Su, Bao Cheng