Fix SHA1 on platforms where Long is 32-bit#44
Merged
Conversation
The SHA1 word arithmetic ran in Long and masked back to 32 bits with (bit-and x 4294967295l). Where Long is 32 bits the literal 4294967295l is -1, so the mask was a no-op and bit-shift-right sign-extended instead of zero-filling. Every digest came out wrong, which broke the WebSocket opening handshake (Sec-WebSocket-Accept is a SHA1 hash) and HMAC-SHA1 / signed cookies with it. 64-bit-Long platforms, including the macOS CI, were unaffected, so it stayed hidden. Do the word arithmetic in Uint32, which is exactly 32 bits with logical shifts everywhere, so the mask is unnecessary. The round constants and init state are composed from two sub-2^16 halves since Carp has no Uint32 literal and Long literals above 2^31 truncate here. The 64-bit length field moves to Uint64 (its shifts also exceeded 32 bits). Verified on this 32-bit-Long machine against the SHA1 vectors for "abc", "" and the pangram, and the RFC 6455 accept-key and RFC 2202 HMAC-SHA1 vectors in the suites; the pre-fix code fails them here.
There was a problem hiding this comment.
Build & Tests
Checked out claude/sha1-uint32 at 2a75c71c (merge-base == origin/main, so the CHANGELOG entry lands under the right section — no misfile). Built and ran both suites on this armhf / 32-bit-Long box — the exact platform where the old code was broken:
test/websocket.carp→ 128 passed, 0 failed, including the three that were red here before the fix:SHA1 of 'abc',SHA1 of empty string, andaccept key matches RFC 6455 example.test/web.carp→ 202 passed, 0 failed, includingSHA1 hex-digest matches the known vector for 'abc',HMAC-SHA1 matches RFC 2202 test vector 2, andsigned-cookie verifies and returns original value.
Findings
No blocking findings. This is a real, client-facing correctness bug and the fix is right. What I checked beyond the suite:
- Root cause confirmed.
mask32 = (bit-and x 4294967295l)— the literal4294967295ltruncates to-1whereLongis 32-bit, so the mask was a no-op andbit-shift-rightsign-extended. Every digest came out wrong, which is whySec-WebSocket-Accept,HMAC-SHA1, and signed cookies were all broken on ILP32 targets while the macOS-only CI (64-bitLong) stayed green. - The
Uint32/Uint64rewrite is algorithmically correct.Uint32gives exact 32-bit width with logical shifts and natural wraparound on every platform, somask32is gone entirely; the message-length field correctly moves toUint64(it shifts by up to 56 bits). The round functions (Ch/Parity/Maj), message schedule, working-variable rotation, and big-endian I/O all match the standard. - All 12 composed constants hand-verified against the originals — e.g.
(u32 26437l 8961l)=0x67452301=1732584193(h0), through(u32 51810l 49622l)=0xCA62C1D6=3395469782(kc3). Every half decomposes correctly, and they're independently pinned by the known-answer tests. - Independent oracle. Beyond the repo's own vectors, I compared the checked-out
SHA1.hex-digestagainst systemsha1sumfor inputs spanning one, two, and three 64-byte blocks (43-, 56-, and 130-byte messages). All three matched exactly (2fd4e1c6…,84983e44…,416714cf…), so the multi-block path and theUint64length field are confirmed correct against an oracle that isn't the PR's own test set. - Scope is tight. Only the
SHA1module's internal arithmetic changed;digest/hex-digestand the HMAC/accept-key callers keep their signatures. CHANGELOG entry is user-visible and correctly filed under### Fixed.
Verdict: merge
Correct, well-scoped, and provable end-to-end on the platform the bug lives on. It's still a draft by your choice — I'm leaving it as a draft for you to steer; flip it to ready whenever you're happy, the code itself is done.
hellerve
marked this pull request as ready for review
July 22, 2026 08:02
hellerve
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
SHA1.digestdid its word arithmetic inLongand masked back to 32 bits withmask32:Where Carp's
Longis 32-bit (any ILP32 target — armhf, wasm32, 32-bit x86…), the literal4294967295ltruncates to-1, so:mask32becomes the identity function — no masking at all, andbit-shift-rightsign-extends instead of zero-filling.So
rotl32and every round mix computed garbage, and every digest came out wrong. BecauseSec-WebSocket-Acceptis a SHA1 hash, the RFC 6455 opening handshake failed on those platforms;HMAC-SHA1(HMAC.sha1) and signed cookies (Cookie.sign) were wrong too. 64-bit-Longplatforms — including this repo's macOS-only CI — happen to work, which is why the tests were green while the digest was broken.The fix
Do the word arithmetic in
Uint32, which is exactly 32-bit unsigned with logical shifts on every platform — somask32is no longer needed at all.rotl32, the round functions, the working variables, the message schedule andh0..h4are allUint32now.Two details worth calling out for review:
Uint32literal, andLongliterals above 2³¹ truncate on a 32-bit-Longbox, so the init state and round constants can't be written directly. Each is composed from two sub-2¹⁶ halves via a smallu32helper ((u32 26437l 8961l)=0x67452301).carp-fmtrewrites hex literals to decimal, hence the decimal halves; they're pinned by the known-answer tests below, so a wrong constant fails CI. Happy to switch representation if you'd prefer something else.Longbreakage), so it moves toUint64.Scope is limited to the
SHA1module arithmetic; the public entry points (digest,hex-digest, and theHMAC/ accept-key callers) keep their types and behaviour.Verification
This is one of the rare bugs provable end-to-end on a 32-bit-
Longmachine (this armhf Pi). I extracted the old and newSHA1side by side and ran both against the standard vectors here:"abc"85defb17…❌a9993e36…✅a9993e364706816aba3e25717850c26c9cd0d89d""04c90ac1…❌da39a3ee…✅da39a3ee5e6b4b0d3255bfef95601890afd80709c5223574…❌2fd4e1c6…✅2fd4e1c67a2d28fced849ee1bb76e7391b93eb12The pre-fix code fails all three on this box; the new code passes. Both repo suites are green here too:
test/websocket.carp— 128/0, incl.SHA1 of 'abc',SHA1 of empty string, andaccept key matches RFC 6455 example(these were red on this box before).test/web.carp— 202/0, incl. theHMAC-SHA1RFC 2202 vector and a newSHA1 hex-digestknown-answer for"abc".Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.