fix(tee): bind the gateway measurement into SEV-SNP and TDX report_data - #563
Conversation
gateway_measurement() folds installed code, the policy bundle and the effective config into one digest, and agentrust-io#432 extended it into a TPM_NT_EXTEND NV index and had the TPM certify it. That path is validated on real Azure Trusted Launch vTPM hardware, and it ran for the tpm provider only: _measure_gateway returned early for every other provider. The stated reason was that SEV-SNP and TDX commit their own binding through the report's fields. That is true and it is not equivalent. Those fields carry the launch measurement, which is fixed at boot and does not move when the Cedar bundle reloads mid-session through PolicyEvaluator._maybe_reload(). So on exactly the platforms whose premise is hardware-rooted policy enforcement, nothing signed said which policy was running. No new commitment scheme is invented. make_measurement_bound_nonce() puts the already-validated digest into the second half of the attestation nonce, in the same 64-byte layout make_audit_bound_nonce already uses: jwk_thumbprint(pubkey) (32) || measurement_digest (32) gateway_measurement().digest is a raw 32-byte SHA-256, so it drops in unreshaped and a verifier compares it against a digest it recomputes rather than against a hash of one. The tpm provider is deliberately outside the set: its NV index keeps an append-only history that report_data cannot. This replaces the 32 random salt bytes on those providers. Freshness survives for a reason worth stating rather than assuming: the gateway generates a new signing key on every start, so report_data[:32] still differs between two starts of byte-identical code, policy and config. Refreshed on every policy-bundle reload, not only on the reloads that moved the hash. report_data holds one value and no history, so nothing in a report says whether it is current: the same digest signed now and that digest signed an hour ago are different assertions, and only the latest report reaches a verifier. PolicyEvaluator._maybe_reload calls the new on_reload hook whenever PolicyStore.reload_if_stale reports the bundle was re-read, and the hook runs refresh_measurement_binding. The cost is bounded by policy_reload_interval_seconds rather than by request rate, because reload_if_stale stamps its clock before the attempt, and it is zero in the default configuration where reloading is off. Recompute-and-compare is what catches a gateway that did not refresh, so it is a check rather than a note in the spec. verify_trace_claim gains an optional expected_gateway_measurement (raw 32 bytes, hex, or sha256:-prefixed) and a MEASUREMENT_NOT_BOUND failure reason. It is opt-in because the expected digest has to be an out-of-band trust input like ApprovedHashes: a check that read it out of the claim would be asking the claim to vouch for itself. The digest is compared directly rather than re-hashed, which is the one way it differs from the AUDIT-006 check beside it, and a mismatch is fatal in software-only mode too since the digest is computed the same way there. A refresh that cannot re-attest logs and keeps the previous report rather than refusing traffic. Failing closed there would trade a detectable weakness for an outage: stale report_data no longer matches the recomputed digest, so a verifier rejects the claim. That is the same trade AUDIT-006 already makes for a failed per-session attestation. Neither ctx.gateway_measurement nor ctx.attestation_report is touched until the new report is in hand, so a failure at any earlier step leaves both unchanged. They are two assignments rather than one atomic swap, which the docstring now states exactly instead of overclaiming: a concurrent reader could see the new measurement beside the old report for one interpreter step. Benign today because nothing reads the pair together, and recorded so it stops being an accident if something later does. Behaviour change: an unmeasurable gateway on sev-snp, tdx or azure-cvm-sev-snp is now fatal at startup in production, as it has been on tpm since agentrust-io#432, because extending the measurement to a platform extends the consequence of not having one. CMCP_DEV_MODE=1 still downgrades it to a warning, which is what an editable install with no RECORD metadata needs. Known limit, stated rather than implied: the binding is on the gateway's startup report. A TRACE Claim for a session carries the per-session report where one was produced, and AUDIT-006 already commits the audit-chain root in the same report_data[32:64]. So on the normal session path a verifier sees the chain-root commitment, not the measurement, and the new check applies to claims that fall back to the startup report. Carrying both in one 64-byte field is not possible as the layout stands, and agentrust-io#552 scopes the audit-chain-root binding out, so this is left for the issue that takes that on. Also out of scope per the issue: validation on real SEV-SNP and TDX silicon. Tests: 40, software-only by design. SoftwareOnlyProvider echoes the nonce into report_data, which is the collector-side shape SEV-SNP and TDX give, so the round trip proves the contract and leaves the silicon question where the issue leaves it. They cover the five properties prototyped on the issue: a deterministic measurement across repeat calls, a one-character Cedar change moving the policy component and only the policy component, a full round trip whose independent recompute matches both halves of report_data, a mid-session reload producing a report that reflects the new measurement, and a stale pre-reload measurement correctly rejected on recompute. Verified by mutation, twelve guards altered one at a time. Each is killed: measuring only on tpm fails 2 tests; never using the measurement-bound nonce fails 1; dropping the post-reload hook call fails 2; refreshing only when the hash changed, which is the non-literal reading of the second bullet, fails 2; firing the hook on evaluations that did not reload fails 3; letting a failing hook escape fails 1; skipping re-attestation on an unchanged digest fails 1; installing the measurement before the report is in hand fails 1; skipping the nonce length check fails 1; re-hashing the digest verifier-side fails 3; accepting an expected digest of any length fails 1; and treating a software-only mismatch as not applicable fails 1. Three checks were added after auditing the change against the issue text rather than against itself. The reload hook was only ever exercised with reload_if_stale stubbed, so a real PolicyStore over a real on-disk bundle now reloads an unchanged bundle and asserts the hook still fires, which is the whole of the second bullet with nothing mocked on the seam. startup._jwk_thumbprint_sha256 and tee.base.jwk_thumbprint each build report_data[:32] and nothing forced them to agree, so a test does. And all three providers are asserted to place the whole 64-byte nonce in report_data, since the binding is only real if it reaches the field the hardware signs. Closes agentrust-io#552. Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Codecov reported 80% patch coverage on agentrust-io#563, 26 added lines with no test. The gap was not evenly spread and one part of it mattered. The whole of step 7c in verify_trace_claim was untested. _check_measurement_binding had unit tests, but nothing ever passed expected_gateway_measurement through the public entry point, so the wiring could have been broken in either direction and the suite would have stayed green. Six tests now cover it: a match landing in verified_fields, a mismatch landing in unverified_fields with the reason, hex and sha256:-prefixed digests, an unparseable expectation failing closed rather than skipping the check, software-only staying advisory, and no expectation supplied leaving the field absent entirely. _make_signed_claim grows an optional report_data so a caller can bind something other than the audit-chain root. The rest were branches rather than wiring. The verifier's fail-closed paths for an undecodable nonce and a nonce too short to hold a commitment, and the software-only variants of both, which return advisory rather than fatal. The guard that stops a provider returning a non-AttestationReport from displacing a good report, which is the same guard AUDIT-006 uses. And the TPM fault paths in _extend_measurement: tpm2-pytss absent, a MeasurementUnavailable from the extend, and any other TPM fault, degrading in dev mode and aborting in production. Those TPM lines are not new behaviour. They read as added because agentrust-io#552 split _measure_gateway into _measure_gateway and _extend_measurement, and the diff attributes the moved body to this branch. Covering them is still right: they were untested before the split too. Every line agentrust-io#563 adds is now covered. Patch coverage measured locally by intersecting the branch diff with coverage's missing-line report, across all six changed source files: 0 uncovered added lines, down from 26. Suite: 50 tests in test_measurement_report_binding.py plus 6 in test_verify.py. Full unit run 1258 passed, with the same 9 pre-existing agent_manifest SDK failures as before this branch. Ruff and mypy clean. The twelve mutations from agentrust-io#552 are all still killed. Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com>
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Reviewed this properly rather than on the description, because it changes what the hardware signs.
The gap is real and stated correctly. SEV-SNP and TDX committing their own binding through the report's fields is true and is not equivalent, and the reason is exactly the one you give: those fields carry a launch measurement. It is fixed at boot and does not move when PolicyEvaluator._maybe_reload() swaps the Cedar bundle. So the policy actually in force on those three platforms was committed to nothing, while the tpm tier has had this since #432.
The layout choice checks out. make_audit_bound_nonce at tee/base.py:117 is jwk_thumbprint(pubkey) || SHA-256(chain_root); make_measurement_bound_nonce at :135 is jwk_thumbprint(pubkey) || measurement_digest through the same helper. Putting the raw digest in rather than a hash of it is the detail that matters, because it lets a verifier compare against a digest it recomputes instead of against a commitment it cannot open. And replacing the random salt without losing freshness holds for the reason you give: the signing key is per start, so report_data[:32] still differs across two starts of byte-identical code, policy and config.
The collision was the thing I came here to raise, and you had already found it. Both commitments target report_data[32:64] with no domain separation between them. Your Not covered says it plainly, scopes it to the issue that takes on the audit-chain-root binding, and does not pretend the layout resolves it. That is the right call. Recording it as a live constraint is worth more than a partial fix would have been.
Verified rather than taken: ran tests/unit/test_measurement_report_binding.py on a detached worktree, 50 passed, then applied your named mutation reverting _measure_gateway to the tpm-only early return. Exactly 2 failures, test_startup_binds_the_measurement_into_the_nonce_on_sev_snp and test_an_unmeasurable_gateway_aborts_startup_on_sev_snp, which is what your table claims. The mutation numbers are honest.
Three things I want to name because they are decisions, not details.
Re-attesting on an unchanged bundle is right, and your reasoning for it is the load-bearing part: report_data holds one value and no history, so "the digest signed now" and "the digest signed an hour ago" are different assertions that the field cannot distinguish. The narrower reading would leave a report standing for as long as policy happens not to change.
Fail-open on refresh against fail-closed at startup is the right asymmetry, and it is safe for the reason you state rather than by luck: stale report_data stops matching the recomputed digest, so a verifier rejects it. The weakness is detectable, and refusing traffic instead would trade that for an outage.
Making an unmeasurable gateway fatal on three more platforms is the one behaviour change here, and calling it out under Decisions worth reviewing rather than burying it in the changelog is what let me agree with it quickly.
On _VALID_PROVIDERS, thank you, and I am taking it rather than leaving it with you. Confirmed: startup.py:57 holds five names without azure-cvm-sev-snp, audit/trace_claim.py:25 holds six with it, and the comment on line 55 says the first mirrors the second. So startup.py:420 raises ATTESTATION_PROVIDER_INVALID and this PR adds binding for a provider that path cannot reach. Follow-up from me, with a test asserting the two sets agree so they cannot drift apart again.
Approving and merging.
* fix(startup): _VALID_PROVIDERS was missing azure-cvm-sev-snp The comment above _VALID_PROVIDERS says it mirrors the keys of _PROVIDER_MAP in audit/trace_claim.py. It did not: the map carries azure-cvm-sev-snp and the set did not, so _validate_attestation_report raised ATTESTATION_PROVIDER_INVALID on every Azure confidential-VM report and the gateway exited at startup. Found by @zohebk8s while wiring #552's report_data measurement binding in #563, which applies to azure-cvm-sev-snp and so added behaviour for a provider that path could not select. The test compares the two sets rather than asserting one membership, so the next provider added to either side cannot drift the same way. Signed-off-by: Imran Siddique <imran.siddique@opaque.co> * style: satisfy ruff SIM300 in the provider-parity assert ruff reads _VALID_PROVIDERS as the constant side because it is uppercase, so the comparison had to put the computed set on the left. Signed-off-by: Imran Siddique <imran.siddique@opaque.co> --------- Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
4 participants
Closes #552. Both bullets of the proposed change, wired for the three providers the issue names. The report the binding lands on is not the one a session claim carries; see Not covered.
Why
gateway_measurement()folds installed code, the policy bundle and the effective config into one digest. #432 extends it into aTPM_NT_EXTENDNV index and certifies it with aTPM2_NV_Certifypair, validated on real Azure Trusted Launch vTPM hardware. The "what to measure" question is answered and proven.It ran for the
tpmprovider only. Atstartup.py:160,_measure_gatewayreturned early ontee_provider.provider_name() != "tpm".The docstring's reason was that SEV-SNP and TDX commit their own binding through the report's fields. That is true and it is not equivalent. Those fields carry the launch measurement, which is boot-time. It does not move when the Cedar bundle reloads mid-session via
PolicyEvaluator._maybe_reload(). So on SEV-SNP, TDX and Azure CVM, the policy actually in force was committed to nothing.What
No new commitment scheme. The existing, already-validated digest goes into those platforms'
report_data.make_measurement_bound_nonce(tee_public_key, measurement_digest)intee/base.py, using the layoutmake_audit_bound_noncealready uses:gateway_measurement().digestis already a raw 32-byte SHA-256, so it drops into the second half unreshaped and a verifier compares it against a digest it recomputes rather than against a hash of one. It sits besidemake_audit_bound_nonceand reuses the samejwk_thumbprinthelper, so the first half is literally the same code path.Applied to
sev-snp,tdxandazure-cvm-sev-snp. All three place the whole 64-byte nonce inreport_data. SEV-SNP and TDX write it straight into REPORT_DATA and REPORTDATA. Azure CVM cannot, since the paravisor owns SNP REPORT_DATA, and commitssha256(nonce)into the AK-signed quote instead, which is whatcmcp_verify.azure_cvmalready re-derives.tpmkeeps the random salt: its NV index has an append-only history thatreport_datadoes not.Freshness survives losing the salt. The gateway generates a new signing key on every start, so
report_data[:32]still differs between two starts of byte-identical code, policy and config.Refreshed on every policy-bundle reload, not only the reloads that moved the hash.
report_dataholds one value and no history, so nothing in a report says whether it is current: the same digest signed now and that digest signed an hour ago are different assertions, and only the latest report reaches a verifier.PolicyEvaluator._maybe_reloadcalls a newon_reloadhook wheneverPolicyStore.reload_if_stalereports the bundle was re-read. The cost is bounded bypolicy_reload_interval_secondsrather than by request rate, becausereload_if_stalestamps its clock before the attempt, and it is zero in the default configuration where reloading is off.Recompute-and-compare is a check, not a note in the spec.
verify_trace_claimgains an optionalexpected_gateway_measurement(raw 32 bytes, hex, orsha256:-prefixed) and aMEASUREMENT_NOT_BOUNDfailure reason, as step 7c. Opt-in, because the expected digest has to be an out-of-band trust input likeApprovedHashes: a check that read it out of the claim would be asking the claim to vouch for itself.Tests
40, software-only by design.
SoftwareOnlyProviderechoes the nonce intoreport_data, which is the collector-side shape SEV-SNP and TDX give, so the round trip proves the contract and leaves the silicon question where the issue leaves it.The five properties prototyped on the issue: a deterministic measurement across repeat calls, a one-character Cedar change moving the policy component and only the policy component, a full round trip whose independent recompute matches both halves of
report_data, a mid-session reload producing a report that reflects the new measurement, and a stale pre-reload measurement correctly rejected on recompute.Verified by mutation, twelve guards altered one at a time, each killed. Restoring the narrower "refresh only when the hash changed" reading fails 2, re-hashing the digest verifier-side fails 3, and reverting to the tpm-only early return fails 2.
Three checks exist because the change was audited against the issue text rather than against itself:
reload_if_stalestubbed, so a realPolicyStoreover a real on-disk bundle now reloads an unchanged bundle and asserts the hook still fires, which is the second bullet with nothing mocked on the seamstartup._jwk_thumbprint_sha256andtee.base.jwk_thumbprinteach buildreport_data[:32]and nothing forced them to agree, so a test doesreport_data, since the binding is only real if it reaches the field the hardware signsFull suite: 1242 passed, 9 skipped. Ruff and mypy clean. Nine failures are pre-existing and unrelated, all from the installed
agent_manifestSDK in my environment (module 'agent_manifest' has no attribute 'COSE_MANIFEST_VERSION');tests/unit/test_agent_manifest_cose.pyandtests/unit/test_intent_binding.pyfail to collect for the same reason.Decisions worth reviewing
An unmeasurable gateway is now fatal at startup on these three providers, in production, as it has been on
tpmsince #432. Extending the measurement to a platform extends the consequence of not having one.CMCP_DEV_MODE=1still downgrades it to a warning, which is what an editable install with noRECORDmetadata needs. This will stop an existing SEV-SNP deployment that runs an editable install without dev mode, so it is the one behaviour change here.A refresh that cannot re-attest is fail-open, unlike startup. It logs and keeps the previous report rather than refusing traffic. Failing closed there would trade a detectable weakness for an outage: stale
report_datano longer matches the recomputed digest, so a verifier rejects the claim. This is the same trade AUDIT-006 already makes for a failed per-session attestation inSessionManager.create_session.Re-attesting on an unchanged bundle is deliberate, and it is the literal reading of the second bullet. The narrower reading, refreshing only when the hash moved, would leave the last report standing for as long as policy happens not to change, which is exactly the staleness the absent history cannot express.
ctx.gateway_measurementandctx.attestation_reportare two assignments, not one atomic swap. Neither is touched until the new report is in hand, so a failure at any earlier step leaves both unchanged, but a concurrent reader could see the new measurement beside the old report for one interpreter step. Benign today because nothing reads the pair together; the docstring records it rather than overclaiming.Not covered
report_data[32:64]. So on the normal session path a verifier sees the chain-root commitment, not the measurement, and step 7c applies to claims that fall back to the startup report. Both cannot occupy one 64-byte field as the layout stands. tee: bind the gateway measurement into SEV-SNP and TDX report_data, refreshed on policy reload #552 scopes the audit-chain-root binding out, so this is left for the issue that takes that on rather than resolved either way here._VALID_PROVIDERSinstartup.pyis missingazure-cvm-sev-snp, though the value is present in both_ALLOWED_PROVIDERSand_PROVIDER_MAP. A gateway that auto-detects the Azure confidential-VM provider hitsATTESTATION_PROVIDER_INVALIDand exits. Not touched here; happy to raise it separately.