test+docs: hinted P-384 review tests and forward-compat documentation

README.md

@@ -143,6 +143,11 @@ integrator (see [docs](docs/hinted-p384-nitro-attestation.md#integrator-responsi
   responsibility.
 - **Revocation** — not supported (consistent with AWS's documented validation process, linked
   above).
+- **Forward-compatibility** — the attestation parser uses a fixed key whitelist (unknown keys
+  revert) and does not support nested indefinite-length `pcrs`/`cabundle`. A future change to the
+  AWS attestation format would make verification revert (never a false accept) until the contract
+  is upgraded. AWS emits the known, definite-length format today. See
+  [docs §10](docs/hinted-p384-nitro-attestation.md#forward-compatibility-attestation-format-changes).
 
 ## Build
 

docs/hinted-p384-nitro-attestation.md

@@ -503,6 +503,30 @@ can never silently diverge.
   must stay in sync with the verifier's execution order. Its DER/CBOR parsing should
   be reviewed for robustness.
 
+### Forward-compatibility (attestation format changes)
+
+The attestation parser accepts a strict, fixed shape: a known whitelist of CBOR keys, and
+definite-length or outer-indefinite-length maps. The two consequences below are deliberate
+parser behaviour and are **liveness-only** — a wrong or unrecognised shape can only cause a
+revert (a genuine attestation failing to verify), never a false accept. They are recorded here
+because the project does not otherwise document them as an accepted trade-off: each would brick
+verification after a future AWS attestation-format change, and resolving it needs a contract
+upgrade.
+
+- **Unknown attestation key → revert (`"invalid attestation key"`).** If AWS adds a new field to
+  the attestation document, every attestation carrying it becomes unverifiable. Tolerating unknown
+  keys would be safe — all fields sit under AWS's COSE signature, so ignoring an unrecognised one
+  cannot forge anything — but the parser has no generic CBOR skip routine. Current behaviour is
+  pinned by `test_neg_unknownKey{Definite,Indefinite}_reverts`; the desired forward-compatible
+  behaviour is captured (skipped) in `test_unknownKey_forwardCompat_tolerated`.
+- **Nested indefinite-length `pcrs` / `cabundle` → revert / empty parse.** Outer-map
+  indefinite-length encoding is supported (via the `0xFF` break marker); nested indefinite-length
+  containers are not. Pinned by `test_edge_innerIndefinitePcrsEmpty_outerBreakTriggered` and
+  `test_neg_nestedIndefiniteNonEmptyArray_reverts`; desired behaviour (skipped) in
+  `test_nestedIndefinitePcrs_forwardCompat_parsed`.
+
+AWS currently emits definite-length CBOR with the known field set, so neither is triggered today.
+
 ### Integrator responsibilities (what the contract does NOT enforce)
 
 Verification proves an attestation is genuine and well-formed. The following are

src/NitroValidator.sol

@@ -188,6 +188,13 @@ contract NitroValidator {
         LibBytes.memcpy(dest + 13 + rawProtectedLength, payloadSrc, rawPayloadLength);
     }
 
+    /// @dev Parses the COSE payload into field pointers using a strict shape: any unrecognised
+    ///      CBOR key reverts ("invalid attestation key"), and nested indefinite-length
+    ///      `pcrs`/`cabundle` are not supported (outer-map indefinite-length IS). These are
+    ///      liveness-only constraints — they can only cause a revert, never a false accept — but
+    ///      they make verification forward-INCOMPATIBLE with attestation-format changes: a new AWS
+    ///      field would require a contract upgrade. Tolerating unknown keys would be safe (every
+    ///      field is under AWS's COSE signature). See docs §10 "Forward-compatibility".
     function _parseAttestation(bytes memory attestationTbs) internal pure returns (Ptrs memory) {
         require(attestationTbs.keccak(0, 18) == ATTESTATION_TBS_PREFIX, "invalid attestation prefix");
 

test/CertManager.t.sol

@@ -80,6 +80,21 @@ contract CertManagerTest is Test {
         return cm.verifyCACertWithHints(cert, parentHash, hints);
     }
 
+    // Cache-griefing liveness edge: `verifiedParent[certHash]` is written once on the cold
+    // path (gated on cert.pubKey.length == 0) and never updated. If AWS renews an intermediate
+    // CA with the SAME signing key but a new validity window, the renewed cert has different
+    // DER bytes -> a different keccak256 -> a different parentCertHash in the chain. A leaf
+    // already cached under the old parent then permanently reverts "parent cert mismatch"
+    // against the renewed parent, with no admin override. (The wrong-parent revert mechanism
+    // itself is covered by HintedNitroAttestationTest.test_Hinted*RejectsCachedParentMismatch.)
+    //
+    // SKIPPED: realising this needs a genuine same-key *renewed* CA cert, which requires an
+    // AWS signing operation and is unavailable as a static fixture. Documented as a known
+    // liveness edge; see security review redteam/r3-cache-replay-malleability.md (Claim 2).
+    function test_CacheGriefingSameKeyCaRenewalBricksCachedLeaf() public {
+        vm.skip(true, "needs a same-key-renewed AWS CA fixture (off-chain signing); documents verifiedParent first-writer-wins");
+    }
+
     function testFuzz_uint384At_LeadingZeros(uint8 numZeros, uint128 hiSeed, uint256 loSeed) public view {
         numZeros = uint8(bound(numZeros, 0, 16));
 

test/IndefiniteLengthCbor.t.sol

@@ -614,4 +614,57 @@ contract NitroValidatorIndefiniteLengthTest is Test {
         vm.expectRevert("unexpected type");
         validator.parseAttestation(_buildTbs(abi.encodePacked(CBOR_MAP_INDEFINITE, entries, CBOR_BREAK)));
     }
+
+    // ══════════════════════════════════════════════════════════
+    //  FORWARD-COMPATIBILITY — known latent liveness risks (SKIPPED)
+    //
+    //  The two reverts above are deliberate parser behaviour, but the project does not
+    //  document the forward-compatibility consequence as an accepted trade-off. They are
+    //  liveness-only (a wrong/unknown shape can only revert, never false-accept), yet a
+    //  future change to the AWS attestation format would brick verification until the
+    //  contract is upgraded. The tests below encode the DESIRED forward-compatible
+    //  behaviour and are skipped until the parser supports it (see docs §10
+    //  "Forward-compatibility").
+    // ══════════════════════════════════════════════════════════
+
+    /// @dev SKIPPED. Today an unknown attestation key reverts (pinned by
+    ///      test_neg_unknownKeyDefinite_reverts). Tolerating it would be SAFE — every field is
+    ///      under AWS's COSE signature, so ignoring an unrecognised key cannot forge anything;
+    ///      the parser simply lacks a generic CBOR skip. If AWS adds any new field, every such
+    ///      attestation becomes unverifiable until a contract upgrade.
+    function test_unknownKey_forwardCompat_tolerated() public {
+        vm.skip(true, "latent liveness risk: unknown attestation key reverts; needs a generic CBOR skip to tolerate new AWS fields");
+
+        // Desired: a known field still parses and the unknown key is ignored.
+        bytes memory entries = abi.encodePacked(
+            hex"696d6f64756c655f6964", // key "module_id"
+            hex"6474657374", //           val "test"
+            hex"63626164", //             key "bad" (unknown)
+            hex"6474657374" //            val "test"
+        );
+        NitroValidator.Ptrs memory p = validator.parseAttestation(_buildTbs(abi.encodePacked(hex"a2", entries)));
+        assertEq(p.moduleID.length(), SYNTH_MODULE_ID_LEN, "known field parsed; unknown key ignored");
+    }
+
+    /// @dev SKIPPED. Outer-map indefinite-length is supported, but a NESTED indefinite-length
+    ///      pcrs map is not (pinned by test_edge_innerIndefinitePcrsEmpty_outerBreakTriggered and
+    ///      test_neg_nestedIndefiniteNonEmptyArray_reverts). If AWS emitted nested
+    ///      indefinite-length containers, verification would brick. This encodes the desired parse.
+    function test_nestedIndefinitePcrs_forwardCompat_parsed() public {
+        vm.skip(true, "latent liveness risk: nested indefinite-length pcrs/cabundle unsupported; would brick on an AWS encoding change");
+
+        // Desired: a nested indefinite-length pcrs map {0: bytes(48)} parses to one PCR.
+        bytes memory entries = abi.encodePacked(
+            hex"696d6f64756c655f6964", // key "module_id"
+            hex"6474657374", //           val "test"
+            hex"6470637273", //           key "pcrs"
+            CBOR_MAP_INDEFINITE, //       nested indefinite-length map
+            hex"00", //                   key 0
+            hex"5830", //                 val bytes(48) header
+            new bytes(48), //             48 zero bytes
+            CBOR_BREAK //                 inner break
+        );
+        NitroValidator.Ptrs memory p = validator.parseAttestation(_buildTbs(abi.encodePacked(hex"a2", entries)));
+        assertEq(p.pcrs.length, 1, "nested indefinite-length pcrs should yield 1 PCR");
+    }
 }