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

attester: tdx: strip CCEL #575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mythi
Copy link
Contributor

@mythi mythi commented Jun 10, 2024

Fixes: #569

The CCEL log is made available through an ACPI sysfs entry and is of size "log_area_minimum_length". OVMF sets it to 64k.

The current tdx-attester code reads the whole blob and it's used as is in encoding and when sent over the wire.

Test runs suggests that it could be beneficial to strip the log before processing it further:

Squeezed from 65536 to 5064 bytes

The stripping follows the same pattern as what eventlog-rs does on the receiving end (we keep the same "stop flag" in the blob to keep things compatible).

@mythi mythi marked this pull request as ready for review October 31, 2024 07:35
@mythi mythi requested a review from a team as a code owner October 31, 2024 07:35
@mythi
Copy link
Contributor Author

mythi commented Oct 31, 2024

I had forgotten this. It's tested and ready now:

[2024-10-31T07:34:34Z INFO  verifier::tdx] CCEL integrity check succeeded. 

Copy link
Member

@dcmiddle dcmiddle left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Thanks for the patch and this would make great sense to efficiency. This is the producer side code. I wonder if any unit tests with ccels that produced by this patch could be added to the verifier side?

@mythi
Copy link
Contributor Author

mythi commented Nov 1, 2024

It's possible to update CCEL_data blob pretty easily if needed.

The CCEL log is made available through an ACPI sysfs
entry and is of size "log_area_minimum_length". OVMF
sets it to 64k.

The current tdx-attester code reads the whole blob and
it's used as is in encoding and when sent over the wire.

Test runs suggests that it could be beneficial to strip the
log before processing it further:

Squeezed from 65536 to 5064 bytes

The stripping follows the same pattern as what eventlog-rs
does on the receiving end (we keep the same "stop flag"
in the blob to keep things compatible).

Signed-off-by: Mikko Ylinen <[email protected]>
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.

tdx-attester: strip CCEL before adding it to the evidence
3 participants