integration for init_tdinfo and servtd_ext#184
Draft
haitaohuang wants to merge 36 commits into
Draft
Conversation
A hostile VMM can complete a tdvmcall_migtd_receive with status=VMM_SUCCESS but data_length=0. The previous implementation let this propagate as Ok(0) through VmcallRaw::recv, which would stall the caller's read loop indefinitely (no forward progress, no error). Reject zero-length success inside the receive poll_fn closure with VmcallRawError::Malformed, which surfaces to upstream callers as a network error instead of an infinite spin. Also stop relying on the post-completion data_length on the send path (per spec, data_length is owned by MigTD when status=0 and is not a meaningful VMM-reported value after completion). The previously returned value was already discarded by VmcallRaw::send, so this is a no-op in behavior but makes the spec contract explicit. Document this in the poll_vmcall_completion doc comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Public connection state variant State::Establised was misspelled. Rename to State::Established across vmcall_raw and vsock. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
When the migration context has been destroyed (no entry in VMCALL_MIG_CONTEXT_FLAGS for the request id), any interrupt injection from the VMM should be rejected. Previously the else branch discarded the error with `let _ = ...`, allowing execution to fall through and treat DMA buffer contents as a valid VMM response. Return an error immediately so the caller rejects the stale injection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
align_up() computed `(size & !(PAGE_SIZE - 1)) + PAGE_SIZE` for non-page-aligned input, which silently wraps to 0 near usize::MAX, and vmcall_raw_transport_enqueue() blindly downcast buf.len() to u32 and added the 12-byte header without overflow checks. A pathological caller could end up allocating a 0-page SharedMemory and writing past it. Switch align_up() to checked_add and propagate None to the caller as VmcallRawError::Illegal; size the data buffer via checked_add(12) and reject buf.len() that does not fit a u32 data_length. Add unit tests for the boundary cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
parse_events() indexed event_data[1..1 + desc_size] for EV_EFI_PLATFORM_FIRMWARE_BLOB2 and event_data[..4] for EV_EVENT_TAG without first validating length. A truncated or malformed event payload caused a panic instead of being skipped. Replace the raw slice indexing with .get(...) and propagate None, so a short event is dropped rather than aborting the parse. Add unit tests for both code paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
HelloPacketPayload is serialized via as_bytes() (a raw pointer cast over the struct) and parsed via read_from_bytes() with hard-coded offsets, both of which assume C layout. Without #[repr(C)] the compiler is free to reorder fields, breaking the wire format. Annotate the struct to match the sibling PreSessionMessage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…om_quote find(PEM_CERT_END) searched from offset 0 of the lossy-decoded quote string, so if any byte sequence matching the END marker appeared before the BEGIN marker (or BEGIN was absent), end_index < start_index and the subsequent slice operation panicked. Search for END strictly after the end of BEGIN, and return InvalidQuote on malformed input. Real hardware quotes never trigger the bug, but the parser should not panic on hostile input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
get_event_log() returns &raw[..size + 1] as a workaround for an upstream bug in cc_measurement::log::CcEvents::next() (uses `<` instead of `<=`), which silently drops the last event when the slice ends exactly on an event boundary. In MigTD that last event is the tagged policy event, so removing the +1 has previously broken AzCVMEmu integration tests. Document the workaround in code next to the slice expression and add last_event_visible_only_with_trailing_padding as a regression test mirroring the upstream reproducer (confidential-containers/td-shim#848). The test fails loudly both if anyone re-applies the "cleanup" and once the upstream iterator is fixed and the workaround can be removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
There was a problem hiding this comment.
Pull request overview
This PR integrates init-TDINFO and SERVTD_EXT handling across MigTD’s SPDM migration and rebinding flows, including policy v2 verification and improved propagation of application-layer errors over SPDM. It also refactors shared SPDM handshake/session-driving logic and updates emulation/CI tooling and supporting utilities.
Changes:
- Add TDINFO_STRUCT plumbing (host-provided + local fallback), SERVTD_EXT capability detection, and new policy-v2 verification paths for migration/rebind SPDM VDM exchanges.
- Refactor SPDM requester/responder common handshake/session logic and improve error propagation by encoding MigrationResult into SPDM ERROR responses.
- Improve robustness/diagnostics across event log parsing, logging output, vmcall-raw transport arithmetic, and emulation build/test scripts.
Reviewed changes
Copilot reviewed 52 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/std-support/sys_time/src/lib.rs | Enables sys_time unit test (now executes RTC port I/O). |
| src/std-support/rust-std-stub/src/io/impls.rs | Changes test module gating for io impl tests. |
| src/std-support/rust-std-stub/src/io/error.rs | Changes test module gating for io error tests. |
| src/std-support/rust-std-stub/Cargo.toml | Removes std_tests feature definition. |
| src/policy/src/v2/collaterals.rs | Fixes PEM end-marker search to be relative to the begin marker. |
| src/migtd/src/spdm/spdm_vdm.rs | Adjusts VDM constants/ops and encodes app error code into SPDM vendor-defined error response. |
| src/migtd/src/spdm/spdm_rsp.rs | Refactors responder context handling, adds init-TDINFO/SERVTD_EXT parsing/verification, and updates SPDM loop wiring. |
| src/migtd/src/spdm/spdm_req.rs | Refactors requester MSK transfer flow, adds buffer zeroization, and updates init-TDINFO/SERVTD_EXT send behavior. |
| src/migtd/src/spdm/spdm_rebind.rs | Refactors SPDM rebind requester/responder to use shared handshake helpers and zeroization. |
| src/migtd/src/spdm/mod.rs | Adds handshake module and TDREPORT REPORTDATA/TH1 binding verifier. |
| src/migtd/src/spdm/handshake.rs | New shared SPDM handshake building blocks for requester/responder paths. |
| src/migtd/src/migration/spdm_session.rs | New helper to run SPDM sessions under timeout and normalize error mapping/shutdown. |
| src/migtd/src/migration/session.rs | Reworks migration session flow to use SPDM session helper; updates init-TDINFO tracing and vmcall-raw sizing logic. |
| src/migtd/src/migration/servtd_ext.rs | Adds SERVTD_EXT support detection via TDCS.ATTRIBUTES; changes read/write APIs to handle unsupported targets. |
| src/migtd/src/migration/pre_session_data.rs | Marks HelloPacketPayload as #[repr(C)] for wire/FFI stability. |
| src/migtd/src/migration/mod.rs | Removes init_data module, adds TDINFO constants/helpers and init-TDINFO parsing APIs. |
| src/migtd/src/migration/logging.rs | Adds log message truncation and tests; adjusts debug-console output formatting. |
| src/migtd/src/migration/init_data.rs | Deletes old init-data envelope parsing/serialization (replaced by raw TDINFO tail). |
| src/migtd/src/migration/data.rs | Updates request/response types and HOB parsing to drop InitData and use MigtdMigrationInformation only. |
| src/migtd/src/mig_policy.rs | Adds init-TDINFO cross-check helpers and destination-side migration auth path using init-TDINFO + SERVTD_EXT; uses quote retry helper. |
| src/migtd/src/event_log.rs | Documents/implements +1 slice workaround; hardens parsing with bounds-safe slicing; adds regression tests. |
| src/migtd/src/bin/migtd/main.rs | Removes local TdInfo-as-bytes helper; tightens feature gating around own-TDINFO verification. |
| src/migtd/src/bin/migtd/cvmemu.rs | Renames variable to migration_source for clarity in emu request setup. |
| src/migtd/Cargo.toml | Adds test-get-quote feature flag. |
| src/devices/vsock/src/stream.rs | Fixes State variant spelling (Established). |
| src/devices/vsock/src/lib.rs | Fixes State variant spelling (Established). |
| src/devices/vmcall_raw/src/transport/vmcall.rs | Hardens size arithmetic and receive validation; adjusts send completion return handling. |
| src/devices/vmcall_raw/src/lib.rs | Makes align_up overflow-safe (returns Option) and adds tests; fixes State variant spelling. |
| src/crypto/src/lib.rs | Adds PEM test chains for peer cert-chain validation tests. |
| src/attestation/src/igvmattest.rs | Avoids expensive full-quote debug formatting; logs a preview instead. |
| src/attestation/src/attest.rs | Increases private attestation heap size with rationale. |
| sh_script/build_AzCVMEmu_policy_and_test.sh | Conditionally updates identity dates based on policy reference. |
| sh_script/Azure/Makefile | Restructures IGVM build targets; adds reject test and mock/get-quote variants. |
| sh_script/Azure/build_azure_mock_test.sh | Adds reject-policy mode and conditional identity date updates. |
| deps/td-shim-AzCVMEmu/tdx-tdcall/src/tdx_emu.rs | Populates ServTD fields in emulation; ignores vm_write mask arg. |
| deps/td-shim-AzCVMEmu/tdx-tdcall/src/lib.rs | Adjusts crate-level no_std gating; ignores tdcall_verify_report arg. |
| deps/td-shim-AzCVMEmu/tdx-tdcall/Cargo.toml | Bumps az-tdx-vtpm dependency version. |
| deps/td-shim-AzCVMEmu/azcvm-extract-report/Cargo.toml | Bumps az-tdx-vtpm dependency version. |
| config/Azure/policy_reject_data_raw.json | Adds a policy-v2 “reject” policy input (bad FMSPC). |
| config/AzCVMEmu/policy_issuer_chain.pem | Removes tracked AzCVMEmu issuer chain PEM (now generated). |
| .gitignore | Ignores review tooling dirs and auto-generated policy artifacts. |
| .github/workflows/integration-emu.yml | Ensures jq/policy generation for scenarios that need generated policy artifacts. |
| .agents/skills/migtd-upstream/SKILL.md | Adds upstreaming workflow documentation (agent skill). |
| .agents/skills/migtd-review/SKILL.md | Adds MigTD review workflow documentation (agent skill). |
| .agents/skills/migtd-review/scripts/run-ci-gauntlet.sh | Adds local CI gauntlet runner script. |
| .agents/skills/migtd-review/scripts/list_open_findings.sh | Adds helper to list open clawpatch findings. |
| .agents/skills/migtd-review/scripts/create_review_issues.sh | Adds template for creating/closing review issues. |
| .agents/skills/migtd-review/references/triage-justifications.md | Adds boilerplate triage text for findings. |
| .agents/skills/migtd-review/references/build-features.md | Adds build feature mapping reference. |
| .agents/init-tdinfo-servtd-ext-summary.md | Adds a narrative summary of init-TDINFO/SERVTD_EXT usage. |
| .agents/AGENT_NOTES.md | Adds agent workflow notes and repository conventions. |
Comments suppressed due to low confidence (1)
src/devices/vmcall_raw/src/transport/vmcall.rs:145
vmcall_service_migtd_sendnow returnsOk(0)on success. Even though the currentVmcallRaw::sendwrapper ignores this value and returnsbuf.len(), the transport API signature isResult<usize, _>and other callers could reasonably interpret the return value as “bytes sent”. Returning 0 on success is a footgun and can break write loops if reused. Prefer returning the originalbuf.len()(ordata_lengthfrom the header you set) to reflect successful progress.
async fn vmcall_service_migtd_send(
mig_request_id: u64,
data_buffer: &mut [u8],
) -> Result<usize, VmcallRawError> {
tdx::tdvmcall_migtd_send(mig_request_id, data_buffer, VMCALL_VECTOR)
.map_err(|_e| VmcallRawError::TdVmcallErr)?;
// Discard `data_length`: not a meaningful VMM-reported value on send.
poll_fn(
|_cx| match poll_vmcall_completion(mig_request_id, data_buffer) {
Poll::Pending => Poll::Pending,
Poll::Ready(Err(e)) => Poll::Ready(Err(e)),
Poll::Ready(Ok((_payload, _data_length))) => Poll::Ready(Ok(0usize)),
},
)
.await
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
30
to
36
| #[cfg(test)] | ||
| mod tests { | ||
| use super::get_sys_time; | ||
| #[test] | ||
| #[ignore = "requires x86 I/O port access (CMOS/RTC), segfaults in userspace"] | ||
| fn it_works() { | ||
| assert_ne!(get_sys_time().unwrap(), 0); | ||
| } |
Comment on lines
+1
to
2
| #[cfg(test)] | ||
| mod tests; |
Comment on lines
+1
to
2
| #[cfg(test)] | ||
| mod tests; |
Comment on lines
8
to
13
| description = "for rustls no_std use, export rustls used functions, traits and structs in std" | ||
| readme = "readme.md" | ||
|
|
||
| [features] | ||
| # Enable to run tests copied from std (requires nightly + full std environment) | ||
| std_tests = [] | ||
|
|
||
| [dependencies] | ||
| sys_time = {path = "../sys_time"} | ||
|
|
27bf961 to
598be55
Compare
Reactivate quote retry during TCB establishment and restore the peer policy certificate chain validation helper. Both were present in the original integration branch but overwritten when upstream Intel changes were merged. - Re-enable retry logic in mig_policy.rs for TCB quote failures - Add verify_peer_policy_cert_chain() to crypto crate for validating issuer certificate chains against CA basic constraints
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Only update tcbDate and issueDate in td_identity.json when they are behind the servtd migtdIdentity tcbDate reference from the policy. Previously these dates were always overwritten to current UTC time. The reference is extracted from policy[].servtd.migtdIdentity.tcbDate. When no servtd reference exists (e.g. allow-all or AzCVMEmu policies), dates are left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After removing the init tcb cache, in a loopback migration scenario, we need call verify_quote_integrity_ex 4 times for each migration. That would cause heap exhaustion inside the c lib. Raise the size of heap Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Two interrelated cleanups:
1. Fold the optional 512-byte initial TDINFO_STRUCT directly into
MigtdMigrationInformation and remove the legacy MIGTDATA TLV envelope:
- mod.rs: add inline init_td_info: [u8; 512] (literal required by
scroll-derive 0.10; TD_INFO_SIZE/offset consts with compile-time
guards), init_td_info_if_present() accessor, td_info_mrowner /
td_info_mrownerconfig / local_init_td_info helpers
- data.rs: drop init_migtd_data field from MigrationInformation
- session.rs: collapse StartMigration dispatch to a single
raw 512-byte TdInfo (no envelope); drop init_migtd_data plumbing;
replace magic 44 with TD_INFO_SIZE
- mig_policy.rs: verify_init_migtd_data_policy_binding now takes
&[u8; TD_INFO_SIZE] directly
- spdm_req.rs: drop init_migtd_data params from transfer_msk and
send_and_receive_sdm_migration_attest_info; use
mig_info.init_td_info_if_present() + local_init_td_info() fallback
2. Unify RebindingInfo into MigtdMigrationInformation. Byte 8 of both
StartMigration and StartRebinding wire payloads carries the same
'this MigTD is source/initiator' semantic; the remaining bytes are
identical:
- rebinding.rs: delete RebindingInfo, REBINDING_INFO_HEADER_SIZE
and its hand-rolled read_from_bytes/Default/accessor + test
scaffolding; switch all start_rebinding / rebinding_old_prepare /
rebinding_new_prepare params to &MigtdMigrationInformation
(info.migration_source replaces info.rebinding_src); port the
8 read_from_bytes unit tests to target MigtdMigrationInformation
- mod.rs: hand-code MigtdMigrationInformation::read_from_bytes for
both the vmcall-raw+policy_v2 path (56|568 short/full form,
validation inlined) and the non-policy_v2 path (single fixed size,
no tail); keep impl_read_from_bytes_with_optional! in its original
single-arm form (only ReportInfo / MigtdDataInfo use it)
- data.rs: WaitForRequestResponse::StartRebinding now carries
MigtdMigrationInformation
- session.rs: StartRebinding dispatch decodes
MigtdMigrationInformation
- spdm_req.rs / spdm_rsp.rs / spdm_rebind.rs: rebind_info params
switched to &MigtdMigrationInformation
- cvmemu.rs: rename local rebinding_src to migration_source for
consistency
Wire format (vmcall-raw):
- StartMigration: 56-byte header [+ 512-byte init_td_info if
has_init_data == 1]
- StartRebinding: identical layout
- GetMigtdData response: raw 512-byte TdInfo
Verification:
- cargo check across vmcall-raw, vmcall-raw+spdm, vmcall-raw+policy_v2,
vmcall-raw+spdm+policy_v2, virtio-vsock+policy_v2+spdm,
AzCVMEmu+policy_v2+spdm+test_mock_report
- cargo test migration::rebinding: 8/8 pass
- integration-emu full suite 14/14 PASS (skip-ra, policy-v2 [+igvm],
rebind [skip-ra, mock], spdm [migration, policy-v2, rebind],
mock-quote-retry verified 5 retries + 1 success per side, all four
key/policy rotation variants)
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
formats the entire 5000+ byte quote as a decimal byte list. Inside VmmLoggerBackend::log this materializes a ~20 KB String on the heap before entrylog drops it at the page-size limit. The formatting work itself stalls the TD for ~11 seconds, during which the SPDM transport cannot send its FINISH message — the migration host then cancels the session with data_status=0x302 (per-receive timeout). Replace the dump with a compact preview that is safe at any log level: total length plus first 8 and last 8 bytes in hex. This keeps the debug breadcrumb useful for verifying quote shape without paying the formatting cost of the full payload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Messages exceeding 64 bytes are truncated to show the first 16 and last 8 bytes with a notice: ", log truncated number of bytes: N". Truncation applies to both logentry and serial/debug console output. Trailing newline is stripped before slicing to avoid double line breaks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
The SPDM rebinding path accepted VMM-provided init_td_info without running the policy-binding verification used by exchange_msk. A hostile VMM could therefore drive rebind attestation over an initial TDINFO whose policy signer hash or SVN would be rejected on the standard migration path. Mirror the exchange_msk check at the start_rebinding entry point: when init_td_info is present, require mrowner == own policy signer key hash and mrownerconfig (init policy SVN) <= own policy SVN before any rebinding exchange happens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
After commit f2ea0ef pivoted policy verification onto TDINFO_STRUCT, the MigPolicyInit element on the rebind attest-info exchange is dead: the responder compares it against td_report_init_vec[112..160] derived from the TdReportInit element in the same VDM message, so both inputs come from the same peer in the same message. Per GHCI 1.5 only init_tdinfo (already carried as TdReportInit) is required. Stop encoding and parsing the element on both sides, decrement the rebind-req element_count from 6 to 5, and drop the unused enum variant (0x15). Wire format change is intentional and atomic; both peers are the same firmware image. Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…cy-layer refactor Signed-off-by: Haitao Huang <haitaohuang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The first failure branch of verify_peer_attestation_v2 (requester) and rsp_verify_peer_attestation_v2 (responder) returned SPDM_STATUS_INVALID_MSG_FIELD without tearing down the SPDM session when mig_policy_hash_peer did not match SHA-384(peer_data). The subsequent failure branches in the same functions (authenticate_remote Err and verify_report_data_binding Err) all explicitly look up the session via get_session_via_id() and call session.teardown() before propagating the same error. Make the hash-mismatch branch follow the same convention so a peer that has already cheated on the policy-hash binding cannot keep the established session alive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
The old build-igvm-accept-all used test_disable_ra_and_accept_all which bypasses attestation entirely. Replace it with the same approach as build-igvm-mock-quote-allow-all: generate an allow-all v2 policy from mock data and build with use-mock-quote feature. This exercises the full attestation flow with an allow-all policy instead of skipping RA. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The regular migration attestation verifies that the peer's REPORTDATA equals SHA384(prefix || TH1), which prevents replay of a TDREPORT from a previous SPDM session. The rebind paths (send_and_receive_sdm_rebind_attest_info on the requester and handle_exchange_rebind_attest_info_req on the responder) accepted the peer TDREPORT without this check, so a VMM MITM could replay an old rebind attestation. Add verify_tdreport_data_binding() that extracts REPORTDATA from the raw TDREPORT structure (offset 128 vs 520 for quote supplemental data) and call it on both sides after policy authentication succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…dm_session
Add migration/spdm_session.rs with SPDM_TIMEOUT, map_spdm_setup_err and
finalize_spdm_session(body, io_ref, mig_request_id). The helper runs the
caller-provided SPDM exchange future under SPDM_TIMEOUT, then tears down
the transport. The caller still owns/constructs the SPDM context so
ResponderContextEx<'a> lifetimes resolve trivially.
Retrofit the four SPDM wrappers to call it:
- migration_src_exchange_msk (SPDM)
- migration_dst_exchange_msk (SPDM)
- rebinding_old_prepare (SPDM)
- rebinding_new_prepare (SPDM)
Eliminates four near-identical copies of the SPDM timeout + transport
shutdown boilerplate. No behavior change.
Drive-by:
- Remove stale BC> ENTER trace logs at migration_src/dst entry.
- Gate Duration/with_timeout/shutdown_transport imports in session.rs
by the cfg of their remaining consumers (TLS path or policy_v2 path),
so the spdm-without-policy_v2 build is warning-clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…elpers
Add spdm/handshake.rs exposing two helpers:
requester_handshake_prelude(spdm_requester) -> Result<u32, SpdmStatus>
Runs version -> capability -> algorithm -> send_and_receive_pub_key
-> key_exchange and returns the negotiated session_id.
run_responder_message_loop(spdm_responder_ex, peer_data, app_context)
Stashes peer_data, encodes app_context into the responder's
app_context_data_buffer, runs rsp_handle_message, then zeroizes the
buffer on success (matches pre-existing behavior — error paths
intentionally do not clear).
Retrofit the four call sites to use the new helpers:
- spdm_requester_transfer_msk
- spdm_requester_rebind_old
- spdm_responder_transfer_msk
- spdm_responder_rebind_new
Each call site now contains only the genuinely path-specific bits:
the two VDM steps, the spdm_finish / end_session calls (one line each,
left inline rather than hidden behind wrappers), and rebind-side
`responder_ex.info = RebindInformation(..)` priming.
Drive-by: the BC> debug trace logs sprinkled through
spdm_requester_transfer_msk and spdm_responder_transfer_msk are dropped
as a natural consequence of the rewrite (they were tied to the specific
intermediate steps that are now folded into the helpers). BC> traces
inside rsp_handle_message are left untouched.
No behavior change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…M session
Tier 3 of the SPDM session consolidation. Pulls the MSK-finalisation chain
(`verify_servtd_attr`, `cal_mig_version`, `set_mig_version`, `write_msk`,
and the policy_v2 `write_approved_servtd_ext_hash`) out of the SPDM
`ExchangeMigrationInfo` VDM handler / requester step and back up into the
post-session orchestrators (`migration_src_exchange_msk` /
`migration_dst_exchange_msk`), matching the structure of the TLS path.
Why
---
Previously the SPDM responder ran the entire TD-state mutation chain
inside libspdm's VDM dispatch (`handle_exchange_mig_info_req`), and the
SPDM requester ran it inside `send_and_receive_sdm_exchange_migration_info`.
That tightly coupled TD-state writes to the SPDM message handler and
made the SPDM and TLS code paths harder to compare. Moving the chain
into the orchestrators normalises the two transports and clears the way
for future shared helpers.
Mechanics
---------
- `finalize_spdm_session` is generalised from `<Fut>` to `<Fut, T>` so
the requester body can surface the negotiated `ExchangeInformation`
while existing rebinding callers continue to use `T = ()` via
inference.
- `ResponderContextExInfo::MigrationInformation` is reshaped into a
struct variant carrying both `mig_info` and the caller-computed
`exchange_information` reference, eliminating the previous
"info set but exchange-info missing" invalid state.
- `ResponderContextEx` gains `remote_information: Option<ExchangeInformation>`,
populated by the VDM handler from `ExchangeMigrationInfoReq` and
consumed by `migration_dst_exchange_msk` after the responder loop
exits.
- `upcast_mut` is ungated from `policy_v2` (migration uses it now too)
and gains an explicit SAFETY comment.
- `spdm_responder_transfer_msk` now takes the pre-computed
`exchange_information` and writes it (alongside `mig_info`) into the
responder `info` variant before driving the loop. The migration
responder no longer encodes the real `mig_info` into
`app_context_data_buffer`; the handler reads from `responder_ex.info`
instead.
- `spdm_requester_transfer_msk` now takes `&ExchangeInformation` and
returns `Result<ExchangeInformation, SpdmStatus>` (the negotiated
remote info).
- `send_and_receive_sdm_exchange_migration_info` likewise takes the
pre-computed `exchange_information` and returns the negotiated
remote `ExchangeInformation`; the chain calls are removed.
- `handle_exchange_mig_info_req` stops calling `exchange_info`,
`verify_servtd_attr`, `cal_mig_version`, `set_mig_version`,
`write_msk`, and `write_approved_servtd_ext_hash`, and instead
stashes the parsed `remote_information` into `responder_ex` for the
caller.
Preserved quirks (tracked as separate behaviour-bug tickets)
------------------------------------------------------------
- **SPDM source uses `is_src=false`** for both `exchange_info(...)` and
`cal_mig_version(...)`. This is asymmetric with the TLS source path
(which uses `true`) and reads IMPORT versions on the source side.
Preserved verbatim so this refactor is behaviour-preserving; flagged
in code comments for a follow-up fix.
- **SPDM source does not call `verify_servtd_attr`**, while SPDM dst and
both TLS sides do. This is the protocol-asymmetry described in
`servtd_ext.rs` comments. Not introduced by this commit; flagged for
a separate ticket.
Post-session failure window
---------------------------
With the chain moved out of the VDM handler, a destination-side
`verify_servtd_attr` / `write_msk` failure now occurs AFTER the
responder has already sent `ExchangeMigrationInfoRsp` successfully.
This matches the failure model the TLS path has always had and is an
inherent consequence of unifying the two flows. The source's
`SecureSessionError` reporting is unchanged from baseline because the
SPDM error is still surfaced via `MigrationResult::SecureSessionError`.
Verification
------------
- `cargo check --no-default-features --features main,vmcall-raw,stack-guard,vmcall-interrupt,oneshot-apic,spdm_attestation,igvm-attest,policy_v2`:
3 pre-existing dead-code warnings in tests; no new warnings.
- `cargo check --no-default-features --features main,vmcall-raw,stack-guard,vmcall-interrupt,oneshot-apic,spdm_attestation`:
same 4 pre-existing warnings as baseline.
- `cargo check --features stack-guard,virtio-vsock,virtio-serial,vmcall-interrupt`
(format.yml workflow set): clean.
- `cargo fmt --check`: only pre-existing logging.rs:{613,672,1636} and
spdm_rsp.rs:{747,771} diffs remain (out of scope of this commit).
- `cargo clippy --lib`: same warning categories as baseline
(`uninlined_format_args`, `map_err` over `inspect_err`); new code
follows the existing style of `exchange_msk`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
send_and_receive_pub_key (called inside requester_handshake_prelude) encodes the requester's ephemeral ECDSA private key into spdm_requester.common.app_context_data_buffer so libspdm's asymmetric signing callback can read it during KEY_EXCHANGE/FINISH. The libspdm SpdmContext provides no automatic cleanup for that field (it is a plain [u8; SPDM_CONTEXT_APP_DATA_BUFFER_SIZE] with no Drop impl), and the PrivateKeyDer ZeroizeOnDrop derive only wipes the temporary local — the encoded byte copy in the libspdm buffer persists for as long as the RequesterContext is alive, and after that the freed heap memory keeps the secret until reuse. Wrap both requester orchestrators (spdm_requester_transfer_msk and spdm_requester_rebind_old) in thin outer functions that call the existing body and then unconditionally call zeroize() on the buffer, covering both Ok and Err returns. The responder side already zeroes the analogous buffer in run_responder_message_loop (handshake.rs:79) — the buffer it holds is all defaults post-Tier-3 so an error-path leak there is harmless, but the requester's is the real secret and deserves the stronger guarantee. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…on destination Signed-off-by: Haitao Huang <haitaohuang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rsp_verify_peer_attestation_v2 and req_verify_peer_attestation_v2 returned SPDM_STATUS_INVALID_MSG_FIELD on policy failure. The VDM dispatch (spdm_vdm.rs) then sent SpdmErrorUnspecified, losing the real error — both sides reported SecureSessionError(6). Fix: - rsp/req handlers return SpdmStatus::from(MigrationResult::PolicyUnsatisfiedError) - spdm_vdm.rs dispatch sends SpdmErrorVendorDefined with app error in param2 - finalize_spdm_session decodes VDM status locally (destination) and extracts param2 from wire error_data (source) Both sides now report PolicyUnsatisfiedError(8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Reverts the cur_servtd_attr == init_attr comparison added in d4336bb (intel/MigTD PR intel#832, issue intel#831). That check compared CURR_SERVTD_ATTR against INIT_ATTR — both read from the same target TD's TDCS. After a rebind, CURR_SERVTD_ATTR could legitimately differ from INIT_ATTR if the new MigTD was bound with different IGNORE flags, causing a false rejection. The hardcoded check (cur == 0x0) remains and is sufficient while all SERVTD_ATTR bits are defined as fixed-zero. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
read_servtd_ext() now checks TDCS.ATTRIBUTES bit 17 before reading SERVTD_EXT fields. Returns Ok(None) when the target TD does not support SERVTD_EXT, Err on actual read failures. When SERVTD_EXT is unavailable: - Source/old MigTD sends zero-length SerVtdExt and TdReportInit elements on the wire (init_tdinfo cannot be verified without init_servtd_info_hash) - write_approved_servtd_ext_hash() accepts Option<&[u8]> and is a no-op when None This replaces the REVERT_ME all-zero init_servtd_info_hash bypass with proper feature detection at the source. Based on the pattern from 6755fe9 (servtdext_disable2 branch). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
…build The old build-igvm-reject-all built without policy_v2 (v1 features only) and relied on empty quote generation to trigger rejection. Replace with the same mock-quote setup as accept-all but using a policy that has a nonexistent FMSPC (DEADBEEF0000) so the platform check will reject the migration at policy evaluation time. New file: config/Azure/policy_reject_data_raw.json New flag: build_azure_mock_test.sh --reject Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a new 'test-get-quote' feature flag that triggers IGVM quote generation and verification at the end of init_policy(). This enables Azure OS CI to test the getquote path during MigTD initialization without requiring a successful migration. - Add test-get-quote feature to src/migtd/Cargo.toml - Call get_local_tcb_evaluation_info() at end of init_policy() when the feature is enabled - Update Makefile to use IGVM_FEATURES_GET_QUOTE_TEST with the new feature for the build-igvm-get-quote target Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move quote data and parsing to reusable module - Remove mock report data and always derive report from the quote - Remove unused constants like TDVMCALL_TDINFO and fix other warnings Signed-off-by: Haitao Huang <haitaohuang@microsoft.com>
Intel PR intel#825 added `mig_policy::verify_own_tdinfo()` (GHCI 1.5) which checks at startup that the running MigTD's TDINFO matches its policy: td_info.mrowner == SHA384(policy signer public key) td_info.mrownerconfig == policy_svn as u32 LE || [0u8; 44] These fields are normally provisioned by the host VMM. AzCVMEmu uses a hardcoded mock TDREPORT whose MROWNER/MROWNERCONFIG are zero, so the check would always fail under emulation and block all policy_v2 + mock-report tests. Skip the call site in main.rs when the `AzCVMEmu` feature is active: #[cfg(all(feature = "vmcall-raw", not(feature = "AzCVMEmu")))] `verify_own_tdinfo()` itself remains compiled under `vmcall-raw`, so real (non-emulator) builds still enforce the GHCI 1.5 binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Import agent notes, skill definitions, and review scripts from origin/integration.
Remove the std_tests cargo feature and associated cfg gates from rust-std-stub, and remove the #[ignore] annotation from sys_time's unit test since it runs correctly in the CI environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
598be55 to
4b93617
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.