[Merge] ~ubuntu-core-dev/shim/+git/shim-signed:self-signed into ~ubuntu-core-dev/shim/+git/shim-signed:master
Steve Langasek
steve.langasek at canonical.com
Wed Mar 28 23:45:59 UTC 2018
Diff comments:
> diff --git a/update-secureboot-policy b/update-secureboot-policy
> index 6f7b0d4..66447da 100755
> --- a/update-secureboot-policy
> +++ b/update-secureboot-policy
> @@ -19,153 +20,268 @@ if [ "$(id -u)" -ne 0 ]; then
> exit 1
> fi
>
> +do_enroll=0
> +do_toggle=0
> +
> +efivars=/sys/firmware/efi/efivars
> +secureboot_var=SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
> +moksb_var=MokSB-605dab50-e046-4300-abb6-3dd810dd8b23
> +moksbstatert_var=MokSBStateRT-605dab50-e046-4300-abb6-3dd810dd8b23
> +
> +SB_KEY="/var/lib/shim-signed/mok/MOK.der"
> +SB_PRIV="/var/lib/shim-signed/mok/MOK.priv"
> +
> OLD_DKMS_LIST="/var/lib/shim-signed/dkms-list"
> NEW_DKMS_LIST="${OLD_DKMS_LIST}.new"
>
> touch $OLD_DKMS_LIST
>
> -args=$@
> -enable_secureboot=0
> dkms_list=$(find /var/lib/dkms -maxdepth 1 -type d -print 2>/dev/null \
> | LC_ALL=C sort)
> dkms_modules=$(echo "$dkms_list" | wc -l)
>
> . /usr/share/debconf/confmodule
>
> -setup_mok_validation()
> +update_dkms_list()
> {
> - local moksbstatert
> - local efivars secureboot_var moksb_var moksbstatert_var
> - local enable_sb action
> - enable_sb=$1
> - efivars=/sys/firmware/efi/efivars
> - secureboot_var=SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
> - moksb_var=MokSB-605dab50-e046-4300-abb6-3dd810dd8b23
> - moksbstatert_var=MokSBStateRT-605dab50-e046-4300-abb6-3dd810dd8b23
> - action=disable
> -
> - if [ $enable_sb -eq 1 ]; then
> - action=enable
> - fi
> + echo "$dkms_list" > $NEW_DKMS_LIST
> +}
>
> - if ! [ -f $efivars/$secureboot_var ] \
> - || [ "$(od -An -t u1 $efivars/$secureboot_var | awk '{ print $NF }')" -ne 1 ]
> - then
> - echo "Secure Boot not enabled on this system." >&2
> - return 0
> - fi
> - moksbstatert=0
> - if [ -f $efivars/$moksb_var ]; then
> - # if MokSB exists we've likely already run mokutil since last boot
> - echo "The Secure Boot policy was already changed since last reboot; nothing to do." >&2
> - return 0
> - 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; }')
> +save_dkms_list()
> +{
> + mv "$NEW_DKMS_LIST" "$OLD_DKMS_LIST"
> +}
> +
> +finish() {
> + save_dkms_list
> +}
> +
> +validate_password()
> +{
> + db_capb
> + if [ "$key" != "$again" ]; then
> + db_fset shim/error/secureboot_key_mismatch seen false
> + db_input critical shim/error/secureboot_key_mismatch || true
> + STATE=$(($STATE - 2))
> + else
> + length=$((`echo "$key" | wc -c` - 1))
> + if [ $length -lt 8 ] || [ $length -gt 16 ]; then
> + db_fset shim/error/bad_secureboot_key seen false
> + db_input critical shim/error/bad_secureboot_key || true
> + STATE=$(($STATE - 2))
> + elif [ $length -ne 0 ]; then
> + return 0
> + fi
> fi
> - # poor man's xor
> - if [ $(($moksbstatert+$enable_sb)) -ne 1 ]; then
> - STATE=1
> - db_settitle shim/title/secureboot
> - while true; do
> - case "$STATE" in
> - 1)
> - db_capb
> - db_fset shim/secureboot_explanation seen false
> - db_input critical shim/secureboot_explanation || true
> +
> + return 1
> +}
> +
> +clear_passwords()
> +{
> + # Always clear secureboot key.
> + db_set shim/secureboot_key ''
> + db_fset shim/secureboot_key seen false
> + db_set shim/secureboot_key_again ''
> + db_fset shim/secureboot_key_again seen false
> +}
> +
> +toggle_validation()
> +{
> + local key=$1
> + local again=$2
> +
> + echo "Enabling shim validation."
> + printf '%s\n%s\n' "$key" "$again" | mokutil --enable-validation >/dev/null || true
> +}
> +
> +enroll_mok()
> +{
> + local key=$1
> + local again=$2
> +
> + echo "Adding '$SB_KEY' to shim:"
> + printf '%s\n%s\n' "$key" "$again" | mokutil --import "$SB_KEY" >/dev/null || true
> +}
> +
> +do_it()
> +{
> + STATE=1
> + db_settitle shim/title/secureboot
> + while true; do
> + case "$STATE" in
> + 1)
> + db_capb
> + db_fset shim/secureboot_explanation seen false
> + db_input critical shim/secureboot_explanation || true
> + ;;
> + 2)
> + if [ "$do_toggle" -eq 1 ]; then
> + # Allow the user to skip toggling Secure Boot.
> + db_fset shim/enable_secureboot seen false
> + db_input critical shim/enable_secureboot || true
> db_go
>
> - # Allow the user to skip disabling Secure Boot.
> - db_fset shim/${action}_secureboot seen false
> - db_input critical shim/${action}_secureboot || true
> - ;;
> - 2)
> - db_get shim/${action}_secureboot
> + db_get shim/enable_secureboot
> if [ "$RET" = "false" ]; then
> break
> fi
> + fi
> + ;;
> + 3)
>
> - db_input critical shim/secureboot_key || true
> - seen_key=$RET
> - db_input critical shim/secureboot_key_again || true
> - ;;
> - 3)
> - db_get shim/secureboot_key
> - key="$RET"
> - db_get shim/secureboot_key_again
> - again="$RET"
> -
> - if [ -z "$key$again" ] && echo "$seen_key" | grep -q ^30; then
> - echo "Running in non-interactive mode, doing nothing." >&2
> -
> - if ! diff -u $OLD_DKMS_LIST $NEW_DKMS_LIST; then
> - exit 1
> - else
> - exit 0
> - fi
> - fi
> + db_input critical shim/secureboot_key || true
> + seen_key=$RET
> + db_input critical shim/secureboot_key_again || true
> + ;;
> + 4)
> + db_get shim/secureboot_key
> + key="$RET"
> + db_get shim/secureboot_key_again
> + again="$RET"
> +
> + if [ -z "$key$again" ] && echo "$seen_key" | grep -q ^30; then
> + echo "Running in non-interactive mode, doing nothing." >&2
>
> - db_capb
> - if [ "$key" != "$again" ]; then
> - db_fset shim/error/secureboot_key_mismatch seen false
> - db_input critical shim/error/secureboot_key_mismatch || true
> - STATE=$(($STATE - 2))
> + if ! diff -u $OLD_DKMS_LIST $NEW_DKMS_LIST; then
> + exit 1
> else
> - length=$((`echo "$key" | wc -c` - 1))
> - if [ $length -lt 8 ] || [ $length -gt 16 ]; then
> - db_fset shim/error/bad_secureboot_key seen false
> - db_input critical shim/error/bad_secureboot_key || true
> - STATE=$(($STATE - 2))
> - elif [ $length -ne 0 ]; then
> - printf '%s\n%s\n' "$key" "$again" | mokutil --${action}-validation >/dev/null || true
> - fi
> + exit 0
> fi
> + fi
>
> - # Always clear secureboot key.
> - db_set shim/secureboot_key ''
> - db_fset shim/secureboot_key seen false
> - db_set shim/secureboot_key_again ''
> - db_fset shim/secureboot_key_again seen false
> - ;;
> - *)
> - break
> - ;;
> - esac
> -
> - if db_go; then
> - STATE=$(($STATE + 1))
> - else
> - STATE=$(($STATE - 1))
> + if validate_password; then
> + if [ $do_toggle -eq 1 ]; then
> + toggle_validation "$key" "$again"
> + fi
> + if [ $do_enroll -eq 1 ]; then
> + enroll_mok "$key" "$again"
> + fi
> fi
> - db_capb backup
> - done
> - db_capb
> +
> + clear_passwords
> + ;;
> + *)
> + break
> + ;;
> + esac
> +
> + if db_go; then
> + STATE=$(($STATE + 1))
> + else
> + STATE=$(($STATE - 1))
> + fi
> + db_capb backup
> + done
> + db_capb
> +}
> +
> +validate_actions() {
> + # Validate any queued actions before we go try to do them.
> + local moksbstatert=0
> +
> + 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
> +
> + # We were asked to enroll a key. This only makes sense if validation
> + # is enabled.
> + if [ $do_enroll -eq 1 ] && [ $moksbstatert -eq 1 ]; then
> + do_toggle=1
> + fi
> +
> + if [ $do_toggle -eq 1 ]; then
> +
> + # Make sure we really do need to do something.
> + if [ $moksbstatert -ne 1 ]; then
> + echo "Shim validation is already in the requested state."
> + do_toggle=0
> + return
> + fi
> +
> + if [ $dkms_modules -le 1 ]; then
> + echo "No DKMS packages installed: not changing Secure Boot validation state."
> + do_toggle=0
> + return
> + fi
> +
> + if [ -f $efivars/$moksb_var ]; then
> + # if MokSB exists we've likely already run mokutil since last boot
> + echo "The Secure Boot policy was already changed since last reboot." >&2
> + do_toggle=0
> + return
> + fi
> + fi
> +}
> +
> +create_mok()
> +{
> + if [ -e "$SB_KEY" ]; then
> + return
> fi
> +
> + echo "Generating a new Secure Boot signing key:"
> + openssl req -config /usr/lib/shim/mok/openssl.cnf \
> + -new -x509 -newkey rsa:2048 \
> + -nodes -days 36500 -outform DER \
> + -keyout "$SB_PRIV" \
> + -out "$SB_KEY"
> }
>
> -if echo "$args" | grep -qc -- '--enable'; then
> - enable_secureboot=1
> -elif echo "$args" | grep -qc -- '--disable'; then
> - enable_secureboot=0
> -elif echo "$args" | grep -qc -- '--help'; then
> - echo "update-secureboot-policy: toggle UEFI Secure Boot in shim"
> - echo
> - echo "\t--enable\tPrompt to enable Secure Boot validation."
> - echo "\t--disable\tPrompt to disable Secure Boot validation (default)."
> - echo "\t--help\t\tThis help text."
> - exit 0
> +update_dkms_list
> +
> +trap finish EXIT
> +
> +if ! [ -f $efivars/$secureboot_var ] \
> + || [ "$(od -An -t u1 $efivars/$secureboot_var | awk '{ print $NF }')" -ne 1 ]
> +then
> + echo "Secure Boot not enabled on this system." >&2
> + exit 0
> fi
>
> -if [ $dkms_modules -gt 1 ] || [ $enable_secureboot -eq 1 ]; then
> - echo "$dkms_list" > $NEW_DKMS_LIST
> - setup_mok_validation $enable_secureboot
> - mv "$NEW_DKMS_LIST" "$OLD_DKMS_LIST"
> -else
> - echo "No DKMS packages installed: not changing Secure Boot validation state."
> +case "$1" in
> +'--enable'|'--disable')
> + echo "Please run mokutil directly to change shim validation behavior."
> + exit 0
> + ;;
> +
> +'--new-key')
> + create_mok
Sorry, I finally re-found the context from where new-key is called: https://bugs.launchpad.net/ubuntu/+source/dkms/+bug/1748983/+attachment/5053788/+files/dkms.debdiff is a patch to dkms that would call --new-key to generate the key before signing modules. It is certainly correct for dkms to generate the key before it tries to use it. This also explains why we are not calling --enroll-key in the shim-signed postinst: enrolling the key serves no purpose if we don't have modules to sign with it, and furthermore, if we flip the policy *before* the dkms modules are signed, we end up in a state that the dkms modules on disk are no longer usable.
If the user upgrades shim-signed but not dkms, they don't get any of their modules signed and they don't get their SecureBoot policy updated. This means that you cannot rely on the version number of the installed shim-signed package to tell you if the new policy is in place. Is that a problem? Should we address this by signing the modules directly, as part of the upgrade? I think this was the actual rationale for the Breaks: dkms that Adam asked you to drop.
It appears that if a user has disabled SecureBoot previously via shim-signed, then removed dkms, then installs the new shim-signed, no changes will be made to their SecureBoot policy. I think this is probably fine.
> + exit 0
> + ;;
> +
> +'--enroll-key')
> + if [ -e "$SB_KEY" ]; then
> + if mokutil --test-key "$SB_KEY" | \
> + grep -qc 'is not'; then
> + do_enroll=1
> + fi
> + else
> + echo "No MOK found."
> + fi
> + ;;
> +
> +*)
> + echo "update-secureboot-policy: toggle UEFI Secure Boot in shim"
> + echo
> + echo "\t--new-key\tCreate a new MOK."
> + echo "\t--enroll-key\tEnroll the new MOK for this system in shim."
> + echo "\t--help\t\tThis help text."
> + exit 0
> +
> +esac
> +
> +validate_actions
> +
> +if [ $(($do_toggle + $do_enroll)) -lt 1 ]; then
> + echo "Nothing to do."
> + exit 0
> fi
>
> +do_it
> +
> exit 0
--
https://code.launchpad.net/~ubuntu-core-dev/shim/+git/shim-signed/+merge/337571
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/shim/+git/shim-signed:master.
More information about the Ubuntu-reviews
mailing list