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

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to this project are documented here. The format is based on
   many certificates revoked, and the owner can rotate the revoker or undo accidental revocations.
 - `CertManager.computeCertId(certDER)`: returns a certificate's `(issuer, serial)` revocation
   identity key.
+- `CborDecode.skipValue`: a generic CBOR data-item skipper used to walk over values of unknown shape.
 
 ### Changed
 - Certificate verification and cached reuse now reject revoked certificates and revoked cached
@@ -24,6 +25,13 @@ All notable changes to this project are documented here. The format is based on
   is never parsed on-chain), while non-root revocation remains delegated to the revoker role.
 - Cold certificate verification rejects submitted cert bytes with trailing data or fields after the
   signature.
+- Attestation parser is now forward-compatible with AWS COSE payload format changes: unrecognised
+  map keys are skipped instead of reverting, and the payload map plus the nested `pcrs` map and
+  `cabundle` array are accepted in both definite- and indefinite-length CBOR encodings. Malformed
+  encodings still revert (reserved headers, odd-length maps, mismatched indefinite-string chunks, a
+  missing break marker). These are liveness-only changes — the whole payload is still verified
+  against AWS's COSE signature, so an ignored or re-encoded field can never change the accept
+  decision.
 
 ### Fixed
 - Reject non-canonical P-384 public key coordinates greater than or equal to the field prime `p`.

README.md

