Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ uapi-version = "0.4.0"
walkdir = "2.3.2"
signal-hook-registry = "1.4.8"

# EFI-only dependencies
[target.'cfg(any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "riscv64"))'.dependencies]
virtfw-libefi = { version = "0.6", features = ["std", "sbdata"] }

[profile.release]
# We assume we're being delivered via e.g. RPM which supports split debuginfo
debug = true
Expand Down
33 changes: 29 additions & 4 deletions src/cli/bootupctl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::bootupd;
#[cfg(efi_arch)]
use crate::secureboot::validate_secureboot_for_update;
use anyhow::Result;
use clap::Parser;
use log::LevelFilter;
Expand Down Expand Up @@ -54,7 +56,7 @@ pub enum CtlVerb {
#[clap(name = "status", about = "Show components status")]
Status(StatusOpts),
#[clap(name = "update", about = "Update all components")]
Update,
Update(UpdateOpts),
#[clap(name = "adopt-and-update", about = "Update all adoptable components")]
AdoptAndUpdate(AdoptAndUpdateOpts),
#[clap(name = "validate", about = "Validate system state")]
Expand Down Expand Up @@ -91,19 +93,34 @@ pub struct StatusOpts {
json: bool,
}

#[derive(Debug, Parser)]
pub struct UpdateOpts {
/// Whether to skip the check for the Microsoft 2023 CA UEFI certificate
/// when updating if Secure Boot is enabled.
///
/// This is a safety feature intended to prevent users from breaking
/// Secure Boot. Therefore, it's recommended to only use this flag
/// if you know what you're doing.
#[cfg(efi_arch)]
#[clap(long, action)]
skip_ms_2023_cert_check: bool,
}

#[derive(Debug, Parser)]
pub struct AdoptAndUpdateOpts {
/// Install the static GRUB config files
#[clap(long, action)]
with_static_config: bool,
#[command(flatten)]
update: UpdateOpts,
}

impl CtlCommand {
/// Run CLI application.
pub fn run(self) -> Result<()> {
match self.cmd {
CtlVerb::Status(opts) => Self::run_status(opts),
CtlVerb::Update => Self::run_update(),
CtlVerb::Update(opts) => Self::run_update(opts),
CtlVerb::AdoptAndUpdate(opts) => Self::run_adopt_and_update(opts),
CtlVerb::Validate => Self::run_validate(),
CtlVerb::Backend(CtlBackend::Generate(opts)) => {
Expand Down Expand Up @@ -141,14 +158,22 @@ impl CtlCommand {
}

/// Runner for `update` verb.
fn run_update() -> Result<()> {
fn run_update(_opts: UpdateOpts) -> Result<()> {
ensure_running_in_systemd()?;
#[cfg(efi_arch)]
if !_opts.skip_ms_2023_cert_check {
validate_secureboot_for_update()?;
}
bootupd::client_run_update()
}

/// Runner for `update` verb.
/// Runner for `adopt-and-update` verb.
fn run_adopt_and_update(opts: AdoptAndUpdateOpts) -> Result<()> {
ensure_running_in_systemd()?;
#[cfg(efi_arch)]
if !opts.update.skip_ms_2023_cert_check {
validate_secureboot_for_update()?;
}
bootupd::client_run_adopt_and_update(opts.with_static_config)
}

Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ mod model;
mod model_legacy;
mod ostreeutil;
mod packagesystem;
#[cfg(efi_arch)]
mod secureboot;
mod sha512string;
mod util;

Expand Down
75 changes: 75 additions & 0 deletions src/secureboot.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
//! Module with checks related to Secure Boot.
//!
//! This is primarily used for checking whether it is safe to update a system which has Secure
//! Boot enabled. There can be complications if updating to a new version of shim which is only
//! signed by Microsoft keys which the user's system does not have matching certificates for,
//! so in that scenario we just block updates until the firmware is updated to match (most likely by
//! fwupd).
//!
//! See https://github.com/coreos/bootupd/issues/1099

use anyhow::{bail, Result};
use fn_error_context::context;
use log::info;
use virtfw_libefi::efivar::{ids, sigdb::EfiSigDB};
use virtfw_libefi::sb::certs;
use virtfw_libefi::varstore::sysfs;

/// Check whether Secure Boot is enabled.
fn is_secureboot_enabled() -> bool {
let Some(var) = sysfs::varstore_read(ids::SECURE_BOOT.name, ids::SECURE_BOOT.guid) else {
info!("Could not read the Secure Boot EFI variable. Assuming Secure Boot is not enabled.");
return false;
};
// First byte represents a boolean for "is_enabled"
match var.data().first() {
Some(&b) => b != 0,
None => false,
}
}

/// Check whether the firmware's signature database contains the Microsoft UEFI CA 2023 certificate.
fn db_contains_ms_2023_cert() -> Result<bool> {
let Some(var) = sysfs::varstore_read(ids::DB.name, ids::DB.guid) else {
// At this stage, we'll have confirmed the user does have Secure Boot enabled.
// Best to be safe and abort any updates if we can't even read the EFI variable.
bail!("Could not read the signature database variable. Assuming it is not safe to update.");
};
let Some(sigdb) = EfiSigDB::new_from_bytes(var.data()) else {
bail!("Failed to parse Secure Boot signature database (unknown layout). Assuming it is not safe to update.");
};

Ok(sigdb
.get_x509_list()
.contains(&certs::MICROSOFT_DB_UEFI_2023))
}

/// Attempt to validate that the system can safely accept an EFI bootloader update (if EFI-booted).
///
/// If Secure Boot is enabled, we can't allow updates if the signature database doesn't contain
/// the Microsoft UEFI CA 2023 certificate.
#[context("Validating Secure Boot certificate compatibility")]
pub(crate) fn validate_secureboot_for_update() -> Result<()> {
if !crate::efi::is_efi_booted()? {
info!("Not EFI-booted, skipping Secure Boot certificate check");
return Ok(());
}

if !is_secureboot_enabled() {
info!("Secure Boot not enabled, skipping Secure Boot certificate check");
return Ok(());
};

match db_contains_ms_2023_cert() {
Comment thread
Rolv-Apneseth marked this conversation as resolved.
Resolved
Ok(true) => {
info!("Secure Boot DB contains Microsoft UEFI CA 2023 certificate. Safe to update.");
Ok(())
}
Ok(false) => bail!(
"Secure Boot is enabled but the Microsoft UEFI CA 2023 certificate was not \
found in the firmware's signature database. Updating the shim could render this system \
unbootable. Please update your system firmware by using, for example, fwupd."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the error message is good, but could be improved. Instead of focus on fwupd it could be a little more generic and give the user other options to be more clear, something like:

""Please update your system firmware using your firmware setup menu, fwupd, or similar tools."

WDYT?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fwupd maintainer did tell me that 99% of updates are done through some kind of UI not fwupdmgr... I was tailoring this more towards FCOS itself but maybe that's reasonable if we want bootupd to be distro-agnostic.

),
Err(e) => Err(e),
}
}
32 changes: 32 additions & 0 deletions tests/kola/test-secureboot
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/bin/bash
## kola:
## # qemu is the only platform where setting a 'secure-boot' tag will cause
## # secureboot to get turned on for that instance.
## platforms: qemu
## # Mark as exclusive since we are requesting secureboot and that won't take effect when we're
## # running non-exclusively.
## exclusive: true
## tags: secure-boot
## architectures: x86_64
## description: Verify that Secure Boot certificate validation works as intended
#
# See https://github.com/coreos/bootupd/issues/1099
#
# NOTE: we're only testing the positive case here, as testing the negative case
# is non-trivial since we need to generate and use a custom vars file with only the 2011 cert.

set -xeuo pipefail

. $KOLA_EXT_DATA/libtest.sh

if [[ "$(mokutil --sb)" != "SecureBoot enabled" ]]; then
fatal "system is not running with Secure Boot enabled"
fi

bootupctl status > /tmp/out.txt
assert_file_has_content_literal /tmp/out.txt 'Component EFI'

# No need to force an update scenario since this should hit the certificate check
# regardless of whether there's any available updates.
bootupctl update
ok "passed the Secure Boot certificate check when the 2023 Microsoft cert is available"