Translate SHA-2 intermediate state for cross-endian peers#468
Open
yosuke-wolfssl wants to merge 1 commit into
Open
Translate SHA-2 intermediate state for cross-endian peers#468yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes mixed-endian client/server interoperability for multi-message SHA-2 hashing by correctly translating the intermediate chaining state (previously treated as an opaque byte array) while preserving the canonical byte order of finalized digests.
Changes:
- Add
_TranslateSha2State()and newwh_MessageCrypto_TranslateSha2Response_ex()/wh_MessageCrypto_TranslateSha2DmaResponse_ex()so callers can specify whetherhash[]is intermediate state (swap per word) or a finalized digest (no swap). - Update server SHA-2 handlers to pass
stateWordSizebased onreq.isLastBlockso only non-final responses have their chaining words swapped. - Add a new direct-translation unit test covering SHA-2 (32/64-bit words), in-place translation, involution, and SHA-3 Keccak state translation; update draft docs accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| wolfhsm/wh_message_crypto.h | Adds _ex translator APIs documenting caller-provided stateWordSize. |
| src/wh_message_crypto.c | Implements SHA-2 state per-word swapping and wraps legacy response translators as digest-only. |
| src/wh_server_crypto.c | Uses _ex response translators with stateWordSize keyed off isLastBlock. |
| test-refactor/wh_test_list.c | Registers the new translation-focused test in the misc suite. |
| test-refactor/misc/wh_test_message_crypto.c | New tests for SHA-2/SHA-3 translation behavior, including foreign-endian and in-place cases. |
| docs/draft/async-crypto.md | Documents the non-opaque nature and endian rules for resumeState.hash and response hashes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #468
Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
No new issues found in the changed files. ✅
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.
Problem
wolfHSM supports a client and server on cores with different byte order: every
message header carries an endian marker and the server byte-swaps each field via
wh_Translate*. The SHA-2 wire structs declare the resume state asuint8_t hash[32]/[64], and the translation helpers skipped it as "just a bytearray". But that array is a verbatim copy of
wc_Sha256.digest/wc_Sha512.digest, which wolfCrypt keeps as host-orderword32/word64. Across-endian pair therefore exchanged byte-flipped chaining words: every
non-final update compressed from garbage and the digest was silently wrong — no
error, no crash. Final digests are unaffected (wolfCrypt emits canonical
big-endian at finalize), so only multi-message streams broke, and same-endian
pairs never saw it. SHA-3 was already correct:
whMessageCrypto_Sha3Stateisuint64_t s[25], swapped per word.Addressed by Fenrir 4041.
Fix (
src/wh_message_crypto.c)New
_TranslateSha2State()swaps the eight chaining words, going through a localword via
memcpy(no alignment or aliasing hazard, safe whensrc == dest).The response direction is ambiguous —
hash[]holds host-order state on anupdate but a canonical digest on the last block, and the struct has no
isLastBlock— so the caller now states which it is. The existing responsesymbols remain as wrappers passing
0; no public signature changed.stateWordSize...Sha256Request,...Sha256DmaRequest4, unconditional...Sha512Request,...Sha512DmaRequest8, unconditional...Sha2Response_ex,...Sha2DmaResponse_ex(new)req.isLastBlock ? 0 : 4/8src/wh_client_crypto.cis untouched: only the server translates, so theclient's copy defines the wire format. Same-endian wire bytes are identical to
before, so there is no cross-version interop break. Closes 4041.
Tests
New
test-refactor/misc/wh_test_message_crypto.c— the translate layer had nocoverage at all. A loopback test cannot observe peers that disagree on byte
order, so it drives the helpers directly with a foreign-endian magic and checks
per-word swapping, byte-for-byte identity under a native magic, involution,
in-place translation, and that a finalized digest is left alone. It covers the
Keccak state too, which validates the test's own swap detector against
already-correct code.
Verification
test-refactor: 38 passed / 0 failed of 63;DMA=142 passed;NOCRYPTO=112passed. Legacy
test/suite clean.-std=c90 -Werror -Wall -Wextra.memcpyfails the new test(
rc=-1).