Skip to content

Conversation

@stunes-ms
Copy link
Contributor

HCL-provisioned vTPMs have a platform-defined AKCert index and the AKCert has to be renewed by OpenHCL. We were planning on using control flags (see, e.g., PR #2036) to control whether OpenHCL will renew the AKCert, but it needs to run on older host OS versions that don't support these flags.

As an alternative, we can detect that the existing vTPM has a platform-defined AKCert index and is 32k in size, which indicates that the vTPM was provisioned by HCL and that OpenHCL should handle renewing the AKCert.

@stunes-ms stunes-ms requested a review from a team as a code owner October 30, 2025 00:12
@stunes-ms stunes-ms requested review from Copilot and tjones60 October 30, 2025 00:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR changes the TpmAkCertType::TrustedPreProvisionedOnly variant from a unit variant to a tuple variant that holds a RequestAkCert helper. This enables OpenHCL to renew AK certificates for host-attested vTPMs when the existing AKCert index is platform-defined (which prevents guests from renewing it themselves).

Key changes:

  • Modified TrustedPreProvisionedOnly to carry a RequestAkCert resource similar to other AK cert types
  • Added logic to detect platform-created AKCert indices and enable automatic renewal
  • Introduced always_renew_ak_cert flag to handle cases where platform-defined indices require renewal from OpenHCL

Reviewed Changes

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

Show a summary per file
File Description
vm/devices/tpm_resources/src/lib.rs Changed TrustedPreProvisionedOnly to hold a Resource<RequestAkCertKind>
vm/devices/tpm/tpm_lib/src/lib.rs Added has_platform_akcert_index() to check if AKCert index is platform-created
vm/devices/tpm/tpm_device/src/resolver.rs Updated resource resolution to handle the new TrustedPreProvisionedOnly tuple variant
vm/devices/tpm/tpm_device/src/lib.rs Added always_renew_ak_cert flag and logic to enable renewal for platform-defined indices
vm/devices/tpm/tpm_device/src/ak_cert.rs Changed TrustedPreProvisionedOnly to tuple variant and updated pattern matches
openhcl/underhill_core/src/worker.rs Updated to pass request_ak_cert resource when creating TrustedPreProvisionedOnly

@github-actions
Copy link

.attempt_ak_cert_callback() =>
{
TpmAkCertTypeResource::Trusted(request_ak_cert)
TpmAkCertTypeResource::Trusted(request_ak_cert, Some(true))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there be a case of Some(false)? If not we can just make this as bool without Option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still open, but we might want to use control flags to explicitly disable AKCert renewal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fits into Trevor's comment above with our older scheme of using two control bits:

  • Bit 1: whether Bit 2 controls AKCert renewal (this would be expressed with None or Some)
  • Bit 2: whether OpenHCL does AKCert renewal (this would be Some(true) or Some(false))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good from the TPM side, do you mind also basically reverting this commit to re-add the control bit to the dps_json? 4ebc209 I can also do it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Just pushed into this PR.

@github-actions
Copy link

mingweishih
mingweishih previously approved these changes Oct 31, 2025
@mingweishih
Copy link
Contributor

Do we want to remove the command-line opt-in (

.with_openhcl_command_line("HCL_ATTEMPT_AK_CERT_CALLBACK=1")
) in vmm tests as part of this PR to get test coverage?

cc @tjones60

@tjones60
Copy link
Contributor

tjones60 commented Nov 4, 2025

Do we want to remove the command-line opt-in (

.with_openhcl_command_line("HCL_ATTEMPT_AK_CERT_CALLBACK=1")

) in vmm tests as part of this PR to get test coverage?
cc @tjones60

Hmm probably, since that will be how it will work in production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants