diff --git a/solidity/contracts/ceremony/CeremonyAttestation.sol b/solidity/contracts/ceremony/CeremonyAttestation.sol index 74249dc..1334575 100644 --- a/solidity/contracts/ceremony/CeremonyAttestation.sol +++ b/solidity/contracts/ceremony/CeremonyAttestation.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; +import {CeremonyFields} from "./CeremonyFields.sol"; + /// @title CeremonyAttestation /// @notice Decoder for the attested-data layout the launch profiles pin. /// @dev THE LAYOUT IS THE PROFILE'S, NOT THE SPECIFICATION'S. REQ-COMMON-18 @@ -142,7 +144,8 @@ library CeremonyAttestation { /// @dev The normalized, line-anchored needle REQ-COMMON-39 counts. bytes internal constant AUTHORIZATION_NEEDLE = "\r\nauthorization:bearer"; - /// @notice The one commitment framed by exactly these revealed bytes. + /// @notice The one commitment framed by these revealed bytes, JSON + /// whitespace aside. /// /// @dev For a direction that is NOT exactly covered, where several ranges /// are hidden and only the anchors around one of them are revealed. @@ -163,12 +166,21 @@ library CeremonyAttestation { pure returns (RangeCommitment memory framed) { + // The prefix at most once across everything revealed, JSON whitespace + // removed: a second one, in any spelling, is a second place the framing + // could point, whether or not a commitment sits behind it. + if (_occurrences(CeremonyFields.normalizeJsonBytes(concatRevealed(block_)), prefix) > 1) { + revert AmbiguousFraming(); + } + uint256 found = type(uint256).max; for (uint256 i = 0; i < block_.commitments.length; ++i) { RangeCommitment memory c = block_.commitments[i]; - if (c.start < prefix.length) continue; - bytes memory before_ = _revealedSlice(block_, c.start - uint32(prefix.length), c.start); - if (keccak256(before_) != keccak256(prefix)) continue; + // The one revealed range ending where the commitment starts is the + // anchor, and its bytes with the JSON whitespace removed end with + // the prefix. One range, never a join: a prefix assembled across a + // seam is one the platform never wrote in one piece. + if (!_anchoredBy(block_, c.start, prefix)) continue; bytes memory after_ = _revealedSlice(block_, c.end, c.end + uint32(suffix.length)); if (keccak256(after_) != keccak256(suffix)) continue; @@ -179,6 +191,36 @@ library CeremonyAttestation { return block_.commitments[found]; } + /// @dev Whether a revealed range ends exactly at `at` and, JSON whitespace + /// removed, ends with `prefix`. The whitespace stays revealed at its + /// offsets -- the range is the wire -- and is only ignored to compare. + function _anchoredBy(DirectionBlock memory block_, uint32 at, bytes memory prefix) private pure returns (bool) { + for (uint256 i = 0; i < block_.revealed.length; ++i) { + RevealedRange memory range = block_.revealed[i]; + if (range.end != at) continue; + bytes memory normalized = CeremonyFields.normalizeJsonBytes(range.value); + if (normalized.length < prefix.length) return false; + for (uint256 j = 0; j < prefix.length; ++j) { + if (normalized[normalized.length - prefix.length + j] != prefix[j]) return false; + } + return true; + } + return false; + } + + function _occurrences(bytes memory haystack, bytes memory needle) private pure returns (uint256 count) { + for (uint256 i = 0; i + needle.length <= haystack.length; ++i) { + bool hit = true; + for (uint256 j = 0; j < needle.length; ++j) { + if (haystack[i + j] != needle[j]) { + hit = false; + break; + } + } + if (hit) ++count; + } + } + /// @notice Every check REQ-COMMON-35, -39 and -40 require of an /// identity-session request that commits a credential in an HTTP /// `Authorization` header. diff --git a/solidity/contracts/ceremony/CeremonyFields.sol b/solidity/contracts/ceremony/CeremonyFields.sol index 98e9872..b4dd3a1 100644 --- a/solidity/contracts/ceremony/CeremonyFields.sol +++ b/solidity/contracts/ceremony/CeremonyFields.sol @@ -51,6 +51,7 @@ library CeremonyFields { pure returns (Found found, bytes memory value) { + data = normalizeJsonBytes(data); bytes memory needle = abi.encodePacked('"', name, '":"'); uint256 at; (found, at) = _findUnique(data, needle); @@ -88,6 +89,7 @@ library CeremonyFields { pure returns (Found found, bytes memory digits) { + data = normalizeJsonBytes(data); bytes memory needle = abi.encodePacked('"', name, '":'); uint256 at; (found, at) = _findUnique(data, needle); @@ -110,6 +112,65 @@ library CeremonyFields { return (Found.One, digits); } + /// @notice `data` with the JSON whitespace that touches a structural + /// byte removed. + /// + /// @dev The four bytes JSON lets a writer put between tokens (RFC 8259 + /// section 2): space, tab, line feed, carriage return. GitHub + /// pretty-prints `/user` for the media type the profile pins, so the + /// compact delimiters the readers above match are a grammar, not the + /// bytes on the wire. Removing the whitespace first, the way + /// `CeremonyAttestation.normalizeHeaderBytes` does for a request head, + /// leaves every reader its one exact template and makes a member in + /// any spelling the same member -- so a duplicate spelled with spaces + /// is still counted as one. + /// + /// Only a run that touches `:` `,` `{` `}` `[` or `]` on either side + /// goes, which is exactly where JSON puts insignificant whitespace. + /// A run between two tokens stays: `123 456` must not read as + /// `123456`, and a trailing space after digits must still be the + /// byte the terminator check judges. Stateless on purpose, with no + /// notion of being inside a string: a reader with one is a reader a + /// prover desynchronises by cutting a revealed range mid-value, and a + /// needle then hides where the reader believes a string is open. No + /// needle can be manufactured by this either -- one needs unescaped + /// quotes, and this removes none -- and nothing this reads carries + /// whitespace beside a structural byte inside its value. + function normalizeJsonBytes(bytes memory data) internal pure returns (bytes memory out) { + out = new bytes(data.length); + uint256 n; + uint256 i; + while (i < data.length) { + if (!_isJsonWhitespace(data[i])) { + out[n++] = data[i]; + ++i; + continue; + } + uint256 j = i; + while (j < data.length && _isJsonWhitespace(data[j])) { + ++j; + } + bool touches = (n != 0 && _isStructural(out[n - 1])) || (j < data.length && _isStructural(data[j])); + if (!touches) { + for (uint256 k = i; k < j; ++k) { + out[n++] = data[k]; + } + } + i = j; + } + assembly ("memory-safe") { + mstore(out, n) + } + } + + function _isJsonWhitespace(bytes1 c) private pure returns (bool) { + return c == 0x20 || c == 0x09 || c == 0x0a || c == 0x0d; + } + + function _isStructural(bytes1 c) private pure returns (bool) { + return c == ":" || c == "," || c == "{" || c == "}" || c == "[" || c == "]"; + } + function _findUnique(bytes memory data, bytes memory needle) private pure returns (Found found, uint256 at) { uint256 hit = type(uint256).max; for (uint256 i = 0; i + needle.length <= data.length; ++i) { diff --git a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol index f1a24a5..c303e7d 100644 --- a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol +++ b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol @@ -171,7 +171,9 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa /// through a second delimiter and the duplicate REQ-COMMON-19A exists /// to reject becomes invisible to it. So the value is read per range /// and the occurrences are counted over the concatenation -- where a - /// seam can only over-count, which fails closed. + /// seam can only over-count, which fails closed. Both read bytes with + /// the JSON whitespace removed, so a copy spelled with spaces is a + /// copy. function _delimiterCount(bytes memory joined, bytes memory delimiter) private pure returns (uint256 count) { for (uint256 i = 0; i + delimiter.length <= joined.length; ++i) { bool hit = true; @@ -215,7 +217,7 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa if (matches != 1) revert FieldNotUnique(name, matches); // And the delimiter appears once across the whole revealed set, so a // second copy cannot hide under a range boundary. - uint256 seen = _delimiterCount(joined, abi.encodePacked('"', name, '":"')); + uint256 seen = _delimiterCount(CeremonyFields.normalizeJsonBytes(joined), abi.encodePacked('"', name, '":"')); if (seen != 1) revert FieldNotUnique(name, seen); } @@ -235,7 +237,7 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa } } if (matches != 1) revert FieldNotUnique(name, matches); - uint256 seen = _delimiterCount(joined, abi.encodePacked('"', name, '":')); + uint256 seen = _delimiterCount(CeremonyFields.normalizeJsonBytes(joined), abi.encodePacked('"', name, '":')); if (seen != 1) revert FieldNotUnique(name, seen); } diff --git a/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol b/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol index ab6a885..1b1a12c 100644 --- a/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyAttestation.t.sol @@ -192,4 +192,64 @@ contract CeremonyAttestationTest is Test { ) ); } + + // ─── Framing behind JSON whitespace ───────────────────────────── + + function framed(CeremonyAttestation.DirectionBlock memory block_) external pure returns (bytes32) { + return CeremonyAttestation.requireFramedCommitment(block_, '"access_token":"', '"').commitment; + } + + /// @dev The anchor range carries the prefix with the platform's spaces, + /// tab and newline inside, ends where the commitment starts, and the + /// closing quote follows. The whitespace stays revealed at its + /// offsets -- the range is the wire -- and is only ignored to compare. + function test_framesABearerBehindJsonWhitespace() public { + bytes memory prefix = bytes('"access_token" \t: \r\n"'); + uint32 start = uint32(prefix.length); + CeremonyAttestation.DirectionBlock memory block_; + block_.revealed = new CeremonyAttestation.RevealedRange[](2); + block_.revealed[0] = CeremonyAttestation.RevealedRange({start: 0, end: start, value: prefix}); + block_.revealed[1] = CeremonyAttestation.RevealedRange({start: start + 5, end: start + 6, value: '"'}); + block_.commitments = new CeremonyAttestation.RangeCommitment[](1); + block_.commitments[0] = + CeremonyAttestation.RangeCommitment({start: start, end: start + 5, commitment: bytes32(uint256(7))}); + assertEq(this.framed(block_), bytes32(uint256(7))); + + // One byte off: no revealed range ends where the commitment starts. + block_.commitments[0].start = start + 1; + vm.expectRevert(CeremonyAttestation.NoFramedCommitment.selector); + this.framed(block_); + + // A second prefix, compact, elsewhere among the revealed bytes is + // ambiguous even with no second commitment behind it. + block_.commitments[0].start = start; + block_.revealed[1].value = bytes('" "access_token":"'); + block_.revealed[1].end = block_.revealed[1].start + uint32(block_.revealed[1].value.length); + vm.expectRevert(CeremonyAttestation.AmbiguousFraming.selector); + this.framed(block_); + } + + /// @dev The prefix split across two revealed ranges around a committed + /// byte. Joined and normalized it would read as the prefix; it is + /// refused, because no single range ends at the commitment with it, + /// and a prefix assembled across a seam is one the platform never + /// wrote in one piece. + function test_refusesAPrefixSplitAcrossRanges() public { + bytes memory first = bytes('"access_token" '); + bytes memory second = bytes(': \t"'); + uint32 split = uint32(first.length); + uint32 bearer = split + 1 + uint32(second.length); + CeremonyAttestation.DirectionBlock memory block_; + block_.revealed = new CeremonyAttestation.RevealedRange[](3); + block_.revealed[0] = CeremonyAttestation.RevealedRange({start: 0, end: split, value: first}); + block_.revealed[1] = CeremonyAttestation.RevealedRange({start: split + 1, end: bearer, value: second}); + block_.revealed[2] = CeremonyAttestation.RevealedRange({start: bearer + 5, end: bearer + 6, value: '"'}); + block_.commitments = new CeremonyAttestation.RangeCommitment[](2); + block_.commitments[0] = + CeremonyAttestation.RangeCommitment({start: split, end: split + 1, commitment: bytes32(uint256(8))}); + block_.commitments[1] = + CeremonyAttestation.RangeCommitment({start: bearer, end: bearer + 5, commitment: bytes32(uint256(7))}); + vm.expectRevert(CeremonyAttestation.NoFramedCommitment.selector); + this.framed(block_); + } } diff --git a/solidity/contracts/ceremony/test/CeremonyFields.t.sol b/solidity/contracts/ceremony/test/CeremonyFields.t.sol index e7689b5..0b1b6fc 100644 --- a/solidity/contracts/ceremony/test/CeremonyFields.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyFields.t.sol @@ -176,4 +176,41 @@ contract CeremonyFieldsTest is Test { assertFalse(CeremonyFields.isSerializerSafe(bytes(""))); assertFalse(CeremonyFields.isSerializerSafe(hex"c3a9")); // non-ASCII } + + // ─── JSON whitespace ──────────────────────────────────────────── + + /// @dev JSON whitespace between tokens is not part of any token. The + /// readers remove it before they look, so a pretty-printed member + /// reads as its compact spelling does. + function test_readsMembersThroughJsonWhitespace() public view { + bytes memory body = bytes('{\n "login" \t: "alice",\r\n "id" : 123 \n}'); + assertEq(string(this.jsonString(body, "login")), "alice"); + assertEq(string(this.jsonInteger(body, "id")), "123"); + } + + /// @dev And a duplicate in another spelling is still a duplicate, for a + /// string and for an integer alike. + function test_countsADuplicateInAnotherWhitespaceSpelling() public { + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.AmbiguousField.selector, "login")); + this.jsonString(bytes('{"login":"alice","login" : "bob"}'), "login"); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.AmbiguousField.selector, "id")); + this.jsonInteger(bytes('{"id":123,"id" : 456}'), "id"); + } + + /// @dev Only the four bytes JSON calls whitespace are removed. A vertical + /// tab is not one of them, and a member spelled with it is no member. + function test_removesOnlyJsonWhitespace() public { + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.FieldNotFound.selector, "login")); + this.jsonString(bytes.concat(bytes('{"login":'), hex"0b", bytes('"alice"}')), "login"); + } + + /// @dev Whitespace before a brace is JSON's and goes; whitespace between + /// two runs of digits touches no structural byte, stays, and is the + /// terminator the reader then refuses. `123 4` does not read as + /// `1234`. + function test_stillRefusesDigitsAfterWhitespace() public { + assertEq(string(this.jsonInteger(bytes('{"id":123 }'), "id")), "123"); + vm.expectRevert(abi.encodeWithSelector(CeremonyFields.BadIntegerTerminator.selector, "id", bytes1(0x20))); + this.jsonInteger(bytes('{"id":123 4}'), "id"); + } } diff --git a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol index 0868daf..9deb9f2 100644 --- a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol @@ -277,7 +277,10 @@ contract GitHubPlatformVerifierTest is Test { /// rather than a prefix of a longer one. function test_rejectsAnIdWithoutAStructuralTerminator() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); - s.identitySession = _identity('{"login":"octocat","id":583231 }', CeremonyProfile.AUTHORITY_GITHUB_API); + // A space before the brace is JSON's own and reads through; a space + // before more digits touches no structural byte and is the + // terminator, which is not one. + s.identitySession = _identity('{"login":"octocat","id":583231 4}', CeremonyProfile.AUTHORITY_GITHUB_API); vm.expectPartialRevert(CeremonyFields.BadIntegerTerminator.selector); this.run{value: quote}(s); } @@ -331,6 +334,30 @@ contract GitHubPlatformVerifierTest is Test { this.run{value: quote}(s); } + /// @dev GitHub pretty-prints `/user` for the media type the profile pins: + /// a newline and two spaces before every member, a space after every + /// colon. The readers remove JSON whitespace before they look, so the + /// compact delimiters they match are the grammar, not the bytes. + function test_readsTheIdentityGitHubPrettyPrints() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = _identity( + '{\n "login": "octocat",\n "id": 583231,\n "node_id": "MDQ6VXNlcjU4MzIzMQ==",\n "name": "The Octocat"\n}', + CeremonyProfile.AUTHORITY_GITHUB_API + ); + ICeremony.VerifiedClaim memory f = this.run{value: quote}(s); + assertEq(f.userId, "583231"); + assertEq(f.handle, "octocat"); + } + + /// @dev A second `login` in another spelling is a second `login`. + function test_rejectsADuplicateMemberInAnotherWhitespaceSpelling() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity('{"login":"octocat","id":583231,"login" : "mallory"}', CeremonyProfile.AUTHORITY_GITHUB_API); + vm.expectRevert(abi.encodeWithSelector(TlsNotaryVerifierBase.FieldNotUnique.selector, "login", 2)); + this.run{value: quote}(s); + } + /// @dev The wrong authority is still refused before any field is read. function test_rejectsAnIdentityReadFromTheWrongAuthority() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload();