-
Notifications
You must be signed in to change notification settings - Fork 21.8k
Update guest-attestation-confidential-virtual-machines-design.md #127198
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
Update guest-attestation-confidential-virtual-machines-design.md #127198
Conversation
Correct the formatting of the Attestation Report (stored in NV index 0x01400001) and the Runtime Data.
@hyperfinitism : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change. @Mishih |
Learn Build status updates of commit 0c63041: ✅ Validation status: passed
For more details, please refer to the build report. |
@mingweishih - Can you review the proposed changes? IMPORTANT: When the changes are ready for publication, adding a #label:"aq-pr-triaged" |
TestWithin Azure CVM (SEV-SNP), the following commands were run to extract and dump the start of the report binary: sudo tpm2_nvread -C o 0x01400001 > ./report.bin
xxd -g1 -l 32 -s 1216 report.bin Results
Correct Report Format
|
The proposed changes is actually the same as the original format. Not clear on why we need this . |
Thanks for the quick review! Differences
Report format in the current document
Correct report format
|
Runtime Claims’s offset is 1232, which is correct. The confusion here is that the size of the claims is in the first 4 bytes of the claim structure (which is the same as your change from the overall layout). |
ProblemThe Attestation Report table currently mis‐labels one field and omits another, so three different byte counts are conflated.
Note that in the current documentation, the Runtime Claims field starts at byte 1232 and represents Runtime Claims in JSON Format. There is no mention of the first 4 bytes of this field representing the size of Runtime Claims. Moreover, the current documentation describes the Data Size field as the size of Runtime Claims; this field actually represents the size of Runtime Data (minus Data Size field). Why it matters
Fix (this PR)
|
Thanks for the clarification. In this case can you just update the Runtime Data table? That is, retain the 0 - 4 as data size, and add the claim size field after hash type. |
…ormat table Reverted edits in the Attestation Report Format table per reviewer’s request, and applied all necessary updates to the Runtime Data Format table only.
I have addressed your suggestion. I've reverted the changes to the Attestation Report Format table and restored it to its original form. All edits now only apply to the Runtime Data Format table, per your request. Please let me know if there’s anything else you’d like adjusted! |
Learn Build status updates of commit 0753a16: ✅ Validation status: passed
For more details, please refer to the build report. |
@mingweishih Could you review this proposed update to your article and enter Thanks! |
#sign-off |
Invalid command: '#sign-off'. Only the assigned author of one or more file in this PR can sign off. @Mishih |
Correct the formatting of the Attestation Report (stored in NV index 0x01400001) and the Runtime Data.