[Merge] ~juliank/grub/+git/ubuntu:juliank/check-signed-kernels into ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu

Steve Langasek steve.langasek at canonical.com
Wed Jul 11 22:32:06 UTC 2018


Review: Needs Fixing

Comments inline.

One additional concern: the grub maintainer script is not the only place that grub-install might be called.  In particular, shim-signed will also call grub-install --target=x86_64-efi from its postinst - as will grub-efi-amd64-signed, which is from a different source package.  And with the most recent adjustment of the dependencies (grub-efi-amd64-signed now depends on grub-efi-amd64 | grub-pc; which means some users in 18.04 and newer will actually have grub-pc installed, whose postinst /should not/ fail to configure due to the kernel secureboot question), grub-efi-amd64-signed may actually have its dependencies satisfied even though there are unsigned kernels.

So I think the right place for the grub-check-signatures code to run is as an inlined wrapper of grub-install.  Do you agree?

Diff comments:

> diff --git a/debian/postinst.in b/debian/postinst.in
> index b1435d3..c8cf13f 100644
> --- a/debian/postinst.in
> +++ b/debian/postinst.in
> @@ -320,6 +320,10 @@ case "$1" in
>  
>      devicemap_regenerated=
>  
> +    if dpkg --compare-versions "$2" lt-nl 2.02-2ubuntu11; then
> +      /usr/share/grub/grub-check-signatures || exit 1

The '|| exit 1' should be redundant since this is set -e.

Shouldn't this be guarded by a check for the platform?  This code should only be run for grub-efi-amd64, not all grub targets.

> +    fi
> +
>      if egrep -q '^[[:space:]]*post(inst|rm)_hook[[:space:]]*=[[:space:]]*(/sbin/|/usr/sbin/)?update-grub' /etc/kernel-img.conf 2>/dev/null; then
>        echo 'Removing update-grub hooks from /etc/kernel-img.conf in favour of' >&2
>        echo '/etc/kernel/ hooks.' >&2
> diff --git a/debian/templates.in b/debian/templates.in
> index e261bb3..d566d81 100644
> --- a/debian/templates.in
> +++ b/debian/templates.in
> @@ -65,3 +65,9 @@ _Description: /boot/grub/device.map has been regenerated
>   .
>   If you do not understand this message, or if there are no custom
>   boot menu entries, you can ignore this message.
> +
> +Template: grub2/unsigned_kernels
> +Type: note
> +_Description: Unsigned kernels
> + Your system does not have any signed kernel installed. If your system
> + uses secure boot or you enable it later, your system might not boot.

- this message is only shown if you have secureboot enabled.
- this message is shown if *any* of your (newer) kernels are *unsigned*.  So it's not accurate to say they don't have any that are signed.
- by default on the graphical frontends, only the short description of the debconf template is shown, so it should give a stand alone explanation of the problem.

Please consider the following wording:

_Description: Cannot upgrade Secure Boot enforcement policy due to unsigned kernels
 Your system has UEFI Secure Boot enabled in firmware, and the following kernels
 present on your system are unsigned:
 .
   $ver1
   $ver2
 .
 These kernels cannot be verified under Secure Boot.  To ensure your system
 remains bootable, GRUB will not be upgraded on your disk until these kernels are
 removed or replaced with signed kernels.

This obviously requires capturing the unsigned kernel versions and passing them into the template with db_subst.



-- 
https://code.launchpad.net/~juliank/grub/+git/ubuntu/+merge/345403
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu.



More information about the Ubuntu-reviews mailing list