@@ -172,6 +172,12 @@ integrator (see [docs](docs/hinted-p384-nitro-attestation.md#integrator-responsi
   operator must monitor AWS CRLs and submit the affected certificate identity keys
   (`keccak256(issuerHash, serialHash)`, computed via `computeCertId` or directly from the CRL's
   issuer/serial entries).
+- **Forward compatibility** — the attestation parser tolerates AWS evolving the COSE payload format:
+  unrecognised map keys are skipped (not rejected), and the payload map plus the nested `pcrs` map
+  and `cabundle` array are accepted in both definite- and indefinite-length CBOR encodings. This is
+  safe because the entire to-be-signed payload is checked against AWS's COSE signature, so unknown or
+  re-encoded content is signed and ignoring it can only ever drop a field the contract does not read
+  — it can never change the accept decision.
 
 ## Build
 

docs/hinted-p384-nitro-attestation.md

@@ -414,6 +414,20 @@ its X.509 validity period has not expired.
 metadata was cached previously; it does not imply the cert is currently trusted, unexpired,
 or unrevoked.
 
+**First-verified parent pinning.** A cached cert is pinned to the parent it was first
+verified under: cold verification records `verifiedParent[certHash]` once, and every
+later warm reuse requires the caller to present that exact `parentCertHash` (a mismatch
+reverts with `parent cert mismatch`). This is a deliberate, conservative binding — warm
+reuse skips signature verification, so it must reflect the precise chain that was
+cryptographically checked, not merely *a* valid same-subject issuer. The liveness
+consequence is that if the same certificate is genuinely issued under two different CA
+objects (for example a same-key CA renewal that produces new DER bytes, hence a new
+parent hash), the cached leaf keeps verifying only through its first parent; a second
+caller chaining it through the renewed parent must wait for the cached entry to expire.
+For AWS Nitro this is effectively a non-issue because leaf certificates are short-lived
+(~3h) and expire long before their issuing CA is rotated, so the binding self-heals; it
+is documented here as a known edge rather than a fixed bug.
+
 **Warm-only guard.** `validateAttestationWithHints` re-runs the cabundle checks with an
 *empty* hint stream. Cached certs return before signature verification; a missing cert
 sends the empty stream into P384 verification and reverts with
@@ -573,6 +587,31 @@ deliberately left to the caller and must be handled in the consuming contract:
   (`keccak256(issuerHash, serialHash)`, via `computeCertId` or computed directly from the
   CRL entry).
 
+### Forward compatibility (attestation format changes)
+
+`_parseAttestation` is deliberately tolerant of AWS evolving the COSE payload, so a
+benign format change cannot brick verification until a contract upgrade:
+
+- **Unknown keys are skipped, not rejected.** A map key the parser does not recognise
+  has its value skipped with a generic CBOR walk (`CborDecode.skipValue`) and parsing
+  continues. Previously an unrecognised key reverted (`"invalid attestation key"`),
+  which meant any new field AWS added would have permanently rejected every attestation.
+- **Definite and indefinite length are both accepted.** The outer payload map and the
+  nested `pcrs` map and `cabundle` array are each parsed 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).
+
+Both behaviours are **liveness-only and cannot cause a false accept**: the entire
+to-be-signed payload is hashed and checked against AWS's COSE signature in
+`validateAttestationWithHints`. Skipped or re-encoded content is therefore signed by
+AWS, and a field the parser ignores cannot influence the accept decision — at most it
+is not surfaced to the caller. Fields the contract *does* read (`pcrs`, `cabundle`,
+`certificate`, `moduleID`, …) are parsed exactly as before. A malformed or truncated
+encoding still reverts (out-of-bounds read or unterminated indefinite container), it
+just never silently mis-parses.
+
 ## 11. On-chain demo
 
 A Base Sepolia run of the §6 cold sequence needs: an RPC URL, a funded broadcaster

src/CborDecode.sol

@@ -94,6 +94,103 @@ library CborDecode {
         return elementAt(cbor, ptr.end(), 0x80, true);
     }
 
+    // Returns the index immediately after the complete CBOR data item that starts at `ix`,
+    // descending into nested arrays/maps and following indefinite-length containers to their
+    // 0xFF break marker. Unlike `elementAt`, this accepts every major type (ints, strings,
+    // arrays, maps, tags, simple/float values), so it can skip over values whose shape is not
+    // known ahead of time — e.g. the value of an unrecognised attestation key. Out-of-bounds
+    // reads (including a truncated indefinite-length container with no break marker) revert.
+    function skipValue(bytes memory cbor, uint256 ix) internal pure returns (uint256) {
+        uint8 b = uint8(cbor[ix]);
+        uint8 major = b >> 5;
+        uint8 ai = b & 0x1f;
+
+        uint256 header;
+        uint64 arg;
+        bool indefinite;
+        if (ai < 24) {
+            header = 1;
+            arg = ai;
+        } else if (ai == 24) {
+            header = 2;
+            arg = uint8(cbor[ix + 1]);
+        } else if (ai == 25) {
+            header = 3;
+            arg = cbor.readUint16(ix + 1);
+        } else if (ai == 26) {
+            header = 5;
+            arg = cbor.readUint32(ix + 1);
+        } else if (ai == 27) {
+            header = 9;
+            arg = cbor.readUint64(ix + 1);
+        } else if (ai == 31) {
+            header = 1;
+            indefinite = true;
+        } else {
+            // additional information 28..30 are reserved per RFC 8949
+            revert("invalid cbor additional info");
+        }
+
+        uint256 p = ix + header;
+
+        if (major == 0 || major == 1) {
+            // unsigned / negative integer: the value lives entirely in the header
+            require(!indefinite, "invalid integer encoding");
+            return p;
+        } else if (major == 2 || major == 3) {
+            // byte string / text string
+            if (indefinite) {
+                // an indefinite-length string is a sequence of definite-length chunks of the SAME
+                // major type (RFC 8949 §3.2.3); reject any other chunk rather than skipping it
+                while (uint8(cbor[p]) != 0xff) {
+                    uint8 chunk = uint8(cbor[p]);
+                    require(chunk >> 5 == major && (chunk & 0x1f) != 31, "invalid indefinite string chunk");
+                    p = skipValue(cbor, p);
+                }
+                return p + 1;
+            }
+            require(p + arg <= cbor.length, "cbor string out of bounds");
+            return p + arg;
+        } else if (major == 4) {
+            // array
+            if (indefinite) {
+                while (uint8(cbor[p]) != 0xff) {
+                    p = skipValue(cbor, p);
+                }
+                return p + 1;
+            }
+            for (uint64 i = 0; i < arg; i++) {
+                p = skipValue(cbor, p);
+            }
+            return p;
+        } else if (major == 5) {
+            // map: each entry is a key item followed by a value item
+            if (indefinite) {
+                // a map must have an even number of items (key/value pairs); a dangling key before
+                // the break marker is malformed and must revert
+                while (uint8(cbor[p]) != 0xff) {
+                    p = skipValue(cbor, p); // key
+                    require(uint8(cbor[p]) != 0xff, "odd cbor map item count");
+                    p = skipValue(cbor, p); // value
+                }
+                return p + 1;
+            }
+            for (uint64 i = 0; i < arg; i++) {
+                p = skipValue(cbor, p);
+                p = skipValue(cbor, p);
+            }
+            return p;
+        } else if (major == 6) {
+            // tag: a single tagged data item follows the header
+            require(!indefinite, "invalid tag encoding");
+            return skipValue(cbor, p);
+        } else {
+            // major == 7: simple value / float; ai==31 (break) is not a standalone value
+            require(!indefinite, "unexpected break");
+            return p;
+        }
+    }
+
     function elementAt(bytes memory cbor, uint256 ix, uint8 expectedType, bool required)
         internal
         pure

src/CertManager.sol

@@ -50,7 +50,13 @@ contract CertManager is ICertManager {
 
     // certHash -> VerifiedCert
     mapping(bytes32 => bytes) public verified;
-    // certHash -> parent cert hash used during cold verification
+    // certHash -> parent cert hash used during cold verification.
+    // A cached cert is pinned to the parent it was FIRST verified under: warm reuse requires the
+    // caller to present this exact parent (see the mismatch check in `_verifyCert`). This is
+    // intentional — warm reuse skips signature verification, so it must reflect the precise chain
+    // that was cryptographically checked. A same-key CA renewal (new DER, new parent hash) cannot
+    // re-bind an already-cached descendant until its cache entry expires; harmless for Nitro because
+    // leaves are short-lived. See docs/hinted-p384-nitro-attestation.md "First-verified parent pinning".
     mapping(bytes32 => bytes32) internal verifiedParent;
     // certHash -> revocation identity key (keccak256(issuerHash, serialHash)), recorded at cold
     // verification so the warm path and parent-chain walk can check revocation without re-parsing.

src/NitroValidator.sol

@@ -188,17 +188,36 @@ contract NitroValidator {
         LibBytes.memcpy(dest + 13 + rawProtectedLength, payloadSrc, rawPayloadLength);
     }
 
+    /// @dev Parses the COSE payload into pointers, without copying. Forward-compatibility notes:
+    ///      - Unknown map keys are skipped, not rejected, so AWS adding new attestation fields does
+    ///        not brick verification. This is safe because the whole TBS is later checked against
+    ///        AWS's COSE signature, so unknown content is signed and ignoring it cannot change the
+    ///        accept decision.
+    ///      - The outer payload map and the nested `pcrs` map / `cabundle` array are each accepted in
+    ///        both definite-length and indefinite-length CBOR form.
     function _parseAttestation(bytes memory attestationTbs) internal pure returns (Ptrs memory) {
         require(attestationTbs.keccak(0, 18) == ATTESTATION_TBS_PREFIX, "invalid attestation prefix");
 
         CborElement payload = attestationTbs.byteStringAt(18);
-        CborElement current = attestationTbs.mapAt(payload.start());
+        uint256 mapHeaderIx = payload.start();
+        CborElement current = attestationTbs.mapAt(mapHeaderIx);
+        bool outerIndefinite = _isIndefinite(attestationTbs, mapHeaderIx);
+        uint256 entryCount = current.value(); // entry count for a definite-length map
 
         Ptrs memory ptrs;
         uint256 end = payload.end();
-        while (current.end() < end) {
-            // Break marker (0xFF) terminates indefinite-length maps
-            if (uint8(attestationTbs[current.end()]) == 0xff) break;
+        for (uint256 entry = 0;; entry++) {
+            if (outerIndefinite) {
+                // An indefinite-length map is terminated only by a 0xFF break marker. Require it to
+                // be present (do not silently stop at the payload end on a missing break), and stop
+                // exclusively on it.
+                require(current.end() < end, "missing break marker");
+                if (uint8(attestationTbs[current.end()]) == 0xff) break;
+            } else {
+                // A definite-length map ends after exactly `entryCount` entries; a stray 0xFF must
+                // not terminate it early (it would be parsed as a key and rejected as a non-string).
+                if (entry == entryCount) break;
+            }
             current = attestationTbs.nextTextString(current);
             bytes32 keyHash = attestationTbs.keccak(current);
             if (keyHash == MODULE_ID_KEY) {
@@ -223,28 +242,98 @@ contract NitroValidator {
                 current = attestationTbs.nextPositiveInt(current);
                 ptrs.timestamp = uint64(current.value());
             } else if (keyHash == CABUNDLE_KEY) {
-                current = attestationTbs.nextArray(current);
-                ptrs.cabundle = new CborElement[](current.value());
-                for (uint256 i = 0; i < ptrs.cabundle.length; i++) {
-                    current = attestationTbs.nextByteString(current);
-                    ptrs.cabundle[i] = current;
-                }
+                (ptrs.cabundle, current) = _parseCabundle(attestationTbs, current);
             } else if (keyHash == PCRS_KEY) {
-                current = attestationTbs.nextMap(current);
-                ptrs.pcrs = new CborElement[](current.value());
-                for (uint256 i = 0; i < ptrs.pcrs.length; i++) {
-                    current = attestationTbs.nextPositiveInt(current);
-                    uint256 key = current.value();
-                    require(key < ptrs.pcrs.length, "invalid pcr key value");
-                    require(CborElement.unwrap(ptrs.pcrs[key]) == 0, "duplicate pcr key");
-                    current = attestationTbs.nextByteString(current);
-                    ptrs.pcrs[key] = current;
-                }
+                (ptrs.pcrs, current) = _parsePcrs(attestationTbs, current);
             } else {
-                revert("invalid attestation key");
+                // Forward-compatibility: skip (rather than reject) keys this parser does not
+                // recognise. The entire TBS is covered by AWS's COSE signature verified in
+                // {validateAttestationWithHints}, so an unknown key cannot be injected without
+                // invalidating that signature, and ignoring it can only ever drop a field we do
+                // not read — never change the accept decision. Rejecting unknown keys instead
+                // would brick verification the moment AWS adds a new attestation field.
+                uint256 nextIx = attestationTbs.skipValue(current.end());
+                current = LibCborElement.toCborElement(0x00, nextIx, 0);
             }
         }
 
         return ptrs;
     }
+
+    /// @dev Parses the `cabundle` array (definite- or indefinite-length) starting from the key
+    ///      element `keyPtr`. Returns the parsed cert pointers and the cursor positioned after the
+    ///      array (past the break marker, for indefinite encoding).
+    function _parseCabundle(bytes memory tbs, CborElement keyPtr)
+        private
+        pure
+        returns (CborElement[] memory cabundle, CborElement current)
+    {
+        uint256 headerIx = keyPtr.end();
+        current = tbs.nextArray(keyPtr);
+        bool indefinite = _isIndefinite(tbs, headerIx);
+        uint256 count = indefinite ? _countIndefiniteItems(tbs, current.end()) : current.value();
+        cabundle = new CborElement[](count);
+        for (uint256 i = 0; i < count; i++) {
+            current = tbs.nextByteString(current);
+            cabundle[i] = current;
+        }
+        if (indefinite) current = _consumeBreak(tbs, current);
+    }
+
+    /// @dev Parses the `pcrs` map (definite- or indefinite-length) starting from the key element
+    ///      `keyPtr`. Returns the parsed pcr pointers (indexed by pcr key) and the cursor positioned
+    ///      after the map (past the break marker, for indefinite encoding).
+    function _parsePcrs(bytes memory tbs, CborElement keyPtr)
+        private
+        pure
+        returns (CborElement[] memory pcrs, CborElement current)
+    {
+        uint256 headerIx = keyPtr.end();
+        current = tbs.nextMap(keyPtr);
+        bool indefinite = _isIndefinite(tbs, headerIx);
+        uint256 count;
+        if (indefinite) {
+            // each map entry is a key/value pair, so a well-formed indefinite map holds an even
+            // number of items (2 per pcr); an odd count is malformed and must revert rather than
+            // silently drop the trailing item.
+            uint256 items = _countIndefiniteItems(tbs, current.end());
+            require(items % 2 == 0, "invalid pcrs map");
+            count = items / 2;
+        } else {
+            count = current.value();
+        }
+        pcrs = new CborElement[](count);
+        for (uint256 i = 0; i < count; i++) {
+            current = tbs.nextPositiveInt(current);
+            uint256 key = current.value();
+            require(key < count, "invalid pcr key value");
+            require(CborElement.unwrap(pcrs[key]) == 0, "duplicate pcr key");
+            current = tbs.nextByteString(current);
+            pcrs[key] = current;
+        }
+        if (indefinite) current = _consumeBreak(tbs, current);
+    }
+
+    /// @dev True if the CBOR container header at `headerIx` uses indefinite-length encoding (ai=31).
+    function _isIndefinite(bytes memory cbor, uint256 headerIx) private pure returns (bool) {
+        return (uint8(cbor[headerIx]) & 0x1f) == 31;
+    }
+
+    /// @dev Counts the data items of an indefinite-length container whose first item starts at `ix`,
+    ///      stopping at the 0xFF break marker. Reverts on truncated input (no break before the end).
+    function _countIndefiniteItems(bytes memory cbor, uint256 ix) private pure returns (uint256 count) {
+        while (uint8(cbor[ix]) != 0xff) {
+            ix = cbor.skipValue(ix);
+            count++;
+        }
+    }
+
+    /// @dev Verifies the byte immediately after `ptr` is the 0xFF break marker and returns a
+    ///      zero-length element positioned just past it, so the caller's cursor skips the consumed
+    ///      break. Reverts if the marker is absent (e.g. a nested indefinite container that did not
+    ///      close where the fill loop ended), turning a malformed encoding into a revert.
+    function _consumeBreak(bytes memory cbor, CborElement ptr) private pure returns (CborElement) {
+        require(uint8(cbor[ptr.end()]) == 0xff, "expected break marker");
+        return LibCborElement.toCborElement(0x00, ptr.end() + 1, 0);
+    }
 }