feat: forward-compatible attestation parsing (skip unknown keys, indefinite-length CBOR)

[leanthebean]

Summary

Makes the Nitro attestation parser forward-compatible with benign changes to AWS's COSE payload format, so such a change cannot brick verification until a contract upgrade. Addresses two liveness findings from a security review (L1, L2) and documents two by-design edges (L4, M1–M3).

Both code changes are liveness-only and cannot cause a false accept: the entire to-be-signed payload is still hashed and checked against AWS's COSE signature in validateAttestationWithHints, so any skipped or re-encoded content is signed by AWS, and a field the parser ignores can never change the accept decision. Fields the contract reads (pcrs, cabundle, certificate, moduleID, …) are parsed exactly as before; malformed/truncated input still reverts.

What changed

L1 — unknown attestation keys are skipped, not rejected.
Previously an unrecognised map key reverted with "invalid attestation key", meaning any new field AWS added would permanently reject every attestation. Now the value is skipped via a new generic CBOR walker and parsing continues.

L2 — definite and indefinite-length CBOR are both accepted.
The outer payload map and the nested pcrs map and cabundle array each parse in either definite-length form or indefinite-length form (0xBF / 0x9F … 0xFF). Previously the nested containers assumed a definite count, so an encoder switch to indefinite length would have silently produced an empty pcrs / cabundle and (via a leaked inner break marker) truncated the rest of the map.

• src/CborDecode.sol: add skipValue, a generic, bounds-checked CBOR data-item skipper (all major types, definite + indefinite).
• src/NitroValidator.sol: rework _parseAttestation for both behaviors; add _isIndefinite, _countIndefiniteItems, _consumeBreak helpers and forward-compat NatSpec.

Docs only (no logic change):

• L4 — document that a cached cert is pinned to the parent it was first verified under (CertManager.verifiedParent comment + "First-verified parent pinning" section in the design doc). This is intentional: warm reuse skips signature verification, so it must reflect the exact chain that was cryptographically checked. Harmless for Nitro because short-lived (~3h) leaves self-heal.
• M1–M3 / forward-compat — README "Security considerations" + design-doc "Forward compatibility" section.

Out of scope (intentionally)

• L3 (expired-CA / freshness). No change. The leaf's ~3h validity window comfortably covers the 5-tx cold flow, and the design already fails closed — the final validation re-walks the bundle and re-checks every cert's expiry against the current block.timestamp, so a mid-flow expiry reverts rather than silently passing. Historical/retrospective verification remains an off-chain concern; no grace window or asOf parameter is introduced.
• L4 logic. A real fix would require re-verifying the signature against the alternate parent (defeating the warm path) for a self-healing, low-risk edge.

Testing

• forge test — 105 passed, 0 failed.
• forge fmt --check src test — clean.
• NITRO_RUN_FFI=true forge test --ffi --match-test test_OffchainWitness — passes (off-chain ↔ on-chain witness parity).
• NitroValidator runtime size well under the EIP-170 limit.

The four behavior-pinning tests in test/IndefiniteLengthCbor.t.sol that previously asserted the old revert-on-unknown-key / break-on-indefinite behavior are flipped to assert correct parsing, and skipValue unit tests plus nested-indefinite and unknown-key coverage are added.

🤖 Generated with Claude Code