fix(migtd): complete SPDM cleanup and temporary GetTDReport compatibility

.agents/knowledge/domain-facts.md

@@ -56,6 +56,15 @@ bypassed under which build feature.
 - Hold `spdm::AppContextGuard` across migration and rebind exchanges so dropping
   either role's future wipes the buffer. Keep cancellation coverage for all
   four entry points in `src/migtd/src/spdm/tests.rs`.
+- SPDM `FINISH` clears `runtime_info.last_session_id` while the established
+  session still holds keys. Retire every exchange-owned `SpdmContext::session`
+  slot on return or cancellation, not just the last handshake ID
+  (`deps/spdm-rs/spdmlib/src/requester/finish_req.rs`, `responder/context.rs`;
+  upstream cleanup: `7b40eccb`).
+- The guard also tears down those slots before `finalize_spdm_session` attempts
+  transport shutdown. Shutdown must run on protocol failure or timeout without
+  replacing the primary error. Keep the teardown assertions for both roles,
+  handshaking/established states, cancellation, and repeated cleanup.
 
 ## TDINFO / MROwner / MROwnerConfig semantics
 

doc/MigTD_Functionality_Summary.md

@@ -84,7 +84,11 @@ Implemented in `src/migtd/src/migration/{data.rs, session.rs, event.rs}`:
   - `StartMigration` — run the MSK key-exchange flow.
   - `StartRebinding` — approve rebinding the user TD to a new MigTD (policy v2).
   - `GetTdReport` — return a TD report using MigTD's fixed report data; the
