[Fikret Can (canfikret)]

Changes

• Add vtpm_util for provisioning file-backed vTPM state using the Microsoft TPM 2.0 reference implementation version 1.38.
• Add commands to create vTPM state blobs, export SRK public material, and calculate TPM key names.
• Generate RSA-2048 keys with DER PKCS#1 public keys and TPM 2.0 import blobs.
• Correct SRK serialization and RSA exponent handling.
• Extend TPM library and protocol definitions needed for state provisioning and key serialization.
• Document supported commands, artifact formats, TPM version compatibility, and handling of private key material.
• Update workspace dependencies and align the implementation with Rust 2024.

Validation

• cargo clippy --all-targets -p cvmutil
• cargo doc --no-deps -p cvmutil
• cargo test -p cvmutil
• cargo xtask fmt --fix

[Copilot]

🟡 Changes recommended

Critical sealing, key export/import, serialization, and simulator correctness issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds cvmutil support for vTPM provisioning, sealed-key workflows, TPM import/export, and socket-based TPM simulation.

Changes:

• Extends TPM protocol and library helpers.
• Adds sealing, unsealing, SRK, and import/export workflows.
• Adds persistent vTPM socket simulator support.
• Registers the new workspace package and dependencies.

File summaries

File	Description
vm/devices/tpm/tpm_protocol/src/tpm20proto.rs	TPM compatibility parsing and structure updates
vm/devices/tpm/tpm_protocol/Cargo.toml	Adds tracing dependency
vm/devices/tpm/tpm_lib/src/lib.rs	TPM helpers, SRK templates, and tests
vm/devices/tpm/tpm_device/src/lib.rs	TPM handle and index constants
vm/cvmutil/src/vtpm_sock_server.rs	Socket-based TPM simulator
vm/cvmutil/src/vtpm_helper.rs	vTPM engine and state callbacks
vm/cvmutil/src/marshal.rs	Sealed-key and AF-split serialization
vm/cvmutil/src/main.rs	CLI workflows and TPM operations
vm/cvmutil/Cargo.toml	New utility package dependencies
Cargo.toml	Workspace membership
Cargo.lock	Dependency lock updates

Review details

Suppressed comments (5)

vm/cvmutil/Cargo.toml:7

• The newly added cvmutil exposes multiple user-facing provisioning, sealing, import/export, and socket-server commands, but there is no corresponding Guide developer-tool page or SUMMARY/mapping entry. This leaves the documented command contract and security-sensitive workflow out of the repository documentation.

[package]
name = "cvmutil"
edition.workspace = true
rust-version.workspace = true

vm/cvmutil/src/main.rs:35

• The new cvmutil command-line tool introduces multiple user-facing commands, but it has no corresponding Guide documentation or entry. Please add a developer-tool/reference page covering the flags, argument order, blob formats, and socket ports, and link it from Guide/src/SUMMARY.md; this is needed to keep the user-facing interface discoverable and synchronized.

#[derive(Parser, Debug)]
#[clap(name = "cvmutil", about = "Tool to interact with vTPM blobs.")]

vm/cvmutil/src/vtpm_sock_server.rs:315

• The Stop command only returns success and breaks the control-client loop; it never changes running, so the server and data listener continue accepting connections. A simulator client requesting Stop cannot actually stop this server.

        ControlCommand::Stop => {
            tracing::info!("TPM Stop requested");
            vec![0x00, 0x00, 0x00, 0x00] // Success
        }

vm/cvmutil/src/vtpm_sock_server.rs:46

• The documented bind address accepts port 65535, but adding one for the control listener overflows u16 and panics. Reject the maximum data port (or choose a separately validated control port) before performing this addition.

    // Parse the bind address to extract host and port
    let (host, data_port) = parse_bind_address(bind_addr);
    let ctrl_port = data_port + 1; // Control port is typically data_port + 1

    tracing::info!("Data port: {}, Control port: {}", data_port, ctrl_port);

vm/cvmutil/src/vtpm_sock_server.rs:355

• The TPM simulator acknowledges MS_SIM_NV_ON with success without changing any TPM state. Clients use this control command to enable NV access before issuing commands; reporting success while leaving NV disabled can make subsequent provisioning fail or behave differently from the advertised simulator protocol. Implement the state transition or return an unsupported/error response.

        ControlCommand::NvOn => {
            tracing::info!("MS_SIM_NV_ON (TPM NV Enable) requested");

            // This is the command that was failing
            // Enable NV storage in the TPM
            // The ms-tpm-20-ref might have specific methods for this

            // For now, acknowledge success
            vec![0x00, 0x00, 0x00, 0x00] // Success

• Files reviewed: 10/11 changed files
• Comments generated: 14
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Two moderate issues remain in destination path handling and public-area length validation.

1 open finding

• Handle bare destination paths with empty parents · New

2 resolved since last review

• Clean up partial files after write failures
• Use atomic temporary writes for key artifacts

Previously missed (1)

In code that hasn't changed since last review

Validate unique length before fixed-size indexing

vm/​vtpm_util/​src/​lib.rs:269

Tpm2bPublic::deserialize accepts any encoded unique length, but this loop unconditionally indexes 256 bytes. A validly parsed SHA-256 public-area file with a short or empty unique field therefore panics instead of returning an error. Validate that this is an RSA-2048 public area (and its modulus length) before indexing, or iterate over the stored slice.

🧠 Review effort: Lite

[Steven Malis (smalis-msft)]

LGTM after updating to the new main commit, the last copilot comment, and fixing whatever CI is unhappy about.

[Copilot]

🟡 Changes recommended

The import blob is malformed and the new library currently fails to compile.

1 open finding

• Malformed TPM2B_PRIVATE serialization omits required length prefix · New

1 resolved since last review

• Handle bare destination paths with empty parents

🧠 Review effort: Lite

[Steven Malis (smalis-msft)]

I think this hit a conflict with some other changes in main, may need a rebase

[Copilot]

🔵 Needs a closer look

Fix the vtpm_util compilation error, add end-to-end import coverage, and correct the validation commands.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Validation commands reference nonexistent cvmutil package

vm/​vtpm_util/​Cargo.toml:5

The validation section names -p cvmutil, but this new workspace package is vtpm_util (and there is no cvmutil package). Please correct the validation commands and rerun clippy, docs, and tests for vtpm_util; otherwise the stated validation does not cover this crate.

1 resolved since last review

• Malformed TPM2B_PRIVATE serialization omits required length prefix

_{Review effort: Lite}

[Copilot]

🟡 Changes recommended

Four unresolved findings remain, including three compilation failures and a critical rollback race.

3 open issues

• Propagate to_components error before accessing key component fields · New
• Rollback can delete an unrelated private key after a TOCTOU race · New
• Add Clap value_enum attribute to KeyAlgorithm · New

_{Review effort: Lite}

[Copilot]

🔵 Needs a closer look

Fix the missing size_of import and add interoperability integration coverage for generated TPM import artifacts.

2 open issues

• Add Clap value_enum attribute to KeyAlgorithm
• Rollback can delete an unrelated private key after a TOCTOU race

1 resolved since last review

• Propagate to_components error before accessing key component fields

_{Review effort: Lite}

[Copilot]

🟡 Changes recommended

An unresolved compile error and missing end-to-end coverage remain, and the validation checklist needs correction.

1 open issue

• Add end-to-end TPM key provisioning test · New

2 resolved since last review

• Add Clap value_enum attribute to KeyAlgorithm
• Rollback can delete an unrelated private key after a TOCTOU race

_{Review effort: Lite}