"Disable secure boot" workflow is broken

Bug #1558438 reported by Colin Watson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dkms (Ubuntu)
Fix Released
Critical
Mathieu Trudel-Lapierre
Xenial
Fix Released
Critical
Mathieu Trudel-Lapierre
grub2 (Ubuntu)
Fix Released
Critical
Mathieu Trudel-Lapierre
Xenial
Fix Released
Critical
Mathieu Trudel-Lapierre

Bug Description

I upgraded to grub2 2.02~beta2-36ubuntu1 and was presented with the new prompt to disable secure boot, since I have a dkms package installed. The password I entered was 14 characters long. On the terminal, I see:

Installing for x86_64-efi platform.
Installation finished. No error reported.
password should be 8~16 characters
password should be 8~16 characters
password should be 8~16 characters
Abort

Looking at the code:

                        db_get dkms/secureboot_key
                        length=`echo $RET | wc -c`
                        if [ $length -lt 8 ] || [ $length -gt 16 ]; then
                            db_fset dkms/text/bad_secureboot_key seen false
                            db_input critical dkms/text/bad_secureboot_key
                            STATE=$(($STATE - 2))
                        elif [ $length -ne 0 ]; then
                            echo "${RET}\n${RET}" | mokutil --disable-validation >/dev/null || true
                        fi

There are a few problems here:

 * You *must* use echo "$RET" rather than echo $RET; the password could contain metacharacters. In general you should always surround any $-expansion in a shell script with "" unless you specifically know that you're in one of the special cases where you need to not do so.
 * This is a /bin/bash script for historical reasons. echo "${RET}\n${RET}" is non-portable syntax and only works in shells such as dash with the other style of echo. You should use this instead: printf '%s\n%s\n' "$RET" "$RET"
 * While you're here, it seems to me that a password confirmation page would be a good idea, given that you obviously can't see what you're typing.

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

dkms would be affected the same way, they both use the same code.

I will fix today.

Changed in dkms (Ubuntu):
importance: Undecided → Critical
status: New → Triaged
Changed in grub2 (Ubuntu):
status: New → Triaged
Changed in dkms (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (mathieu-tl)
Changed in grub2 (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (mathieu-tl)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package dkms - 2.2.0.3-2ubuntu11

---------------
dkms (2.2.0.3-2ubuntu11) xenial; urgency=medium

  * debian/patches/shim_secureboot_support.patch: (LP: #1558438)
    - fix quoting variables for setup_mok_validation() to account for passwords
      that might have special characters.
    - use printf rather than straight echo to pass values to mokutil.
    - ask the user to confirm password; not just write it once, this will avoid
      issues with typos in the Secure Boot keys.
  * debian/templates:
    - rename dkms/text/bad_secureboot_key to dkms/error/bad_secureboot_key.
    - add dkms/text/secureboot_key_mismatch.
    - add dkms/secureboot_key_again.

 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 18 Mar 2016 20:54:11 -0400

Changed in dkms (Ubuntu Xenial):
status: Triaged → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grub2 - 2.02~beta2-36ubuntu2

---------------
grub2 (2.02~beta2-36ubuntu2) xenial; urgency=medium

  * debian/postinst.in: (LP: #1558438)
    - fix quoting variables for setup_mok_validation() to account for passwords
      that might have special characters.
    - use printf rather than straight echo to pass values to mokutil.
    - ask the user to confirm password; not just write it once, this will avoid
      issues with typos in the Secure Boot keys.

 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 18 Mar 2016 21:35:50 -0400

Changed in grub2 (Ubuntu Xenial):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.