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

podvm: truncate initdata digest to 32 bytes on az #2186

Merged

Conversation

mkulke
Copy link
Collaborator

@mkulke mkulke commented Dec 5, 2024

depends on #2187 to be merged first

According to initdata spec the digest needs to be truncated/padded according to the requirements of the TEE. for az tpm we use the sha256 bank of TPM for initdata.

This will fix a bug when a initdata body with alg=sha384+ was used and the PCR8 value in the TEE evidence will not be extended, since you cannot extend sha256 w/ a digest that's bigger than 32 bytes.

An e2e test for azure was added to assert this behaviour.

@mkulke mkulke requested a review from a team as a code owner December 5, 2024 12:02
@mkulke mkulke force-pushed the mkulke/fix-sha384-initdata-in-az branch from 8c10d90 to 864a014 Compare December 5, 2024 12:33
@mkulke mkulke marked this pull request as draft December 5, 2024 12:34
@mkulke mkulke force-pushed the mkulke/fix-sha384-initdata-in-az branch 2 times, most recently from 4333a0c to f106d5c Compare December 5, 2024 20:51
@mkulke mkulke marked this pull request as ready for review December 5, 2024 20:52
Copy link
Member

@bpradipt bpradipt left a comment

Choose a reason for hiding this comment

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

/lgtm

@mkulke mkulke force-pushed the mkulke/fix-sha384-initdata-in-az branch from f106d5c to a95db14 Compare December 6, 2024 15:48
According to initdata spec the digest needs to be truncated/padded
according to the requirements of the TEE. for az tpm we use the sha256
bank of TPM for initdata.

This will fix a bug when a initdata body with alg=sha384+ was used and
the PCR8 value in the TEE evidence will not be extended, since you
cannot extend sha256 w/ a digest that's bigger than 32 bytes.

An e2e test for azure was added to assert this behaviour.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
@mkulke mkulke force-pushed the mkulke/fix-sha384-initdata-in-az branch from a95db14 to eb0e87b Compare December 6, 2024 15:49
@mkulke mkulke requested a review from Copilot December 9, 2024 08:48

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 4 changed files in this pull request and generated no suggestions.

Files not reviewed (2)
  • src/cloud-api-adaptor/podvm-mkosi/mkosi.skeleton/usr/lib/systemd/system/afterburn-checkin.service.d/10-override.conf: Language not supported
  • src/cloud-api-adaptor/podvm-mkosi/mkosi.skeleton/usr/lib/systemd/system/process-user-data.service.d/10-override.conf: Language not supported
@stevenhorsman stevenhorsman added the test_e2e_libvirt Run Libvirt e2e tests label Dec 9, 2024
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me.

@mkulke mkulke merged commit 99f24dd into confidential-containers:main Dec 10, 2024
43 checks passed
@mkulke mkulke deleted the mkulke/fix-sha384-initdata-in-az branch December 10, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_e2e_libvirt Run Libvirt e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants