From 6755fe981d6fcd1d431b7a633b099f96217f8ffd Mon Sep 17 00:00:00 2001 From: Haitao Huang Date: Fri, 27 Feb 2026 10:55:12 -0800 Subject: [PATCH] MigTD: handle target TD without SERVTD_EXT On platforms with rebind support but no support or TD opts out for SERVTD_EXT, TDG.servtd.rd on the SERVTD_EXT fields in TDCS of a target TD would return zeros. This change reads TDCS.ATTRIBUTES to check bit 17 and makes SERVTD_EXT optional throughout the rebinding flow: - read_servtd_ext() reads TDCS.ATTRIBUTES via tdcall_servtd_rd and returns None when SERVTDEXT bit is not set - write_approved_servtd_ext_hash() accepts Option and is a no-op when None - Certificate generation/verification: servtd_ext extension is conditionally included and tolerated when missing - Policy verification (authenticate_rebinding_old) skips init report verification entirely when servtd_ext is unavailable, since the init TD report cannot be verified without servtd_info_hash - SPDM VDM messages send zero-length servtd_ext element when not available; receiver handles it gracefully - All TDCS write operations (write_servtd_rebind_attr, write_approved_servtd_ext_hash) are skipped when servtd_ext is None Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang --- src/migtd/src/mig_policy.rs | 61 ++++----- src/migtd/src/migration/rebinding.rs | 31 +++-- src/migtd/src/migration/servtd_ext.rs | 68 ++++++++--- src/migtd/src/ratls/server_client.rs | 170 +++++++++++++------------- src/migtd/src/spdm/spdm_req.rs | 14 ++- src/migtd/src/spdm/spdm_rsp.rs | 30 +++-- 6 files changed, 217 insertions(+), 157 deletions(-) diff --git a/src/migtd/src/mig_policy.rs b/src/migtd/src/mig_policy.rs index fd6dd7ed..4ca144f9 100644 --- a/src/migtd/src/mig_policy.rs +++ b/src/migtd/src/mig_policy.rs @@ -270,7 +270,7 @@ mod v2 { init_policy: &[u8], init_event_log: &[u8], init_td_report: &[u8], - servtd_ext_src: &[u8], + servtd_ext_src: Option<&[u8]>, ) -> Result, PolicyError> { let policy_issuer_chain = get_policy_issuer_chain().ok_or(PolicyError::InvalidParameter)?; @@ -284,34 +284,39 @@ mod v2 { )?; let policy = get_verified_policy().ok_or(PolicyError::InvalidParameter)?; - // Verify the td report init / event log init / policy init - let servtd_ext_src_obj = - ServtdExt::read_from_bytes(servtd_ext_src).ok_or(PolicyError::InvalidParameter)?; - let init_tdreport = verify_init_tdreport(init_td_report, &servtd_ext_src_obj)?; - let _engine_svn = policy - .servtd_tcb_mapping - .get_engine_svn_by_measurements(&Measurements::new_from_bytes( - &init_tdreport.td_info.mrtd, - &init_tdreport.td_info.rtmr0, - &init_tdreport.td_info.rtmr1, - None, - None, - )) - .ok_or(PolicyError::SvnMismatch)?; - let verified_policy_init = verify_policy_and_event_log( - init_event_log, - init_policy, - policy_issuer_chain, - &get_rtmrs_from_tdreport(&init_tdreport)?, - )?; + // Verify the init TD report only when servtd_ext is available. + // Without servtd_ext (target TD ATTRIBUTES.SERVTDEXT=0), we cannot verify + // the init TD report's servtd_info_hash, so skip init report verification + // entirely and only rely on the current peer TD report verification above. + if let Some(ext_bytes) = servtd_ext_src { + let servtd_ext_src_obj = + ServtdExt::read_from_bytes(ext_bytes).ok_or(PolicyError::InvalidParameter)?; + let init_tdreport = verify_init_tdreport(init_td_report, &servtd_ext_src_obj)?; + let _engine_svn = policy + .servtd_tcb_mapping + .get_engine_svn_by_measurements(&Measurements::new_from_bytes( + &init_tdreport.td_info.mrtd, + &init_tdreport.td_info.rtmr0, + &init_tdreport.td_info.rtmr1, + None, + None, + )) + .ok_or(PolicyError::SvnMismatch)?; + let verified_policy_init = verify_policy_and_event_log( + init_event_log, + init_policy, + policy_issuer_chain, + &get_rtmrs_from_tdreport(&init_tdreport)?, + )?; - let relative_reference = - get_init_tcb_evaluation_info(&init_tdreport, &verified_policy_init)?; - policy.policy_data.evaluate_policy_common( - &evaluation_data_src, - &relative_reference, - true, - )?; + let relative_reference = + get_init_tcb_evaluation_info(&init_tdreport, &verified_policy_init)?; + policy.policy_data.evaluate_policy_common( + &evaluation_data_src, + &relative_reference, + true, + )?; + } // If backward policy exists, evaluate the migration src based on it. let relative_reference = get_local_tcb_evaluation_info()?; diff --git a/src/migtd/src/migration/rebinding.rs b/src/migtd/src/migration/rebinding.rs index f9d28ed9..cd729d05 100644 --- a/src/migtd/src/migration/rebinding.rs +++ b/src/migtd/src/migration/rebinding.rs @@ -604,7 +604,7 @@ async fn rebinding_old_prepare( data: &mut Vec, remote_policy: Vec, ) -> Result<(), MigrationResult> { - let servtd_ext = read_servtd_ext(info.binding_handle, &info.target_td_uuid)?; + let servtd_ext = read_servtd_ext(info.binding_handle, &info.target_td_uuid); let init_policy_hash = digest_sha384(&init_migtd_data.init_policy)?; // TLS client @@ -614,7 +614,7 @@ async fn rebinding_old_prepare( &init_policy_hash, &init_migtd_data.init_report, &init_migtd_data.init_event_log, - &servtd_ext, + servtd_ext.as_ref(), ) .map_err(|_| { #[cfg(feature = "vmcall-raw")] @@ -677,8 +677,15 @@ async fn rebinding_new_prepare( // The TLS session is established; we can now extract servtd_ext from the peer certificates. let servtd_ext = get_servtd_ext_from_cert(&ratls_server.peer_certs())?; write_rebinding_session_token(&rebind_token.token)?; - write_servtd_rebind_attr(&servtd_ext.cur_servtd_attr)?; - write_approved_servtd_ext_hash(&servtd_ext.calculate_approved_servtd_ext_hash()?)?; + if let Some(ext) = &servtd_ext { + write_servtd_rebind_attr(&ext.cur_servtd_attr)?; + } + write_approved_servtd_ext_hash( + servtd_ext + .map(|ext| ext.calculate_approved_servtd_ext_hash()) + .transpose()? + .as_deref(), + )?; shutdown_transport(ratls_server.transport_mut(), info.mig_request_id).await?; Ok(()) @@ -689,7 +696,7 @@ async fn rebinding_new_finalize( _data: &mut Vec, ) -> Result<(), MigrationResult> { write_rebinding_session_token(&[0u8; 32])?; - write_approved_servtd_ext_hash(&[0u8; SHA384_DIGEST_SIZE])?; + write_approved_servtd_ext_hash(Some(&[0u8; SHA384_DIGEST_SIZE]))?; Ok(()) } @@ -733,7 +740,9 @@ pub fn approve_rebinding( Ok(()) } -fn get_servtd_ext_from_cert(certs: &Option>) -> Result { +fn get_servtd_ext_from_cert( + certs: &Option>, +) -> Result, MigrationResult> { if let Some(cert_chain) = certs { if cert_chain.is_empty() { return Err(MigrationResult::SecureSessionError); @@ -748,10 +757,14 @@ fn get_servtd_ext_from_cert(certs: &Option>) -> Result bytes, + None => return Ok(None), + }; - ServtdExt::read_from_bytes(servtd_ext).ok_or(MigrationResult::InvalidParameter) + ServtdExt::read_from_bytes(servtd_ext) + .ok_or(MigrationResult::InvalidParameter) + .map(Some) } else { Err(MigrationResult::SecureSessionError) } diff --git a/src/migtd/src/migration/servtd_ext.rs b/src/migtd/src/migration/servtd_ext.rs index c8325163..d24ebcc9 100644 --- a/src/migtd/src/migration/servtd_ext.rs +++ b/src/migtd/src/migration/servtd_ext.rs @@ -9,6 +9,11 @@ use tdx_tdcall::tdx::{tdcall_servtd_rd, tdcall_vm_write}; use crate::migration::MigrationResult; +/// Target TD’s ATTRIBUTES field in TDCS (readable via TDG.SERVTD.RD) +pub const TDCS_FIELD_ATTRIBUTES: u64 = 0x1110000300000000; +/// Bit 17 of ATTRIBUTES indicates SERVTD_EXT support +const ATTRIBUTES_SERVTDEXT_BIT: u64 = 1 << 17; + /// SERVTD_EXT_STRUCT fields in target TD’s TDCS pub const TDCS_FIELD_SERVTD_INIT_SERVTD_INFO_HASH: u64 = 0x191000030000020E; pub const TDCS_FIELD_SERVTD_INIT_ATTR: u64 = 0x191000030000020D; @@ -75,10 +80,25 @@ pub struct TeeModel { reservtd: [u8; 8], } -pub fn read_servtd_ext( - binding_handle: u64, - target_td_uuid: &[u64], -) -> Result { +/// Try to read ServtdExt from the target TD's TDCS. +/// Returns `None` if the target TD does not support SERVTD_EXT +/// (i.e., TDCS.ATTRIBUTES.SERVTDEXT bit 17 is zero). +pub fn read_servtd_ext(binding_handle: u64, target_td_uuid: &[u64]) -> Option { + // Check TDCS.ATTRIBUTES bit 17 (SERVTDEXT) to determine support. + let attributes = tdcall_servtd_rd(binding_handle, TDCS_FIELD_ATTRIBUTES, target_td_uuid) + .map_err(|e| { + log::error!("Failed to read TDCS.ATTRIBUTES: {e:?}\n"); + e + }) + .ok()?; + if (attributes.content & ATTRIBUTES_SERVTDEXT_BIT) == 0 { + log::info!( + "Target TD does not support SERVTD_EXT (ATTRIBUTES=0x{:x}).\n", + attributes.content + ); + return None; + } + let read_field = |field_base: u64, elem_size: usize, buf: &mut [u8]| -> Result<(), MigrationResult> { for (idx, chunk) in buf.chunks_mut(elem_size).enumerate() { @@ -99,19 +119,24 @@ pub fn read_servtd_ext( let mut cur_servtd_info_hash = [0u8; 48]; let mut cur_servtd_attr = [0u8; 8]; - read_field( + if read_field( TDCS_FIELD_SERVTD_INIT_SERVTD_INFO_HASH, 8, &mut init_servtd_info_hash, - )?; - read_field(TDCS_FIELD_SERVTD_INIT_ATTR, 8, &mut init_attr)?; - read_field(TDCS_FIELD_INIT_CPUSVN, 8, &mut init_cpusvn)?; - read_field(TDCS_FIELD_INIT_TEE_TCB_SVN, 8, &mut init_tee_tcb_svn)?; - read_field(TDCS_FIELD_INIT_TEE_MODEL, 4, &mut init_tee_model)?; - read_field(TDCS_FIELD_SERVTD_INFO_HASH, 8, &mut cur_servtd_info_hash)?; - read_field(TDCS_FIELD_SERVTD_ATTR, 8, &mut cur_servtd_attr)?; - - Ok(ServtdExt { + ) + .is_err() + || read_field(TDCS_FIELD_SERVTD_INIT_ATTR, 8, &mut init_attr).is_err() + || read_field(TDCS_FIELD_INIT_CPUSVN, 8, &mut init_cpusvn).is_err() + || read_field(TDCS_FIELD_INIT_TEE_TCB_SVN, 8, &mut init_tee_tcb_svn).is_err() + || read_field(TDCS_FIELD_INIT_TEE_MODEL, 4, &mut init_tee_model).is_err() + || read_field(TDCS_FIELD_SERVTD_INFO_HASH, 8, &mut cur_servtd_info_hash).is_err() + || read_field(TDCS_FIELD_SERVTD_ATTR, 8, &mut cur_servtd_attr).is_err() + { + log::error!("Failed to read SERVTD_EXT fields.\n"); + return None; + } + + Some(ServtdExt { init_servtd_info_hash, init_attr, init_cpusvn, @@ -124,12 +149,20 @@ pub fn read_servtd_ext( }) } -pub fn write_approved_servtd_ext_hash(servtd_ext_hash: &[u8]) -> Result<(), MigrationResult> { - if servtd_ext_hash.len() != SHA384_DIGEST_SIZE { +/// Write the approved SERVTD_EXT hash to the target TD's TDCS. +/// If `servtd_ext_hash` is `None`, this is a no-op (target TD does not support SERVTD_EXT). +pub fn write_approved_servtd_ext_hash( + servtd_ext_hash: Option<&[u8]>, +) -> Result<(), MigrationResult> { + let hash = match servtd_ext_hash { + Some(h) => h, + None => return Ok(()), + }; + if hash.len() != SHA384_DIGEST_SIZE { return Err(MigrationResult::InvalidParameter); } - for (idx, chunk) in servtd_ext_hash.chunks_exact(size_of::()).enumerate() { + for (idx, chunk) in hash.chunks_exact(size_of::()).enumerate() { let elem = u64::from_le_bytes(chunk.try_into().unwrap()); tdcall_vm_write( TDCS_FIELD_SERVTD_ACCEPT_SERVTD_EXT_HASH + idx as u64, @@ -141,6 +174,7 @@ pub fn write_approved_servtd_ext_hash(servtd_ext_hash: &[u8]) -> Result<(), Migr Ok(()) } +#[cfg(test)] mod test { use super::ServtdExt; diff --git a/src/migtd/src/ratls/server_client.rs b/src/migtd/src/ratls/server_client.rs index b805c167..6d9243bf 100644 --- a/src/migtd/src/ratls/server_client.rs +++ b/src/migtd/src/ratls/server_client.rs @@ -186,7 +186,7 @@ pub fn client_rebinding( init_policy_hash: &[u8], init_td_report: &[u8], init_event_log: &[u8], - servtd_ext: &ServtdExt, + servtd_ext: Option<&ServtdExt>, ) -> Result> { let signing_key = EcdsaPk::new().map_err(|e| { log::error!( @@ -411,7 +411,7 @@ fn create_certificate_for_rebinding_old( init_policy_hash: &[u8], init_tdreport: &[u8], init_event_log: &[u8], - servtd_ext: &ServtdExt, + servtd_ext: Option<&ServtdExt>, ) -> Result> { let pub_key = signing_key.public_key().map_err(|e| { log::error!( @@ -462,111 +462,114 @@ fn create_certificate_for_rebinding_old( e })?; - // If policy_v2 feature is enabled, add policy extension #[cfg(feature = "policy_v2")] - let x509_builder = x509_builder - .add_extension( - Extension::new(EXTNID_MIGTD_POLICY_HASH, Some(false), Some(&policy_hash)).map_err( - |e| { - log::error!( - "gen_cert policy_v2 add_extension failed with error {:?}.\n", + let x509_builder = { + let mut builder = x509_builder + .add_extension( + Extension::new(EXTNID_MIGTD_POLICY_HASH, Some(false), Some(&policy_hash)).map_err( + |e| { + log::error!( + "gen_cert policy_v2 add_extension failed with error {:?}.\n", + e + ); e - ); - e - }, - )?, - ) - .map_err(|e| { - log::error!( - "gen_cert policy_v2 add_extension for policy hash failed with error {:?}.\n", - e - ); - e - })? - .add_extension( - Extension::new( - EXTNID_MIGTD_SERVTD_EXT, - Some(false), - Some(servtd_ext.as_bytes()), + }, + )?, ) .map_err(|e| { log::error!( - "gen_cert policy_v2 add_extension failed with error {:?}.\n", + "gen_cert policy_v2 add_extension for policy hash failed with error {:?}.\n", e ); e - })?, - ) - .map_err(|e| { - log::error!( - "gen_cert policy_v2 add_extension for servtd_ext failed with error {:?}.\n", - e - ); - e - })? - .add_extension( - Extension::new( - EXTNID_MIGTD_TDREPORT_INIT, - Some(false), - Some(&init_tdreport), + })?; + + if let Some(ext) = servtd_ext { + builder = builder + .add_extension( + Extension::new(EXTNID_MIGTD_SERVTD_EXT, Some(false), Some(ext.as_bytes())) + .map_err(|e| { + log::error!( + "gen_cert policy_v2 add_extension failed with error {:?}.\n", + e + ); + e + })?, + ) + .map_err(|e| { + log::error!( + "gen_cert policy_v2 add_extension for servtd_ext failed with error {:?}.\n", + e + ); + e + })?; + } + + builder + .add_extension( + Extension::new( + EXTNID_MIGTD_TDREPORT_INIT, + Some(false), + Some(&init_tdreport), + ) + .map_err(|e| { + log::error!( + "gen_cert policy_v2 add_extension failed with error {:?}.\n", + e + ); + e + })?, ) .map_err(|e| { log::error!( - "gen_cert policy_v2 add_extension failed with error {:?}.\n", + "gen_cert policy_v2 add_extension for tdreport init failed with error {:?}.\n", e ); e - })?, - ) - .map_err(|e| { - log::error!( - "gen_cert policy_v2 add_extension for tdreport init failed with error {:?}.\n", - e - ); - e - })? - .add_extension( - Extension::new( - EXTNID_MIGTD_EVENT_LOG_INIT, - Some(false), - Some(&init_event_log), + })? + .add_extension( + Extension::new( + EXTNID_MIGTD_EVENT_LOG_INIT, + Some(false), + Some(&init_event_log), + ) + .map_err(|e| { + log::error!( + "gen_cert policy_v2 add_extension failed with error {:?}.\n", + e + ); + e + })?, ) .map_err(|e| { log::error!( - "gen_cert policy_v2 add_extension failed with error {:?}.\n", + "gen_cert policy_v2 add_extension for event log init failed with error {:?}.\n", e ); e - })?, - ) - .map_err(|e| { - log::error!( - "gen_cert policy_v2 add_extension for event log init failed with error {:?}.\n", - e - ); - e - })? - .add_extension( - Extension::new( - EXTNID_MIGTD_INIT_POLICY_HASH, - Some(false), - Some(&init_policy_hash), + })? + .add_extension( + Extension::new( + EXTNID_MIGTD_INIT_POLICY_HASH, + Some(false), + Some(&init_policy_hash), + ) + .map_err(|e| { + log::error!( + "gen_cert policy_v2 add_extension failed with error {:?}.\n", + e + ); + e + })?, ) .map_err(|e| { log::error!( - "gen_cert policy_v2 add_extension failed with error {:?}.\n", - e - ); - e - })?, - ) - .map_err(|e| { - log::error!( "gen_cert policy_v2 add_extension for init policy hash failed with error {:?}.\n", e ); - e - })?; + e + })? + }; let x509_cert_der = sign_tls_tbs(x509_builder, &signing_key)?; Ok(x509_cert_der) @@ -1007,10 +1010,7 @@ mod verify { log::error!("Failed to find init policy hash extension.\n"); CryptoError::ParseCertificate })?; - let servtd_ext = find_extension(extensions, &EXTNID_MIGTD_SERVTD_EXT).ok_or_else(|| { - log::error!("Failed to find servtd ext extension.\n"); - CryptoError::ParseCertificate - })?; + let servtd_ext = find_extension(extensions, &EXTNID_MIGTD_SERVTD_EXT); let remote_policy_size = u32::from_le_bytes( pre_session_data diff --git a/src/migtd/src/spdm/spdm_req.rs b/src/migtd/src/spdm/spdm_req.rs index a6fc18d6..b1521ed8 100644 --- a/src/migtd/src/spdm/spdm_req.rs +++ b/src/migtd/src/spdm/spdm_req.rs @@ -883,19 +883,21 @@ pub async fn send_and_receive_sdm_rebind_attest_info( } let init_migtd_data = rebind_info.init_migtd_data.as_ref().unwrap(); - let servtd_ext = read_servtd_ext(binding_handle, target_td_uuid) - .map_err(|_| SPDM_STATUS_INVALID_STATE_LOCAL)?; + let servtd_ext = read_servtd_ext(binding_handle, target_td_uuid); + let servtd_ext_bytes = servtd_ext.as_ref().map(|ext| ext.as_bytes()); let servtd_ext_element = VdmMessageElement { element_type: VdmMessageElementType::SerVtdExt, - length: servtd_ext.as_bytes().len() as u32, + length: servtd_ext_bytes.map_or(0, |b| b.len()) as u32, }; cnt += servtd_ext_element .encode(&mut writer) .map_err(|_| SPDM_STATUS_BUFFER_FULL)?; - cnt += writer - .extend_from_slice(servtd_ext.as_bytes()) - .ok_or(SPDM_STATUS_BUFFER_FULL)?; + if let Some(bytes) = servtd_ext_bytes { + cnt += writer + .extend_from_slice(bytes) + .ok_or(SPDM_STATUS_BUFFER_FULL)?; + } //TD report init let tdreport_init = &init_migtd_data.init_report; diff --git a/src/migtd/src/spdm/spdm_rsp.rs b/src/migtd/src/spdm/spdm_rsp.rs index a69ec2c9..e5be459a 100644 --- a/src/migtd/src/spdm/spdm_rsp.rs +++ b/src/migtd/src/spdm/spdm_rsp.rs @@ -922,7 +922,7 @@ pub fn handle_exchange_rebind_attest_info_req( .ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?; let mig_policy_hash_src_vec = mig_policy_hash_src.to_vec(); - // SERVTD_EXT + // SERVTD_EXT (may be zero-length if target TD does not support SERVTD_EXT) let vdm_element = VdmMessageElement::read(reader).ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?; if vdm_element.element_type != VdmMessageElementType::SerVtdExt { error!( @@ -931,10 +931,14 @@ pub fn handle_exchange_rebind_attest_info_req( ); return Err(SPDM_STATUS_INVALID_MSG_FIELD); }; - let servtd_ext = reader - .take(vdm_element.length as usize) - .ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?; - let servtd_ext_vec = servtd_ext.to_vec(); + let servtd_ext_vec = if vdm_element.length > 0 { + let servtd_ext = reader + .take(vdm_element.length as usize) + .ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?; + Some(servtd_ext.to_vec()) + } else { + None + }; // TD report init let vdm_element = VdmMessageElement::read(reader).ok_or(SPDM_STATUS_INVALID_MSG_SIZE)?; @@ -1033,7 +1037,7 @@ pub fn handle_exchange_rebind_attest_info_req( init_policy, &event_log_init_vec, &td_report_init_vec, - &servtd_ext_vec, + servtd_ext_vec.as_deref(), ); if let Err(e) = &policy_check_result { error!("Policy v2 check failed, below is the detail information:\n"); @@ -1049,7 +1053,9 @@ pub fn handle_exchange_rebind_attest_info_req( unsafe { let spdm_responder_ex = upcast_mut(responder_context); - spdm_responder_ex.servtd_ext = ServtdExt::read_from_bytes(&servtd_ext_vec); + spdm_responder_ex.servtd_ext = servtd_ext_vec + .as_ref() + .and_then(|v| ServtdExt::read_from_bytes(v)); }; let mut writer = Writer::init(vendor_defined_rsp_payload); @@ -1193,14 +1199,14 @@ pub fn handle_exchange_rebind_info_req( let servtd_ext = unsafe { let spdm_responder_ex = upcast_mut(responder_context); - spdm_responder_ex - .servtd_ext - .ok_or(SPDM_STATUS_INVALID_STATE_LOCAL)? + spdm_responder_ex.servtd_ext }; write_rebinding_session_token(&token)?; - write_approved_servtd_ext_hash(&servtd_ext.calculate_approved_servtd_ext_hash()?)?; - write_servtd_rebind_attr(&servtd_ext.cur_servtd_attr)?; + if let Some(ext) = servtd_ext { + write_approved_servtd_ext_hash(Some(&ext.calculate_approved_servtd_ext_hash()?))?; + write_servtd_rebind_attr(&ext.cur_servtd_attr)?; + } token.zeroize(); let mut writer = Writer::init(vendor_defined_rsp_payload);