Ubuntu
shim-signed package

shim-signed trigger should not fail when attempting to re-prompt noninteractively and we've prompted before

Bug #1695578 reported by Steve Langasek
84
This bug affects 10 people
Affects Status Importance Assigned to Milestone
shim-signed (Ubuntu)
Fix Released
Critical
Mathieu Trudel-Lapierre
Ubuntu ubuntu-17.06
Trusty
Fix Released
Undecided
Unassigned
Xenial
Fix Released
Undecided
Unassigned
Yakkety
Won't Fix
Undecided
Unassigned
Zesty
Fix Released
Undecided
Unassigned

Bug Description

[Impact]
Users may require updates to DKMS modules while shim validation is disabled, and expect to do these updates non-interactively, such as via unattended-upgrades.

[Test Case]
== Interactive ==
(on an EFI system with proper Secure Boot support)
1- Install an old version of a DKMS package (such as bbswitch-dkms or r8168-dkms)
2- Follow the prompts on install to disable shim validation; reboot.

The system should allow you to install the dkms module and prompt you for a password to disable Secure Boot validation in shim. This behavior should remain the same with or without the patch and serves as an extra verification to guard against regressions.

== Uninteractive ==
(on an EFI system with proper Secure Boot support)
1- Install an old version of a DKMS package (such as bbswitch-dkms or r8168-dkms)
2- Follow the prompts on install to disable shim validation; reboot.
3- Run 'sudo apt update'
4- Enable unattended-upgrades (see https://help.ubuntu.com/lts/serverguide/automatic-updates.html)
4b - If necessary, using PPA packages for testing, allow the PPA source in /etc/apt/apt.conf.d/50unattended-upgrades.
5- Wait/trigger unattended upgrades; observe the behavior:

The user has not installed any new DKMS modules, and we're noninteractive, the trigger should silently pass. Without the patch, this test case should fail (upgrade fails) because update-secureboot-policy is unable to prompt the user.

== New DKMS package install uninteractively ==
1- Enable unattended upgrades
2- Prepare a new PPA package update that adds a dependency on a -dkms package.
3- Wait/trigger unattended upgrades; observe the behavior.

The user is in effect installing a new DKMS module that was not already present on the system and being upgraded. In this case, the upgrade should fail, because update-secureboot-policy is unable to prompt the user. Without the patch, the same behavior occurs (this serves as another regression guard)

[Regression Potential]
Failure to complete an update in non-interactive (unattended-upgrades) when DKMS modules are in use, triggering a failure in apt's upgrade process would be a regression of this update. Also, being unable to correctly recognize the current state of Secure Boot and shim validation, or incorrectly returning before prompting for the password required to toggle shim validation when the shim validation state make sense to be changed (ie. prompting to enable when it is disabled only, prompting to disable only if it's currently enabled).

[Background information]
The upgrades should run non-interactively without issue whenever possible, but forcefully prompt the user if a new DKMS package is being installed uninteractively:

 - If the user installs a new DKMS module, we should not silently proceed. Either the user should be prompted, or if we're noninteractive, the trigger should fail.
 - If the user has not installed any new DKMS modules, but we have an interactive frontend, we should prompt.
 - If the user has not installed any new DKMS modules, and we're noninteractive, the trigger should silently pass.

---

We currently always fail with an error when update-secureboot-policy has been called, we detect that secureboot needs to be disabled for a dkms module, and we don't have an interactive debconf frontend. However, as a result this means that if the user has previously made a conscious decision *not* to disable secureboot, despite having dkms modules installed, a non-interactive package upgrade will fail.

It doesn't make sense for a non-interactive package upgrade to fail merely because the user's secureboot setting is ill-advised.

We should ensure that:

 - If the user installs a new DKMS module, we should not silently proceed. Either the user should be prompted, or if we're noninteractive, the trigger should fail.
 - If the user has not installed any new DKMS modules, but we have an interactive frontend, we should prompt.
 - If the user has not installed any new DKMS modules, and we're noninteractive, the trigger should silently pass.

To know whether new DKMS modules have been installed, we should capture the list from /var/lib/dkms and store it under /var/lib/shim-signed on each successful invocation.

For upgrade purposes, the shim-signed postinst should detect that we are upgrading from a version of the package that did not yet record the list in /var/lib/shim-signed, and record any DKMS modules present, so that these are not considered "new". We only want to do this on upgrade, not on a new install of shim-signed; on a new install, the trigger should already handle this for us.

See original description
Steve Langasek (vorlon)
Changed in shim-signed (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)
importance: Undecided → Critical
milestone: none → ubuntu-17.06
status: New → Triaged
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package shim-signed - 1.30

