[Merge] ~juliank/grub/+git/ubuntu:juliank/check-signed-kernels into ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu
Steve Langasek
steve.langasek at canonical.com
Thu Jul 12 21:07:59 UTC 2018
A few final comments inline, nothing that should block landing this. Please resolve these comments as you see fit and land this change.
Diff comments:
> diff --git a/debian/grub-check-signatures b/debian/grub-check-signatures
> new file mode 100755
> index 0000000..633ad2d
> --- /dev/null
> +++ b/debian/grub-check-signatures
> @@ -0,0 +1,90 @@
> +#!/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 sized
s/sized/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
> + this_uname_r="$(echo "$kernel" | sed -nr 's#^/boot/vmlinuz-(.*)#\1#p' | sed 's#\.efi\.signed$##')"
micro-optimization, I think?
this_uname_r="$(echo "$kernel" | sed -r 's#^/boot/vmlinuz-(.*)#\1#; s#\.efi\.signed$##')"
since there should be no cases where $kernel doesn't match, therefore the -n + s///p is superfluous.
Have you checked that this code DTRT in the case that /boot/vmlinuz-* matches nothing?
> + 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 [ "$unsigned" ]; then
style nit, I'd prefer if [ -n "$unsigned" ] there
> + show_warning "$unsigned"
> + fi
> +fi
--
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