Re: [PATCH] make grub-install on efi idempotent by omitting PE timestamp

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

On 20.03.2015 15:39, Daniel Kahn Gillmor wrote:
> I'm using grub-efi on a debian amd64 system, with the ESP mounted at
> /boot/efi.
>
> when i run:
>
>    grub-install
>
> the digest of /boot/efi/EFI/debian/grubx64.efi changes:
>
> root@pops:~# sha1sum /boot/efi/EFI/debian/grubx64.efi
> eafbe63218ecb7f7e69522e55ff7bc9551002c08  /boot/efi/EFI/debian/grubx64.efi
> root@pops:~# grub-install
> Installing for x86_64-efi platform.
> Installation finished. No error reported.
> root@pops:~# sha1sum /boot/efi/EFI/debian/grubx64.efi
> b3ec06be87d356479e652e6515814ed4fd8f35cc  /boot/efi/EFI/debian/grubx64.efi
> root@pops:~#
>
> I compared the two versions, and it looks like they only differ in the
> PE timestamp:
>
> root@pops:~# cp /boot/efi/EFI/debian/grubx64.efi{,.bak}
> root@pops:~# grub-install
> Installing for x86_64-efi platform.
> Installation finished. No error reported.
> root@pops:~# sha1sum /boot/efi/EFI/debian/grubx64.efi{,.bak}
> a2fe7412a8fff69b033c5b11eb3a78927ea084e1  /boot/efi/EFI/debian/grubx64.efi
> b3ec06be87d356479e652e6515814ed4fd8f35cc  /boot/efi/EFI/debian/grubx64.efi.bak
> root@pops:~# diff -u <(hd < /boot/efi/EFI/debian/grubx64.efi.bak ) <(hd < /boot/efi/EFI/debian/grubx64.efi )
> --- /dev/fd/63	2015-03-20 09:32:36.784878070 -0400
> +++ /dev/fd/62	2015-03-20 09:32:36.784878070 -0400
> @@ -6,7 +6,7 @@
>   00000050  69 73 20 70 72 6f 67 72  61 6d 20 63 61 6e 6e 6f  |is program canno|
>   00000060  74 20 62 65 20 72 75 6e  20 69 6e 20 44 4f 53 20  |t be run in DOS |
>   00000070  6d 6f 64 65 2e 0d 0d 0a  24 00 00 00 00 00 00 00  |mode....$.......|
> -00000080  50 45 00 00 64 86 04 00  30 21 0c 55 00 00 00 00  |PE..d...0!.U....|
> +00000080  50 45 00 00 64 86 04 00  42 21 0c 55 00 00 00 00  |PE..d...B!.U....|
>   00000090  00 00 00 00 f0 00 0e 02  0b 02 00 00 00 96 00 00  |................|
>   000000a0  00 2c 01 00 00 00 00 00  00 04 00 00 00 04 00 00  |.,..............|
>   000000b0  00 00 00 00 00 00 00 00  00 02 00 00 00 02 00 00  |................|
> root@pops:~#
>
> These are definitely timestamps, as the two files were generated about
> 20 seconds apart, and 0x42 - 0x30 = 0x12 = 18.  Here's the
> timestamp-to-date conversion:
>
> $ date -d \@$(printf '%d' 0x550c2130)
> Fri Mar 20 09:31:28 EDT 2015
> $
>
> This seems like a spurious change to me, and one that makes it harder to
> do things like compare checksums to make sure no one has tampered with
> the bootloader, or keeping a signature valid for the bootloader as blob.
>
> Is there a reason to embed the timestamps here?  in practice, PE
> executables seem to be able to work with an all-zero timestamp
> (e.g. see ld's --no-insert-timestamp option).
>
> It looks like we're also embedding the timestamp for UBOOT images.
>
> This should be fixable with the following change, which covers both efi
> (first hunk) and uboot (second hunk):
>
> ----------
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 7821dc5..4b0f4de 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -1439,7 +1439,7 @@ grub_install_generate_image (const char *dir, const char *prefix,
>   	c->machine = grub_host_to_target16 (image_target->pe_target);
>
>   	c->num_sections = grub_host_to_target16 (4);
> -	c->time = grub_host_to_target32 (time (0));
> +	c->time = grub_host_to_target32 (0);
>   	c->characteristics = grub_host_to_target16 (GRUB_PE32_EXECUTABLE_IMAGE
>   						    | GRUB_PE32_LINE_NUMS_STRIPPED
>   						    | ((image_target->voidp_sizeof == 4)
> @@ -1782,7 +1782,7 @@ grub_install_generate_image (const char *dir, const char *prefix,
>
>         memset (hdr, 0, sizeof (*hdr));
>         hdr->ih_magic = grub_cpu_to_be32_compile_time (GRUB_UBOOT_IH_MAGIC);
> -      hdr->ih_time = grub_cpu_to_be32 (time (0));
> +      hdr->ih_time = grub_cpu_to_be32 (0);
>         hdr->ih_size = grub_cpu_to_be32 (core_size);
>         hdr->ih_load = grub_cpu_to_be32 (image_target->link_addr);
>         hdr->ih_ep = grub_cpu_to_be32 (image_target->link_addr);
> ----------
>
> I notice that we're already embedding 0 for the timestamp for MIPS_ARC
> on util/mkimage:1859, so this isn't without precedent.
>
I've already detailed in another post in another post why embedding 0 is 
probably bad and that it's better to embed either 2015-01-01 00:00:00 or 
the date of last commit which can be put into modinfo.sh during compile 
time.

> If we want to get fancier, we could make an --embed-timestamp=SECONDS
> option to grub-install, which would allow the user to explicitly set the
> timestamp to a known value, or an --embed-current-timestamp option to
> actually use the output of time(0), but i wouldn't get that fancy
> without first hearing a strong argument for why we need an embedded
> timestamp in the first place.
>
> what do you think?
>
>           --dkg
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>