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

[Julian Andres Klode]

Check that kernels are signed

[Steve Langasek (vorlon)]

review: Needs Fixing

[Steve Langasek (vorlon)]

should actually check both /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c != 0 && /sys/firmware/efi/efivars/MokSBStateRT-605dab50-e046-4300-abb6-3dd810dd8b23 != 1. mokutil unhelpfully gives no information about the latter, so you'll need to directly read the files. See /usr/sbin/update-secureboot-policy for examples.

*Ideally*, we would verify that the kernel is not just signed, but signed with a key that's trusted by the firmware (so: found in db, or in MokListRT). Requires a bit more code, but I believe it's warranted.

[Julian Andres Klode (juliank)]

It now looks at all kernels in /boot, rather then finding via grub.cfg (so we can run before update-grub), checks the policy in the postinst, and only if we are on secure boot.

I have not found any solution to check if it's signed with the right key, though. What I figured out is that sbverify needs a certificate, and mokutil can give me a list of keys.

[Steve Langasek (vorlon)]

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?

review: Needs Fixing

[Julian Andres Klode (juliank)]

> 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?

Your review on the last diff said to only check when we are switching the secure boot policy in grub - the only place we can do this is from the grub maintainer scripts, as we need to have the grub version to check against.

We specifically can't do that in grub-install, since grub-install may be used for a lot of stuff other than installing stuff to the ESP. Hence the previous approach checked it in update-grub.

I don't see why grub-pc should not fail due to secure boot if it's on an EFI system in secure boot mode.

[Steve Langasek (vorlon)]

A few final comments inline, nothing that should block landing this. Please resolve these comments as you see fit and land this change.

diff --git a/debian/changelog b/debian/changelog
index e02ed3f..7c43119 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+grub2 (2.02-2ubuntu11) UNRELEASED; urgency=medium
+
+  [ Julian Andres Klode ]
+  * Verify that the current and newer kernels are signed when grub is updated, to
+    make sure people do not accidentally shutdown without a signed kernel.
+
+ -- Julian Andres Klode <[e-mail]>  Wed, 20 Jun 2018 13:04:33 +0200
+
 grub2 (2.02-2ubuntu10) cosmic; urgency=medium
 
   * debian/patches/grub-shell-test-helper-disable-seabios-sercon.patch: In the
diff --git a/debian/grub-check-signatures b/debian/grub-check-signatures
new file mode 100755
index 0000000..3a466ec
--- /dev/null
+++ b/debian/grub-check-signatures
@@ -0,0 +1,94 @@
+#!/bin/sh
+
+set -e
+
+. /usr/share/debconf/confmodule
+
+# Check if we are on an EFI system
+efivars=/sys/firmware/efi/efivars
+secureboot_var=SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
+moksbstatert_var=MokSBStateRT-605dab50-e046-4300-abb6-3dd810dd8b23
+
+on_secure_boot() {
+	# Validate any queued actions before we go try to do them.
+	local moksbstatert=0
+
+	if ! [ -d $efivars ]; then
+		return 1
+	fi
+
+	if ! [ -f $efivars/$secureboot_var ] \
+		|| [ "$(od -An -t u1 $efivars/$secureboot_var | awk '{ print $NF }')" -ne 1 ]
+	then
+		return 1
+	fi
+
+	if [ -f /proc/sys/kernel/moksbstate_disabled ]; then
+		moksbstatert=$(cat /proc/sys/kernel/moksbstate_disabled 2>/dev/null || echo 0)
+	elif [ -f $efivars/$moksbstatert_var ]; then
+		# MokSBStateRT set to 1 means validation is disabled
+		moksbstatert=$(od -An -t u1 $efivars/$moksbstatert_var | \
+					   awk '{ print $NF; }')
+	fi
+
+	if [ $moksbstatert -eq 1 ]; then
+		return 1
+	fi
+
+	return 0
+}
+
+# Check if a given kernel image is signed
+is_signed() {
+	tmp=$(mktemp)
+	sbattach --detach $tmp $1 >/dev/null  	# that's ugly...
+	test "$(wc -c < $tmp)" -ge 16	# Just _some_ minimum size
+	result=$?
+	rm $tmp
+	return $result
+}
+
+# Check that our current kernel and every newer one is signed
+find_unsigned() {
+	uname_r="$(uname -r)"
+	for kernel in $(ls -1 /boot/vmlinuz-* | sort -V -r); do
+		# no kernels :(
+		if [ "$kernel" = "/boot/vmlinuz-*" ]; then
+			break
+		fi
+		this_uname_r="$(echo "$kernel" | sed -r 's#^/boot/vmlinuz-(.*)#\1#; s#\.efi\.signed$##')"
+		if dpkg --compare-versions "$this_uname_r" lt "$uname_r"; then
+			continue
+		fi
+		if ! is_signed $kernel; then
+			echo "$this_uname_r"
+		fi
+	done
+}
+
+# Only reached from show_warning
+error() {
+	echo "E: Your kernels are unsigned. This system will fail to boot in a secure boot environment." >&2
+	exit 1
+}
+
+# Either shows a debconf note or prints an error with error() above if
+# that fails
+show_warning() {
+	# kernels should be an indented list of one version per line
+	escaped="$(printf "%s" "$unsigned" | sed "s#^#     #" | debconf-escape -e )"
+	db_capb escape
+	db_settitle grub2/unsigned_kernels_title || error
+	db_fset grub2/unsigned_kernels seen 0 || error
+	db_subst grub2/unsigned_kernels unsigned_versions "$escaped" || error
+	db_input critical grub2/unsigned_kernels || error
+	db_go || error
+	error
+}
+
+if on_secure_boot; then
+	unsigned="$(find_unsigned)"
+	if [ -n "$unsigned" ]; then
+		show_warning "$unsigned"
+	fi
+fi
diff --git a/debian/grub-common.install.in b/debian/grub-common.install.in
index f06320d..59d3462 100644
--- a/debian/grub-common.install.in
+++ b/debian/grub-common.install.in
@@ -1,5 +1,6 @@
 ../../debian/apport/source_grub2.py	usr/share/apport/package-hooks/
 ../../debian/grub.d			etc
+../../debian/grub-check-signatures usr/share/grub/
 
 etc/bash_completion.d
 etc/grub.d
diff --git a/debian/postinst.in b/debian/postinst.in
index b1435d3..5c8c93d 100644
--- a/debian/postinst.in
+++ b/debian/postinst.in
@@ -320,6 +320,10 @@ case "$1" in
 
     devicemap_regenerated=
 
+    if [ @PACKAGE@ = "grub-efi-amd64" ] && dpkg --compare-versions "$2" lt-nl 2.02-2ubuntu11; then
+      /usr/share/grub/grub-check-signatures
+    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..4ae3b6e 100644
--- a/debian/templates.in
+++ b/debian/templates.in
@@ -65,3 +65,19 @@ _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_title
+Type: title
+_Description: unsigned kernels
+
+Template: grub2/unsigned_kernels
+Type: note
+_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:
+ .
+  ${unsigned_versions}
+ .
+ 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.