Debian
sbsigntool package

sbkeysync fails to return non-zero on error

Bug #1892797 reported by dann frazier
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
sbsigntool (Debian)
Confirmed
Unknown
sbsigntool (Ubuntu)
Fix Released
Undecided
Unassigned
Bionic
Won't Fix
Undecided
Unassigned
Focal
Won't Fix
Undecided
Unassigned
Groovy
Won't Fix
Undecided
Unassigned

Bug Description

[Impact]
sbkeysync may exit with exitcode 0 even if it failed to update keys. The secureboot-db service will report no error in this case. This can lead a user to believe they have protected themselves against known insecure bootloaders when they have not.

An example of when this can happen - and where I noticed it - is if you have a system w/ limited variable store space and you try to import a new DBX update file. This is the case today if you pull in the latest DBX for boothole on an OVMF VM w/ a 2M NV variable store (we've since added 4M images - see bug 1885662).

[Test Case]

Boot a secureboot VM w/ 2MB flash, e.g.:
$ cat > user-data.yaml << EOF
#cloud-config
password: ubuntu
chpasswd: { expire: False }
ssh_pwauth: True
EOF

$ cloud-localds seed.img user-data.yaml
$ wget https://cloud-images.ubuntu.com/focal/current/focal-server-cloudimg-amd64.img
$ virt-install --name test --boot loader=/usr/share/OVMF/OVMF_CODE.secboot.fd,loader_ro=yes,loader_type=pflash --import --disk path=test.img --disk path=test-seed.img --ram 4096 --vcpus 4 --os-type linux --os-variant ubuntu18.04 --graphics none --console pty,target_type=serial --network network:default --feature smm=on

Then, from within the guest:
$ wget https://uefi.org/sites/default/files/resources/dbxupdate_x64.bin
$ sudo cp dbxupdate_x64.bin /usr/share/secureboot/updates/dbx
$ sudo service secureboot-db stop
$ sudo service secureboot-db start
$ sudo systemctl status secureboot-db.service
<...>
 /usr/share/secureboot/updates --verbose (code=exited, status=0/SUCCESS)
   Main PID: 2271 (code=exited, status=0/SUCCESS)

Aug 25 16:41:07 ubuntu sbkeysync[2271]: Error syncing keystore file /usr/share/secureboot/updates/dbx/dbxupdate_x64.bin
<...>

[Fix]
https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git/commit/?id=f12484869c9590682ac3253d583bf59b890bb826

[Regression Potential]
It's possible that causing a command to fail that previously did not will lead to other issues. For example, if someone has a 'set -e' shell script that restarts the secureboot-db service, and then does other things, those other things would no longer happen after the secureboot-db servic restart begins to fail.

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

This bug was fixed in the package sbsigntool - 0.9.2-2ubuntu3

---------------
sbsigntool (0.9.2-2ubuntu3) groovy; urgency=medium

  * sbkeysync: exit non-zero upon key insertion failure. (LP: #1892797)

 -- dann frazier <email address hidden> Mon, 24 Aug 2020 18:35:41 -0600

Changed in sbsigntool (Ubuntu):
status: New → Fix Released
dann frazier (dannf)
description: updated
Changed in sbsigntool (Ubuntu Focal):
status: New → In Progress
Revision history for this message
Steve Langasek (vorlon) wrote :

I agree in principle with having the sbkeysync tool return non-zero when it fails to update. However, as implemented, my understanding is that this will also cause the systemd unit to go into a failed state, and to leave the entire system boot in a 'degraded' state, and I don't think at all that this is something we want - because there will be various cases where, based on the realities of the system firmware, we will be unable to apply the secureboot db updates, and I do not think we should have such systems show in degraded state in perpetuity.

I particularly don't think such a behavior change is appropriate for an SRU.

Revision history for this message
dann frazier (dannf) wrote :

Fair enough, I suppose I should think more about a way to let a user know that the db update failed - and the consequences of it - that do not involve failing a systemd unit. No objections to rejected the SRUs.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

sbsigntool (0.9.2-2ubuntu3) groovy; urgency=medium

  * sbkeysync: exit non-zero upon key insertion failure. (LP: #1892797)

 -- dann frazier <email address hidden> Mon, 24 Aug 2020 18:35:41 -0600

Please revert this!

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

If you want this in, then one must adjust secureboot-db package service unit to ignore the error from sbkeysync, and/or declare the relevant error codes as normal.

This behaviour has been discussed on the grub_distros keybase channel, without any objections raised.

And no, seeing that package update / sbkeysync succeeded once, is not good enough. As one has to verify that on every boot. Becuase dbx variable store can be reverted/reset between each boot back to stock defaults. Thus a single success from sbkeysync, can only give a false sense of security.

Changed in sbsigntool (Ubuntu Groovy):
status: Fix Released → Triaged
Changed in sbsigntool (Ubuntu Focal):
status: In Progress → Won't Fix
Changed in sbsigntool (Ubuntu Bionic):
status: New → Won't Fix
Revision history for this message
Steve Langasek (vorlon) wrote :

Dann, I see that you've uploaded secureboot-db to change the systemd unit. I think secureboot-db also needs to declare a Breaks: on versions of secureboot-db that precede this change.

Revision history for this message
Steve Langasek (vorlon) wrote :

Sorry, I mean sbsigntool needs to declare a Breaks: on versions of secureboot-db that precede this change.

Revision history for this message
dann frazier (dannf) wrote :

Sure thing, I'll prepare one.

Revision history for this message
dann frazier (dannf) wrote :

sbsigntool w/ "Breaks: secureboot-db (<< 1.7~)" now uploaded to groovy.

Revision history for this message
Brian Murray (brian-murray) wrote : Proposed package upload rejected

An upload of sbsigntool to focal-proposed has been rejected from the upload queue for the following reason: "This is missing the Breaks that went into Groovy.".

Revision history for this message
Robie Basak (racb) wrote :

There's still an sbsigntool upload sitting in Bionic unapproved. Is this also now to be superseded by a new upload? With both the Bionic and Focal tasks marked Won't Fix, are there any plans remaining to proceed with SRUs?

Revision history for this message
dann frazier (dannf) wrote :

@rcb: No plans, please reject the bionic upload as well.

Bug Watch Updater (bug-watch-updater)
Changed in sbsigntool (Debian):
status: Unknown → Confirmed
Dimitri John Ledkov (xnox)
Changed in sbsigntool (Ubuntu Groovy):
status: Triaged → Won't Fix
Changed in sbsigntool (Ubuntu):
status: Triaged → Fix Released
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.