AAEL eventlog replay support for az-snp-vtpm - #1565
Conversation
Decode the az-snp-vtpm evidence's cc_eventlog, parse it into a CcEventLog, build a reference measurement against PCR17 from the TPM quote, and call replay_and_match. Insert the resulting parsed event log as a uefi_event_logs claim, analogous to the CSV verifier. Add an initial_value field to the shared ReferenceMeasurement type in the eventlog crate (default empty/zero) so a caller can seed a register's replay from a non-zero reset value -- used here to seed PCR17's vTPM DRTM reset value (0xFF-repeated). Existing TDX/CSV callers are unaffected; an empty initial_value keeps the prior all-zero seed. Signed-off-by: enrique.larraia@openchip.com <enrique.larraia@openchip.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Read each event's target PCR register from the eventlog itself instead of hardcoding PCR17, and seed the replay per-register (0xFF for DRTM PCRs 17-22, zero otherwise). Azure's vTPM rejects locality-0 extends on PCR17, so real deployments target PCR23 instead, which resets to zero, not 0xFF -- the original wiring was internally consistent but verified the wrong register with the wrong seed for how this is actually deployed. Signed-off-by: Àlex Ramírez <ext_alexandre.ramirez@openchip.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fitzthum
left a comment
There was a problem hiding this comment.
First, please see community AI policy, particularly the sections about PR descriptions and comments. This will help the PR reviewed fsater.
Second, please provide some references for the various claims about which PCRs are used.
Also, can you mention that the AZ TDX verifier already exposes the eventlog tcb claim, but does it not have the same problem that you are targeting in the second commit?
| /// buffer. Some measurement registers (e.g. a vTPM DRTM PCR) reset to a | ||
| /// non-zero value before any event extends them. Leave empty to fall | ||
| /// back to an all-zero seed of the algorithm's digest size. | ||
| pub initial_value: Vec<u8>, |
There was a problem hiding this comment.
Should this be an Option? Also, please provide some documentation for this nonzero reset claim.
There was a problem hiding this comment.
@aramirez-capacity This non-zero case would cause a problem. If allow non-zero initial value, an malwared attester can fake this field with a non-zero value. Also, trim the eventlog items with only later "good parts", and squash previous "bad parts" into a initial_value here.
Now we assume all fields to be zero-initialized, thus no such attack could happen - as all eventlog entries should be included so reflect the PCR final value.
If we want this field, we should let the verifier explicitly know the expected value and check it. One way is to add this into the parsed claims and let the policy cover this. Also, I suggest to change default policy to assert these fields to be all zeros. This prevents users from inadvertently allowing malicious initial PCR values due to a lack of configuration.
There was a problem hiding this comment.
The non-zero initial value is part of the spec: https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf (see Table 7).
It's not an arbitrary value; it's either 0 or -1, depending on the PCR.
I added a mention in the comment.
There was a problem hiding this comment.
Oh ok. I misunderstood this. Thanks for the explanation. Another question is: Is DRTM the always behavior for Azure thus 17~22 be reset to -1?
There was a problem hiding this comment.
Confirmed there's no DRTM launch on Azure's SEV-SNP vTPM (az-snp-vtpm).
Here's a live quote showing PCR23 holds a real measurement while PCR17-22 still contain their initial values (0xFF):
"tpm": {
"pcr00": "13177a6535badf19415c06589705dd5a1890f73545c4a9fef7acfe2c6177a2b7",
"pcr01": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr02": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr03": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr04": "c35ccf52ab2960710d0b0d90fa81596654dbd25cd21912bb26b2debbf5f14209",
"pcr05": "0a3d1a92e6542d7a6bc71d199e5fc37f69a5b302bd0f5b4a1a1b15e7f6fc7683",
"pcr06": "f6a2145d299e1372a74a218637e18fc2f72d791bcd41cead5ffa3697ff64fbeb",
"pcr07": "3b20e022416fdf61d72e4da32b4354781be3de0608116976d28ffdad8c341d2a",
"pcr08": "b6bf8a1bd74334f4a79e1b19002af49f4789eb4b25211b0c86d8f01466f0ef42",
"pcr09": "4282b879e0120876b9f9b3800cf4b72f6eb45178365041223da972fdd07bbbbd",
"pcr10": "484b9766c2d0615ac214cbce90c36a0b81d89e6fa7f485bce3abc36b40fc7165",
"pcr11": "04785baf77f36dee740f41bd6758bd2d0a7b48ef7f732d5d1f6268cba008f0f1",
"pcr12": "f1a142c53586e7e2223ec74e5f4d1a4942956b1fd9ac78fafcdf85117aa345da",
"pcr13": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr14": "306f9d8b94f17d93dc6e7cf8f5c79d652eb4c6c4d13de2dddc24af416e13ecaf",
"pcr15": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr16": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr17": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr18": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr19": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr20": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr21": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr22": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr23": "dd21afeb2b2ee07d6754d2fe9be856813aa023e5875445bc46cb1234b7508d42"
}
There was a problem hiding this comment.
cc @mkulke please help to check if this is expected from white box view.
| /// Decode and replay the AAEL runtime eventlog (if present) against PCR17, | ||
| /// and insert the parsed per-event log as a claim analogous to the CSV/TDX | ||
| /// verifiers' `uefi_event_logs` claim. | ||
| fn extend_eventlog_claim( |
There was a problem hiding this comment.
This doesn't seem like the right name. This function is doing a lot more than simply adding the eventlog to the claims.
There was a problem hiding this comment.
Would something like verify_and_extend_eventlog_claim be better?
There was a problem hiding this comment.
fn names that have contain do_x_and_y are a bit smelly, if possible let's have verify_eventlog() and extend_eventlog_claim().
There was a problem hiding this comment.
Fair. Split the function into verify_eventlog and extend_eventlog_claim.
Xynnn007
left a comment
There was a problem hiding this comment.
Thanks @aramirez-capacity . Before diving into the code, an overall design comment.
Also, could you help to make a separate PR for the document updation? I think it can get merged soon.
| /// buffer. Some measurement registers (e.g. a vTPM DRTM PCR) reset to a | ||
| /// non-zero value before any event extends them. Leave empty to fall | ||
| /// back to an all-zero seed of the algorithm's digest size. | ||
| pub initial_value: Vec<u8>, |
There was a problem hiding this comment.
@aramirez-capacity This non-zero case would cause a problem. If allow non-zero initial value, an malwared attester can fake this field with a non-zero value. Also, trim the eventlog items with only later "good parts", and squash previous "bad parts" into a initial_value here.
Now we assume all fields to be zero-initialized, thus no such attack could happen - as all eventlog entries should be included so reflect the PCR final value.
If we want this field, we should let the verifier explicitly know the expected value and check it. One way is to add this into the parsed claims and let the policy cover this. Also, I suggest to change default policy to assert these fields to be all zeros. This prevents users from inadvertently allowing malicious initial PCR values due to a lack of configuration.
| /// Platform Firmware Profile). These reset to all-0xFF, not all-zero, | ||
| /// before any event extends them -- unlike general-purpose registers |
There was a problem hiding this comment.
I'm curious if you see all-zeros for the measured boot range <16? My understanding is DRTM is not really applicable in this Azure setup.
There was a problem hiding this comment.
Not quite. DRTM is not applicable in Azure, but SRTM is.
Here's a quote that shows a real measurement in PCR23 while keeping PCR17-22 (DRTM) at their default values (0xFF), but PCR00-15 do have real measurements too (SRTM):
"tpm": {
"pcr00": "13177a6535badf19415c06589705dd5a1890f73545c4a9fef7acfe2c6177a2b7",
"pcr01": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr02": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr03": "3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969",
"pcr04": "c35ccf52ab2960710d0b0d90fa81596654dbd25cd21912bb26b2debbf5f14209",
"pcr05": "0a3d1a92e6542d7a6bc71d199e5fc37f69a5b302bd0f5b4a1a1b15e7f6fc7683",
"pcr06": "f6a2145d299e1372a74a218637e18fc2f72d791bcd41cead5ffa3697ff64fbeb",
"pcr07": "3b20e022416fdf61d72e4da32b4354781be3de0608116976d28ffdad8c341d2a",
"pcr08": "b6bf8a1bd74334f4a79e1b19002af49f4789eb4b25211b0c86d8f01466f0ef42",
"pcr09": "4282b879e0120876b9f9b3800cf4b72f6eb45178365041223da972fdd07bbbbd",
"pcr10": "484b9766c2d0615ac214cbce90c36a0b81d89e6fa7f485bce3abc36b40fc7165",
"pcr11": "04785baf77f36dee740f41bd6758bd2d0a7b48ef7f732d5d1f6268cba008f0f1",
"pcr12": "f1a142c53586e7e2223ec74e5f4d1a4942956b1fd9ac78fafcdf85117aa345da",
"pcr13": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr14": "306f9d8b94f17d93dc6e7cf8f5c79d652eb4c6c4d13de2dddc24af416e13ecaf",
"pcr15": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr16": "0000000000000000000000000000000000000000000000000000000000000000",
"pcr17": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr18": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr19": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr20": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr21": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr22": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"pcr23": "dd21afeb2b2ee07d6754d2fe9be856813aa023e5875445bc46cb1234b7508d42"
}
There was a problem hiding this comment.
From confidential-containers/guest-components#1605 I see that writes to PCR17-22 fail. What prevents us to use one of the PCRs that are not used at the moment? Alternatively, we could include full /sys/kernel/security/tpm0/binary_bios_measurements and get the full log the way TDX and CCEL uses replay of all MRs.
There was a problem hiding this comment.
I agree, if we are able to use the standard tcg eventlog, that would be preferable over a custom coco log (if that’s feasible)
There was a problem hiding this comment.
I believe we are getting sidetracked here. Your comments seem to concern the guest-components side of things. This PR has no say in what PCRs are used or what are the contents of the logs. It simply validates the eventlog (be it CCEL or AAEL) and uses the PCRs defined in the log events themselves.
But to answer your questions:
- PCR23 ("Application Support" in the TCG spec) is already the designated unused / general-purpose register, and it's already what Azure's config targets.
- guest-components'
read_eventlog()already tries CCEL first, the same standard mechanism TDX uses, and only falls back to the custom AAEL runtime file when CCEL isn't present. Azure SEV-SNP CVMs just don't expose a CCEL table at all, so there's nothing standard to include there. We don't make an explicit choice to bypass it. - The AAEL already uses the standard TCG eventlog format. The only custom bit is the payload describing an image pull, because CCEL has no such concept.
Still, I think this is a conversation to be had in the guest-components PR: confidential-containers/guest-components#1605
There was a problem hiding this comment.
@aramirez-capacity I believe these two PRs are related so not completely side-tracking here. What's relevant for this PR is the question whether how to handle the replay of 17-22 for Azure vTPMs since they don't seem to work and has impact to initial_value
This is true for the baremetal TDX only. We don't have the confidential computing eventlog available for Azure TDX VMs. |
Added a reference to the spec about the init values for different PCRs. Also added a note to empirical evidence of PCR17 being rejected. Signed-off-by: Àlex Ramírez <ext_alexandre.ramirez@openchip.com>
Created a separate PR for the document: #1568 |
extend_eventlog_claim did both verification (decode, build per-register reference measurements, replay_and_match) and claim mutation, which reads as a do-x-and-y function from its call sites. Split into verify_eventlog() -- decode/parse/replay, returns the verified log or None -- and extend_eventlog_claim() -- pure claim insertion, matching the existing extend_claim/extend_using_custom_claims convention. No behavior change; existing tests updated to call both functions in sequence. Signed-off-by: Àlex Ramírez <ext_alexandre.ramirez@openchip.com>
Xynnn007
left a comment
There was a problem hiding this comment.
Some code structures are common case. I will take a refactor once this get merged.
|
|
||
| // Mirrors guest-components' `EL_HEADER` (attestation-agent/attester/src/utils.rs): | ||
| // a TCG_PCR_EVENT `EV_NO_ACTION` entry whose TCG_EfiSpecIDEvent declares | ||
| // SHA-256 (0x000B, 32 bytes), SHA-384 (0x000C, 48 bytes) and SM3 (0x0012, 32 bytes). | ||
| #[rustfmt::skip] | ||
| const TEST_EL_HEADER: [u8; 73] = [ | ||
| 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x29, 0x00, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, | ||
| 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, | ||
| 0x0B, 0x00, 0x20, 0x00, 0x0C, 0x00, 0x30, 0x00, 0x12, 0x00, | ||
| 0x20, 0x00, 0x00, | ||
| ]; |
There was a problem hiding this comment.
When I see lots of code here are about the eventlog generation, I suspect if it's good to be under the azure snp vtpm verifier part. A possible way, is to include some evidence fixtures with eventlog field Option::Some. And do a simple verify_eventlog rstest matrix here.
There was a problem hiding this comment.
Did the rstest matrix.
Didn't move the helper as neither the TDX nor CSV verifier tests have an equivalent helper today, so I'm not sure how you would like this to be organized.
There was a problem hiding this comment.
Yes. I mean no fixture generation code here, but only leave evidence fixtures in https://github.com/confidential-containers/trustee/tree/main/deps/verifier/test_data/az-snp-vtpm
There was a problem hiding this comment.
Ah, I see. I added the fixtures.
| /// Insert a verified eventlog as a `uefi_event_logs` claim, analogous to | ||
| /// the CSV/TDX verifiers -- a no-op if `ccel` is `None`. |
There was a problem hiding this comment.
We do not need to mention other verifiers here
There was a problem hiding this comment.
Dropped the reference.
|
|
||
| /// Insert a verified eventlog as a `uefi_event_logs` claim, analogous to | ||
| /// the CSV/TDX verifiers -- a no-op if `ccel` is `None`. | ||
| fn extend_eventlog_claim( |
There was a problem hiding this comment.
We could bring this function to https://github.com/confidential-containers/trustee/blob/main/deps/verifier/src/lib.rs as a common helper and use it here.
There was a problem hiding this comment.
Moved the function to lib.rs.
Address review feedback on the az-snp-vtpm eventlog claim: - Move extend_eventlog_claim to the verifier crate root as a shared pub(crate) helper (gated on az-snp-vtpm-verifier, its only caller today), instead of a copy local to az_snp_vtpm. CSV/TDX keep their existing inline inserts -- out of scope here. - Drop the doc comment's comparison to the CSV/TDX verifiers. - Collapse the three near-identical single-register replay tests (DRTM seed success, zero-seed success on the app-support PCR, zero-seed failure on a DRTM PCR) into one rstest case matrix. Signed-off-by: Àlex Ramírez <ext_alexandre.ramirez@openchip.com>
# Conflicts: # attestation-service/docs/tcb_claims.md
Replace the inline AAEL eventlog builder helpers (TEST_EL_HEADER, make_sha256_event, make_aael, make_aael_multi) with three checked-in binary fixtures under test_data/az-snp-vtpm/, loaded via include_bytes! the same way the rest of this test module's evidence fixtures are -- no fixture-generation code left in the crate. Also fix a pre-existing trailing-whitespace-only line in deps/eventlog/src/lib.rs (unrelated to this PR's changes, but it was failing the fmt/clippy/test CI job). Signed-off-by: Àlex Ramírez <ext_alexandre.ramirez@openchip.com>

Wires the
az_snp_vtpmverifier into trustee's generic CoCo eventlog replay, so parsed AAEL events surface as a structureduefi_event_logsclaim — matching what the CSV and TDX verifiers already expose. Previouslyaz_snp_vtpmexposed no eventlog claim at all.The verifier reads each event's target PCR register directly from the eventlog rather than assuming a fixed one, and replays each targeted register independently. A new
initial_valuefield on the sharedReferenceMeasurementtype (deps/eventlog) lets a register seed from a non-zero reset value instead of the default all-zero — used here to seed vTPM DRTM registers (PCR17-22) with their TCG-specified0xFFreset value (see Table 7: https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf). Existing TDX/CSV callers are unaffected (emptyinitial_valuekeeps the old zero-seed behavior).In practice, Azure's vTPM rejects locality-0 extends on PCR17-22 (as discussed here: confidential-containers/guest-components#1605 (comment)), so real AAEL events land on PCR23 instead — reading the target register from the eventlog itself handles this correctly without hardcoding either PCR.
Depends on confidential-containers/guest-components#1605:
az_snp_vtpm'sget_evidence()doesn't currently attach an eventlog at all, so there's nothing to replay without it. Its own hardware validation hit the same PCR17 locality-0 rejection independently.Testing
cargo build / test / clippy / fmt --check (eventlog, verifier --features az-snp-vtpm-verifier); one pre-existing clippy warning in an unrelated, untouched test
new unit tests: register-agnostic PCR replay seeding (DRTM vs. non-DRTM registers); initial_value behavior in the shared eventlog crate
validated end-to-end against real Azure SEV-SNP + vTPM hardware: replay succeeds against both an empty eventlog and a populated one (a real image-pull event on PCR23, digest_matches_event: true)