Skip to content

add SHA3 support#384

Open
bigbrett wants to merge 4 commits into
wolfSSL:mainfrom
bigbrett:sha3
Open

add SHA3 support#384
bigbrett wants to merge 4 commits into
wolfSSL:mainfrom
bigbrett:sha3

Conversation

@bigbrett

Copy link
Copy Markdown
Contributor

Adds support for SHA3 to wolfHSM. Supports blocking wolfCrypt and async native API surface.

Copilot AI review requested due to automatic review settings May 28, 2026 16:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds SHA3 support across wolfHSM’s crypto message formats, client APIs, server handlers, cryptocb dispatch, and tests for blocking, async, and DMA paths.

Changes:

  • Introduces SHA3 wire formats, translation helpers, and comm-buffer sizing checks.
  • Adds SHA3 client/server handling for SHA3-224/256/384/512, including DMA and Keccak rejection/fallback behavior.
  • Expands crypto tests and test configuration to exercise SHA3 variants and async paths.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
wolfhsm/wh_message_crypto.h Adds SHA3/SHA3-DMA message structs, sizing macros, static asserts, and translation prototypes.
wolfhsm/wh_client_crypto.h Adds SHA3 include and public client API declarations.
src/wh_message_crypto.c Implements SHA3 and SHA3-DMA message translation helpers.
src/wh_client_crypto.c Implements shared SHA3 client helpers, per-variant APIs, and DMA APIs.
src/wh_client_cryptocb.c Routes SHA3 cryptocb requests to wolfHSM client APIs with Keccak fallback.
src/wh_server_crypto.c Adds SHA3 and SHA3-DMA server handlers and dispatch cases.
test/wh_test_crypto.c Adds SHA3 functional, async, DMA, bad-argument, and Keccak behavior tests.
test/wh_test_check_struct_padding.c Adds SHA3 message structs to padding checks.
test/config/user_settings.h Enables hash flags for SHA3 Keccak-path test coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/wh_client_crypto.c
Comment thread src/wh_client_crypto.c
Comment thread test/wh_test_crypto.c
Comment thread test/wh_test_crypto.c
Comment thread test/wh_test_crypto.c
Comment thread test/wh_test_crypto.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-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.

Fenrir Automated Review — PR #384

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread wolfhsm/wh_message_crypto.h
Comment thread src/wh_server_crypto.c
Comment thread wolfhsm/wh_message_crypto.h
Comment thread src/wh_server_crypto.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-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.

Fenrir Automated Review — PR #384

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

No new issues found in the changed files. ✅

@bigbrett bigbrett marked this pull request as ready for review July 7, 2026 23:39
@bigbrett bigbrett requested a review from Frauschi July 7, 2026 23:39

@Frauschi Frauschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only minor issues, otherwise LGTM.

Just one question: is SHAKE128/SHAKE256 missing on purpose?

Comment thread wolfhsm/wh_message_crypto.h Outdated
Comment thread src/wh_client_crypto.c
@bigbrett

bigbrett commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@Frauschi didn't even think about SHAKE tbh. I can add in a subsequent PR. This was largely driven by a customer request for SHA3 who never mentioned Shake

@Frauschi

Frauschi commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@bigbrett Ah ok, I just asked because SHAKE is the one required for the PQC algos we talked about a few weeks ago.

@Frauschi Frauschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@bigbrett bigbrett assigned AlexLanzano and unassigned Frauschi Jul 13, 2026
@bigbrett bigbrett requested a review from AlexLanzano July 13, 2026 17:55
Comment thread src/wh_client_crypto.c
}
}

/* Pack as many whole blocks from input as fit inline. */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Fix grammar?

Comment thread src/wh_client_crypto.c
Comment on lines +8534 to +8535
while (i < inLen && sha->i < v->blockSize) {
sha->t[sha->i++] = in[i++];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we do an additional bounds check prior to this to see that:
v->blockSize < MIN(sizeof(sha->t), sizeof(in))

Comment thread src/wh_client_crypto.c
Comment on lines +8614 to +8616
for (k = 0; k < 25; k++) {
sha->s[k] = res->resumeState.s[k];
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: use memcpy instead? Same for other instances

Comment thread src/wh_client_crypto.c
Comment on lines +8760 to +8803
/* Per-variant public APIs - thin wrappers over the shared helpers. */

#define WH_SHA3_VARIANT_API(NN) \
int wh_Client_Sha3_##NN(whClientContext* ctx, wc_Sha3* sha, \
const uint8_t* in, uint32_t inLen, uint8_t* out) \
{ \
return _Sha3Oneshot(ctx, sha, &whSha3_##NN, in, inLen, out); \
} \
int wh_Client_Sha3_##NN##UpdateRequest(whClientContext* ctx, wc_Sha3* sha, \
const uint8_t* in, uint32_t inLen, \
bool* requestSent) \
{ \
return _Sha3UpdateRequest(ctx, sha, &whSha3_##NN, in, inLen, \
requestSent); \
} \
int wh_Client_Sha3_##NN##UpdateResponse(whClientContext* ctx, \
wc_Sha3* sha) \
{ \
return _Sha3UpdateResponse(ctx, sha, &whSha3_##NN); \
} \
int wh_Client_Sha3_##NN##FinalRequest(whClientContext* ctx, wc_Sha3* sha) \
{ \
return _Sha3FinalRequest(ctx, sha, &whSha3_##NN); \
} \
int wh_Client_Sha3_##NN##FinalResponse(whClientContext* ctx, wc_Sha3* sha, \
uint8_t* out) \
{ \
return _Sha3FinalResponse(ctx, sha, &whSha3_##NN, out); \
}

#ifndef WOLFSSL_NOSHA3_224
WH_SHA3_VARIANT_API(224)
#endif
#ifndef WOLFSSL_NOSHA3_256
WH_SHA3_VARIANT_API(256)
#endif
#ifndef WOLFSSL_NOSHA3_384
WH_SHA3_VARIANT_API(384)
#endif
#ifndef WOLFSSL_NOSHA3_512
WH_SHA3_VARIANT_API(512)
#endif

#undef WH_SHA3_VARIANT_API

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not the biggest fan of this. Would rather just lay these functions out explicitly to keep them grepable and easy to jump to with LSPs or other debugging tools.

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.

5 participants