-    request payload contains only the migration request ID.
+    canonical request payload is the 8-byte migration request ID.
+    **REVERT_ME (OS-transition testing only):** temporarily also accept the
+    legacy 72-byte payload with a trailing 64-byte `REPORT_DATA`. That tail
+    is discarded silently, never stored or used to generate the report.
+    Remove this compatibility path once older test OS versions are retired.
   - `EnableLogArea` — enable/raise the VMM log level for a request.
   - `GetMigtdData` — return MigTD attestation data (policy v2).
 - `ReportStatus` returns the per-request result (`ReportStatusResponse` carries

src/migtd/src/migration/mod.rs

@@ -345,10 +345,16 @@ impl ReportInfo {
         data_length: u32,
         payload: &[u8],
     ) -> core::result::Result<Self, MigrationResult> {
-        if data_length != core::mem::size_of::<Self>() as u32 {
+        let request_id_size = core::mem::size_of::<Self>() as u32;
+        let legacy_size =
+            request_id_size + tdx_tdcall::tdreport::TD_REPORT_ADDITIONAL_DATA_SIZE as u32;
+        // REVERT_ME: tolerate the retired REPORT_DATA tail during OS-transition testing.
+        if data_length != request_id_size && data_length != legacy_size {
             return Err(MigrationResult::InvalidParameter);
         }
         payload
+            .get(..data_length as usize)
+            .ok_or(MigrationResult::InvalidParameter)?
             .pread(0)
             .map_err(|_| MigrationResult::InvalidParameter)
     }

src/migtd/src/migration/session.rs

@@ -1463,7 +1463,7 @@ mod test {
         use crate::migration::MIGTD_MIGRATION_INFO_HEADER_SIZE;
         use crate::migration::{
             data::{RequestDataBufferHeader, WaitForRequestResponse},
-            EnableLogAreaInfo, MigrationResult,
+            EnableLogAreaInfo, MigrationResult, ReportInfo,
         };
         use core::mem::size_of;
         use core::task::Poll;
@@ -1653,33 +1653,84 @@ mod test {
                 }
                 _ => panic!("Expected GetTdReport, got unexpected variant"),
             }
+            assert!(pending.is_none());
             cleanup_request(request_id);
         }
 
         #[test]
-        fn test_parse_get_td_report_legacy_report_data_rejected() {
+        fn test_parse_get_td_report_legacy_report_data_ignored() {
+            assert_eq!(size_of::<ReportInfo>(), size_of::<u64>());
             let request_id: u64 = 0xCC00_0000_0000_0003;
-            let mut payload = vec![0xCC; 72];
-            payload[0..8].copy_from_slice(&request_id.to_le_bytes());
-            let buf = build_request_buffer(3, &payload);
-            let mut pending = None;
-            let result = parse_request(&buf, HDR_LEN, &mut pending);
-            assert!(matches!(
-                result,
-                Poll::Ready(Err(MigrationResult::InvalidParameter))
-            ));
+            for report_data_byte in [0x00, 0x5a, 0xff] {
+                let mut payload = vec![report_data_byte; 72];
+                payload[0..8].copy_from_slice(&request_id.to_le_bytes());
+                let buf = build_request_buffer(3, &payload);
+                let mut pending = None;
+                let result = parse_request(&buf, HDR_LEN, &mut pending);
+                match result {
+                    Poll::Ready(Ok(WaitForRequestResponse::GetTdReport(info))) => {
+                        assert_eq!(info.mig_request_id, request_id);
+                    }
+                    _ => panic!("Expected GetTdReport for legacy payload"),
+                }
+                assert!(pending.is_none());
+                cleanup_request(request_id);
+            }
         }
 
         #[test]
         fn test_parse_get_td_report_wrong_size_rejected() {
-            // Only the exact 8-byte request ID is accepted.
-            let buf = build_request_buffer(3, &[0u8; 16]);
+            for payload_size in 0..=80 {
+                if payload_size == 8 || payload_size == 72 {
+                    continue;
+                }
+                let buf = build_request_buffer(3, &vec![0u8; payload_size]);
+                let mut pending = None;
+                let result = parse_request(&buf, HDR_LEN, &mut pending);
+                assert!(matches!(
+                    result,
+                    Poll::Ready(Err(MigrationResult::InvalidParameter))
+                ));
+            }
+        }
+
+        #[test]
+        fn test_parse_get_td_report_truncated_declared_payload_rejected() {
+            for data_length in [8, 72] {
+                for payload_size in 0..data_length as usize {
+                    let payload = vec![0u8; payload_size];
+                    assert!(matches!(
+                        ReportInfo::read_from_bytes(data_length, &payload),
+                        Err(MigrationResult::InvalidParameter)
+                    ));
+                    let buf = build_raw_buffer(0x0301, data_length, &payload);
+                    let mut pending = None;
+                    assert!(matches!(
+                        parse_request(&buf, HDR_LEN, &mut pending),
+                        Poll::Ready(Err(MigrationResult::InvalidParameter))
+                    ));
+                }
+            }
+        }
+
+        #[cfg(feature = "policy_v2")]
+        #[test]
+        fn test_parse_get_migtd_data_retains_report_data() {
+            let request_id: u64 = 0xCD00_0000_0000_0004;
+            let report_data = [0x5a; 64];
+            let mut payload = request_id.to_le_bytes().to_vec();
+            payload.extend_from_slice(&report_data);
+            let buf = build_request_buffer(5, &payload);
             let mut pending = None;
-            let result = parse_request(&buf, HDR_LEN, &mut pending);
-            assert!(matches!(
-                result,
-                Poll::Ready(Err(MigrationResult::InvalidParameter))
-            ));
+            match parse_request(&buf, HDR_LEN, &mut pending) {
+                Poll::Ready(Ok(WaitForRequestResponse::GetMigtdData(info))) => {
+                    assert_eq!(info.mig_request_id, request_id);
+                    assert_eq!(info.reportdata, report_data);
+                }
+                _ => panic!("Expected GetMigtdData, got unexpected variant"),
+            }
+            assert!(pending.is_none());
+            cleanup_request(request_id);
         }
 
         #[test]

src/migtd/src/migration/spdm_session.rs

@@ -31,14 +31,15 @@ pub(super) fn map_spdm_setup_err(mig_request_id: u64) -> MigrationResult {
     MigrationResult::SecureSessionError
 }
 
-/// Run an SPDM session `body` under [`SPDM_TIMEOUT`], then shut down the
-/// transport associated with `io_ref`. Returns any value produced by `body`.
+/// Run an SPDM session `body` under [`SPDM_TIMEOUT`] and always attempt transport
+/// shutdown. A protocol error or timeout takes precedence over a shutdown error.
 ///
 /// `body` is the already-constructed future returned by an SPDM exchange
 /// function (e.g. `spdm_requester_transfer_msk`, `spdm_responder_rebind_new`).
 /// The caller owns the SPDM context that `body` borrows; this helper only
 /// drives `body` to completion and then takes the device-IO lock to invoke
-/// `shutdown_transport`.
+/// `shutdown_transport`. The exchange's `AppContextGuard` retires its session
+/// keys before shutdown, including when the timeout drops `body`.
 pub(super) async fn finalize_spdm_session<Fut, T>(
     body: Fut,
     io_ref: SpdmDeviceIoArc<TransportType>,
@@ -47,26 +48,30 @@ pub(super) async fn finalize_spdm_session<Fut, T>(
 where
     Fut: Future<Output = core::result::Result<T, SpdmStatus>>,
 {
-    let value = with_timeout(SPDM_TIMEOUT, body)
+    let session_result = with_timeout(SPDM_TIMEOUT, body)
         .await
         .map_err(|e| {
             log::error!(
                 migration_request_id = mig_request_id;
                 "finalize_spdm_session: body timeout: {e:?}\n"
             );
             MigrationResult::from(e)
-        })?
-        .map_err(|e| {
-            log::error!(
-                migration_request_id = mig_request_id;
-                "finalize_spdm_session: body error: {e:?}\n"
-            );
-            crate::spdm::decode_spdm_session_err(e)
-        })?;
+        })
+        .and_then(|result| {
+            result.map_err(|e| {
+                log::error!(
+                    migration_request_id = mig_request_id;
+                    "finalize_spdm_session: body error: {e:?}\n"
+                );
+                crate::spdm::decode_spdm_session_err(e)
+            })
+        });
 
     let mut transport_lock = io_ref.lock();
     let transport = transport_lock.deref_mut();
-    shutdown_transport(&mut transport.transport, mig_request_id).await?;
+    let shutdown_result = shutdown_transport(&mut transport.transport, mig_request_id).await;
 
+    let value = session_result?;
+    shutdown_result?;
     Ok(value)
 }

src/migtd/src/spdm/handshake.rs

@@ -57,8 +57,9 @@ pub(super) async fn requester_handshake_prelude(
 ///
 /// This helper does NOT zeroize `app_context_data_buffer` itself: the buffer
 /// may carry the caller's ephemeral private key, so callers hold an
-/// `AppContextGuard` across the await to wipe it on success, error, or
-/// cancellation (see `spdm_responder_transfer_msk` / `spdm_responder_rebind_new`).
+/// `AppContextGuard` across the await to wipe it and retire session keys on
+/// success, error, or cancellation (see `spdm_responder_transfer_msk` /
+/// `spdm_responder_rebind_new`).
 ///
 /// Callers that need to expose extra context to VDM handlers (e.g. setting
 /// `spdm_responder_ex.info = RebindInformation(..)`) must do so before

src/migtd/src/spdm/mod.rs

@@ -23,7 +23,7 @@ use codec::Codec;
 use codec::Reader;
 use codec::Writer;
 use log::error;
-use spdmlib::common::SpdmDeviceIo;
+use spdmlib::common::{SpdmContext, SpdmDeviceIo};
 use spdmlib::error::*;
 use spdmlib::protocol::{SpdmDigestStruct, SPDM_MAX_HASH_SIZE};
 use spin::Mutex;
@@ -50,17 +50,26 @@ use crate::spdm::vmcall_msg::VMCALL_SPDM_MESSAGE_HEADER_SIZE;
 
 pub(crate) type SpdmDeviceIoArc<T> = Arc<Mutex<MigtdTransport<T>>>;
 
-// The raw application buffer holds the ephemeral signing key. Borrow the whole
-// context so the exchange can keep using it while the buffer is wiped on both
-// normal return and future cancellation.
+// Borrow the whole context so the exchange can keep using it while its session
+// keys and raw application-buffer signing key are cleared on return or cancellation.
 struct AppContextGuard<'a, T> {
     context: &'a mut T,
-    buffer: fn(&mut T) -> &mut [u8],
+    common: fn(&mut T) -> &mut SpdmContext,
 }
 
 impl<T> Drop for AppContextGuard<'_, T> {
     fn drop(&mut self) {
-        (self.buffer)(self.context).zeroize();
+        let common = (self.common)(self.context);
+        teardown_sessions(common);
+        common.app_context_data_buffer.zeroize();
+    }
+}
+
+pub(crate) fn teardown_sessions(context: &mut SpdmContext) {
+    // FINISH clears last_session_id while the established session still holds keys.
+    // Each context belongs to one exchange, so retire every slot before shutdown.
+    for session in &mut context.session {
+        session.teardown();
     }
 }
 

src/migtd/src/spdm/spdm_rebind.rs

@@ -21,7 +21,7 @@ pub async fn spdm_requester_rebind_old(
 ) -> Result<(), SpdmStatus> {
     let guard = super::AppContextGuard {
         context: spdm_requester,
-        buffer: |context| &mut context.common.app_context_data_buffer,
+        common: |context| &mut context.common,
     };
     spdm_requester_rebind_old_inner(guard.context, rebind_info, peer_data).await
 }
@@ -61,7 +61,7 @@ pub async fn spdm_responder_rebind_new<'a>(
 ) -> Result<(), SpdmStatus> {
     let guard = super::AppContextGuard {
         context: spdm_responder_ex,
-        buffer: |context| &mut context.responder_context.common.app_context_data_buffer,
+        common: |context| &mut context.responder_context.common,
     };
     let spdm_responder_ex = &mut *guard.context;
 

src/migtd/src/spdm/spdm_req.rs

@@ -91,7 +91,7 @@ pub async fn spdm_requester_transfer_msk(
 ) -> Result<ExchangeInformation, SpdmStatus> {
     let guard = super::AppContextGuard {
         context: spdm_requester,
-        buffer: |context| &mut context.common.app_context_data_buffer,
+        common: |context| &mut context.common,
     };
     spdm_requester_transfer_msk_inner(
         guard.context,

src/migtd/src/spdm/spdm_rsp.rs

@@ -169,7 +169,7 @@ pub async fn spdm_responder_transfer_msk<'a>(
 ) -> Result<(), SpdmStatus> {
     let guard = super::AppContextGuard {
         context: spdm_responder_ex,
-        buffer: |context| &mut context.responder_context.common.app_context_data_buffer,
+        common: |context| &mut context.responder_context.common,
     };
     let spdm_responder_ex = &mut *guard.context;
 
@@ -180,6 +180,7 @@ pub async fn spdm_responder_transfer_msk<'a>(
         mig_info,
         exchange_information,
     };
+    spdm_responder_ex.mig_info_exchanged = false;
     spdm_responder_ex.remote_information = None;
 
     // The VDM handler reads `mig_info` / `exchange_information` from
@@ -209,17 +210,7 @@ pub async fn rsp_handle_message(spdm_responder: &mut ResponderContext) -> Result
 
         match res {
             Ok(Ok(_)) => {}
-            Ok(Err(spdm_status)) => {
-                if spdm_status.severity == StatusSeverity::ERROR
-                    && matches!(spdm_status.status_code, StatusCode::VDM(_))
-                {
-                    return Err(spdm_status);
-                }
-                if spdm_status == SPDM_STATUS_INVALID_STATE_LOCAL {
-                    // Terminate the responder upon invalid state.
-                    return Err(spdm_status);
-                }
-            }
+            Ok(Err(spdm_status)) => return Err(spdm_status),
             Err(_) => return Err(SPDM_STATUS_RECEIVE_FAIL),
         }