[Merge] ~ubuntu-core-dev/shim/+git/shim-signed:self-signed into ~ubuntu-core-dev/shim/+git/shim-signed:master

Mathieu Trudel-Lapierre mathieu.tl at gmail.com
Wed Apr 18 15:17:08 UTC 2018



Diff comments:

> diff --git a/debian/shim-signed.postinst b/debian/shim-signed.postinst
> index 9e082e0..7fcf6e0 100644
> --- a/debian/shim-signed.postinst
> +++ b/debian/shim-signed.postinst
> @@ -44,7 +44,26 @@ case $1 in
>  		| LC_ALL=C sort	> /var/lib/shim-signed/dkms-list
>  	fi
>  
> -	SHIM_NOTRIGGER=y update-secureboot-policy
> +	# Upgrade case, migrate all existing kernels/dkms module combinations
> +	# to self-signed modules.
> +	if dpkg --compare-versions "$2" lt-nl "1.34" \

Is this likely to happen? Installing shim-signed for the first time would mean some migration from  Legacy mode to UEFI mode, which we "don't support"?

But I'll make the change anyway.

> +	   && [ -d /var/lib/dkms ]
> +	then
> +		SHIM_NOTRIGGER=y update-secureboot-policy --new-key
> +		for kern in `dpkg -l linux-image-[0-9]\* | grep ^ii | awk '{print $2;}' | sed -e 's/linux-image-//'`;

Thanks!

> +		do
> +			for dkms in `dkms status -k $(uname -r) | grep 'installed' | awk -F,\  '{print $1"/"$2}'`;
> +			do
> +				dkms uninstall "${dkms}"
> +				kmodsign sha512 \
> +					/var/lib/shim-signed/mok/MOK.priv \
> +					/var/lib/shim-signed/mok/MOK.der \
> +					/var/lib/dkms/${dkms}/${kern}/$(uname -m)/module/${dkms%%/*}.ko
> +				dkms install "${dkms}"
> +			done
> +		done
> +		SHIM_NOTRIGGER=y update-secureboot-policy --enroll-key
> +	fi
>  	;;
>  esac
>  
> diff --git a/debian/templates b/debian/templates
> index 604cda1..7fe3ac5 100644
> --- a/debian/templates
> +++ b/debian/templates
> @@ -8,39 +8,36 @@ _Description: Invalid password
>   The Secure Boot key you've entered is not valid. The password used must be
>   between 8 and 16 characters.
>  
> -Template: shim/disable_secureboot
> -Type: boolean
> -Default: true
> -_Description: Disable UEFI Secure Boot?
> - If Secure Boot remains enabled on your system, your system may still boot but
> - any hardware that requires third-party drivers to work correctly may not be
> - usable.
> -
>  Template: shim/enable_secureboot
>  Type: boolean
>  Default: false
> -_Description: Enable UEFI Secure Boot?
> - If Secure Boot is enabled on your system, your system may still boot but
> - any hardware that requires third-party drivers to work correctly may not be
> - usable.
> +_Description: Enroll a new Machine-Owner Key?
> + A new Machine-Owner key has been used to sign drivers. This key now needs to
> + be enrolled in your firmware, which will be done at the next reboot.
> + .
> + If Secure Boot validation was previously disabled on your system, validation
> + will also be re-enabled as part of this key enrollment process.
>  
>  Template: shim/secureboot_explanation
>  Type: note
>  _Description: Your system has UEFI Secure Boot enabled.
> - UEFI Secure Boot is not compatible with the use of third-party drivers.
> + UEFI Secure Boot requires additional configuration to work with third-party
> + drivers.
>   .
> - The system will assist you in toggling UEFI Secure Boot. To ensure that this
> - change is being made by you as an authorized user, and not by an attacker,
> - you must choose a password now and then use the same password after reboot
> - to confirm the change.
> + The system will assist you in configuring UEFI Secure Boot. To permit the
> + use of the third-party drivers that are currently installed, a new

Do we care? People who'd do this know would know what they are doing, especially since we no longer do --enable/--disable?

> + Machine-Owner Key (MOK) has been used to sign these drivers. This key now
> + needs to be enrolled in your system's firmware.
>   .
> - If you choose to proceed but do not confirm the password upon reboot, Ubuntu
> - will still be able to boot on your system but the Secure Boot state will not
> - be changed.
> + To ensure that this change is being made by you as an authorized user, and
> + not by an attacker, you must choose a password now and then confirm the
> + change after reboot using the same password, in both the "Enroll MOK" and
> + "Change Secure Boot state" menus that will be presented to you when this
> + system reboots.

I'm afraid of people missing steps, and reaching an half-configured state. If you don't Change Secure Boot state, you'll still have validation disabled, and if you only do it without Enroll MOK, you'll be left with unworking drivers. Do you have suggestions on how to better present this, or do you think it's really superfluous?

>   .
> - If Secure Boot remains enabled on your system, your system may still boot but
> - any hardware that requires third-party drivers to work correctly may not be
> - usable.
> + If you proceed but do not confirm the password upon reboot, Ubuntu
> + will still be able to boot on your system but any hardware that requires
> + third-party drivers to work correctly may not be usable.
>  
>  Template: shim/secureboot_key
>  Type: string
> diff --git a/update-secureboot-policy b/update-secureboot-policy
> index 6f7b0d4..4e1c1ee 100755
> --- a/update-secureboot-policy
> +++ b/update-secureboot-policy
> @@ -1,5 +1,6 @@
>  #!/bin/sh
>  set -e
> +#set -x

Yup.

>  
>  if  test $# = 0                                                 \
>      && test x"$SHIM_NOTRIGGER" = x                              \
> @@ -19,153 +20,247 @@ 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

There's no particular harm in re-running mokutil, especially since we're always enabling validation, not toggling it -- in this sense, you'll just be left with the very last "action" applicable. Indeed $moksb_var would be superfluous now.

> -        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"
> +}
> +
> +clear_new_dkms_list()
> +{
> +    rm "$NEW_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
> +                    clear_new_dkms_list
> +                    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
> +
> +            save_dkms_list
> +            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 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
> +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 \
> +        -subj "/CN=`hostname -s` Secure Boot Module Signature key" \
> +        -new -x509 -newkey rsa:2048 \
> +        -nodes -days 36500 -outform DER \
> +        -keyout "$SB_PRIV" \
> +        -out "$SB_KEY"
> +}
> +
> +update_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
> +    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
> +    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."

I don't mind either way. This was a non-error in my mind because one might be running --enroll-key manually before having generated one, and we do warn. I'll change it to non-zero.

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