diff --git a/Cargo.lock b/Cargo.lock index 91d671da..2a211482 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -62,7 +62,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -73,7 +73,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -88,6 +88,17 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" +[[package]] +name = "bitfield-struct" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ca6739863c590881f038d033a146c51ddae239186a4327014839fd864f44ed5" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "bitflags" version = "2.11.0" @@ -192,6 +203,7 @@ dependencies = [ "signal-hook-registry", "tempfile", "uapi-version", + "virtfw-libefi", "walkdir", ] @@ -201,6 +213,12 @@ version = "3.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "bytes" version = "1.11.1" @@ -230,7 +248,7 @@ dependencies = [ "maybe-owned", "rustix", "rustix-linux-procfs", - "windows-sys 0.61.2", + "windows-sys 0.60.2", "winx", ] @@ -271,7 +289,7 @@ dependencies = [ "rustix", "rustix-linux-procfs", "uuid", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -326,9 +344,11 @@ version = "4.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" dependencies = [ + "anstream", "anstyle", "clap_lex", "strsim", + "terminal_size", ] [[package]] @@ -427,7 +447,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -805,7 +825,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -1061,7 +1081,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -1230,7 +1250,17 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.59.0", +] + +[[package]] +name = "terminal_size" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" +dependencies = [ + "rustix", + "windows-sys 0.59.0", ] [[package]] @@ -1372,6 +1402,12 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "849f6b1fe8a0fb07170737d7f3acf72cac5462fb3f4e86614474a49f7fac3b65" +[[package]] +name = "uguid" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c8352f8c05e47892e7eaf13b34abd76a7f4aeaf817b716e88789381927f199c" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -1420,6 +1456,22 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "virtfw-libefi" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8508c830cd7bd67ed6103ddc02a04b7aabfe6d2d57b1a1767ff8ec20b706a39" +dependencies = [ + "bitfield-struct", + "byteorder", + "chrono", + "clap", + "libc", + "log", + "uguid", + "zerocopy", +] + [[package]] name = "walkdir" version = "2.5.0" @@ -1539,7 +1591,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e3ccd79e..9b72ce7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 diff --git a/src/bootupd.rs b/src/bootupd.rs index bdad9ff5..b1bd99ae 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -20,6 +20,8 @@ use crate::freezethaw::fsfreeze_thaw_cycle; ))] use crate::grubconfigs::{ensure_grub_permissions, GRUB2DIR}; use crate::model::{ComponentStatus, ComponentUpdatable, ContentMetadata, SavedState, Status}; +#[cfg(efi_arch)] +use crate::secureboot::validate_secureboot_for_update; use crate::{ostreeutil, util}; use anyhow::{anyhow, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; @@ -756,6 +758,8 @@ fn prep_before_update() -> Result { let path = "/"; let sysroot = Dir::open_ambient_dir(path, ambient_authority()).context("Opening root dir")?; let device = list_dev_current_root()?; + #[cfg(efi_arch)] + validate_secureboot_for_update()?; Ok(RootContext::new(sysroot, path, device)) } diff --git a/src/main.rs b/src/main.rs index 918366a1..1246c65d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -45,6 +45,8 @@ mod model; mod model_legacy; mod ostreeutil; mod packagesystem; +#[cfg(efi_arch)] +mod secureboot; mod sha512string; mod util; diff --git a/src/secureboot.rs b/src/secureboot.rs new file mode 100644 index 00000000..ea0d34bc --- /dev/null +++ b/src/secureboot.rs @@ -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() -> Result { + let Some(var) = sysfs::varstore_read(ids::SECURE_BOOT.name, ids::SECURE_BOOT.guid) else { + bail!("Could not read the Secure Boot EFI var after confirming the system is EFI-booted."); + }; + + let Some(&is_enabled_byte) = var.data().first() else { + bail!("Secure Boot data is empty or unreadable"); + }; + + Ok(is_enabled_byte != 0) +} + +/// Check whether the firmware's signature database contains the Microsoft UEFI CA 2023 certificate. +fn db_contains_ms_2023_cert() -> Result { + 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() { + 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." + ), + Err(e) => Err(e), + } +} diff --git a/tests/kola/test-secureboot b/tests/kola/test-secureboot new file mode 100755 index 00000000..6868d820 --- /dev/null +++ b/tests/kola/test-secureboot @@ -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"