---------------
shim-signed (1.30) artful; urgency=medium

  * update-secureboot-policy: track the installed DKMS modules so we can skip
    failing unattended upgrades if they hasn't changed (ie. if no new DKMS
    modules have been installed, just honour the user's previous decision to
    not disable shim validation). (LP: #1695578)
  * update-secureboot-policy: allow re-enabling shim validation when no DKMS
    packages are installed. (LP: #1673904)
  * debian/source_shim-signed.py: add the textual representation of SecureBoot
    and MokSBStateRT EFI variables rather than just adding the files directly;
    also, make sure we include the relevant EFI bits from kernel log.
    (LP: #1680279)

 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 23 Jun 2017 14:37:21 -0400

Changed in shim-signed (Ubuntu):
status: Triaged → Fix Released
Mathieu Trudel-Lapierre (cyphermox)
description: updated
Mathieu Trudel-Lapierre (cyphermox)
description: updated
Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello Steve, or anyone else affected,

Accepted shim-signed into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shim-signed/1.32~17.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-zesty to verification-done-zesty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-zesty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in shim-signed (Ubuntu Zesty):
status: New → Fix Committed
tags: added: verification-needed verification-needed-zesty
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Steve, or anyone else affected,

Accepted shim-signed into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shim-signed/1.32~16.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-yakkety to verification-done-yakkety. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-yakkety. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in shim-signed (Ubuntu Yakkety):
status: New → Fix Committed
tags: added: verification-needed-yakkety
Revision history for this message
Steve Langasek (vorlon) wrote : Proposed package upload rejected

An upload of shim-signed to trusty-proposed has been rejected from the upload queue for the following reason: "needs adjusted versioned dep on grub2-common; drop ref to LP: #1624096 from changelog".

Steve Langasek (vorlon)
Changed in shim-signed (Ubuntu Xenial):
status: New → Fix Committed
tags: added: verification-needed-xenial
Revision history for this message
Steve Langasek (vorlon) wrote : Please test proposed package

Hello Steve, or anyone else affected,

Accepted shim-signed into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shim-signed/1.32~14.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in shim-signed (Ubuntu Trusty):
status: New → Fix Committed
tags: added: verification-needed-trusty
Revision history for this message
Steve Langasek (vorlon) wrote :

Hello Steve, or anyone else affected,

Accepted shim-signed into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shim-signed/1.32~14.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Mathieu Trudel-Lapierre (cyphermox)
tags: added: verification-done-trusty
removed: verification-needed-trusty
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Verification done for trusty:

I've verified that interactive, non-interactive (old dkms update) and non-interactive (new pulled update) behave correctly, using shim-signed 1.32~14.04.2

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

Verification-done for xenial, using shim-signed 1.32~16.04.1:

I have verified that interactive, non-interactive (old dkms update) and non-interactive (new pulled update) behave correctly: interactive package installs always prompt the user, non-interactive installs attempt to prompt the user but can't due to running non-interactively, so they fail the unattended upgrade in the case where a new DKMS package is being installed, or continue (without a failure, but also not promting, along with a notice to that effect in the logs) on successfully.

description: updated
tags: added: verification-done-xenial
removed: verification-needed-xenial
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Verification-done for xenial, using shim-signed 1.32~17.04.1:

Interactive, non-interactive (old dkms update) and non-interactive (new pulled update) react as expected. I have tested that the interactive installs always prompt the user; but also that any unattended-upgrade run correctly fails if trying to install a new DKMS module that was not previously present (as it can't prompt the user), but complete successfully if upgrading an already-installed DKMS module.

tags: added: verification-done-zesty
removed: verification-needed-yakkety verification-needed-zesty
tags: removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package shim-signed - 1.32~16.04.1

---------------
shim-signed (1.32~16.04.1) xenial; urgency=medium

  * Backport shim-signed 1.32 to 16.04. (LP: #1700170)

shim-signed (1.32) artful; urgency=medium

  * Handle cleanup of /var/lib/shim-signed on package purge.

shim-signed (1.31) artful; urgency=medium

  * Fix regression in postinst when /var/lib/dkms does not exist.
    (LP#1700195)
  * Sort the list of dkms modules when recording.

shim-signed (1.30) artful; urgency=medium

  * update-secureboot-policy: track the installed DKMS modules so we can skip
    failing unattended upgrades if they hasn't changed (ie. if no new DKMS
    modules have been installed, just honour the user's previous decision to
    not disable shim validation). (LP: #1695578)
  * update-secureboot-policy: allow re-enabling shim validation when no DKMS
    packages are installed. (LP: #1673904)
  * debian/source_shim-signed.py: add the textual representation of SecureBoot
    and MokSBStateRT EFI variables rather than just adding the files directly;
    also, make sure we include the relevant EFI bits from kernel log.
    (LP: #1680279)

shim-signed (1.29) artful; urgency=medium

  * Makefile: Generate BOOT$arch.CSV, for use with fallback.
  * debian/rules: make sure we can do per-arch EFI files.

 -- Mathieu Trudel-Lapierre <email address hidden> Mon, 10 Jul 2017 17:43:10 -0400

Changed in shim-signed (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Steve Langasek (vorlon) wrote : Update Released

The verification of the Stable Release Update for shim-signed has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package shim-signed - 1.32~17.04.1

---------------
shim-signed (1.32~17.04.1) zesty; urgency=medium

  * Backport shim-signed 1.32 to 17.04. (LP: #1700170)

shim-signed (1.32) artful; urgency=medium

  * Handle cleanup of /var/lib/shim-signed on package purge.

shim-signed (1.31) artful; urgency=medium

  * Fix regression in postinst when /var/lib/dkms does not exist.
    (LP#1700195)
  * Sort the list of dkms modules when recording.

shim-signed (1.30) artful; urgency=medium

  * update-secureboot-policy: track the installed DKMS modules so we can skip
    failing unattended upgrades if they hasn't changed (ie. if no new DKMS
    modules have been installed, just honour the user's previous decision to
    not disable shim validation). (LP: #1695578)
  * update-secureboot-policy: allow re-enabling shim validation when no DKMS
    packages are installed. (LP: #1673904)
  * debian/source_shim-signed.py: add the textual representation of SecureBoot
    and MokSBStateRT EFI variables rather than just adding the files directly;
    also, make sure we include the relevant EFI bits from kernel log.
    (LP: #1680279)

shim-signed (1.29) artful; urgency=medium

  * Makefile: Generate BOOT$arch.CSV, for use with fallback.
  * debian/rules: make sure we can do per-arch EFI files.

 -- Mathieu Trudel-Lapierre <email address hidden> Mon, 10 Jul 2017 17:10:08 -0400

Changed in shim-signed (Ubuntu Zesty):
status: Fix Committed → Fix Released
Steve Langasek (vorlon)
Changed in shim-signed (Ubuntu Yakkety):
status: Fix Committed → Won't Fix
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package shim-signed - 1.32~14.04.2

---------------
shim-signed (1.32~14.04.2) trusty; urgency=medium

  * Backport shim-signed 1.32 to 14.04. (LP: #1700170)

shim-signed (1.32) artful; urgency=medium

  * Handle cleanup of /var/lib/shim-signed on package purge.

shim-signed (1.31) artful; urgency=medium

  * Fix regression in postinst when /var/lib/dkms does not exist.
    (LP #1700195)
  * Sort the list of dkms modules when recording.

shim-signed (1.30) artful; urgency=medium

  * update-secureboot-policy: track the installed DKMS modules so we can skip
    failing unattended upgrades if they hasn't changed (ie. if no new DKMS
    modules have been installed, just honour the user's previous decision to
    not disable shim validation). (LP: #1695578)
  * update-secureboot-policy: allow re-enabling shim validation when no DKMS
    packages are installed. (LP: #1673904)
  * debian/source_shim-signed.py: add the textual representation of SecureBoot
    and MokSBStateRT EFI variables rather than just adding the files directly;
    also, make sure we include the relevant EFI bits from kernel log.
    (LP: #1680279)

shim-signed (1.29) artful; urgency=medium

  * Makefile: Generate BOOT$arch.CSV, for use with fallback.
  * debian/rules: make sure we can do per-arch EFI files.

shim-signed (1.28) zesty; urgency=medium

  * Adjust apport hook to include key files that tell us about the system's
    current SB state. LP: #1680279.

shim-signed (1.27) zesty; urgency=medium

  [ Steve Langasek ]
  * Update to the signed 0.9+1474479173.6c180c6-1ubuntu1 binary from
    Microsoft.
  * update-secureboot-policy:
    - detect when we have no debconf prompting and error out instead of ending
      up in an infinite loop. LP: #1673817.
    - refactor to make the code easier to follow.
    - remove a confusing boolean that would always re-prompt on a request to
      --enable, but not on a request to --disable.

  [ Mathieu Trudel-Lapierre ]
  * update-secureboot-policy:
    - some more fixes to properly handle non-interactive mode. (LP: #1673817)

shim-signed (1.23) zesty; urgency=medium

  * debian/control: bump the Depends on grub2-common since that's needed to
    install with the new updated EFI binaries filenames.

shim-signed (1.22) yakkety; urgency=medium

  * Update to the signed 0.9+1474479173.6c180c6-0ubuntu1 binary from Microsoft.
  * Update paths now that the shim binary has been renamed to include the
    target architecture.
  * debian/shim-signed.postinst: clean up old MokManager.efi from EFI/ubuntu;
    since it's being replaced by mm$arch.efi.

shim-signed (1.21.3) vivid; urgency=medium

  * No-change rebuild for shim 0.9+1465500757.14a5905.is.0.8-0ubuntu3.

shim-signed (1.21.2) vivid; urgency=medium

  * Revert to signed shim from 0.8-0ubuntu2.
    - shim.efi.signed originally built from shim 0.8-0ubuntu2 in wily.

shim-signed (1.20) yakkety; urgency=medium

  * Update to the signed 0.9+1465500757.14a5905-0ubuntu1 binary from Microsoft.
    (LP: #1581299)

 -- Mathieu Trudel-Lapierre <email address hidden> Mon, 10 Jul 2017 20:29:28 -0400

Changed in shim-signed (Ubuntu Trusty):
status: Fix Committed → Fix Released
Francis Ginther (fginther)
tags: added: id-594aec8b127c3c2ebe70c166
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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