Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improvements to TPM measurements #361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
105 changes: 78 additions & 27 deletions rust/uefi/linux-bootloader/src/measure.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use alloc::{string::ToString, vec::Vec};
use alloc::{collections::BTreeMap, string::ToString};
use log::info;
use uefi::{
cstr16,
proto::tcg::PcrIndex,
table::{runtime::VariableAttributes, Boot, SystemTable},
CString16,
};

use crate::{
Expand Down Expand Up @@ -37,43 +38,93 @@ pub fn measure_image(system_table: &SystemTable<Boot>, image: &PeInMemory) -> ue
let pe = goblin::pe::PE::parse(pe_binary).map_err(|_err| uefi::Status::LOAD_ERROR)?;

let mut measurements = 0;
for section in pe.sections {
let section_name = section.name().map_err(|_err| uefi::Status::UNSUPPORTED)?;
if let Ok(unified_section) = UnifiedSection::try_from(section_name) {
// UNSTABLE: && in the previous if is an unstable feature
// https://github.com/rust-lang/rust/issues/53667
if unified_section.should_be_measured() {
// Here, perform the TPM log event in ASCII.
if let Some(data) = pe_section_data(pe_binary, &section) {
info!("Measuring section `{}`...", section_name);
if tpm_log_event_ascii(
boot_services,
TPM_PCR_INDEX_KERNEL_IMAGE,
data,
section_name,
)? {
measurements += 1;
}
}
}

// Match behaviour of systemd-stub (see src/boot/efi/stub.c in systemd)
// The encoding as well as the ordering of measurements is critical.
//
// References:
//
// "TPM2 PCR Measurements Made by systemd", https://systemd.io/TPM2_PCR_MEASUREMENTS/
// Section: PCR Measurements Made by systemd-stub (UEFI)
// - PCR 11, EV_IPL, “PE Section Name”
// - PCR 11, EV_IPL, “PE Section Data”
//
// Unified Kernel Image (UKI) specification, UAPI Group,
// https://uapi-group.org/specifications/specs/unified_kernel_image/#uki-tpm-pcr-measurements
//
// Citing from "UKI TPM PCR Measurements":
// On systems with a Trusted Platform Module (TPM) the UEFI boot stub shall measure the sections listed above,
// starting from the .linux section, in the order as listed (which should be considered the canonical order).
// The .pcrsig section is not measured.
//
// For each section two measurements shall be made into PCR 11 with the event code EV_IPL:
// - The section name in ASCII (including one trailing NUL byte)
// - The (binary) section contents
//
// The above should be repeated for every section defined above, so that the measurements are interleaved:
// section name followed by section data, followed by the next section name and its section data, and so on.

// NOTE: The order of measurements is important, so the use of BTreeMap is intentional here.
let ordered_sections: BTreeMap<_, _> = pe
.sections
.iter()
.filter_map(|section| {
let section_name = section.name().ok()?;
let unified_section = UnifiedSection::try_from(section_name).ok()?;
unified_section
.should_be_measured()
.then_some((unified_section, section))
})
.collect();

for (unified_section, section) in ordered_sections {
let section_name = unified_section.name();

info!("Measuring section `{}`...", section_name);

// First measure the section name itself
// This needs to be an UTF-8 encoded string with a trailing null byte
//
// As per reference:
// "Measured hash covers the PE section name in ASCII (including a trailing NUL byte!)."
let section_name_cstr_utf8 = unified_section.name_cstr();

if tpm_log_event_ascii(
RaitoBezarius marked this conversation as resolved.
Show resolved Hide resolved
boot_services,
TPM_PCR_INDEX_KERNEL_IMAGE,
section_name_cstr_utf8.as_bytes_with_nul(),
section_name,
)? {
measurements += 1;
}

// Then measure the section contents.
let Some(data) = pe_section_data(pe_binary, section) else {
continue;
};
Comment on lines +102 to +104

Choose a reason for hiding this comment

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

Is this a case where we should continue? Or should we just fail?

Suggested change
let Some(data) = pe_section_data(pe_binary, section) else {
continue;
};
let data = pe_section_data(pe_binary, section).with_context(|| "Failed to extract section: {section_name}")?;

Copy link
Member

Choose a reason for hiding this comment

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

If some section is not available for measurement, we should probably continue until we actually execute things and this is where things can ultimately crash.


if tpm_log_event_ascii(
boot_services,
TPM_PCR_INDEX_KERNEL_IMAGE,
data,
RaitoBezarius marked this conversation as resolved.
Show resolved Hide resolved
section_name,
)? {
measurements += 1;
}
}

if measurements > 0 {
let pcr_index_encoded = TPM_PCR_INDEX_KERNEL_IMAGE
.0
.to_string()
.encode_utf16()
.flat_map(|c| c.to_le_bytes())
.collect::<Vec<u8>>();
let pcr_index_encoded =
CString16::try_from(TPM_PCR_INDEX_KERNEL_IMAGE.0.to_string().as_str())
.map_err(|_err| uefi::Status::UNSUPPORTED)?;

// If we did some measurements, expose a variable encoding the PCR where
// we have done the measurements.
runtime_services.set_variable(
cstr16!("StubPcrKernelImage"),
&BOOT_LOADER_VENDOR_UUID,
VariableAttributes::BOOTSERVICE_ACCESS | VariableAttributes::RUNTIME_ACCESS,
&pcr_index_encoded,
pcr_index_encoded.as_bytes(),
)?;
}

Expand Down
25 changes: 25 additions & 0 deletions rust/uefi/linux-bootloader/src/unified_sections.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use alloc::ffi::CString;

/// List of PE sections that have a special meaning with respect to
/// UKI specification.
/// This is the canonical order in which they are measured into TPM
/// PCR 11.
/// !!! DO NOT REORDER !!!
#[derive(PartialEq, Eq, PartialOrd, Ord)]
#[repr(u8)]
pub enum UnifiedSection {
Linux = 0,
Expand Down Expand Up @@ -37,4 +40,26 @@ impl UnifiedSection {
pub fn should_be_measured(&self) -> bool {
!matches!(self, UnifiedSection::PcrSig)
}

/// The canonical section name.
pub fn name(&self) -> &'static str {
match self {
UnifiedSection::Linux => ".linux",
UnifiedSection::OsRel => ".osrel",
UnifiedSection::CmdLine => ".cmdline",
UnifiedSection::Initrd => ".initrd",
UnifiedSection::Splash => ".splash",
UnifiedSection::Dtb => ".dtb",
UnifiedSection::PcrSig => ".pcrsig",
UnifiedSection::PcrPkey => ".pcrpkey",
}
}

/// The section name as a `CString`.
pub fn name_cstr(&self) -> CString {
// This should never panic:
// CString::new() only returns an error on strings containing a null byte,
// and we only call it on strings we specified above
CString::new(self.name()).expect("section name should not contain a null byte")
}
}