From 944581eaefe4c6887790f2b8ed39c9ee76146c55 Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 17 Dec 2024 11:58:01 +0100 Subject: [PATCH] rust/pvimg: Add upper estimates for the Secure Execution header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Secure Execution header V1 can be at maximum two pages large, optional items are not supported, and the size of the encrypted part cannot be larger than the total size of the Secure Execution header add this as Deku assertions and additional conditions to the code. In addition, add a check for the number of key slots. Fixes: f4cf4ae6ebb1 ("rust: Add a new tool called 'pvimg'") Reviewed-by: Steffen Eiden Signed-off-by: Marc Hartmayer Signed-off-by: Jan Höppner --- rust/pvimg/src/pv_utils/error.rs | 3 + rust/pvimg/src/pv_utils/se_hdr/brb.rs | 50 +++++++++++++--- rust/pvimg/src/pv_utils/se_hdr/builder.rs | 10 +++- rust/pvimg/src/pv_utils/se_hdr/hdr_v1.rs | 71 ++++++++++++++++++++--- rust/pvimg/src/pv_utils/uvdata.rs | 18 ++++-- 5 files changed, 130 insertions(+), 22 deletions(-) diff --git a/rust/pvimg/src/pv_utils/error.rs b/rust/pvimg/src/pv_utils/error.rs index 2a176276..a12c4a22 100644 --- a/rust/pvimg/src/pv_utils/error.rs +++ b/rust/pvimg/src/pv_utils/error.rs @@ -30,6 +30,9 @@ pub enum Error { #[error("Invalid Secure Execution header")] InvalidSeHdr, + #[error("Secure Execution header size {given} is larger than the maximum of {maximum} bytes")] + InvalidSeHdrTooLarge { given: usize, maximum: usize }, + #[error("Invalid component metadata.")] InvalidComponentMetadata, diff --git a/rust/pvimg/src/pv_utils/se_hdr/brb.rs b/rust/pvimg/src/pv_utils/se_hdr/brb.rs index ac3a2e6e..b8dadba1 100644 --- a/rust/pvimg/src/pv_utils/se_hdr/brb.rs +++ b/rust/pvimg/src/pv_utils/se_hdr/brb.rs @@ -171,8 +171,8 @@ impl AeadCipherTrait for SeHdr { } impl AeadDataTrait for SeHdr { - fn aad(&self) -> Vec { - [serialize_to_bytes(&self.common).unwrap(), self.data.aad()].concat() + fn aad(&self) -> Result> { + Ok([serialize_to_bytes(&self.common)?, self.data.aad()?].concat()) } fn data(&self) -> Vec { @@ -265,7 +265,7 @@ impl SeHdr { data.resize(sehs, 0); reader.read_exact(&mut data[common_size..])?; - Self::try_from_data(&data) + Self::try_from_data(&data).map_err(|_| Error::InvalidSeHdr) } } @@ -342,13 +342,13 @@ impl UvDataPlainTrait for SeHdrPlain { } impl AeadPlainDataTrait for SeHdrPlain { - fn aad(&self) -> Vec { - let data_aad = self.data.aad(); + fn aad(&self) -> Result> { + let data_aad = self.data.aad()?; - [serialize_to_bytes(&self.common).unwrap(), data_aad].concat() + Ok([serialize_to_bytes(&self.common)?, data_aad].concat()) } - fn data(&self) -> Confidential> { + fn data(&self) -> Result>> { self.data.data() } @@ -387,5 +387,41 @@ mod tests { ])), Err(Error::InvalidSeHdr) )); + + // Invalid SeHdr as the `sehs` is too large. + assert!(matches!( + SeHdr::try_from_io(Cursor::new([ + 73, 66, 77, 83, 101, 99, 69, 120, 0, 0, 1, 0, 0, 0, 1, 255, 65, 65, 65, 65, 67, 0, + 65, 17, 65, 0, 65, 65, 65, 65, 65, 65, 91, 91, 180, 91, 91, 91, 91, 91, 91, 91, 91, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 241, 241, + 241, 241, 241, 91, 91, 91, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 80, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, + 112, 112, 112, 112, 91, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, + 112, 112, 112, 112, 112, 112, 112, 0, 0, 0, 0, 101, 99, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 65, 65, 65, 65, 67, 0, 65, 17, 65, 0, 65, 65, 65, 65, + 65, 65, 91, 91, 180, 91, 91, 91, 91, 91, 91, 91, 91, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 241, 241, 241, 241, 241, 91, 91, 91, 112, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 80, 112, 112, 112, 112, 112, 112, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 91, 112, 112, + 112, 112, 112, 112, 112, 112, 112, 112, 112, 112, 73, 66, 77, 83, 101, 99, 69, 120, + 0, 112, 112, 0, 1, 0, 0, 0, 0, 101, 99, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 65, 65, 65, 65, 67, 0, 65, 17, 65, 0, 65, 65, 65, 65, 65, 65, 91, + 91, 180, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, 91, + 91, 91, 112, 112, 112, 112, 112, 73, 66, 77, 83, 101, 99, 69, 120, 0, 0, 1, 0, 0, + 0, 0, 48, 53, 53, 53, 53, 53, 53, 53, 91, 91, 91, 241, 241, 46, 49, 49, 0, 49, 49, + 0, 0, 112, 112, 112, 91, 0, 0, 0, 0, 9, 0, 49, 50, 22, 241, 241, 241, 241, 241, + 241, 241, 241, 241, 241, 241, 91, 91, 91, 91, 91, 255, 251, 0, 0, 91, 91, 91, 91, + 91, 91, 91, 91, 91, 91, 91, 0, 0, 91, 0, 0, 10, 91, 91, 91, 65, 65, 65, 65 + ])), + Err(Error::InvalidSeHdr) + )); } } diff --git a/rust/pvimg/src/pv_utils/se_hdr/builder.rs b/rust/pvimg/src/pv_utils/se_hdr/builder.rs index ba6de898..93bcc7af 100644 --- a/rust/pvimg/src/pv_utils/se_hdr/builder.rs +++ b/rust/pvimg/src/pv_utils/se_hdr/builder.rs @@ -230,8 +230,14 @@ mod tests { let decrypted = bin.decrypt(&prot_key).expect("BUG"); assert_eq!(bin.common, decrypted.common); - assert_eq!(bin.aad(), decrypted.aad()); - assert_ne!(&bin.data(), decrypted.data().value()); + assert_eq!( + bin.aad().expect("should not fail"), + decrypted.aad().expect("should not fail") + ); + assert_ne!( + &bin.data(), + decrypted.data().expect("should not fail").value() + ); let _decrypted_hdrv1: SeHdrDataV1 = decrypted.data.try_into().expect("BUG"); } diff --git a/rust/pvimg/src/pv_utils/se_hdr/hdr_v1.rs b/rust/pvimg/src/pv_utils/se_hdr/hdr_v1.rs index a7f2f609..b179d50d 100644 --- a/rust/pvimg/src/pv_utils/se_hdr/hdr_v1.rs +++ b/rust/pvimg/src/pv_utils/se_hdr/hdr_v1.rs @@ -19,6 +19,7 @@ use serde::{Serialize, Serializer}; use super::keys::phkh_v1; use crate::{ error::Error, + misc::PAGESIZE, pv_utils::{ error::Result, se_hdr::{ @@ -51,11 +52,14 @@ struct HdrSizesV1 { #[derive(Debug, Clone, PartialEq, Eq, DekuRead, DekuWrite, Serialize)] #[deku(endian = "endian", ctx = "endian: Endian", ctx_default = "Endian::Big")] struct SeHdrAadV1 { + #[deku(assert = "*sehs <= SeHdrDataV1::MAX_SIZE.try_into().unwrap()")] sehs: u32, #[serde(serialize_with = "ser_hex")] iv: [u8; SymKeyType::AES_256_GCM_IV_LEN], res1: u32, + #[deku(assert = "*nks <= (*sehs).into()", update = "self.keyslots.len()")] nks: u64, + #[deku(assert = "*sea <= (*sehs).into()")] sea: u64, nep: u64, #[serde(serialize_with = "ser_lower_hex")] @@ -118,6 +122,7 @@ pub struct SeHdrConfV1 { psw: PSW, #[serde(serialize_with = "ser_lower_hex")] scf: u64, + #[deku(assert_eq = "0")] noi: u32, res2: u32, #[deku(count = "noi")] @@ -200,6 +205,7 @@ where } impl SeHdrDataV1 { + const MAX_SIZE: usize = 2 * PAGESIZE; const PCF_DEFAULT: u64 = 0x0; const SCF_DEFAULT: u64 = 0x0; @@ -241,7 +247,14 @@ impl SeHdrDataV1 { tag: SeHdrTagV1::default(), }; let hdr_size = ret.size()?; - ret.aad.sehs = hdr_size.phs.try_into()?; + let phs = hdr_size.phs.try_into()?; + if phs > Self::MAX_SIZE { + return Err(Error::InvalidSeHdrTooLarge { + given: phs, + maximum: Self::MAX_SIZE, + }); + } + ret.aad.sehs = phs.try_into()?; ret.aad.sea = hdr_size.sea; Ok(ret) } @@ -494,8 +507,8 @@ impl KeyExchangeTrait for SeHdrBinV1 { } impl AeadDataTrait for SeHdrBinV1 { - fn aad(&self) -> Vec { - serialize_to_bytes(&self.aad).unwrap() + fn aad(&self) -> Result> { + serialize_to_bytes(&self.aad) } fn data(&self) -> Vec { @@ -508,12 +521,12 @@ impl AeadDataTrait for SeHdrBinV1 { } impl AeadPlainDataTrait for SeHdrDataV1 { - fn aad(&self) -> Vec { - serialize_to_bytes(&self.aad).unwrap() + fn aad(&self) -> Result> { + serialize_to_bytes(&self.aad) } - fn data(&self) -> Confidential> { - serialize_to_bytes(self.data.value()).unwrap().into() + fn data(&self) -> Result>> { + Ok(serialize_to_bytes(self.data.value())?.into()) } fn tag(&self) -> Vec { @@ -610,4 +623,48 @@ mod tests { assert_eq!(psw, hdr_data_v1.data.value().psw); assert_eq!(cck.value(), hdr_data_v1.data.value().cck.value()); } + + #[test] + fn max_size_sehdr_test() { + const MAX_HOST_KEYS: usize = 95; + + let (_, host_key) = get_test_key_and_cert(); + let pub_key = host_key.public_key().unwrap(); + let host_keys_max: Vec<_> = (0..MAX_HOST_KEYS).map(|_| pub_key.clone()).collect(); + let too_many_host_keys: Vec<_> = (0..MAX_HOST_KEYS + 1).map(|_| pub_key.clone()).collect(); + let xts_key = Confidential::new([0x3; SymKeyType::AES_256_XTS_KEY_LEN]); + let meta = ComponentMetadataV1 { + ald: [0x1; SHA_512_HASH_LEN], + pld: [0x2; SHA_512_HASH_LEN], + tld: [0x3; SHA_512_HASH_LEN], + nep: 3, + key: xts_key, + }; + let psw = PSW { + addr: 1234, + mask: 5678, + }; + + let mut builder = SeHdrBuilder::new(SeHdrVersion::V1, psw.clone(), meta.clone()) + .expect("should not fail"); + builder + .add_hostkeys(&host_keys_max) + .expect("should not fail") + .with_components(meta.clone()) + .expect("should not fail"); + let bin = builder.build().expect("should not fail"); + assert_eq!(bin.common.version, SeHdrVersion::V1); + let hdr_v1: SeHdrBinV1 = bin.data.try_into().expect("should not fail"); + assert_eq!(hdr_v1.aad.sehs, 8160); + + let mut builder = SeHdrBuilder::new(SeHdrVersion::V1, psw.clone(), meta.clone()) + .expect("should not fail"); + + builder + .add_hostkeys(&too_many_host_keys) + .expect("should not fail") + .with_components(meta) + .expect("should not fail"); + assert!(matches!(builder.build(), Err(Error::InvalidSeHdr))); + } } diff --git a/rust/pvimg/src/pv_utils/uvdata.rs b/rust/pvimg/src/pv_utils/uvdata.rs index b0ec355a..c6ed9567 100644 --- a/rust/pvimg/src/pv_utils/uvdata.rs +++ b/rust/pvimg/src/pv_utils/uvdata.rs @@ -34,7 +34,7 @@ pub trait AeadCipherTrait { #[enum_dispatch] pub trait AeadDataTrait { /// Returns the authenticated associated data. - fn aad(&self) -> Vec; + fn aad(&self) -> Result>; /// Returns the encrypted data. fn data(&self) -> Vec; @@ -47,10 +47,10 @@ pub trait AeadDataTrait { #[enum_dispatch] pub trait AeadPlainDataTrait { /// Returns the authenticated associated data. - fn aad(&self) -> Vec; + fn aad(&self) -> Result>; /// Returns the unencrypted data. - fn data(&self) -> Confidential>; + fn data(&self) -> Result>>; /// Returns the tag data. fn tag(&self) -> Vec; @@ -124,8 +124,14 @@ pub trait UvDataPlainTrait: expected: self.aead_key_type().to_string(), }); } - let aad = self.aad(); - let unecrypted_data = self.data(); + let aad = self.aad().map_err(|err| match err { + Error::Deku(_) => Error::InvalidSeHdr, + err => err, + })?; + let unecrypted_data = self.data().map_err(|err| match err { + Error::Deku(_) => Error::InvalidSeHdr, + err => err, + })?; let iv = self.iv(); let result = encrypt_aead(key, iv, &aad, unecrypted_data.value())?; Self::C::try_from_data(&result.into_buf()) @@ -169,7 +175,7 @@ pub trait UvDataTrait: AeadDataTrait + AeadCipherTrait + KeyExchangeTrait + Clon } let tag_size = self.aead_tag_size(); - let aad = self.aad(); + let aad = self.aad()?; let unecrypted_data = self.data(); let iv = self.iv(); let tag = self.tag();