From fd82e1f66c407631621f8f6a5263357076af8bae Mon Sep 17 00:00:00 2001
From: Haitao Huang <haitaohuang@microsoft.com>
Date: Wed, 28 Oct 2026 07:54:56 +0000
Subject: [PATCH] fix(migtd/spdm): reject success status on error paths

SPDM vendor-defined error responses carry an application status byte, but value zero maps to MigrationResult::Success. Decoding that value on an error path can therefore report a failed migration or rebind operation as successful.

Centralize error-path decoding so Success and unknown values map to SecureSessionError, and cover direct decoding plus both SPDM conversion paths with zero application-status tests.

Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Assisted-by: GitHub Copilot CLI:GPT-5.6 Sol
---
 src/migtd/src/spdm/mod.rs   | 21 +++++++++++----
 src/migtd/src/spdm/tests.rs | 51 +++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/migtd/src/spdm/mod.rs b/src/migtd/src/spdm/mod.rs
index 24e4f5b88..67f1ddec8 100644
--- a/src/migtd/src/spdm/mod.rs
+++ b/src/migtd/src/spdm/mod.rs
@@ -401,8 +401,7 @@ impl From<SpdmStatus> for MigrationResult {
         if spdm_status.severity == StatusSeverity::SUCCESS {
             MigrationResult::Success
         } else if let StatusCode::VDM(vdm_error) = &spdm_status.status_code {
-            MigrationResult::try_from(vdm_error.vdm_error_code as u8)
-                .unwrap_or(MigrationResult::SecureSessionError)
+            decode_error_path_status(vdm_error.vdm_error_code as u8)
         } else {
             MigrationResult::SecureSessionError
         }
@@ -429,6 +428,20 @@ impl From<MigrationResult> for SpdmStatus {
     }
 }
 
+/// Decode a raw application status code that arrived on a path that is only
+/// ever taken for SPDM *errors* (a `StatusCode::VDM` error status, or the
+/// vendor-defined Param2 of an SPDM ERROR message). The wire encoding reuses
+/// `MigrationResult::Success` (discriminant 0) as an ordinary enum value, so
+/// a bare `TryFrom<u8>` decode could otherwise turn a peer-reported error
+/// into a false `Success`. Reject that case (and any undecodable value) so
+/// an error path can never produce `Success`.
+fn decode_error_path_status(value: u8) -> MigrationResult {
+    match MigrationResult::try_from(value) {
+        Ok(MigrationResult::Success) | Err(()) => MigrationResult::SecureSessionError,
+        Ok(result) => result,
+    }
+}
+
 /// Decode an SpdmStatus returned by an SPDM session body into a
 /// MigrationResult, preserving application-level errors carried over the wire
 /// as `SpdmErrorVendorDefined`.
@@ -448,9 +461,7 @@ pub(crate) fn decode_spdm_session_err(e: SpdmStatus) -> MigrationResult {
         if ed.length >= 4
             && ed.data[2] == spdmlib::message::SpdmErrorCode::SpdmErrorVendorDefined.get_u8()
         {
-            if let Ok(decoded) = MigrationResult::try_from(ed.data[3]) {
-                return decoded;
-            }
+            return decode_error_path_status(ed.data[3]);
         }
     }
     MigrationResult::SecureSessionError
diff --git a/src/migtd/src/spdm/tests.rs b/src/migtd/src/spdm/tests.rs
index 839824f3d..980966322 100644
--- a/src/migtd/src/spdm/tests.rs
+++ b/src/migtd/src/spdm/tests.rs
@@ -61,3 +61,54 @@ fn responder_teardown_clears_handshaking_and_established_sessions() {
     let (mut responder, _) = spdm_responder(TestTransport).unwrap();
     assert_teardown_clears_sessions(&mut responder.responder_context.common);
 }
+
+#[test]
+fn decode_error_path_status_never_returns_success() {
+    // App-code 0 collides with `MigrationResult::Success`'s discriminant.
+    // On an error path this must be treated as an undecodable/invalid code,
+    // not as a successful result.
+    assert_eq!(
+        decode_error_path_status(0),
+        MigrationResult::SecureSessionError
+    );
+    // A genuine, non-zero error code still decodes to its real variant.
+    assert_eq!(
+        decode_error_path_status(MigrationResult::InvalidParameter as u8),
+        MigrationResult::InvalidParameter
+    );
+    // An undecodable value falls back to a generic session error rather
+    // than panicking or silently becoming Success.
+    assert_eq!(
+        decode_error_path_status(0xAA),
+        MigrationResult::SecureSessionError
+    );
+}
+
+#[test]
+fn from_spdm_status_vdm_error_never_decodes_to_success() {
+    let status = SpdmStatus {
+        severity: StatusSeverity::ERROR,
+        status_code: StatusCode::VDM(StatusCodeVdmError { vdm_error_code: 0 }),
+        error_data: None,
+    };
+    assert_eq!(
+        MigrationResult::from(status),
+        MigrationResult::SecureSessionError
+    );
+}
+
+#[test]
+fn decode_spdm_session_err_vendor_defined_zero_app_code_never_decodes_to_success() {
+    let mut data = [0u8; spdmlib::error::SPDM_ERROR_MESSAGE_DATA_MAX_LENGTH];
+    data[2] = spdmlib::message::SpdmErrorCode::SpdmErrorVendorDefined.get_u8();
+    data[3] = MigrationResult::Success as u8;
+    let status = SpdmStatus {
+        severity: StatusSeverity::ERROR,
+        status_code: StatusCode::VDM(StatusCodeVdmError { vdm_error_code: 0 }),
+        error_data: Some(spdmlib::error::SpdmErrorMessageData { length: 4, data }),
+    };
+    assert_eq!(
+        decode_spdm_session_err(status),
+        MigrationResult::SecureSessionError
+    );
+}