Skip to content

Fix SHA1 on platforms where Long is 32-bit#44

Merged
hellerve merged 1 commit into
mainfrom
claude/sha1-uint32
Jul 22, 2026
Merged

Fix SHA1 on platforms where Long is 32-bit#44
hellerve merged 1 commit into
mainfrom
claude/sha1-uint32

Conversation

@carpentry-agent

Copy link
Copy Markdown
Contributor

The bug

SHA1.digest did its word arithmetic in Long and masked back to 32 bits with mask32:

(defn mask32 [x] (bit-and x 4294967295l))

Where Carp's Long is 32-bit (any ILP32 target — armhf, wasm32, 32-bit x86…), the literal 4294967295l truncates to -1, so:

  • mask32 becomes the identity function — no masking at all, and
  • bit-shift-right sign-extends instead of zero-filling.

So rotl32 and every round mix computed garbage, and every digest came out wrong. Because Sec-WebSocket-Accept is 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-Long platforms — 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 — so mask32 is no longer needed at all. rotl32, the round functions, the working variables, the message schedule and h0..h4 are all Uint32 now.

Two details worth calling out for review:

  • Constants. Carp has no Uint32 literal, and Long literals above 2³¹ truncate on a 32-bit-Long box, so the init state and round constants can't be written directly. Each is composed from two sub-2¹⁶ halves via a small u32 helper ((u32 26437l 8961l) = 0x67452301). carp-fmt rewrites 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.
  • Length field. The 64-bit big-endian message-length also shifted by ≥32 bits (same 32-bit-Long breakage), so it moves to Uint64.

Scope is limited to the SHA1 module arithmetic; the public entry points (digest, hex-digest, and the HMAC / accept-key callers) keep their types and behaviour.

Verification

This is one of the rare bugs provable end-to-end on a 32-bit-Long machine (this armhf Pi). I extracted the old and new SHA1 side by side and ran both against the standard vectors here:

input pre-fix (here) post-fix expected
"abc" 85defb17… a9993e36… a9993e364706816aba3e25717850c26c9cd0d89d
"" 04c90ac1… da39a3ee… da39a3ee5e6b4b0d3255bfef95601890afd80709
pangram c5223574… 2fd4e1c6… 2fd4e1c67a2d28fced849ee1bb76e7391b93eb12

The 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, and accept key matches RFC 6455 example (these were red on this box before).
  • test/web.carp — 202/0, incl. the HMAC-SHA1 RFC 2202 vector and a new SHA1 hex-digest known-answer for "abc".

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

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.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.carp128 passed, 0 failed, including the three that were red here before the fix: SHA1 of 'abc', SHA1 of empty string, and accept key matches RFC 6455 example.
  • test/web.carp202 passed, 0 failed, including SHA1 hex-digest matches the known vector for 'abc', HMAC-SHA1 matches RFC 2202 test vector 2, and signed-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 literal 4294967295l truncates to -1 where Long is 32-bit, so the mask was a no-op and bit-shift-right sign-extended. Every digest came out wrong, which is why Sec-WebSocket-Accept, HMAC-SHA1, and signed cookies were all broken on ILP32 targets while the macOS-only CI (64-bit Long) stayed green.
  • The Uint32/Uint64 rewrite is algorithmically correct. Uint32 gives exact 32-bit width with logical shifts and natural wraparound on every platform, so mask32 is gone entirely; the message-length field correctly moves to Uint64 (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-digest against system sha1sum for 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 the Uint64 length field are confirmed correct against an oracle that isn't the PR's own test set.
  • Scope is tight. Only the SHA1 module's internal arithmetic changed; digest/hex-digest and 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
hellerve marked this pull request as ready for review July 22, 2026 08:02
@hellerve
hellerve merged commit 1698466 into main Jul 22, 2026
1 check passed
@hellerve
hellerve deleted the claude/sha1-uint32 branch July 22, 2026 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant