From 309f891a24ab569ac4469e97770045b372656ac4 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Wed, 25 Dec 2024 02:28:37 +0000 Subject: [PATCH] Fix panic if the quote doesn't contains pck cert chain --- src/collateral.rs | 28 ++++++++------- src/lib.rs | 35 ------------------- src/quote.rs | 36 +++++++++++-------- src/utils.rs | 89 ++++++++++++++++++++++++----------------------- src/verify.rs | 48 ++++++++++++++----------- 5 files changed, 109 insertions(+), 127 deletions(-) diff --git a/src/collateral.rs b/src/collateral.rs index ac31d38..f43ad92 100644 --- a/src/collateral.rs +++ b/src/collateral.rs @@ -1,5 +1,5 @@ use alloc::string::{String, ToString}; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use scale::Decode; use crate::quote::Quote; @@ -12,7 +12,7 @@ fn get_header(resposne: &reqwest::Response, name: &str) -> Result { let value = resposne .headers() .get(name) - .ok_or(anyhow!("Missing {name}"))? + .ok_or_else(|| anyhow!("Missing {name}"))? .to_str()?; let value = urlencoding::decode(value)?; Ok(value.into_owned()) @@ -36,7 +36,7 @@ pub async fn get_collateral( #[cfg(not(feature = "js"))] timeout: Duration, ) -> Result { let quote = Quote::decode(&mut quote)?; - let fmspc = hex::encode_upper(quote.fmspc().map_err(|_| anyhow!("get fmspc error"))?); + let fmspc = hex::encode_upper(quote.fmspc().context("Failed to get FMSPC")?); let builder = reqwest::Client::builder(); #[cfg(not(feature = "js"))] let builder = builder.danger_accept_invalid_certs(true).timeout(timeout); @@ -63,29 +63,31 @@ pub async fn get_collateral( }; let tcb_info_json: serde_json::Value = - serde_json::from_str(&raw_tcb_info).map_err(|_| anyhow!("TCB Info should a JSON"))?; + serde_json::from_str(&raw_tcb_info).context("TCB Info should be valid JSON")?; let tcb_info = tcb_info_json["tcbInfo"].to_string(); let tcb_info_signature = tcb_info_json .get("signature") - .ok_or(anyhow!("TCB Info should has `signature` field"))? + .context("TCB Info missing 'signature' field")? .as_str() - .ok_or(anyhow!("TCB Info signature should a hex string"))?; + .context("TCB Info signature must be a string")?; let tcb_info_signature = hex::decode(tcb_info_signature) - .map_err(|_| anyhow!("TCB Info signature should a hex string"))?; + .ok() + .context("TCB Info signature must be valid hex")?; - let qe_identity_json: serde_json::Value = serde_json::from_str(raw_qe_identity.as_str()) - .map_err(|_| anyhow!("QE Identity should a JSON"))?; + let qe_identity_json: serde_json::Value = + serde_json::from_str(&raw_qe_identity).context("QE Identity should be valid JSON")?; let qe_identity = qe_identity_json .get("enclaveIdentity") - .ok_or(anyhow!("QE Identity should has `enclaveIdentity` field"))? + .context("QE Identity missing 'enclaveIdentity' field")? .to_string(); let qe_identity_signature = qe_identity_json .get("signature") - .ok_or(anyhow!("QE Identity should has `signature` field"))? + .context("QE Identity missing 'signature' field")? .as_str() - .ok_or(anyhow!("QE Identity signature should a hex string"))?; + .context("QE Identity signature must be a string")?; let qe_identity_signature = hex::decode(qe_identity_signature) - .map_err(|_| anyhow!("QE Identity signature should a hex string"))?; + .ok() + .context("QE Identity signature must be valid hex")?; Ok(QuoteCollateralV3 { tcb_info_issuer_chain, diff --git a/src/lib.rs b/src/lib.rs index 96df7b7..8722757 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,43 +39,8 @@ extern crate alloc; use scale::{Decode, Encode}; -use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; -#[derive(Encode, Decode, TypeInfo, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum Error { - InvalidCertificate, - InvalidSignature, - CodecError, - - // DCAP - TCBInfoExpired, - KeyLengthIsInvalid, - PublicKeyIsInvalid, - RsaSignatureIsInvalid, - DerEncodingError, - UnsupportedDCAPQuoteVersion, - UnsupportedDCAPAttestationKeyType, - UnsupportedQuoteAuthData, - UnsupportedDCAPPckCertFormat, - LeafCertificateParsingError, - CertificateChainIsInvalid, - CertificateChainIsTooShort, - IntelExtensionCertificateDecodingError, - IntelExtensionAmbiguity, - CpuSvnLengthMismatch, - CpuSvnDecodingError, - PceSvnDecodingError, - PceSvnLengthMismatch, - FmspcLengthMismatch, - FmspcDecodingError, - FmspcMismatch, - QEReportHashMismatch, - IsvEnclaveReportSignatureIsInvalid, - DerDecodingError, - OidIsMissing, -} - #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] pub struct QuoteCollateralV3 { pub tcb_info_issuer_chain: String, diff --git a/src/quote.rs b/src/quote.rs index 48d348e..c2441bd 100644 --- a/src/quote.rs +++ b/src/quote.rs @@ -1,11 +1,11 @@ use alloc::string::String; use alloc::vec::Vec; -use anyhow::Result; +use anyhow::{anyhow, bail, Context, Result}; use scale::{Decode, Input}; use serde::{Deserialize, Serialize}; -use crate::{constants::*, utils, Error}; +use crate::{constants::*, utils}; #[derive(Debug, Clone)] pub struct Data { @@ -232,7 +232,7 @@ fn decode_auth_data(ver: u16, input: &mut &[u8]) -> Result Err(scale::Error::from("unsupported auth data version")), + _ => Err(scale::Error::from("Unsupported auth data version")), } } @@ -296,7 +296,7 @@ impl Decode for Quote { TEE_TYPE_TDX => { report = Report::TD10(TDReport10::decode(input)?); } - _ => return Err(scale::Error::from("invalid tee type")), + _ => return Err(scale::Error::from("Invalid TEE type")), }, 5 => { let body = Body::decode(input)?; @@ -310,10 +310,10 @@ impl Decode for Quote { BODY_TD_REPORT15_TYPE => { report = Report::TD15(TDReport15::decode(input)?); } - _ => return Err(scale::Error::from("unsupported body type")), + _ => return Err(scale::Error::from("Unsupported body type")), } } - _ => return Err(scale::Error::from("unsupported quote version")), + _ => return Err(scale::Error::from("Unsupported quote version")), } let data = Data::::decode(input)?; let auth_data = decode_auth_data(header.version, &mut &data.data[..])?; @@ -334,18 +334,26 @@ impl Quote { } /// Get the raw certificate chain from the quote. - pub fn raw_cert_chain(&self) -> &[u8] { - match &self.auth_data { - AuthData::V3(data) => &data.certification_data.body.data, - AuthData::V4(data) => &data.qe_report_data.certification_data.body.data, + pub fn raw_cert_chain(&self) -> Result<&[u8]> { + let cert_data = match &self.auth_data { + AuthData::V3(data) => &data.certification_data, + AuthData::V4(data) => &data.qe_report_data.certification_data, + }; + if cert_data.cert_type != 5 { + bail!("Unsupported cert type: {}", cert_data.cert_type); } + Ok(&cert_data.body.data) } /// Get the FMSPC from the quote. - pub fn fmspc(&self) -> Result { - let raw_cert_chain = self.raw_cert_chain(); - let certs = utils::extract_certs(raw_cert_chain)?; - let extension_section = utils::get_intel_extension(&certs[0])?; + pub fn fmspc(&self) -> Result { + let raw_cert_chain = self + .raw_cert_chain() + .context("Failed to get raw cert chain")?; + let certs = utils::extract_certs(raw_cert_chain).context("Failed to extract certs")?; + let cert = certs.get(0).ok_or(anyhow!("Invalid certificate"))?; + let extension_section = + utils::get_intel_extension(cert).context("Failed to get Intel extension")?; utils::get_fmspc(&extension_section) } diff --git a/src/utils.rs b/src/utils.rs index 72a08b0..9ae9068 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,5 @@ use alloc::vec::Vec; +use anyhow::{anyhow, bail, Context, Result}; use asn1_der::{ typed::{DerDecodable, Sequence}, DerObject, @@ -7,11 +8,10 @@ use webpki::types::CertificateDer; use x509_cert::Certificate; use crate::constants::*; -use crate::Error; -pub fn get_intel_extension(der_encoded: &[u8]) -> Result, Error> { - let cert: Certificate = der::Decode::from_der(der_encoded) - .map_err(|_| Error::IntelExtensionCertificateDecodingError)?; +pub fn get_intel_extension(der_encoded: &[u8]) -> Result> { + let cert: Certificate = + der::Decode::from_der(der_encoded).context("Failed to decode certificate")?; let mut extension_iter = cert .tbs_certificate .extensions @@ -21,88 +21,89 @@ pub fn get_intel_extension(der_encoded: &[u8]) -> Result, Error> { .filter(|e| e.extn_id == oids::SGX_EXTENSION) .map(|e| e.extn_value.clone()); - let extension = extension_iter - .next() - .ok_or(Error::IntelExtensionAmbiguity)?; + let extension = extension_iter.next().context("Intel extension not found")?; if extension_iter.next().is_some() { //"There should only be one section containing Intel extensions" - return Err(Error::IntelExtensionAmbiguity); + bail!("Intel extension ambiguity"); } Ok(extension.into_bytes()) } -pub fn find_extension(path: &[&[u8]], raw: &[u8]) -> Result, Error> { - let obj = DerObject::decode(raw).map_err(|_| Error::DerDecodingError)?; - let subobj = get_obj(path, obj)?; +pub fn find_extension(path: &[&[u8]], raw: &[u8]) -> Result> { + let obj = DerObject::decode(raw).context("Failed to decode DER object")?; + let subobj = get_obj(path, obj).context("Failed to get subobject")?; Ok(subobj.value().to_vec()) } -fn get_obj<'a>(path: &[&[u8]], mut obj: DerObject<'a>) -> Result, Error> { +fn get_obj<'a>(path: &[&[u8]], mut obj: DerObject<'a>) -> Result> { for oid in path { - let seq = Sequence::load(obj).map_err(|_| Error::DerDecodingError)?; - obj = sub_obj(oid, seq)?; + let seq = Sequence::load(obj).context("Failed to load sequence")?; + obj = sub_obj(oid, seq).context("Failed to get subobject")?; } Ok(obj) } -fn sub_obj<'a>(oid: &[u8], seq: Sequence<'a>) -> Result, Error> { +fn sub_obj<'a>(oid: &[u8], seq: Sequence<'a>) -> Result> { for i in 0..seq.len() { - let entry = seq.get(i).map_err(|_| Error::OidIsMissing)?; - let entry = Sequence::load(entry).map_err(|_| Error::DerDecodingError)?; - let name = entry.get(0).map_err(|_| Error::OidIsMissing)?; - let value = entry.get(1).map_err(|_| Error::OidIsMissing)?; + let entry = seq.get(i).context("Failed to get entry")?; + let entry = Sequence::load(entry).context("Failed to load sequence")?; + let name = entry.get(0).context("Failed to get name")?; + let value = entry.get(1).context("Failed to get value")?; if name.value() == oid { return Ok(value); } } - Err(Error::OidIsMissing) + bail!("Oid is missing"); } -pub fn get_fmspc(extension_section: &[u8]) -> Result { - let data = find_extension(&[oids::FMSPC.as_bytes()], extension_section)?; +pub fn get_fmspc(extension_section: &[u8]) -> Result { + let data = find_extension(&[oids::FMSPC.as_bytes()], extension_section) + .context("Failed to find Fmspc")?; if data.len() != 6 { - return Err(Error::FmspcLengthMismatch); + bail!("Fmspc length mismatch"); } - data.try_into().map_err(|_| Error::FmspcDecodingError) + data.try_into() + .map_err(|_| anyhow!("Failed to decode Fmspc")) } -pub fn get_cpu_svn(extension_section: &[u8]) -> Result { +pub fn get_cpu_svn(extension_section: &[u8]) -> Result { let data = find_extension( &[oids::TCB.as_bytes(), oids::CPUSVN.as_bytes()], extension_section, )?; if data.len() != 16 { - return Err(Error::CpuSvnLengthMismatch); + bail!("CpuSvn length mismatch"); } - data.try_into().map_err(|_| Error::CpuSvnDecodingError) + data.try_into().map_err(|_| anyhow!("Failed to decode CpuSvn")) } -pub fn get_pce_svn(extension_section: &[u8]) -> Result { +pub fn get_pce_svn(extension_section: &[u8]) -> Result { let data = find_extension( &[oids::TCB.as_bytes(), oids::PCESVN.as_bytes()], extension_section, - )?; + ) + .context("Failed to find PceSvn")?; match data.len() { 1 => Ok(u16::from(data[0])), 2 => Ok(u16::from_be_bytes( - data.try_into().map_err(|_| Error::PceSvnDecodingError)?, + data.try_into().map_err(|_| anyhow!("Failed to decode PceSvn"))?, )), - _ => Err(Error::PceSvnLengthMismatch), + _ => bail!("PceSvn length mismatch"), } } -pub fn extract_raw_certs(cert_chain: &[u8]) -> Result>, Error> { +pub fn extract_raw_certs(cert_chain: &[u8]) -> Result>> { Ok(pem::parse_many(cert_chain) - .map_err(|_| Error::CodecError)? + .context("Failed to parse certs")? .iter() .map(|i| i.contents().to_vec()) .collect()) } -pub fn extract_certs<'a>(cert_chain: &'a [u8]) -> Result>, Error> { +pub fn extract_certs<'a>(cert_chain: &'a [u8]) -> Result>> { let mut certs = Vec::>::new(); let raw_certs = extract_raw_certs(cert_chain)?; @@ -117,26 +118,26 @@ pub fn extract_certs<'a>(cert_chain: &'a [u8]) -> Result> /// Encode two 32-byte values in DER format /// This is meant for 256 bit ECC signatures or public keys /// TODO: We may could use `asn1_der` crate reimplement this, so we can remove `der` which overlap with `asn1_der` -pub fn encode_as_der(data: &[u8]) -> Result, Error> { +pub fn encode_as_der(data: &[u8]) -> Result> { if data.len() != 64 { - return Err(Error::KeyLengthIsInvalid); + bail!("Key length is invalid"); } let mut sequence = der::asn1::SequenceOf::::new(); sequence - .add(der::asn1::UintRef::new(&data[0..32]).map_err(|_| Error::PublicKeyIsInvalid)?) - .map_err(|_| Error::PublicKeyIsInvalid)?; + .add(der::asn1::UintRef::new(&data[0..32]).context("Failed to add first element")?) + .context("Failed to add second element")?; sequence - .add(der::asn1::UintRef::new(&data[32..]).map_err(|_| Error::PublicKeyIsInvalid)?) - .map_err(|_| Error::PublicKeyIsInvalid)?; + .add(der::asn1::UintRef::new(&data[32..]).context("Failed to add third element")?) + .context("Failed to add third element")?; // 72 should be enough in all cases. 2 + 2 x (32 + 3) let mut asn1 = alloc::vec![0u8; 72]; let mut writer = der::SliceWriter::new(&mut asn1); writer .encode(&sequence) - .map_err(|_| Error::DerEncodingError)?; + .context("Failed to encode sequence")?; Ok(writer .finish() - .map_err(|_| Error::DerEncodingError)? + .context("Failed to finish writer")? .to_vec()) } @@ -146,7 +147,7 @@ pub fn verify_certificate_chain( leaf_cert: &webpki::EndEntityCert, intermediate_certs: &[CertificateDer], verification_time: u64, -) -> Result<(), Error> { +) -> Result<()> { let time = webpki::types::UnixTime::since_unix_epoch(core::time::Duration::from_secs( verification_time / 1000, )); @@ -161,7 +162,7 @@ pub fn verify_certificate_chain( None, None, ) - .map_err(|_e| Error::CertificateChainIsInvalid)?; + .context("Failed to verify certificate chain")?; Ok(()) } diff --git a/src/verify.rs b/src/verify.rs index 237e5b7..4416982 100644 --- a/src/verify.rs +++ b/src/verify.rs @@ -1,3 +1,4 @@ +use anyhow::{anyhow, bail, Context, Result}; use scale::Decode; use { @@ -6,11 +7,11 @@ use { }; pub use crate::quote::{AuthData, EnclaveReport, Quote}; +use crate::QuoteCollateralV3; use crate::{ quote::Report, utils::{self, encode_as_der, extract_certs, verify_certificate_chain}, }; -use crate::{Error, QuoteCollateralV3}; use serde::{Deserialize, Serialize}; #[cfg(feature = "js")] @@ -62,19 +63,20 @@ pub fn verify( raw_quote: &[u8], quote_collateral: &QuoteCollateralV3, now: u64, -) -> Result { +) -> Result { // Parse data let mut quote = raw_quote; - let quote = Quote::decode(&mut quote).map_err(|_| Error::CodecError)?; + let quote = Quote::decode(&mut quote).context("Failed to decode quote")?; let signed_quote_len = quote.signed_length(); let tcb_info = serde_json::from_str::("e_collateral.tcb_info) - .map_err(|_| Error::CodecError)?; + .context("Failed to decode TcbInfo")?; let next_update = chrono::DateTime::parse_from_rfc3339(&tcb_info.next_update) - .map_err(|_| Error::CodecError)?; + .ok() + .context("Failed to parse next update")?; if now > next_update.timestamp() as u64 { - return Err(Error::TCBInfoExpired); + bail!("TCBInfo expired"); } let now_in_milli = now * 1000; @@ -90,10 +92,10 @@ pub fn verify( // Check TCB info cert chain and signature let leaf_certs = extract_certs(quote_collateral.tcb_info_issuer_chain.as_bytes())?; if leaf_certs.len() < 2 { - return Err(Error::CertificateChainIsTooShort); + bail!("Certificate chain is too short"); } let leaf_cert: webpki::EndEntityCert = webpki::EndEntityCert::try_from(&leaf_certs[0]) - .map_err(|_| Error::LeafCertificateParsingError)?; + .context("Failed to parse leaf certificate")?; let intermediate_certs = &leaf_certs[1..]; verify_certificate_chain(&leaf_cert, intermediate_certs, now_in_milli)?; let asn1_signature = encode_as_der("e_collateral.tcb_info_signature)?; @@ -105,16 +107,16 @@ pub fn verify( ) .is_err() { - return Err(Error::RsaSignatureIsInvalid); + return Err(anyhow!("Rsa signature is invalid")); } // Check quote fields if ![3, 4, 5].contains("e.header.version) { - return Err(Error::UnsupportedDCAPQuoteVersion); + return Err(anyhow!("Unsupported DCAP quote version")); } // We only support ECDSA256 with P256 curve if quote.header.attestation_key_type != ATTESTATION_KEY_TYPE_ECDSA256_WITH_P256_CURVE { - return Err(Error::UnsupportedDCAPAttestationKeyType); + bail!("Unsupported DCAP attestation key type"); } // Extract Auth data from quote @@ -123,16 +125,16 @@ pub fn verify( // We only support 5 -Concatenated PCK Cert Chain (PEM formatted). if certification_data.cert_type != PCK_CERT_CHAIN { - return Err(Error::UnsupportedDCAPPckCertFormat); + bail!("Unsupported DCAP PCK cert format"); } let certification_certs = extract_certs(&certification_data.body.data)?; if certification_certs.len() < 2 { - return Err(Error::CertificateChainIsTooShort); + bail!("Certificate chain is too short"); } // Check certification_data let leaf_cert: webpki::EndEntityCert = webpki::EndEntityCert::try_from(&certification_certs[0]) - .map_err(|_| Error::LeafCertificateParsingError)?; + .context("Failed to parse leaf certificate")?; let intermediate_certs = &certification_certs[1..]; verify_certificate_chain(&leaf_cert, intermediate_certs, now_in_milli)?; @@ -146,12 +148,12 @@ pub fn verify( ) .is_err() { - return Err(Error::RsaSignatureIsInvalid); + return Err(anyhow!("Rsa signature is invalid")); } // Extract QE report from quote let mut qe_report = auth_data.qe_report.as_slice(); - let qe_report = EnclaveReport::decode(&mut qe_report).map_err(|_err| Error::CodecError)?; + let qe_report = EnclaveReport::decode(&mut qe_report).context("Failed to decode QE report")?; // Check QE hash let mut qe_hash_data = [0u8; QE_HASH_DATA_BYTE_LEN]; @@ -159,7 +161,7 @@ pub fn verify( qe_hash_data[ATTESTATION_KEY_LEN..].copy_from_slice(&auth_data.qe_auth_data.data); let qe_hash = ring::digest::digest(&ring::digest::SHA256, &qe_hash_data); if qe_hash.as_ref() != &qe_report.report_data[0..32] { - return Err(Error::QEReportHashMismatch); + bail!("QE report hash mismatch"); } // Check signature from auth data @@ -169,10 +171,12 @@ pub fn verify( ring::signature::UnparsedPublicKey::new(&ring::signature::ECDSA_P256_SHA256_FIXED, pub_key); peer_public_key .verify( - &raw_quote.get(..signed_quote_len).ok_or(Error::CodecError)?, + &raw_quote + .get(..signed_quote_len) + .ok_or(anyhow!("Failed to get signed quote"))?, &auth_data.ecdsa_signature, ) - .map_err(|_| Error::IsvEnclaveReportSignatureIsInvalid)?; + .map_err(|_| anyhow!("Isv enclave report signature is invalid"))?; // Extract information from the quote @@ -181,9 +185,11 @@ pub fn verify( let pce_svn = utils::get_pce_svn(&extension_section)?; let fmspc = utils::get_fmspc(&extension_section)?; - let tcb_fmspc = hex::decode(&tcb_info.fmspc).map_err(|_| Error::CodecError)?; + let tcb_fmspc = hex::decode(&tcb_info.fmspc) + .ok() + .context("Failed to decode TCB FMSPC")?; if fmspc != tcb_fmspc[..] { - return Err(Error::FmspcMismatch); + bail!("Fmspc mismatch"); } // TCB status and advisory ids