fix(verify): authenticate TPM quotes and let evidence reach the verifier (#370) - #469
Conversation
…ier (agentrust-io#370) A TPM quote was unauthenticated. The tpm2 branch of verify_trace_claim called only verify_tpm_measurement, which takes no signature parameter, so a forged TPMS_ATTEST with correct magic and a matching qualifying_data was reported as hardware-attested. verify_tpm_quote_chained already existed and was tested but had no production caller. forged attest, byte-identical input: before: hardware_attestation in verified_fields = True after : hardware_attestation in verified_fields = False VERIFIED is gated on hardware_attestation not being unverified, so this is what enforces the issue's "status must not claim hardware-backed verification". Evidence could not reach the verifier either. RuntimeInfo in agentrust-trace is extra="forbid", so a claim carrying raw_evidence / quote_signature / cert_chain under trace.runtime is CLAIM_MALFORMED before the platform branch runs. Signed evidence now travels as gateway.attestation_evidence -- GatewayAddenda is cmcp's own model, documented for fields outside the canonical TRACE spec, so this needs no upstream release. trace.runtime is still read as a fallback for older claims. Both halves land together on purpose: transport without the signature check would turn a reporting bug into acceptance of a forged attestation. verifier verify_ak_ek_chain() in cmcp_verify/tpm.py, mirroring verify_vcek_chain(); trusted_tpm_ca_pem threaded through verify_trace_claim alongside trusted_ark_pem. Supplied-but-invalid material is fatal; absent material degrades to unverified, as SNP does. agentrust-io#370 named _cert_signed_by, which agentrust-io#447 replaced with agent_manifest.verify_cert_chain. producer quote_signature and cert_chain carried from AttestationReport into the claim; non-bytes treated as absent so providers that do not set them keep working. base64 the gateway writes base64url unpadded, the verifier used standard b64decode; the reader now accepts both. AK->EK is deliberately not attempted: an EK is a restricted decryption key, so it cannot sign, cannot issue an AK certificate, and cannot appear in the AK's issuance path. Only EK->manufacturer-CA is a certificate path, supplied as its own chain. This matches the agentrust-io#431 pivot to the platform-certified AK. The TCG EK EKU 2.23.133.8.1 does not identify an EK -- the committed Azure fixture carries it on its issuing CAs, where it means "may issue EK certificates". Identification requires the EKU and ca=False; matching on the EKU alone credited a chain containing no EK. Not covered: the producer still does not capture an EK certificate, and the SEV-SNP cert_chain path has the same transport defect and is not migrated here. Both noted in LIMITATIONS.md. Tests mirror test_snp_signature_verify.py: valid, tampered, wrong-root and missing-chain, plus an end-to-end claim built through the producer path that reaches status: verified. Signed-off-by: Mohammed Zoheb Shaik <zoheb.shaik7@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
imran-siddique
left a comment
There was a problem hiding this comment.
Reviewed the diff and the reasoning. This closes a real hole: the tpm2 branch called only verify_tpm_measurement, which takes no signature, so a forged TPMS_ATTEST with correct magic and matching qualifying_data reached hardware_attestation verified. The A/B in the description is the right way to demonstrate that.
What I checked and agree with:
- Fail-closed shape matches the SNP path. Absent material degrades to unverified, supplied-but-invalid is fatal with
HARDWARE_ATTESTATION_FAILED, andhardware_attestationis only credited when both the measurement and the chain check out. ek_cert_chainrequiring the TCG EKU andca=Falseis correct and non-obvious. Azure carries2.23.133.8.1on its issuing CAs, so EKU-only matching would have reportedek_cert_chainverified for a chain holding no EK. Good that there is a regression test on it.- Not attempting AK to EK as a certificate path is right. An EK is a restricted decryption key and cannot appear in the AK issuance path. Credential activation is the real binding and is correctly left out of scope.
- Carrying evidence in
gateway.attestation_evidencerather thantrace.runtimeis the correct call givenRuntimeInfoisextra="forbid"upstream. - The base64 mismatch catch is a good find. The gateway wrote base64url unpadded while the verifier used standard
b64decode, so the two halves could never have interoperated.
Two non-blocking notes:
_decode_evidencereturnsNoneon undecodable input, so corrupt base64 inquote_signatureis classified as absent rather than supplied-but-invalid. That takes the non-fatal path, which is inconsistent with the principle stated elsewhere in this PR. It cannot lift a forged claim toVERIFIED, since the field still lands in unverified, so it is a reporting nit rather than a hole. Worth distinguishing decode failure from absence in a follow-up._b64's docstring says a report object that does not carry these fields at all must not break claim construction, butreport.quote_signatureandreport.attestation_key_chain_pemare plain attribute reads. Both exist with defaults onAttestationReport, so real providers are fine; only a hand-rolled test double would raise. Either tighten the docstring or usegetattr.
Approving the code. Holding the merge for one decision that is not mine: this breaks old verifiers in the forward direction, since a new claim carrying gateway.attestation_evidence fails extra="forbid" on any verifier built before the field. That needs a version bump and a release note before it ships, which is the piece you correctly flagged as not yours to decide.
Contributes to #370, the TPM half of the residual scope. Does not close it; see Not covered.
Why
A TPM quote was unauthenticated. The
tpm2branch ofverify_trace_claimcalled onlyverify_tpm_measurement, which takes no signature parameter.verify_tpm_quote_chainedalready existed and was tested, but nothing in the production path called it.A/B on byte-identical input, only
verify.pyandtpm.pydiffering:VERIFIEDis gated onhardware_attestationnot being unverified, so this is what enforces the issue's "status must not claim hardware-backed verification". Before this change the forgery was held belowVERIFIEDonly by an unrelated failure, not by any check of the evidence.Evidence also could not reach the verifier.
RuntimeInfoin agentrust-trace isextra="forbid", so a claim carryingraw_evidence/quote_signature/cert_chainundertrace.runtimewasCLAIM_MALFORMEDbefore the platform branch ran.Both halves land together deliberately. Transport on its own would remove the
CLAIM_MALFORMEDthat was incidentally masking the hole, turning a reporting bug into acceptance of a forged attestation.What
Verifier. New
verify_ak_ek_chain()incmcp_verify/tpm.py, mirroringverify_vcek_chain().trusted_tpm_ca_pemthreaded throughverify_trace_claimalongsidetrusted_ark_pem, at the sites named in the issue. Signature verified with the AK pub taken from the chain leaf. Supplied-but-invalid material is fatal (HARDWARE_ATTESTATION_FAILED); absent material degrades tounverified, as SNP does.Transport. Signed evidence travels as
gateway.attestation_evidence.GatewayAddendais cmcp's own model, documented for fields outside the canonical TRACE spec, so no upstream release is needed.trace.runtimeis still read as a fallback for older claims.base64. The gateway wrote base64url unpadded, the verifier used standard
b64decode, so the two halves could not have interoperated. The reader now accepts both. Nothing caught this because no test carried evidence through both sides.A claim built through the claim builder, with evidence populated the way the gateway populates it, now reaches
status: verifiedwithfailure_reason: None.Decisions worth reviewing
Back-compatibility is broken in one direction. Measured both:
This is not caused by the choice of
gateway.GatewayAddendaandRuntimeClaimare bothextra="forbid", so any additive field anywhere in the claim is rejected by a verifier built before it, andverify_trace_claimnever readscmcp_version, so there is no negotiation path. "Evidence travels with the claim" and "old verifiers keep working" cannot both hold under the current models. Options are laid out in the issue comment; this PR implements the first, minus the version bump, which is not mine to decide.AK->EKis not attempted. An EK is a restricted decryption key: it cannot sign, cannot issue an AK certificate, and cannot appear in the AK's issuance path.verify_cert_chainrejects the shape. OnlyEK->manufacturer-CAis a certificate path, implemented as a separately supplied chain. This matches the #431 pivot to the platform-certified AK at0x81000003.EK identification needs the EKU and
ca=False. The TCG EK EKU2.23.133.8.1appears on Azure's issuing CAs (Azure Cloud Virtual TPM CA - 11,... CA 2025), where it means "may issue EK certificates". Matching on the EKU alone creditedek_cert_chainfor a chain containing no EK. Regression test included._cert_signed_byno longer exists. #447 replaced it withagent_manifest.verify_cert_chain, so this mirrorsverify_vcek_chain()rather than reviving a deleted helper.verify_tpm_quote_chainedis left uncalled. The EK chain is a separate path and the links need naming individually, so the wiring usesverify_ak_ek_chainplusverify_quote_signature. Happy to collapse the two into one entry point if preferred.Not covered
verify_sev_snp_measurementreadscert_chainfromtrace.runtime, whichRuntimeInfoforbids, so the VCEK verification merged in fix(verify): verify SNP report signature + VCEK chain (cmcp#370) #386 does not engage for a schema-valid claim either. Not migrated here since it touches merged code. Worth its own issue.SessionManager.close_session()is not driven end to end with real evidence bytes. Both sides of that seam are unit tested; the existing session tests use mocks.The first three are recorded in
LIMITATIONS.md.Tests
Mirrors
test_snp_signature_verify.py: synthetic chains plus a signed attest, exercised valid / tampered / wrong-root / missing-chain, plus an end-to-end claim through the producer path.