From b9b3372677a1f4fe85fffb5ea81a99cef4106ae7 Mon Sep 17 00:00:00 2001 From: Elena Nadolinski Date: Wed, 17 Jun 2026 08:59:20 -0700 Subject: [PATCH] test: hinted P-384 adversarial soundness tests + cache-griefing spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Focused tests from the security review of the hinted P-384 change (tests only). Revocation landed in #29 and the forward-compatibility implementation + docs in #32, so this carries only the adversarial soundness tests that part of the review surfaced. test/hinted/HintedNitroAttestation.t.sol (via a small U384TestWrapper): - test_UnreducedInverseHintIsSound — a non-canonical inverse hint (inv + p) passes the on-chain b*inv == 1 check and yields the identical reduced result, so it cannot change the accept decision. - test_ModulusConfusionHintReverts — a hint valid mod n is rejected where mod p is expected. - test_CorruptedSignatureNeverAccepts — no hint stream makes a corrupted signature verify: non-self-consistent hints (empty/garbage/original/padded) revert at the inverse-hint gate, and self-consistent hints collected for the corrupted sig pass the gate but fail the final ECDSA curve check (returns false). - test_MalleableTwinSignatureAccepted — (r, n-s) also verifies (intentional, CURVE_LOW_S_MAX = n-1) and decodes to the identical attestation. - test_AttestationReplayHasNoOnChainProtection — the same (tbs, sig) verifies repeatedly; freshness/replay is the integrator's responsibility. test/CertManager.t.sol: - test_CacheGriefingSameKeyCaRenewalBricksCachedLeaf — skipped spec documenting the verifiedParent first-writer-wins liveness edge. Adds P384HintCollector.collectVerifyHintsAllowInvalid so a self-consistent hint stream can be collected for a signature that fails the final check. Generated with Claude Code Co-Authored-By: Claude --- test/CertManager.t.sol | 19 ++ test/helpers/HintedNitroTestHelpers.sol | 42 ++++ test/hinted/HintedNitroAttestation.t.sol | 239 +++++++++++++++++++++++ 3 files changed, 300 insertions(+) diff --git a/test/CertManager.t.sol b/test/CertManager.t.sol index 60ba0c6..687e222 100644 --- a/test/CertManager.t.sol +++ b/test/CertManager.t.sol @@ -80,6 +80,25 @@ 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{CA,Client}CertRejectsCachedParentMismatch.) + // + // 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; the binding is intentional (warm reuse skips signature verification, so it + // must reflect the exact verified chain) and self-heals because Nitro leaves are short-lived. + 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)); diff --git a/test/helpers/HintedNitroTestHelpers.sol b/test/helpers/HintedNitroTestHelpers.sol index 682255f..baa488c 100644 --- a/test/helpers/HintedNitroTestHelpers.sol +++ b/test/helpers/HintedNitroTestHelpers.sol @@ -59,6 +59,48 @@ contract P384HintCollector { } } + /// @dev Like {collectVerifyHints} but does NOT require the signature to verify. The inverse + /// hints are gathered during the verification trace (before the final curve equation is + /// checked), so this returns a complete, self-consistent hint stream even for a signature + /// that ultimately fails — letting a test prove the final ECDSA check (not just the hint + /// gate) is what rejects a well-hinted invalid signature. Returns the hints and `ok` + /// (whether the signature actually verified). + function collectVerifyHintsAllowInvalid(bytes memory hash, bytes memory signature, bytes memory pubKey) + public + returns (bytes memory hints, bool ok) + { + assembly { + tstore(0, 0) + tstore(1, 0) + tstore(2, 0) + tstore(7, 0) + tstore(8, 1) + } + + ok = ECDSA384HintCollectorLib.verify(_hintParams(), hash, signature, pubKey); + + uint256 count; + uint256 inv; + assembly { + count := tload(2) + inv := tload(0) + tstore(8, 0) + } + require(count == inv, "inverse collection mismatch"); + + hints = new bytes(count * 48); + for (uint256 i = 0; i < count; ++i) { + assembly { + let slot_ := add(1000, mul(i, 2)) + let hi := tload(slot_) + let lo := tload(add(slot_, 1)) + let dst_ := add(add(hints, 0x20), mul(i, 48)) + mstore(dst_, shl(128, hi)) + mstore(add(dst_, 0x10), lo) + } + } + } + function collectCertSignatureHints(bytes memory certificate, bytes memory parentPubKey) external returns (bytes memory) diff --git a/test/hinted/HintedNitroAttestation.t.sol b/test/hinted/HintedNitroAttestation.t.sol index f68f60a..b3c63a1 100644 --- a/test/hinted/HintedNitroAttestation.t.sol +++ b/test/hinted/HintedNitroAttestation.t.sol @@ -10,6 +10,59 @@ import {CborDecode} from "../../src/CborDecode.sol"; import {P384Verifier} from "../../src/P384Verifier.sol"; import {Sha2Ext} from "../../src/Sha2Ext.sol"; import {P384HintCollector} from "../helpers/HintedNitroTestHelpers.sol"; +import {U384} from "../../src/vendor/ECDSA384.sol"; + +/// @dev Thin wrapper exposing U384's internal modinv / moddivAssign so hint-soundness +/// properties (non-canonical inverses, modulus confusion) can be tested in isolation. +contract U384TestWrapper { + /// @dev Canonical b^{-1} mod m via the non-hinted Fermat path; returns 48-byte big-endian. + function computeModinvNoHint(bytes memory bBytes, bytes memory mBytes) external view returns (bytes memory result) { + uint256 m = U384.init(mBytes); + uint256 b = U384.init(bBytes); + uint256 call_ = U384.initCall(m); + result = _u384ToBytes(U384.modinv(call_, b, m)); + } + + /// @dev modinv(b, m) consuming a single 48-byte hint; reverts "bad inverse hint" unless + /// b * hint == 1 (mod m). Returns the raw (possibly non-canonical) hint value. + function callModinvWithHint(bytes memory bBytes, bytes memory mBytes, bytes memory hint) + external + view + returns (bytes memory result) + { + require(hint.length == 48, "hint must be 48 bytes"); + uint256 m = U384.init(mBytes); + uint256 b = U384.init(bBytes); + uint256 call_ = U384.initCallWithHints(m, hint); + result = _u384ToBytes(U384.modinv(call_, b, m)); + } + + /// @dev a/b mod p via moddivAssign using a single 48-byte hint for b^{-1} (p is the baked + /// MODEXP modulus). Reverts "bad inverse hint" if b * hint != 1 (mod p). + function callModdivAssignWithHint(bytes memory aBytes, bytes memory bBytes, bytes memory pBytes, bytes memory hint) + external + view + returns (bytes memory result) + { + require(hint.length == 48, "hint must be 48 bytes"); + uint256 p = U384.init(pBytes); + uint256 a = U384.init(aBytes); + uint256 b = U384.init(bBytes); + uint256 call_ = U384.initCallWithHints(p, hint); + U384.moddivAssign(call_, a, b); + result = _u384ToBytes(a); + } + + /// @dev Serialise a U384 pointer (64-byte slot) to 48-byte big-endian bytes. + function _u384ToBytes(uint256 ptr) internal pure returns (bytes memory result) { + result = new bytes(48); + assembly { + let dst := add(result, 0x20) + mstore(dst, shl(128, mload(ptr))) + mstore(add(dst, 0x10), mload(add(ptr, 0x20))) + } + } +} contract NitroValidatorParseHarness is NitroValidator { constructor(CertManager certManager, P384Verifier p384Verifier) NitroValidator(certManager, p384Verifier) {} @@ -31,6 +84,20 @@ contract HintedNitroAttestationTest is Test { NitroValidatorParseHarness parser; P384Verifier p384Verifier; P384HintCollector hintCollector; + U384TestWrapper u384; + + // P-384 field modulus p (48-byte big-endian). + bytes constant CURVE_P = + hex"fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffff0000000000000000ffffffff"; + // P-384 group order n (48-byte big-endian). + bytes constant CURVE_N = + hex"ffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973"; + // 7 + p (7 < 2^128, so this fits in 48 bytes): a non-canonical representative of 7 mod p. + bytes constant SEVEN_PLUS_P = + hex"fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffff000000000000000100000006"; + // P-384 order split into (hi128, lo256) for forming the malleable twin (r, n-s). + uint128 constant N_HI = type(uint128).max; + uint256 constant N_LO = 0xffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973; struct SequenceSummary { ICertManager.VerifiedCert leaf; @@ -52,6 +119,178 @@ contract HintedNitroAttestationTest is Test { validator = new NitroValidator(certManager, p384Verifier); parser = new NitroValidatorParseHarness(certManager, p384Verifier); hintCollector = new P384HintCollector(); + u384 = new U384TestWrapper(); + } + + // ───────────────────────────────────────────────────────────────────────────── + // Hint-verification soundness & signature semantics (from the security review) + // ───────────────────────────────────────────────────────────────────────────── + + /// @dev A non-canonical inverse hint (inv + p, still < 2^384) passes the on-chain + /// `b·inv ≡ 1 (mod p)` check AND yields the identical reduced result as the + /// canonical hint, so unreduced hints can never change the accept decision. + /// Exercised at the U384 level: real 384-bit inverses are ~2^384 and their + /// `inv + p` would overflow 48 bytes, so a small inverse (7) is used to make the + /// non-canonical representative constructible. + function test_UnreducedInverseHintIsSound() public view { + bytes memory seven = new bytes(48); + seven[47] = 0x07; + bytes memory one = new bytes(48); + one[47] = 0x01; + + // b = 7^{-1} mod p, so the canonical inverse of b is 7. + bytes memory b = u384.computeModinvNoHint(seven, CURVE_P); + + // modinv accepts both the canonical hint (7) and the non-canonical (7 + p), returning each verbatim. + assertEq(u384.callModinvWithHint(b, CURVE_P, seven), seven, "canonical hint accepted"); + assertEq(u384.callModinvWithHint(b, CURVE_P, SEVEN_PLUS_P), SEVEN_PLUS_P, "unreduced hint accepted"); + + // moddivAssign(1, b) = 1/b mod p = 7 for BOTH hints — identical reduced result. + bytes memory canon = u384.callModdivAssignWithHint(one, b, CURVE_P, seven); + bytes memory unreduced = u384.callModdivAssignWithHint(one, b, CURVE_P, SEVEN_PLUS_P); + assertEq(canon, seven, "canonical hint computes 7"); + assertEq(unreduced, seven, "unreduced hint computes 7"); + assertEq(canon, unreduced, "unreduced hint yields identical result -> cannot affect accept"); + } + + /// @dev A hint that is a valid inverse modulo n is rejected when consumed where modulo p + /// is expected. Guards against modulus confusion between the mod-n (scalar) and + /// mod-p (field) inversion paths. + function test_ModulusConfusionHintReverts() public { + bytes memory seven = new bytes(48); + seven[47] = 0x07; + + bytes memory inv7modN = u384.computeModinvNoHint(seven, CURVE_N); + // Correct modulus: accepted. + assertEq(u384.callModinvWithHint(seven, CURVE_N, inv7modN), inv7modN, "mod-n hint valid for mod-n"); + // Wrong modulus: 7 * (7^{-1} mod n) mod p != 1 -> rejected. + vm.expectRevert("bad inverse hint"); + u384.callModinvWithHint(seven, CURVE_P, inv7modN); + } + + /// @dev With a corrupted attestation signature, NO attacker-chosen hint stream forces + /// acceptance. Two distinct gates are exercised: + /// 1. hints NOT self-consistent with the corrupted signature (empty / garbage / the + /// original-valid / padded) revert at the inverse-hint check (`b·inv ≡ 1`); and + /// 2. hints that ARE self-consistent with the corrupted signature pass every inverse-hint + /// check, but the signature still fails the final ECDSA curve equation, so verification + /// returns false rather than reverting at the hint gate. + function test_CorruptedSignatureNeverAccepts() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs, bytes memory signature) = validator.decodeAttestationTbs(attestation); + ICertManager.VerifiedCert memory leaf = _cacheCertBundleWithHints(attestationTbs); + bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); + + bytes memory validHints = hintCollector.collectVerifyHints(hash, signature, leaf.pubKey); + + bytes memory corrupted = new bytes(signature.length); + for (uint256 i = 0; i < signature.length; ++i) { + corrupted[i] = signature[i]; + } + // Flip a low bit of s's leading byte. This keeps s well within range, so the gate that + // fires below is the inverse-hint check, not a scalar range check. + corrupted[48] = bytes1(uint8(corrupted[48]) ^ 0x01); + + // (1) Hints not self-consistent with `corrupted` revert at the inverse-hint check. + vm.expectRevert("inverse hint underflow"); + p384Verifier.verifyP384SignatureWithHints(hash, corrupted, leaf.pubKey, ""); + + vm.expectRevert("bad inverse hint"); + p384Verifier.verifyP384SignatureWithHints(hash, corrupted, leaf.pubKey, new bytes(48)); + + vm.expectRevert("bad inverse hint"); + p384Verifier.verifyP384SignatureWithHints(hash, corrupted, leaf.pubKey, validHints); + + vm.expectRevert("bad inverse hint"); + p384Verifier.verifyP384SignatureWithHints( + hash, corrupted, leaf.pubKey, abi.encodePacked(validHints, new bytes(48)) + ); + + // (2) Hints correctly computed FOR the corrupted signature satisfy every inverse-hint check, + // so we get past the hint gate — but the signature is still invalid, so the final curve + // check fails and verification returns false (never accepts). The hint stream is gathered + // during the verification trace, so it is self-consistent even though the trace reports + // the signature as invalid. + (bytes memory corruptedHints, bool traceOk) = + hintCollector.collectVerifyHintsAllowInvalid(hash, corrupted, leaf.pubKey); + assertFalse(traceOk, "corrupted signature must not pass the collector's verification trace"); + assertFalse( + p384Verifier.verifyP384SignatureWithHints(hash, corrupted, leaf.pubKey, corruptedHints), + "corrupted signature with self-consistent hints must not verify" + ); + } + + /// @dev Signature malleability is intentional (CURVE_LOW_S_MAX = n-1): for a valid (r, s) + /// the twin (r, n-s) also verifies. Both are accepted and decode to the identical + /// attestation, so integrators must NOT use raw signature bytes as a uniqueness key. + function test_MalleableTwinSignatureAccepted() public { + bytes memory attestationTbs; + bytes memory sig; + { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (attestationTbs, sig) = validator.decodeAttestationTbs(attestation); + } + ICertManager.VerifiedCert memory leaf = _cacheCertBundleWithHints(attestationTbs); + bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); + + bytes memory twin = _computeTwinSig(sig); + assertTrue(keccak256(sig) != keccak256(twin), "twin must differ from original"); + + bytes memory hints = hintCollector.collectVerifyHints(hash, sig, leaf.pubKey); + bytes memory twinHints = hintCollector.collectVerifyHints(hash, twin, leaf.pubKey); + + assertTrue(p384Verifier.verifyP384SignatureWithHints(hash, sig, leaf.pubKey, hints), "original verifies"); + assertTrue( + p384Verifier.verifyP384SignatureWithHints(hash, twin, leaf.pubKey, twinHints), + "malleable twin (r, n-s) also verifies" + ); + + NitroValidator.Ptrs memory p1 = validator.validateAttestationWithHints(attestationTbs, sig, hints); + NitroValidator.Ptrs memory p2 = validator.validateAttestationWithHints(attestationTbs, twin, twinHints); + assertEq(p1.timestamp, p2.timestamp, "both twins decode to the same attestation"); + } + + /// @dev validateAttestationWithHints has no on-chain anti-replay: the same (tbs, sig) + /// verifies repeatedly until the leaf cert expires. Freshness/replay protection is + /// the integrator's responsibility (documented in NatSpec). + function test_AttestationReplayHasNoOnChainProtection() public { + bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64())); + (bytes memory attestationTbs, bytes memory sig) = validator.decodeAttestationTbs(attestation); + ICertManager.VerifiedCert memory leaf = _cacheCertBundleWithHints(attestationTbs); + bytes memory hash = Sha2Ext.sha384(attestationTbs, 0, attestationTbs.length); + bytes memory hints = hintCollector.collectVerifyHints(hash, sig, leaf.pubKey); + + uint256 ts1 = validator.validateAttestationWithHints(attestationTbs, sig, hints).timestamp; + uint256 ts2 = validator.validateAttestationWithHints(attestationTbs, sig, hints).timestamp; + assertEq(ts1, ts2, "same attestation accepted twice (no replay protection)"); + } + + /// @dev Builds the malleable twin (r, n-s) from a 96-byte packed (r||s) signature. + function _computeTwinSig(bytes memory sig) internal pure returns (bytes memory twin) { + require(sig.length == 96, "sig must be 96 bytes"); + uint128 rhi; + uint256 rlo; + uint128 shi; + uint256 slo; + assembly { + let data := add(sig, 0x20) + rhi := shr(128, mload(data)) + rlo := mload(add(data, 0x10)) + shi := shr(128, mload(add(data, 0x30))) + slo := mload(add(data, 0x40)) + } + uint128 thi; + uint256 tlo; + if (N_LO >= slo) { + tlo = N_LO - slo; + thi = N_HI - shi; + } else { + unchecked { + tlo = N_LO - slo; + } + thi = N_HI - shi - 1; + } + twin = abi.encodePacked(rhi, rlo, thi, tlo); } function test_HintedAttestationRejectsSurplusHint() public {