TPM: Update TPM 1.85 - #4472
TPM: Update TPM 1.85#4472
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Update the SymCrypt feature configuration and Nix dependency pin before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates TPM 1.85 dependencies and OpenVMM artifacts for full SymCrypt support.
Changes:
- Bumps the TPM dependency revision.
- Updates
openvmm-depsto0.3.0-141. - Refreshes the lockfile revision.
File summaries
| File | Summary |
|---|---|
flowey/flowey_lib_hvlite/src/_jobs/cfg_versions.rs |
Updates the dependency artifact version; Nix remains pinned to the older release. |
Cargo.toml |
Updates the TPM revision; consumer feature selections still require adjustment for SymCrypt. |
Cargo.lock |
Records the updated TPM dependency revision. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
TPM 1.85 builds still use OpenSSL configuration, so full SymCrypt support is not yet wired.
Review details
Suppressed comments (1)
Cargo.toml:573
- This revision adds the complete SymCrypt backend, but the repository still configures
ms-tcg-tpm-syswithfeatures = ["openssl"]for both musl and non-musl OpenHCL builds (openhcl/openvmm_hcl/Cargo.toml:52,56), and only providestcg-tpm-openssllibrary paths. As a result, the builds and tests continue using OpenSSL and this PR does not deliver the stated full SymCrypt support. Please wire the SymCrypt feature and matching library paths for TPM 1.85 (while keeping TPM 1.38 on OpenSSL if required).
ms-tcg-tpm-sys = { git = "https://github.com/microsoft/ms-tcg-tpm-sys.git", rev = "259c64582d942e70bbd8a575f08e41a2eef85852", default-features = false }
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical Hyper-V compatibility and moderate SymCrypt feature-selection issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Cargo.toml:573
- This revision makes
ms-tcg-tpm-sys'ssymcryptfeature fully independent of OpenSSL, but this repository still selectsfeatures = ["openssl", ...]for the TPM 1.85 dependency inopenvmmandopenvmm_hcl, and the musl build configuration still resolvestcg-tpm-openssl. As a result, no OpenVMM/OpenHCL build exercises the full SymCrypt TPM backend described by the PR; update the target-specific feature selection and matching prebuilt-library paths (while retaining OpenSSL for TPM 1.38), or add an explicit build/test target for it.
ms-tcg-tpm-sys = { git = "https://github.com/microsoft/ms-tcg-tpm-sys.git", rev = "259c64582d942e70bbd8a575f08e41a2eef85852", default-features = false }
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Hyper-V TPM version selection remains disabled, so the requested versions are not applied.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
SymCrypt is not enabled by current consumers, and Hyper-V TPM-version flags are not yet propagated to test VMs.
Review details
Suppressed comments (2)
Cargo.toml:573
- The new
ms-tcg-tpm-sysrevision adds the SymCrypt backend, but the in-tree consumers still selectfeatures = ["openssl", ...](openvmm/openvmm/Cargo.toml:50andopenhcl/openvmm_hcl/Cargo.toml:52), and the build environment still points attcg-tpm-openssl. As a result, the updated dependency is not actually used with SymCrypt, so this does not provide the full SymCrypt support stated in the PR description; switch the relevant OpenHCL build to the SymCrypt feature and matching artifact/library plumbing, or update the description.
ms-tcg-tpm-sys = { git = "https://github.com/microsoft/ms-tcg-tpm-sys.git", rev = "90d23db6e415fa7f88d917b448f46f2e6a90ca56", default-features = false }
petri/src/vm/hyperv/powershell.rs:571
- With this setter commented out,
PetriVmBuilder::with_tpm_version(V138/V185)is never propagated to Hyper-V; thetpm138matrix still runs Hyper-V cases, but those VMs use Hyper-V's default TPM version rather than the version requested by the test. This leaves the newly added flags unused and makes the Hyper-V TPM-version coverage incorrect. Please either wire the flags for supported host/UEFI combinations or gate the affected Hyper-V cases until the ARM UEFI fix is available.
//.with_tpm_version(tpm.as_ref().map(|t| t.version))
- Files reviewed: 8/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
SymCrypt wiring and Hyper-V TPM version test coverage remain incomplete.
Review details
Suppressed comments (3)
Cargo.toml:573
- This revision adds a SymCrypt feature to
ms-tcg-tpm-sys, but the workspace still selectsfeatures = ["openssl"]for this dependency in bothopenhcl/openvmm_hcl/Cargo.tomlandopenvmm/openvmm/Cargo.toml, and the prebuilt TCG library paths are stilltcg-tpm-openssl. Consequently the TPM 1.85 backend remains OpenSSL even in the existing SymCrypt build jobs, so this does not deliver the full SymCrypt support described by the PR. Please wire the dependency and its prebuilt-library environment through the SymCrypt configuration, or narrow the PR description/scope.
ms-tcg-tpm-sys = { git = "https://github.com/microsoft/ms-tcg-tpm-sys.git", rev = "90d23db6e415fa7f88d917b448f46f2e6a90ca56", default-features = false }
openhcl/underhill_attestation/src/lib.rs:2004
test_attestation_configduplicates the existingnew_attestation_vm_confighelper below at lines 2148-2163 with the same field values. Keeping two copies makes futureAttestationVmConfigtest defaults easy to desynchronize; reuse the existing helper and update these four call sites instead of adding a second copy.
fn test_attestation_config() -> AttestationVmConfig {
AttestationVmConfig {
current_time: None,
root_cert_thumbprint: String::new(),
console_enabled: false,
petri/src/vm/hyperv/powershell.rs:571
- The requested TPM version is still discarded on the Hyper-V path:
from_configis the only place this backend consumesconfig.tpm.version, and this call remains commented. Consequently the Hyper-V TPM 1.85 tests also reach Underhill with both TPM-default bits clear, whichunderhill_core/src/worker.rs:1882-1890maps to TPM 1.38; the 1.38 tests likewise do not verify their requested version. Until the ARM UEFI fix makes this safe, these tests should be gated/disabled or the version flags should be plumbed through on supported hosts so the suite does not report coverage for the wrong TPM implementation.
// TODO Once UEFI is fixed
//.with_tpm_version(tpm.as_ref().map(|t| t.version))
- Files reviewed: 8/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
dd14e12
into
microsoft:main
This will bring full symcrypt support to the internal repo.
This also comes with setting the TPM version through Hyper-V petri tests, and a fix to flowey for a renamed artifact. The Hyper-V bit is commented out for now due to an ARM UEFI bug, will be uncommented once that fix is ingested.