Igvmfilegen: Support CoRIM generation and patch command by mingweishih · Pull Request #3088 · microsoft/openvmm

mingweishih · 2026-03-20T22:28:26Z

This PR adds the support of writing CoRIM to an existing IGVM file, allowing for injecting the CoRIM files post build in the pipeline.

By IGVM design, a signed CoRIM is expected to be broken into payload and signed envelope with payload detached and written into CORIM_DOCUMENT and CORIM_SIGNATURE entries correspondingly. The idea is decoupling the payload generation and the signing process. However, given that the CoRIM detached signing might not always be supported by the signing infrastructure, the tool additionally supports taking a signed CoRIM as input and handles the decoupling before writing to the IGVM file.

Notes to the PR

Pin to igvm main: The PR consumes the upstream CoRIM header support that hasn't been released to crates.io yet. Tracked separately — will switch back to a crates.io version once igvm 0.5.0 ships.
Few changes related to switching from igvm 0.4.0 to git main:
- SNP SevVmsa size loosening: igvm crate's SevVmsa is padded out to a full 4K page.
- Adapt AArch64Register variants (X2-X7) and a new IgvmDirectiveHeader::AArch64CcaVpContext variant.

Copilot

Pull request overview

Adds CoRIM (Concise Reference Integrity Manifest) patching/extraction capabilities to igvmfilegen, enabling post-build injection of CoRIM document/signature data into an existing IGVM file (including support for splitting bundled COSE_Sign1 inputs).

Changes:

Add new DumpCorim and PatchCorim CLI subcommands to inspect and patch CoRIM headers in IGVM files.
Introduce a new corim module implementing IGVM directive patching plus minimal COSE_Sign1 split/validation utilities (with unit tests).
Switch igvm/igvm_defs workspace deps to a git fork/branch that includes the required CoRIM header support.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
vm/loader/igvmfilegen/src/main.rs	Adds new CLI commands and implements CoRIM header dumping/patching entrypoints.
vm/loader/igvmfilegen/src/corim/mod.rs	Adds IGVM-level CoRIM patching logic and compatibility-mask helpers.
vm/loader/igvmfilegen/src/corim/cose.rs	Adds COSE_Sign1 parsing/splitting/validation helpers plus unit tests.
vm/loader/igvmfilegen/Cargo.toml	Adds `open_enum` dependency for the new COSE/CBOR helpers.
Cargo.toml	Switches `igvm` and `igvm_defs` from crates.io to a git fork/branch.
Cargo.lock	Updates lock entries for the new git-based `igvm` crates and incidental dependency resolution changes.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 8 comments.

+igvm = { git = "https://github.com/mingweishih/igvm", rev = "c389b9f", features = ["corim"] }
+igvm_defs = { git = "https://github.com/mingweishih/igvm", rev = "c389b9f", default-features = false }


Copilot

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.

github-actions · 2026-06-10T19:06:15Z

27 Petri tests failed (5 unstable)

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.

mebersol

[Copilot] Review feedback on PR #3088

Overall this is a substantial and well-tested change — the documentation in the new modules is excellent, the pre-mutation verification + atomic rename in patch_corim_signature is the right shape, and the test suite (especially the multi-platform and end-to-end paths) covers a lot of meaningful failure modes. A few things I'd push back on or want clarified before merge:

High-impact

