Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

az-snp/tdx-vtpm-verifier: add PCRs to claims map #334

Merged

Conversation

mkulke
Copy link
Contributor

@mkulke mkulke commented Feb 21, 2024

PCR values are added in a "tpm": { "pcr0": ..., "pcrN": ... } hierarchy to the claims map so they can be compared to reference values.

@mkulke mkulke requested a review from sameo as a code owner February 21, 2024 13:48
@mkulke mkulke marked this pull request as draft February 21, 2024 13:48
@mkulke mkulke added the test_e2e Authorize TEE e2e test run label Feb 21, 2024
@mkulke mkulke force-pushed the mkulke/add-pcrs-to-vtpm-claim branch from bbf9843 to e832896 Compare February 23, 2024 14:46
@mkulke mkulke marked this pull request as ready for review February 23, 2024 14:48
};

let mut tpm_values = serde_json::Map::new();
for (i, pcr) in quote.pcrs_sha256().enumerate() {
Copy link
Member

@portersrc portersrc Feb 23, 2024

Choose a reason for hiding this comment

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

Where's pcrs_sha256 coming from? I couldn't track it down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@portersrc

https://docs.rs/az-snp-vtpm/latest/az_snp_vtpm/vtpm/struct.Quote.html#method.pcrs_sha256

those are the measurement digests from the CVM's vTPM. The launch measurements in TD/SNP reports do not tell us much beyond the HW state of the guest here, since the reports are fetched at early boot. Measurement of e.g. kernel, initrd, rootfs and more is deferred to the TPM registers. So we will have reference values for, say quote.tpm.pcr11, to verify the UKI of the confidential guest.

Copy link
Member

@portersrc portersrc Feb 28, 2024

Choose a reason for hiding this comment

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

Thanks @mkulke, that info is helpful. Yeah I was sanity-checking the element iteration/ordering when I asked this question, I believe. LGTM, nice PR.

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

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

LGTM. Nice PR!

@mkulke mkulke force-pushed the mkulke/add-pcrs-to-vtpm-claim branch from e832896 to 7b908df Compare February 26, 2024 10:48
Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

LGTM. I've merged confidential-containers/guest-components#486

Do we need to bump the version in the client Cargo file to pick that up?

attestation-service/docs/parsed_claims.md Outdated Show resolved Hide resolved
@mkulke mkulke force-pushed the mkulke/add-pcrs-to-vtpm-claim branch from 7b908df to 8c5c470 Compare February 28, 2024 12:35
PCR values are added in a `"tpm": { "pcr0": ..., "pcrN": ... }`
hierarchy, to the claims map so they can be compared to reference
values.

Signed-off-by: Magnus Kulke <[email protected]>
@mkulke mkulke force-pushed the mkulke/add-pcrs-to-vtpm-claim branch from 8c5c470 to d155be0 Compare February 28, 2024 12:37
@mkulke mkulke merged commit 497a63a into confidential-containers:main Feb 28, 2024
17 checks passed
@mkulke mkulke deleted the mkulke/add-pcrs-to-vtpm-claim branch February 28, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_e2e Authorize TEE e2e test run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants