Skip to content

snp: validate certificate table bounds #413

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.

Sign up for GitHub

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

Draft
brongan wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

snp: validate certificate table bounds #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1afeb95
guest: handle small certificate buffers
brongan Nov 10, 2026
5876667
snp: validate certificate table bounds
brongan Nov 10, 2026
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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

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

75 changes: 34 additions & 41 deletions src/firmware/guest/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ use crate::error::*;
#[cfg(target_os = "linux")]
use crate::firmware::{
host::CertTableEntry,
linux::{
guest::{ioctl::*, types::*},
host as HostFFI,
},
linux::guest::{ioctl::*, types::*},
};

#[cfg(target_os = "linux")]
Expand Down Expand Up @@ -126,7 +123,7 @@ impl Firmware {
let mut report_response = ReportRsp::default();

// Define a buffer to store the certificates in.
let mut certificate_bytes: Vec<u8>;
let mut certificate_bytes: Vec<u8> = vec![];

// Due to the complex buffer allocation, we will take the ReportReq
// provided by the caller, and create an extended report request object
Expand All @@ -143,36 +140,32 @@ impl Firmware {
&mut report_response,
);

// KEEP for Kernels before 47894e0f (5.19), as userspace broke at that hash.
if SNP_GET_EXT_REPORT
.ioctl(&mut self.0, &mut guest_request)
.is_err()
{
match guest_request.fw_err.into() {
// The kernel patch by [email protected] in kernel hash 47894e0f
// changed the ioctl return to succeed instead of returning an
// error when encountering an invalid certificate length. This was
// done to keep the cryptography safe, so we will now just check
// the guest_request.fw_err for a new value.
//
// Check to see if the buffer needs to be resized. If it does, the
// we need to resize the buffer to the correct size, and
// re-request for the certificates.
VmmError::InvalidCertificatePageLength => {
certificate_bytes = vec![0u8; ext_report_request.certs_len as usize];
ext_report_request.certs_address = certificate_bytes.as_mut_ptr() as u64;
let mut guest_request_retry: GuestRequest<ExtReportReq, ReportRsp> =
GuestRequest::new(
message_version,
&mut ext_report_request,
&mut report_response,
);
SNP_GET_EXT_REPORT
.ioctl(&mut self.0, &mut guest_request_retry)
.map_err(|_| map_fw_err(guest_request_retry.fw_err.into()))?;
let result = SNP_GET_EXT_REPORT.ioctl(&mut self.0, &mut guest_request);

// Depending on the kernel version, a certificate buffer that is too
// small either fails the ioctl or returns success. Both cases set
// fw_err, and this code checks it in either case.
match guest_request.fw_err.into() {
// Resize the buffer to the length the kernel asked for and
// request the certificates again.
VmmError::InvalidCertificatePageLength => {
certificate_bytes = vec![0u8; ext_report_request.certs_len as usize];
ext_report_request.certs_address = certificate_bytes.as_mut_ptr() as u64;
let mut guest_request_retry: GuestRequest<ExtReportReq, ReportRsp> =
GuestRequest::new(
message_version,
&mut ext_report_request,
&mut report_response,
);
let retry_result = SNP_GET_EXT_REPORT.ioctl(&mut self.0, &mut guest_request_retry);
if retry_result.is_err() || guest_request_retry.fw_err != 0 {
Err(map_fw_err(guest_request_retry.fw_err.into()))?
}
_ => Err(map_fw_err(guest_request.fw_err.into()))?,
}
_ if result.is_err() || guest_request.fw_err != 0 => {
Err(map_fw_err(guest_request.fw_err.into()))?
}
_ => {}
}

// Make sure response status is successful
Expand All @@ -184,16 +177,16 @@ impl Firmware {
return Ok((report_response.report.to_vec(), None));
}

let mut certificates: Vec<CertTableEntry>;

unsafe {
let entries = (ext_report_request.certs_address as *mut HostFFI::types::CertTableEntry)
.as_mut()
.ok_or(CertError::EmptyCertBuffer)?;
certificates = HostFFI::types::CertTableEntry::parse_table(entries)?;
certificates.sort();
// The kernel writes certificates only into a buffer that this
// function allocated. Without one, certs_address still holds the
// u64::MAX placeholder from ExtReportReq::new.
if certificate_bytes.is_empty() {
Err(CertError::EmptyCertBuffer)?
}

let mut certificates = CertTableEntry::vec_bytes_to_cert_table(&mut certificate_bytes)?;
certificates.sort();

// Return both the Attestation Report, as well as the Cert Table.
Ok((report_response.report.to_vec(), Some(certificates)))
}
Expand Down
5 changes: 1 addition & 4 deletions src/firmware/host/types/snp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,7 @@ impl CertTableEntry {
/// Takes in bytes in kernel CertTable format and returns in user API CertTable format.
#[cfg(target_os = "linux")]
pub fn vec_bytes_to_cert_table(bytes: &mut [u8]) -> Result<Vec<Self>, CertError> {
let cert_bytes_ptr: *mut FFI::types::CertTableEntry =
bytes.as_mut_ptr() as *mut FFI::types::CertTableEntry;

Ok(unsafe { FFI::types::CertTableEntry::parse_table(cert_bytes_ptr).unwrap() })
FFI::types::CertTableEntry::parse_table(bytes)
}
}

Expand Down
111 changes: 63 additions & 48 deletions src/firmware/linux/host/types/snp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,52 +166,42 @@ impl CertTableEntry {
/// };
/// ```
///
/// The table ends at the first entry whose GUID is all zeroes. Each entry's
/// `offset` is relative to the start of `bytes`, and `offset + length` must
/// not exceed `bytes.len()`.
///
/// # Errors
///
/// Returns [`CertError::BufferOverflow`] if the table has no terminating
/// entry inside `bytes`, or if an entry points outside `bytes`.
#[cfg(target_os = "linux")]
pub unsafe fn parse_table(
mut data: *mut CertTableEntry,
) -> Result<Vec<UAPI::CertTableEntry>, uuid::Error> {
// Helpful Constance for parsing the data
const ZERO_GUID: Uuid = Uuid::from_bytes([0x0; 16]);
pub fn parse_table(bytes: &[u8]) -> Result<Vec<UAPI::CertTableEntry>, CertError> {
const ENTRY_LEN: usize = std::mem::size_of::<CertTableEntry>();

// Pre-defined re-usable variables.
let table_ptr: *mut u8 = data as *mut u8;

// Create a location to store the final data.
let mut retval: Vec<UAPI::CertTableEntry> = vec![];
let mut remaining = bytes;

// Start parsing the PSP data from the pointers.
let mut entry: CertTableEntry;

loop {
// Dereference the pointer to parse the table data.
entry = *data;
let guid: Uuid = Uuid::from_slice(entry.guid.as_slice())?;

// Once we find a zeroed GUID, we are done.
if guid == ZERO_GUID {
break;
}

// Calculate the beginning and ending pointers of the raw certificate data.
let mut cert_bytes: Vec<u8> = vec![];
let mut cert_addr: *mut u8 = table_ptr.offset(entry.offset as isize);
let cert_end: *mut u8 = cert_addr.add(entry.length as usize);
while let Some((entry, rest)) = remaining.split_first_chunk::<ENTRY_LEN>() {
remaining = rest;

// Gather the certificate bytes.
while cert_addr != cert_end {
cert_bytes.push(*cert_addr);
cert_addr = cert_addr.add(1usize);
let [guid @ .., o0, o1, o2, o3, l0, l1, l2, l3] = *entry;
let guid = Uuid::from_bytes(guid);
if guid.is_nil() {
return Ok(retval);
}

// Build the Rust-friendly structure and append vector to be returned when
// we are finished.
retval.push(UAPI::CertTableEntry::from_guid(&guid, cert_bytes.clone())?);
let start = u32::from_ne_bytes([o0, o1, o2, o3]) as usize;
let length = u32::from_ne_bytes([l0, l1, l2, l3]) as usize;
let end = start.checked_add(length).ok_or(CertError::BufferOverflow)?;
let data = bytes.get(start..end).ok_or(CertError::BufferOverflow)?;

// Move the pointer ahead to the next value.
data = data.offset(1isize);
retval.push(
UAPI::CertTableEntry::from_guid(&guid, data.to_vec())
.map_err(|_| CertError::InvalidGUID)?,
);
}

Ok(retval)
Err(CertError::BufferOverflow)
}
}

Expand Down Expand Up @@ -527,6 +517,7 @@ mod test {
#[cfg(target_os = "linux")]
mod cert_table_entry {

use crate::error::CertError;
use crate::firmware::host as UAPI;
use crate::firmware::linux::host::types::CertTableEntry;
use uuid::Uuid;
Expand Down Expand Up @@ -565,7 +556,7 @@ mod test {

#[test]
fn test_parse_table_regular() {
let mut cert_bytes: Vec<u8> = vec![
let cert_bytes: Vec<u8> = vec![
192, 180, 6, 164, 168, 3, 73, 82, 151, 67, 63, 182, 1, 76, 208, 174, 120, 0, 0, 0,
25, 0, 0, 0, 74, 183, 179, 121, 187, 172, 79, 228, 160, 47, 5, 174, 243, 39, 199,
130, 145, 0, 0, 0, 25, 0, 0, 0, 99, 218, 117, 141, 230, 100, 69, 100, 173, 197,
Expand All @@ -577,21 +568,17 @@ mod test {
5, 5, 5, 7, 7, 7, 7, 7, 7,
];

let cert_bytes_ptr: *mut CertTableEntry =
cert_bytes.as_mut_ptr() as *mut CertTableEntry;

let actual: Vec<UAPI::CertTableEntry> =
unsafe { CertTableEntry::parse_table(cert_bytes_ptr).unwrap() };
CertTableEntry::parse_table(&cert_bytes).unwrap();

let expected: Vec<UAPI::CertTableEntry> = build_vec_uapi_cert_table();

assert_eq!(expected, actual);
}

#[test]
#[should_panic]
fn test_parse_table_offset_short() {
let mut cert_bytes: Vec<u8> = vec![
let cert_bytes: Vec<u8> = vec![
192, 180, 6, 164, 168, 3, 73, 82, 151, 67, 63, 182, 1, 76, 208, 174, 120, 0, 0, 0,
1, 0, 0, 0, 74, 183, 179, 121, 187, 172, 79, 228, 160, 47, 5, 174, 243, 39, 199,
130, 145, 0, 0, 0, 25, 0, 0, 0, 99, 218, 117, 141, 230, 100, 69, 100, 173, 197,
Expand All @@ -603,18 +590,46 @@ mod test {
5, 5, 5, 7, 7, 7, 7, 7, 7,
];

let cert_bytes_ptr: *mut CertTableEntry =
cert_bytes.as_mut_ptr() as *mut CertTableEntry;

let actual: Vec<UAPI::CertTableEntry> =
unsafe { CertTableEntry::parse_table(cert_bytes_ptr).unwrap() };
CertTableEntry::parse_table(&cert_bytes).unwrap();

let expected: Vec<UAPI::CertTableEntry> = build_vec_uapi_cert_table();

assert_eq!(
assert_ne!(
expected, actual,
"Invalid certificate offset encountered..."
);
}

#[test]
fn test_parse_table_entry_out_of_bounds() {
let mut cert_bytes: Vec<u8> =
CertTableEntry::uapi_to_vec_bytes(&build_vec_uapi_cert_table()).unwrap();

// Make the last certificate's length run one byte past the buffer.
cert_bytes[92..96].copy_from_slice(&7u32.to_ne_bytes());

assert_eq!(
CertTableEntry::parse_table(&cert_bytes),
Err(CertError::BufferOverflow)
);
}

#[test]
fn test_parse_table_empty() {
let cert_bytes: Vec<u8> = CertTableEntry::uapi_to_vec_bytes(&[]).unwrap();

assert_eq!(CertTableEntry::parse_table(&cert_bytes), Ok(vec![]));
}

#[test]
fn test_parse_table_missing_terminator() {
let cert_bytes: Vec<u8> = CertTableEntry::uapi_to_vec_bytes(&[]).unwrap();

assert_eq!(
CertTableEntry::parse_table(&cert_bytes[..23]),
Err(CertError::BufferOverflow)
);
}
}
}
Loading