vaportpm-verify: harden cert-chain validation + guard trust anchor#8
Open
HarryR wants to merge 1 commit into
Open
vaportpm-verify: harden cert-chain validation + guard trust anchor#8HarryR wants to merge 1 commit into
HarryR wants to merge 1 commit into
Conversation
Adversarial review follow-ups on the attestation verification path: - Enforce signatureAlgorithm consistency (RFC 5280 4.1.1.2): each cert's outer signatureAlgorithm must equal its inner tbsCertificate.signature, closing an algorithm-substitution gap since the verify alg is chosen from the outer field. New ChainValidationReason::SignatureAlgorithmMismatch. - Require the Key Usage extension on CA certs (not just leaves), removing the leaf/CA asymmetry. BasicConstraints.cA remains the authoritative CA gate; KU is defence in depth. New ChainValidationReason::CaMissingKeyUsage. - Add root-anchor consistency tests: re-derive AWS_NITRO_ROOT_HASH and GCP_EKAK_ROOT_HASH from the embedded root PEMs and assert equality, so the precomputed constants (kept hardcoded to save zkVM circuit cycles) can never silently drift from the certs they represent. - Docs/comments: correct validate_tpm_cert_chain to match enforcement (EKU is intentionally unchecked); document why extract_basic_constraints hand- parses a minimal ASN.1 subset; document the SHA-256 pcrDigest invariant in verify_pcr_digest_matches (holds even for SHA-384 PCR banks). Adds negative tests for both new checks. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from an adversarial review of the attestation verification path (the single point of trust for the zkVM attestation flow). All changes are in
vaportpm-verify.Changes
Enforce
signatureAlgorithmconsistency (RFC 5280 §4.1.1.2)Each certificate's outer
signatureAlgorithmmust equal its innertbsCertificate.signature. The chain validator selects the verification algorithm from the outer field but verifies over the TBS bytes (which carry the inner field), so requiring equality closes an algorithm-substitution gap between the two. NewChainValidationReason::SignatureAlgorithmMismatch.Require Key Usage on CA certificates (not just leaves)
Previously a non-leaf cert with no Key Usage extension skipped the
keyCertSigncheck entirely (RFC 5280 §6.1.4(n) only mandates the check when the extension is present). We now require the extension on every cert, removing the leaf/CA asymmetry.BasicConstraints.cAremains the authoritative CA gate; Key Usage is defence in depth. NewChainValidationReason::CaMissingKeyUsage.Guard the trust anchor against drift
AWS_NITRO_ROOT_HASH/GCP_EKAK_ROOT_HASHare kept hardcoded (deriving them in-guest costs significant zkVM circuit cycles), which means nothing at runtime checks they match the certs they represent. Added tests that re-derive each hash from the embedded root PEM and assert equality, turning silent drift into a CI failure. Both pass today — the constants are in sync.Docs / comments brought in line with enforcement
validate_tpm_cert_chaindoc corrected: EKU is intentionally not checked (GCP AK leaves carry only Key Usage;BasicConstraints.cAis authoritative).extract_basic_constraintshand-parses a minimal ASN.1 subset (circuit size + smaller, auditable trust surface; fail-closed by construction).pcrDigestinvariant inverify_pcr_digest_matches(holds even for SHA-384 PCR banks, because the digest follows the AK signing-scheme hash, not the bank algorithm) — with a "do not change" warning.Tests
Negative tests added for both new checks (
test_reject_signature_algorithm_mismatch,test_reject_ca_without_key_usage) plus the two anchor-consistency tests. Full suite green: 103 lib + 16 GCP + 13 Nitro.🤖 Generated with Claude Code