All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] make grub-install on efi idempotent by omitting PE timestamp
@ 2015-03-20 14:39 Daniel Kahn Gillmor
  2015-03-20 15:25 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Kahn Gillmor @ 2015-03-20 14:39 UTC (permalink / raw)
  To: Grub 2 Development List

[-- Attachment #1: Type: text/plain, Size: 4402 bytes --]

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 948 bytes --]

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] make grub-install on efi idempotent by omitting PE timestamp
  2015-03-20 14:39 [PATCH] make grub-install on efi idempotent by omitting PE timestamp Daniel Kahn Gillmor
@ 2015-03-20 15:25 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2015-03-20 16:16   ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 3+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-03-20 15:25 UTC (permalink / raw)
  To: The development of GNU GRUB

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
>



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] make grub-install on efi idempotent by omitting PE timestamp
  2015-03-20 15:25 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2015-03-20 16:16   ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Kahn Gillmor @ 2015-03-20 16:16 UTC (permalink / raw)
  To: Vladimir 'φ-coder/phcoder' Serbinenko,
	The development of GNU GRUB

On Fri 2015-03-20 11:25:25 -0400, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> 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.

whoops, sorry!  I missed that the first time through.

I just followed up on that thread with a patch using 2015-01-01
00:00:00.

I can work up another patch with modinfo.sh approach if you like, but i
think a stable/permanent timestamp is better.  If i recompile grub with
a patch affecting some other piece of code (code that doesn't affect the
core image), and then i use the new grub to produce an image, i'd like
to be able to see that the resultant image is identical.

   --dkg


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-03-20 16:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-20 14:39 [PATCH] make grub-install on efi idempotent by omitting PE timestamp Daniel Kahn Gillmor
2015-03-20 15:25 ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-03-20 16:16   ` Daniel Kahn Gillmor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.