From 04edf70aa046994733f49ac7c8cb542add12496f Mon Sep 17 00:00:00 2001 From: HarryR <303926+HarryR@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:56:06 +0000 Subject: [PATCH] vaportpm-verify: harden cert-chain validation + guard trust anchor 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) --- crates/vaportpm-verify/src/error.rs | 6 ++ crates/vaportpm-verify/src/lib.rs | 46 ++++++++++ crates/vaportpm-verify/src/tpm.rs | 7 ++ crates/vaportpm-verify/src/x509.rs | 134 ++++++++++++++++++++++++++-- 4 files changed, 188 insertions(+), 5 deletions(-) diff --git a/crates/vaportpm-verify/src/error.rs b/crates/vaportpm-verify/src/error.rs index 3ad03bb..377fcad 100644 --- a/crates/vaportpm-verify/src/error.rs +++ b/crates/vaportpm-verify/src/error.rs @@ -242,9 +242,15 @@ pub enum ChainValidationReason { #[error("Leaf certificate missing Key Usage extension")] LeafMissingKeyUsage, + #[error("Certificate {index} (CA) missing Key Usage extension")] + CaMissingKeyUsage { index: usize }, + #[error("Certificate {index} issuer does not match parent subject")] IssuerMismatch { index: usize }, + #[error("Certificate {index} outer signatureAlgorithm does not match inner tbsCertificate.signature")] + SignatureAlgorithmMismatch { index: usize }, + #[error("Certificate {index} signature verification failed")] SignatureVerificationFailed { index: usize }, diff --git a/crates/vaportpm-verify/src/lib.rs b/crates/vaportpm-verify/src/lib.rs index 1094a33..64c726a 100644 --- a/crates/vaportpm-verify/src/lib.rs +++ b/crates/vaportpm-verify/src/lib.rs @@ -325,6 +325,52 @@ pub fn verify_attestation_json(json: &str) -> Result [u8; 32] { + let certs = parse_cert_chain_pem(pem).expect("embedded root PEM should parse"); + let root = certs + .last() + .expect("embedded PEM has at least one certificate"); + let pubkey = extract_public_key(root).expect("root certificate has a public key"); + hash_public_key(&pubkey) + } + + #[test] + fn aws_nitro_root_hash_matches_embedded_cert() { + assert_eq!( + embedded_root_pubkey_hash(AWS_NITRO_ROOT_PEM), + AWS_NITRO_ROOT_HASH, + "AWS_NITRO_ROOT_HASH is out of sync with the embedded AWS Nitro root certificate" + ); + } + + #[test] + fn gcp_ekak_root_hash_matches_embedded_cert() { + assert_eq!( + embedded_root_pubkey_hash(GCP_EKAK_ROOT_PEM), + GCP_EKAK_ROOT_HASH, + "GCP_EKAK_ROOT_HASH is out of sync with the embedded GCP EK/AK root certificate" + ); + } +} + #[cfg(test)] mod ephemeral_gcp_tests; #[cfg(test)] diff --git a/crates/vaportpm-verify/src/tpm.rs b/crates/vaportpm-verify/src/tpm.rs index fdec8d7..6ed74dd 100644 --- a/crates/vaportpm-verify/src/tpm.rs +++ b/crates/vaportpm-verify/src/tpm.rs @@ -242,6 +242,13 @@ impl<'a> SafeCursor<'a> { /// /// The TPM Quote contains a PCR digest (SHA-256 of concatenated PCR values). /// PcrBank guarantees all 24 values in order, so we just hash them sequentially. +/// +/// INVARIANT: the digest is always SHA-256, even when the PCR *bank* is SHA-384 +/// (the Nitro case). A TPM2_Quote computes `pcrDigest` using the hash of the +/// AK's signing scheme, not the hash of the PCR bank. Both paths use a P-256 AK +/// signing with ECDSA-SHA256 (see `verify_ecdsa_p256`), so the digest is SHA-256 +/// over the concatenated bank values regardless of bank algorithm. Do NOT switch +/// this to SHA-384 for the Nitro path — that would break verification. pub(crate) fn verify_pcr_digest_matches( quote_info: &TpmQuoteInfo, pcrs: &PcrBank, diff --git a/crates/vaportpm-verify/src/x509.rs b/crates/vaportpm-verify/src/x509.rs index 96a5dbd..5b8a0a8 100644 --- a/crates/vaportpm-verify/src/x509.rs +++ b/crates/vaportpm-verify/src/x509.rs @@ -74,6 +74,19 @@ pub(crate) fn extract_key_usage(cert: &Certificate) -> Option { /// Extract Basic Constraints extension from a certificate (OID 2.5.29.19) /// /// Returns None if the extension is not present. +/// +/// This deliberately hand-parses a minimal ASN.1 subset (short-form lengths, +/// the `cA BOOLEAN` and optional `pathLenConstraint INTEGER`) rather than using +/// the full `der`/`x509_cert` typed decoder. Two reasons: +/// 1. Pulling the general DER decoder into the verification path significantly +/// inflates zkVM circuit size. +/// 2. A constrained parser over this one known-shape extension is a smaller, +/// more auditable trust surface than the general decoder. +/// +/// It is fail-closed by construction: any byte it does not understand yields the +/// default (`cA = false`), which `validate_tpm_cert_chain` rejects for any +/// non-leaf certificate. So a malformed Basic Constraints can only cause a CA to +/// be rejected, never a non-CA to be accepted as a CA. pub(crate) fn extract_basic_constraints(cert: &Certificate) -> Option { let extensions = cert.tbs_certificate.extensions.as_ref()?; @@ -264,22 +277,27 @@ pub(crate) struct ChainValidationResult { /// **Time validity:** /// - Certificate validity periods include the specified time /// +/// **signatureAlgorithm consistency (RFC 5280 §4.1.1.2):** +/// - Each certificate's outer `signatureAlgorithm` must equal its inner +/// `tbsCertificate.signature` +/// /// **Basic Constraints (OID 2.5.29.19):** /// - Leaf certificate must have `CA:FALSE` (or no Basic Constraints) /// - Intermediate/root certificates must have `CA:TRUE` /// - Path length constraints are honored /// /// **Key Usage (OID 2.5.29.15):** +/// - Every certificate must carry the Key Usage extension /// - Leaf certificate must have `digitalSignature` bit set /// - CA certificates must have `keyCertSign` bit set /// -/// **Extended Key Usage (OID 2.5.29.37):** -/// - CA certificates should have TPM EK Certificate EKU (2.23.133.8.1) -/// - Note: GCP AK leaf certificates don't have EKU, only Key Usage -/// /// **Name chaining:** /// - Each certificate's Issuer must match its parent's Subject /// +/// Extended Key Usage (OID 2.5.29.37) is intentionally NOT checked: the GCP AK +/// leaf certificates carry only Key Usage, and `BasicConstraints.cA` is the +/// authoritative gate for CA capability. +/// /// Chain should be leaf-first, root-last. pub(crate) fn validate_tpm_cert_chain( chain: &[Certificate], @@ -303,6 +321,17 @@ pub(crate) fn validate_tpm_cert_chain( let is_leaf = i == 0; let is_root = i == chain.len() - 1; + // 0. signatureAlgorithm consistency (RFC 5280 §4.1.1.2) + // + // The outer signatureAlgorithm must equal the inner + // tbsCertificate.signature. We select the verification algorithm from + // the outer field below but verify over the TBS bytes (which carry the + // inner field); requiring equality here forecloses any algorithm + // substitution between the two. + if cert.signature_algorithm != cert.tbs_certificate.signature { + return Err(ChainValidationReason::SignatureAlgorithmMismatch { index: i }.into()); + } + // 1. Basic Constraints validation if let Some(bc) = extract_basic_constraints(cert) { if is_leaf && bc.ca { @@ -339,6 +368,14 @@ pub(crate) fn validate_tpm_cert_chain( } // 2. Key Usage validation + // + // BasicConstraints.cA (checked above) is the *authoritative* gate for + // whether a certificate may act as a CA. Key Usage is enforced here as + // defence in depth. RFC 5280 §6.1.4(n) only requires the keyCertSign + // check when a Key Usage extension is present; we additionally require + // the extension itself on every certificate (leaf and CA) so that a CA + // can never sign a child without an explicit keyCertSign bit, and a leaf + // can never be used to verify the Quote without digitalSignature. if let Some(ku) = extract_key_usage(cert) { if is_leaf && !ku.digital_signature { return Err(ChainValidationReason::LeafMissingDigitalSignature.into()); @@ -347,8 +384,9 @@ pub(crate) fn validate_tpm_cert_chain( return Err(ChainValidationReason::CaMissingKeyCertSign { index: i }.into()); } } else if is_leaf { - // Leaf certificate MUST have Key Usage for signing return Err(ChainValidationReason::LeafMissingKeyUsage.into()); + } else { + return Err(ChainValidationReason::CaMissingKeyUsage { index: i }.into()); } // 3. Subject/Issuer name chaining @@ -699,6 +737,92 @@ mod tests { assert!(!ku.key_cert_sign, "keyCertSign should not be set for leaf"); } + #[test] + fn test_reject_ca_without_key_usage() { + use pki_types::UnixTime; + use std::time::Duration; + + // Root CA with cA:TRUE but NO Key Usage extension (empty key_usages → + // rcgen omits the extension entirely). BasicConstraints alone would + // accept it as a CA; the new Key Usage requirement must reject it. + let mut ca_params = + rcgen::CertificateParams::new(vec!["No-KU Test Root".to_string()]).unwrap(); + ca_params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + ca_params.key_usages = vec![]; + let ca_key = rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + let ca_cert = ca_params.self_signed(&ca_key).unwrap(); + + // Leaf signed by that CA, with a valid Key Usage of its own. + let mut leaf_params = + rcgen::CertificateParams::new(vec!["No-KU Test Leaf".to_string()]).unwrap(); + leaf_params.is_ca = rcgen::IsCa::NoCa; + leaf_params.key_usages = vec![rcgen::KeyUsagePurpose::DigitalSignature]; + let leaf_key = rcgen::KeyPair::generate_for(&rcgen::PKCS_ECDSA_P256_SHA256).unwrap(); + let leaf_cert = leaf_params.signed_by(&leaf_key, &ca_cert, &ca_key).unwrap(); + + let chain = vec![ + Certificate::from_der(leaf_cert.der()).unwrap(), + Certificate::from_der(ca_cert.der()).unwrap(), + ]; + + let time = UnixTime::since_unix_epoch(Duration::from_secs( + crate::test_support::EPHEMERAL_TIMESTAMP_SECS, + )); + let result = validate_tpm_cert_chain(&chain, time); + assert!( + matches!( + result, + Err(VerifyError::ChainValidation( + ChainValidationReason::CaMissingKeyUsage { index: 1 } + )) + ), + "CA without Key Usage must be rejected, got: {:?}", + result + ); + } + + #[test] + fn test_reject_signature_algorithm_mismatch() { + use pki_types::UnixTime; + use std::time::Duration; + + // Real P-256 leaf+root chain (leaf signed ecdsa-with-SHA256). + let keys = crate::test_support::generate_gcp_chain(); + + // Corrupt the leaf so its inner tbsCertificate.signature algorithm + // differs from the outer signatureAlgorithm. The ecdsa-with-SHA256 OID + // (1.2.840.10045.4.3.2) is DER-encoded as 06 08 2A 86 48 CE 3D 04 03 02; + // the first occurrence in the certificate is the inner tbs.signature. + // Flip its final byte to 0x03 (ecdsa-with-SHA384 OID) so inner != outer. + let pattern = [0x06u8, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x04, 0x03, 0x02]; + let mut leaf_der = keys.leaf_der.clone(); + let pos = leaf_der + .windows(pattern.len()) + .position(|w| w == pattern) + .expect("ecdsa-with-SHA256 OID present in leaf cert"); + leaf_der[pos + pattern.len() - 1] = 0x03; + + let chain = vec![ + Certificate::from_der(&leaf_der).unwrap(), + Certificate::from_der(&keys.root_der).unwrap(), + ]; + + let time = UnixTime::since_unix_epoch(Duration::from_secs( + crate::test_support::EPHEMERAL_TIMESTAMP_SECS, + )); + let result = validate_tpm_cert_chain(&chain, time); + assert!( + matches!( + result, + Err(VerifyError::ChainValidation( + ChainValidationReason::SignatureAlgorithmMismatch { index: 0 } + )) + ), + "mismatched inner/outer signature algorithm must be rejected, got: {:?}", + result + ); + } + #[test] fn test_extract_key_usage_ca() { // GCP intermediate certificate with Key Usage: Certificate Sign, CRL Sign