diff --git a/solidity/contracts/ceremony/CeremonyFields.sol b/solidity/contracts/ceremony/CeremonyFields.sol index 588105c9..997023db 100644 --- a/solidity/contracts/ceremony/CeremonyFields.sol +++ b/solidity/contracts/ceremony/CeremonyFields.sol @@ -567,4 +567,29 @@ library CeremonyFields { } return true; } + + /// @notice Whether `value` is the canonical decimal spelling of a nonzero + /// unsigned 64-bit integer: `^[1-9][0-9]{0,19}$`, at most + /// `2^64 - 1`. + /// + /// @dev REQ-PLAT-06, the grammar of an X or GitHub `userId`: every role + /// must record one account under the same bytes. + function isCanonicalNonzeroUint64(bytes memory value) internal pure returns (bool) { + uint256 n = value.length; + if (n == 0 || n > 20) return false; + // Twenty bytes fit in one word, read from its most significant byte. + uint256 w = _word(value, 0); + if (w >> 248 == 0x30) return false; + uint256 number; + unchecked { + for (uint256 i = 0; i < n; ++i) { + // A byte below `0` wraps past 9, so one comparison bounds both ends. + uint256 digit = ((w >> (248 - 8 * i)) & 0xff) - 0x30; + if (digit > 9) return false; + // At most twenty digits: no product here can overflow. + number = number * 10 + digit; + } + } + return number <= type(uint64).max; + } } diff --git a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol index 47237f78..128f9a1e 100644 --- a/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol +++ b/solidity/contracts/ceremony/TlsNotaryVerifierBase.sol @@ -93,6 +93,9 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa error ClientIdentifierNotSerializerSafe(bytes found); /// @dev A field was found in no revealed range, or in more than one. error FieldNotUnique(string name, uint256 rangesMatching); + /// @dev The id read is not a nonzero unsigned 64-bit integer in its + /// canonical decimal spelling (REQ-PLAT-06). + error NoncanonicalUserId(bytes found); /// @dev The first revealed range does not begin the transcript, so nothing /// says the bytes read as a request line ARE the request line. error RequestLineNotAtOrigin(uint32 start); @@ -173,7 +176,8 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa /// Declaring the field names and leaving the reading to the base makes /// all three unwritable rather than forbidden by a comment. A new /// profile supplies two strings and a shape; it never touches an - /// attestation. + /// attestation. Whatever the shape, the id must spell a nonzero + /// `uint64` in decimal (REQ-PLAT-06), as X and GitHub ids do. function _identityFields() internal pure @@ -435,11 +439,13 @@ abstract contract TlsNotaryVerifierBase is IPlatformVerifier, PlatformVerifierBa } bytes memory joined = CeremonyFields.normalizeJsonBytes(CeremonyAttestation.concatRevealed(data.received)); (string memory idField, IdShape idShape, string memory handleField) = _identityFields(); - userId = string( - idShape == IdShape.JsonString - ? _uniqueJsonString(ranges, joined, idField) - : _uniqueJsonInteger(ranges, joined, idField) - ); + bytes memory id = idShape == IdShape.JsonString + ? _uniqueJsonString(ranges, joined, idField) + : _uniqueJsonInteger(ranges, joined, idField); + // The readers hold the id to its JSON syntax only: X's quotes admit + // any string, GitHub's digits a lone `0` and any length. + if (!CeremonyFields.isCanonicalNonzeroUint64(id)) revert NoncanonicalUserId(id); + userId = string(id); handle = string(_uniqueJsonString(ranges, joined, handleField)); } diff --git a/solidity/contracts/ceremony/test/CeremonyFields.t.sol b/solidity/contracts/ceremony/test/CeremonyFields.t.sol index 6e173de6..e6788ec7 100644 --- a/solidity/contracts/ceremony/test/CeremonyFields.t.sol +++ b/solidity/contracts/ceremony/test/CeremonyFields.t.sol @@ -122,8 +122,12 @@ contract CeremonyFieldsTest is Test { this.jsonInteger(bytes('{"id":1.5}'), "id"); } - function test_zeroIsCanonical() public view { - assertEq(string(this.jsonInteger(bytes('{"id":0}'), "id")), "0"); + /// @dev `0` is a JSON integer in its one spelling, so the reader returns + /// it; the `userId` grammar is what refuses it. + function test_readsZeroWhichIsNoUserId() public view { + bytes memory zero = this.jsonInteger(bytes('{"id":0}'), "id"); + assertEq(string(zero), "0"); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64(zero)); } function test_refusesAQuotedIntegerForTheIntegerTemplate() public { @@ -132,6 +136,38 @@ contract CeremonyFieldsTest is Test { this.jsonInteger(bytes('{"id":"1"}'), "id"); } + // ─── The userId grammar (REQ-PLAT-06) ─────────────────────────── + + function test_acceptsANonzeroUint64UpToItsMaximum() public pure { + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("1")); + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("2244994945")); + assertTrue(CeremonyFields.isCanonicalNonzeroUint64("18446744073709551615")); + } + + function test_refusesAValuePastUint64() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("18446744073709551616")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("99999999999999999999")); + // Twenty-one digits. + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("100000000000000000000")); + } + + function test_refusesZeroAndALeadingZero() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("0")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("007")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("01844674407370955161")); + } + + function test_refusesAnythingButDigits() public pure { + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("abc")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("12a")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("-1")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("+1")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("1.5")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64("1e3")); + assertFalse(CeremonyFields.isCanonicalNonzeroUint64(" 1")); + } + // ─── Form fields ──────────────────────────────────────────────── function test_readsAnXTokenRequestBody() public view { diff --git a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol index 93b492d4..d472fd8d 100644 --- a/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/GitHubPlatformVerifier.t.sol @@ -622,6 +622,26 @@ contract GitHubPlatformVerifierTest is Test { this.run{value: quote}(s); } + /// @dev REQ-PLAT-06. Each is a JSON integer the reader returns, and none + /// is a nonzero `uint64`. + function test_rejectsAnIdOutsideTheGrammar() public { + string[3] memory ids = ["0", "18446744073709551616", "100000000000000000000"]; + for (uint256 i = 0; i < ids.length; ++i) { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity(string.concat('{"login":"octocat","id":', ids[i], "}"), CeremonyProfile.AUTHORITY_GITHUB_API); + vm.expectRevert(abi.encodeWithSelector(TlsNotaryVerifierBase.NoncanonicalUserId.selector, bytes(ids[i]))); + this.run{value: quote}(s); + } + } + + function test_acceptsTheLargestId() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = + _identity('{"login":"octocat","id":18446744073709551615}', CeremonyProfile.AUTHORITY_GITHUB_API); + assertEq(this.run{value: quote}(s).userId, "18446744073709551615"); + } + // ─── The handle field is `login` ──────────────────────────────── function test_rejectsAResponseWithNoLogin() public { diff --git a/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol b/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol index abbb2374..981ba5e2 100644 --- a/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol +++ b/solidity/contracts/ceremony/test/TranscriptEquivalence.t.sol @@ -151,6 +151,10 @@ contract LiveHelpers { return CeremonyFields.isSerializerSafe(value); } + function isCanonicalNonzeroUint64(bytes memory value) external pure returns (bool) { + return CeremonyFields.isCanonicalNonzeroUint64(value); + } + function occurrences(bytes memory haystack, bytes memory needle) external pure returns (uint256) { return CeremonyFields.occurrences(haystack, needle); } @@ -218,6 +222,10 @@ contract RefHelpers { return RefCeremonyFields.isSerializerSafe(value); } + function isCanonicalNonzeroUint64(bytes memory value) external pure returns (bool) { + return RefCeremonyFields.isCanonicalNonzeroUint64(value); + } + function occurrences(bytes memory haystack, bytes memory needle) external pure returns (uint256) { return RefCeremonyAttestation._occurrences(haystack, needle); } @@ -731,8 +739,8 @@ library Gen { { bytes memory ws = oneOf(r, list("", " ", "\n ", "\t")); bytes memory idValue = integerId - ? (chance(r, 80) ? bytes("293919812") : oneOf(r, list("0", "007", "12 3"))) - : (chance(r, 80) ? bytes('"1051915704843333634"') : oneOf(r, list('""', '"7', "7"))); + ? (chance(r, 80) ? bytes("293919812") : oneOf(r, list("0", "007", "12 3", "18446744073709551616"))) + : (chance(r, 80) ? bytes('"1051915704843333634"') : oneOf(r, list('""', '"7', "7", '"abc"'))); bytes memory idMember = abi.encodePacked('"id"', ws, ":", ws, idValue); bytes memory handleMember = abi.encodePacked('"', handleField, '"', ws, ":", ws, '"', oneOf(r, list("alice", "Bob_1", "", "a b")), '"'); @@ -959,6 +967,16 @@ contract TranscriptEquivalenceTest is Test { _same(address(live), address(ref), abi.encodeCall(LiveHelpers.isSerializerSafe, (value))); } + /// forge-config: default.fuzz.runs = 2000 + function testFuzz_userIdGrammarMatchesReference(uint256 seed, bytes memory raw) public view { + Gen.Rng memory r = Gen.Rng(seed); + // Digit runs either side of twenty, and one-byte edits of `2^64 - 1`. + bytes memory value = r.chance(25) + ? raw + : r.chance(50) ? r.soup("0123456789", 22) : r.mutate("18446744073709551615", "0123456789a"); + _same(address(live), address(ref), abi.encodeCall(LiveHelpers.isCanonicalNonzeroUint64, (value))); + } + /// forge-config: default.fuzz.runs = 2000 function testFuzz_revealedJoinMatchesReference(uint256 seed) public view { Gen.Rng memory r = Gen.Rng(seed); diff --git a/solidity/contracts/ceremony/test/TranscriptReference.sol b/solidity/contracts/ceremony/test/TranscriptReference.sol index 3562465b..9964bdfd 100644 --- a/solidity/contracts/ceremony/test/TranscriptReference.sol +++ b/solidity/contracts/ceremony/test/TranscriptReference.sol @@ -234,7 +234,8 @@ library RefCeremonyAttestation { } /// @notice The field readers of `CeremonyFields` as of commit 7a63c20, code -/// unchanged. +/// unchanged, and the REQ-PLAT-06 `userId` grammar, written as a digit +/// scan and a comparison with the decimal of `2^64 - 1`. library RefCeremonyFields { error AmbiguousField(string name); error FieldNotFound(string name); @@ -449,6 +450,20 @@ library RefCeremonyFields { || c == "_" || c == "-"; } + function isCanonicalNonzeroUint64(bytes memory value) internal pure returns (bool) { + bytes memory max = "18446744073709551615"; + if (value.length == 0 || value.length > max.length || value[0] == "0") return false; + for (uint256 i = 0; i < value.length; ++i) { + if (value[i] < "0" || value[i] > "9") return false; + } + if (value.length < max.length) return true; + // Equal lengths of digits order as their numbers do. + for (uint256 i = 0; i < max.length; ++i) { + if (value[i] != max[i]) return value[i] < max[i]; + } + return true; + } + function _matchesAt(bytes memory data, bytes memory needle, uint256 at) private pure returns (bool) { for (uint256 j = 0; j < needle.length; ++j) { if (data[at + j] != needle[j]) return false; @@ -458,8 +473,9 @@ library RefCeremonyFields { } /// @notice The transcript checks `TlsNotaryVerifierBase` ran on each session -/// as of commit 7a63c20, code unchanged: everything between -/// authenticating an attestation and reading its time. +/// as of commit 7a63c20: everything between authenticating an +/// attestation and reading its time. Code unchanged except the +/// REQ-PLAT-06 check on the id the identity session reads. /// @dev A profile's hooks are parameters here: `Profile` carries the /// constants `XPlatformVerifier` and `GitHubPlatformVerifier` return. library RefTranscript { @@ -467,6 +483,7 @@ library RefTranscript { error CodeVerifierMismatch(); error ClientIdentifierNotSerializerSafe(bytes found); error FieldNotUnique(string name, uint256 rangesMatching); + error NoncanonicalUserId(bytes found); error RequestLineNotAtOrigin(uint32 start); error WrongTokenRequestLayout(uint256 revealedRanges, uint256 commitments); error NoHeadBoundary(uint256 occurrences); @@ -579,11 +596,11 @@ library RefTranscript { RefCeremonyAttestation.requireExactCoverage(data.received, data.recvTranscriptLength); bytes memory joined = RefCeremonyAttestation.concatRevealed(data.received); - userId = string( - !profile.idIsInteger - ? _uniqueJsonString(data.received, joined, profile.idField) - : _uniqueJsonInteger(data.received, joined, profile.idField) - ); + bytes memory id = !profile.idIsInteger + ? _uniqueJsonString(data.received, joined, profile.idField) + : _uniqueJsonInteger(data.received, joined, profile.idField); + if (!RefCeremonyFields.isCanonicalNonzeroUint64(id)) revert NoncanonicalUserId(id); + userId = string(id); handle = string(_uniqueJsonString(data.received, joined, profile.handleField)); } diff --git a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol index 7747a0f9..4b606d12 100644 --- a/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol +++ b/solidity/contracts/ceremony/test/XPlatformVerifier.t.sol @@ -1781,6 +1781,24 @@ contract XPlatformVerifierTest is Test { this.run{value: quote}(s); } + /// @dev REQ-PLAT-06. The string reader returns whatever lies between the + /// quotes, so the id grammar alone refuses each of these. + function test_rejectsAnIdOutsideTheGrammar() public { + string[6] memory ids = ["0", "007", "abc", "-1", "18446744073709551616", "100000000000000000000"]; + for (uint256 i = 0; i < ids.length; ++i) { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = _identityAttestation(ids[i], "alice", ""); + vm.expectRevert(abi.encodeWithSelector(TlsNotaryVerifierBase.NoncanonicalUserId.selector, bytes(ids[i]))); + this.run{value: quote}(s); + } + } + + function test_acceptsTheLargestId() public { + TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); + s.identitySession = _identityAttestation("18446744073709551615", "alice", ""); + assertEq(this.run{value: quote}(s).userId, "18446744073709551615"); + } + function test_theIdentityAttestationsOwnTimeIsNotEvidenceTime() public { TlsNotaryVerifierBase.TlsNotaryProof memory s = _payload(); bytes memory a = s.identitySession.attestedData; diff --git a/solidity/snapshots/claim-verification.json b/solidity/snapshots/claim-verification.json index 87dd4011..eae91dd5 100644 --- a/solidity/snapshots/claim-verification.json +++ b/solidity/snapshots/claim-verification.json @@ -1,6 +1,6 @@ { "BearerLinkHonkVerifier.verify": "720457", - "GitHubPlatformVerifier.verify": "1146243", + "GitHubPlatformVerifier.verify": "1147664", "OidcGoogleHonkVerifier.verify": "755087", - "XPlatformVerifier.verify": "1094893" + "XPlatformVerifier.verify": "1097665" } \ No newline at end of file