igvm/igvm_defs switched from crates.io to a git pin (rev = "e6c3ff1"). This is a release-blocker concern for downstream consumers — pinning to a specific commit of microsoft/igvm means anyone building openvmm now needs network access to that exact git rev, and any cargo update lottery can regress. The PR description doesn't mention a plan for re-publishing 0.5.0 to crates.io. Could you either (a) cut an igvm 0.5.0 release first and depend on that, or (b) explicitly call out in the PR body that this is a temporary pin and link the tracking issue?
SNP SevVmsa size handling silently loosened in file_loader.rs. The old check required exact equality; the new check allows any data.len() <= size_of::<SevVmsa>() and zero-pads to the new (page-aligned, 4K) size. The comment explains the size mismatch but doesn't address the measurement implication: SEV-SNP measures the VMSA page contents, and PSP-spec measurement is over the architectural 1648-byte form. If the padding bytes end up in the measured digest because the new SevVmsa definition is page-sized, you've silently changed measurements (and they'll only diverge at attestation time on real hardware). Please confirm:
- The test_snp_measurement reference digest was regenerated against real hardware-validated output, not just updated to match whatever the new code produces.
- The padded VMSA bytes are not included in the hashed region for the launch digest, OR the spec-mandated bytes are byte-identical pre/post.
Possibly incorrect libssl-dev dependency in build_igvmfilegen.rs. The comment says libssl-dev + pkg-config are required, but crypto is enabled with features = ["native", "vendored"]. Per support/crypto/Cargo.toml, vendored translates to openssl?/vendored which builds OpenSSL from source — system libssl-dev should not be needed in that configuration. pkg-config may still be needed by openssl-sys's build script, but libssl-dev likely isn't. Worth verifying — if the build works without libssl-dev, drop it; if it doesn't, the vendored feature is being defeated somewhere and that needs fixing instead.

Behavior nuances worth surfacing

PatchCorimSignature --corim-bundle silently discards the bundle's payload. Only the detached signature is taken from a bundled input; the embedded CoRIM document is dropped on the floor and the IGVM-embedded document is what gets signed against. The CLI help mentions it, but in practice if a user supplies a bundle whose embedded payload differs from the IGVM-embedded document, they'll get a cryptographic verification failed error with no hint about why (the document mismatch). Consider either: (a) compare the two documents and emit a targeted error ("bundled document does not match IGVM-embedded document"), or (b) log the discard at info level so it's visible in build logs.
CoRIM signature header is reordered relative to other initialization headers. In corim_signature::mod::patch, when an existing CorimDocument is matched it's dropped from the iteration and re-pushed at the end, then the new signature is appended. This moves the document past any subsequent non-CoRIM init headers (e.g. GuestPolicy, RelocatableRegion). The igvm crate may or may not care about absolute init-header ordering — please confirm with the upstream maintainer that only the relative (doc-before-sig) ordering matters, and that re-anchoring at the tail doesn't change semantics. If you can preserve the document at its original position and only move/insert the signature adjacent to it, that's strictly safer.
Inconsistency: VBS legacy signing-algo says ECDSA_P384, CoRIM signing-algo restricted to PS384. In measurement_diag::log_vbs you hard-code signing_algo: VbsSigningAlgorithm::ECDSA_P384.0, but the CoRIM verify path in envelope::verify_corim_signature only accepts CoseAlgorithm::Ps384 (RSA-PSS). Two different signing paths, two different algorithms is fine, but a sentence in the PR description explaining "legacy VBS endorsement is ECDSA P-384 over the boot-measurement struct; CoRIM endorsement is PS384 over the CoRIM document, by separate keys" would prevent future readers from staring at this and assuming a bug.
Crypto backend mismatch between test and prod. crypto::ensure_single_backend!() is gated #[cfg(not(test))]. If tests run under a different backend than the binary, the cryptographic verify path being exercised by verify_ps384_round_trip is not the one shipping. Please confirm the test backend (RustCrypto?) and the prod backend (OpenSSL via native+vendored) both round-trip PS384 identically.

Minor

build_endorsement_json uses unreachable! for non-measurable platforms. Invariant is enforced at the call site, not the type system. Consider taking a small enum like enum MeasurablePlatform { Snp, Tdx, Vbs } from the call site so the function signature itself can't be misused — would eliminate the .expect() digests too.
Platform CLI enum duplicates IgvmPlatformType for clap. Cosmetic. Could be a single newtype with a value_parser, but the current approach is also fine.
CoRIM crate at 0.1.3 (0.y.z). Pre-1.0 SemVer means any 0.x bump can break — given how central this is to the patch path, consider pinning to =0.1.3 until upstream stabilizes, to avoid silent breakage on cargo update.
SNP_FAMILY_ID = *b"msft\0..." and SNP_IMAGE_ID = *b"underhill\0..." are now duplicated in measurement_diag.rs having moved from signed_measurement/snp.rs. Worth confirming the bytes are byte-identical to the original (looks correct in the diff, but a one-line test asserting these constants would catch a future typo since the values are baked into externally-issued SNP ID blocks).

Test coverage

The test suite is thorough — particularly test_e2e_real_corim_build_and_patch exercising the full add_corim → sign → patch flow. Two additions that would tighten it:

A negative test where the bundle's embedded payload differs from the IGVM-embedded document (covers the failure mode in point 4 above).
A test that asserts the patched IGVM file's IGVM_FIXED_HEADER.checksum (CRC32) actually validates — test_patch_corim_output_is_valid_igvm_header checks magic/version/size but not the checksum.

Petri test failures

The bot reports 27 (5 unstable) Petri failures on the latest push, 38 (15 unstable) on the one before. Worth checking whether any of those are CoRIM/measurement-related vs pre-existing flakiness, and calling that out in the PR.

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.

mingweishih · 2026-06-12T21:03:57Z

Review responses

High-Impact issues:

igvm git pin - Leave as-is
The pinning is required because the PR consumes the upstream CoRIM support that has not been released yet. Will switch back to crate.io version once new release ships.
SNP SevVmsa size loosening - Leave as-is
The zero padding is introduced by upstream igvm (igvm: Update SevVmsa structure definition. Pad to full 4K. igvm#109). There is no impact on the measurement because SNP's calculation always operates on 4k pages.
libssl-dev in build_igvmfilegen.rs - Leave as-is
Kept for consistency with the existing build_vmgstool.rs pattern. Will revisit as a cleanup follow-up.

Behavior nuances worth surfacing

bundle/IGVM-doc mismatch produces an opaque crypto error - Fixed
Add explicit comparison between the CoRIM document from the bundle against the built-in one and provider clearer error message.
re-anchoring CorimDocument at the tail - Comment updated
igvm only enforces that CorimDocument appears before CorimSignature for the same mask. There are no constraints on ordering relative to other init headers.
VBS vs CoRIM algorithm difference. - Won't Fix
The comment is incorrect - VBS and CoRIM signing targets different payload.
test vs prod crypto backend - Won't Fix
Following the existing patterns.

Minor

unreachable! in build_endorsement_json. - Leave as-is
The suggestion requires significant refactoring. Not worth.
Platform CLI enum duplication. - Leave as-is
The duplication is intentional to isolate clap-derive concerns from the upstream IgvmPlatformType type.
pinning corim = "=0.1.3" - Leave as-is
Following the project pattern. The dependency is tracked by Cargo.lock that effectively does the pinning.
SNP_FAMILY_ID/SNP_IMAGE_ID byte-identity - Fixed
Add a test case to prevent any change to the identity values.

Test Coverage

CRC32 round-trip assertion. - Added test cases

Petri failures

failures are unrelated to this PR.

github-actions · 2026-06-12T23:31:54Z

1 Petri tests failed (1 unstable)

Copilot

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.

github-actions · 2026-06-18T02:31:13Z

3 Petri tests failed (2 unstable)

Bump the workspace igvm/igvm_defs dependencies to microsoft/igvm rev e6c3ff1 (current main tip). This pulls in: - PR microsoft#109 which pads `SevVmsa` out to a full 4 KiB page, whereas `x86defs::snp::SevVmsa` produced by the VP context builder is the architectural 1648-byte structure. - PR microsoft#122 which adds CoRIM launch-measurement APIs. - New `AArch64Register` variants (X2-X7) and a new `IgvmDirectiveHeader::AArch64CcaVpContext` variant. Adapt local callers: - igvmfilegen's `import_pages` SNP branch zero-pads the incoming VMSA up to the igvm crate's expected size before calling `SevVmsa::read_from_bytes`, and the strict size check is relaxed to allow any input no larger than the padded size. - `vm/loader` and `vmm_core/vm_loader` mirror the new `AArch64Register` X2-X7 variants. - `openvmm_core`'s IGVM directive matcher adds `AArch64CcaVpContext` arms (todo-stubbed; no callers exercise CCA yet). This is a no-op for current functionality and is a prerequisite for the upcoming CoRIM endorsement support. Signed-off-by: Ming-Wei Shih <mishih@microsoft.com>

Signed-off-by: Ming-Wei Shih <mishih@microsoft.com>

Copilot

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.

github-actions · 2026-06-22T23:16:39Z

1 Petri tests failed (0 unstable)

Copilot AI review requested due to automatic review settings March 20, 2026 22:28

Copilot started reviewing on behalf of mingweishih March 20, 2026 22:29 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 20, 2026 22:45

Copilot started reviewing on behalf of mingweishih March 20, 2026 22:46 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/corim/mod.rs Outdated

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

Copilot AI review requested due to automatic review settings March 24, 2026 18:00

Copilot started reviewing on behalf of mingweishih March 24, 2026 18:02 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

Comment thread vm/loader/igvmfilegen/src/corim/mod.rs Outdated

Comment thread vm/loader/igvmfilegen/src/corim/mod.rs Outdated

Copilot AI review requested due to automatic review settings March 24, 2026 18:15

Copilot started reviewing on behalf of mingweishih March 24, 2026 18:16 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 26, 2026 20:59

Copilot started reviewing on behalf of mingweishih March 26, 2026 21:00 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 26, 2026 21:08

Copilot started reviewing on behalf of mingweishih March 26, 2026 21:10 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/corim_signature/mod.rs

Comment thread vm/loader/igvmfilegen/src/main.rs

Comment thread vm/loader/igvmfilegen/src/corim/cose.rs Outdated

benhillis added the enhancement New feature or request label Apr 6, 2026

Copilot AI review requested due to automatic review settings May 29, 2026 18:46

Copilot started reviewing on behalf of mingweishih May 29, 2026 18:46 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/measurement_diag.rs

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

Comment thread Cargo.toml Outdated

mingweishih force-pushed the igvmfilegen_corim_write branch from 568d310 to 6c10547 Compare May 29, 2026 19:14

Copilot AI review requested due to automatic review settings May 29, 2026 19:24

mingweishih force-pushed the igvmfilegen_corim_write branch from 6c10547 to 6e2ff44 Compare May 29, 2026 19:24

Copilot started reviewing on behalf of mingweishih May 29, 2026 19:24 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 30, 2026 05:12

Copilot started reviewing on behalf of mingweishih May 30, 2026 05:12 View session

Copilot AI reviewed May 30, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/measurement_diag.rs

Comment thread vm/loader/igvmfilegen/src/main.rs

Comment thread Cargo.toml Outdated

mingweishih force-pushed the igvmfilegen_corim_write branch from f77bbff to 9d34a96 Compare June 10, 2026 17:43

Copilot AI review requested due to automatic review settings June 10, 2026 21:13

mingweishih force-pushed the igvmfilegen_corim_write branch from 9d34a96 to 2948e70 Compare June 10, 2026 21:13

Copilot started reviewing on behalf of mingweishih June 10, 2026 21:13 View session

Copilot AI reviewed Jun 10, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/measurement_diag.rs

mingweishih force-pushed the igvmfilegen_corim_write branch from 2948e70 to 992252b Compare June 10, 2026 21:43

mingweishih changed the title ~~Igvmfilegen: Support patch-CoRIM command~~ Igvmfilegen: Support CoRIM generation and patch command Jun 10, 2026

mebersol reviewed Jun 12, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 12, 2026 17:51

mingweishih force-pushed the igvmfilegen_corim_write branch from 992252b to 3a7b319 Compare June 12, 2026 17:51

Copilot started reviewing on behalf of mingweishih June 12, 2026 17:51 View session

Copilot AI reviewed Jun 12, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/corim_signature/mod.rs

Comment thread openvmm/openvmm_core/src/worker/vm_loaders/igvm.rs

Comment thread openvmm/openvmm_core/src/worker/vm_loaders/igvm.rs

Comment thread vm/loader/igvmfilegen/src/main.rs

Comment thread vm/loader/igvmfilegen/src/main.rs

mingweishih force-pushed the igvmfilegen_corim_write branch from 3a7b319 to eaf8b8f Compare June 12, 2026 18:30

mingweishih requested review from Copilot and mebersol June 12, 2026 23:34

mingweishih force-pushed the igvmfilegen_corim_write branch from eaf8b8f to fd8b2ed Compare June 17, 2026 20:48

Copilot started reviewing on behalf of mingweishih June 17, 2026 20:50 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/measurement_diag.rs

Comment thread vm/loader/igvmfilegen/src/main.rs Outdated

mingweishih force-pushed the igvmfilegen_corim_write branch from fd8b2ed to 224346f Compare June 17, 2026 21:00

mingweishih added 2 commits June 22, 2026 22:02

igvmfilegen: Add CoRIM endorsement support

bf3969b

Signed-off-by: Ming-Wei Shih <mishih@microsoft.com>

Copilot AI review requested due to automatic review settings June 22, 2026 22:03

mingweishih force-pushed the igvmfilegen_corim_write branch from 224346f to bf3969b Compare June 22, 2026 22:03

Copilot started reviewing on behalf of mingweishih June 22, 2026 22:04 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread vm/loader/igvmfilegen/src/measurement_diag.rs

		igvm = { git = "https://github.com/mingweishih/igvm", rev = "c389b9f", features = ["corim"] }
		igvm_defs = { git = "https://github.com/mingweishih/igvm", rev = "c389b9f", default-features = false }

Uh oh!

Conversation

mingweishih commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

mingweishih commented Mar 20, 2026 •

edited

Loading