Skip to content

Fix sha512 buffer copy #333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
padelsbach wants to merge 1 commit into
base: main
Choose a base branch
from
+240 −30
Open

Fix sha512 buffer copy #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions src/wh_client_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5249,11 +5249,10 @@ static int _xferSha512BlockAndUpdateDigest(whClientContext* ctx,
}

int wh_Client_Sha512(whClientContext* ctx, wc_Sha512* sha512, const uint8_t* in,
uint32_t inLen, uint8_t* out)
uint32_t inLen, uint8_t* out, int hashType)
{
int ret = 0;
uint8_t* sha512BufferBytes = (uint8_t*)sha512->buffer;
int hashType = WC_HASH_TYPE_SHA512;

/* Caller invoked SHA Update:
* wc_CryptoCb_Sha512Hash(sha512, data, len, NULL) */
Expand Down Expand Up @@ -5291,17 +5290,33 @@ int wh_Client_Sha512(whClientContext* ctx, wc_Sha512* sha512, const uint8_t* in,
/* Caller invoked SHA finalize:
* wc_CryptoCb_Sha512Hash(sha512, NULL, 0, * hash) */
if (ret == 0 && out != NULL) {
word32 digestSz;

ret = _xferSha512BlockAndUpdateDigest(ctx, sha512, 1);

/* Use the hashType from the dispatcher (info->hash.type) to
* select the correct output size. This is more reliable than
* sha512->hashType which depends on the wolfSSL port setting
* it during init. */
switch (hashType) {
case WC_HASH_TYPE_SHA512_224:
digestSz = WC_SHA512_224_DIGEST_SIZE;
break;
case WC_HASH_TYPE_SHA512_256:
digestSz = WC_SHA512_256_DIGEST_SIZE;
break;
default:
digestSz = WC_SHA512_DIGEST_SIZE;
break;
}

/* Copy out the final hash value */
if (ret == 0) {
memcpy(out, sha512->digest, WC_SHA512_DIGEST_SIZE);
memcpy(out, sha512->digest, digestSz);
}
/* keep hashtype before initialization */
hashType = sha512->hashType;
/* reset the state of the sha context (without blowing away devId and
* hashType)
*/

/* Reset the sha context for potential reuse, calling the
* variant-appropriate init to preserve devId and hashType */
switch (hashType) {
case WC_HASH_TYPE_SHA512_224:
(void)wc_InitSha512_224_ex(sha512, NULL, sha512->devId);
Expand All @@ -5320,7 +5335,7 @@ int wh_Client_Sha512(whClientContext* ctx, wc_Sha512* sha512, const uint8_t* in,

#ifdef WOLFHSM_CFG_DMA
int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,
uint32_t inLen, uint8_t* out)
uint32_t inLen, uint8_t* out, int hashType)
{
int ret = WH_ERROR_OK;
wc_Sha512* sha512 = sha;
Expand All @@ -5332,6 +5347,20 @@ int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,
uintptr_t inAddr = 0;
uintptr_t outAddr = 0;
uintptr_t stateAddr = 0;
word32 digestSz;

/* Select digest size based on variant */
switch (hashType) {
case WC_HASH_TYPE_SHA512_224:
digestSz = WC_SHA512_224_DIGEST_SIZE;
break;
case WC_HASH_TYPE_SHA512_256:
digestSz = WC_SHA512_256_DIGEST_SIZE;
break;
default:
digestSz = WC_SHA512_DIGEST_SIZE;
break;
}

/* Get data pointer from the context to use as request/response storage */
dataPtr = (uint8_t*)wh_CommClient_GetDataPtr(ctx->comm);
Expand All @@ -5341,12 +5370,12 @@ int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,

/* Setup generic header and get pointer to request data */
req = (whMessageCrypto_Sha2DmaRequest*)_createCryptoRequest(
dataPtr, WC_HASH_TYPE_SHA512, ctx->cryptoAffinity);
dataPtr, hashType, ctx->cryptoAffinity);
Copy link
Copy Markdown
Contributor

@rizlik rizlik Apr 14, 2026

Choose a reason for hiding this comment

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

neither wh_Server_HandleCryptoDmaRequest nor wh_Server_HandleCryptoRequest hanlde SHA512 length variants (

wolfHSM/src/wh_server_crypto.c

Lines 4700 to 4711 in 752ca63

#if defined(WOLFSSL_SHA512)
case WC_HASH_TYPE_SHA512:
WH_DEBUG_SERVER("SHA512 req recv. type:%u\n",
rqstHeader.algoType);
ret = _HandleSha512(ctx, magic, devId, cryptoDataIn,
cryptoInSize, cryptoDataOut,
&cryptoOutSize);
if (ret != 0) {
WH_DEBUG_SERVER("SHA512 ret = %d\n", ret);
}
break;
#endif /* WOLFSSL_SHA512 */
).
This request is never handled by the server.

Test fallback to client side, that's the reason tests are not failing.


if (in != NULL || out != NULL) {
req->state.sz = sizeof(*sha512);
req->input.sz = inLen;
req->output.sz = WC_SHA512_DIGEST_SIZE; /* not needed, but YOLO */
req->output.sz = digestSz;

/* Perform address translations */
ret = wh_Client_DmaProcessClientAddress(
Expand Down Expand Up @@ -5396,10 +5425,10 @@ int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,
if (ret == WH_ERROR_OK) {
/* Get response structure pointer, validates generic header
* rc */
ret = _getCryptoResponse(dataPtr, WC_HASH_TYPE_SHA512,
ret = _getCryptoResponse(dataPtr, hashType,
(uint8_t**)&resp);
/* Nothing to do on success, as server will have updated the context
* in client memory */
/* Nothing to do on success, as server will have updated the
* context in client memory */
}
}

Expand Down Expand Up @@ -5427,10 +5456,10 @@ int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,
if (ret == WH_ERROR_OK) {
/* Get response structure pointer, validates generic header
* rc */
ret = _getCryptoResponse(dataPtr, WC_HASH_TYPE_SHA512,
ret = _getCryptoResponse(dataPtr, hashType,
(uint8_t**)&resp);
/* Nothing to do on success, as server will have updated the output
* hash in client memory */
/* Nothing to do on success, as server will have updated the
* output hash in client memory */
}
}

Expand All @@ -5442,12 +5471,12 @@ int wh_Client_Sha512Dma(whClientContext* ctx, wc_Sha512* sha, const uint8_t* in,
ctx, (uintptr_t)in, (void**)&inAddr, inLen,
WH_DMA_OPER_CLIENT_READ_POST, (whDmaFlags){0});
(void)wh_Client_DmaProcessClientAddress(
ctx, (uintptr_t)out, (void**)&outAddr, WC_SHA512_DIGEST_SIZE,
ctx, (uintptr_t)out, (void**)&outAddr, digestSz,
WH_DMA_OPER_CLIENT_WRITE_POST, (whDmaFlags){0});
}
return ret;
}
#endif /* WOLFHSM_CFG_DMA */
#endif /* WOLFHSM_CFG_DMA */
#endif /* WOLFSSL_SHA512 */

#ifdef HAVE_DILITHIUM
Expand Down
26 changes: 21 additions & 5 deletions src/wh_client_cryptocb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,13 +514,21 @@ int wh_Client_CryptoCb(int devId, wc_CryptoInfo* info, void* inCtx)
} break;
#endif /* WOLFSSL_SHA384 */
#if defined(WOLFSSL_SHA512) && defined(WOLFSSL_SHA512_HASHTYPE)
case WC_HASH_TYPE_SHA512: {
case WC_HASH_TYPE_SHA512:
#if !defined(WOLFSSL_NOSHA512_224)
case WC_HASH_TYPE_SHA512_224:
#endif
#if !defined(WOLFSSL_NOSHA512_256)
case WC_HASH_TYPE_SHA512_256:
#endif
{
wc_Sha512* sha = info->hash.sha512;
const uint8_t* in = info->hash.in;
uint32_t inLen = info->hash.inSz;
uint8_t* out = info->hash.digest;

ret = wh_Client_Sha512(ctx, sha, in, inLen, out);
ret = wh_Client_Sha512(ctx, sha, in, inLen,
out, info->hash.type);
} break;
#endif /* WOLFSSL_SHA512 && WOLFSSL_SHA512_HASHTYPE */
default:
Expand Down Expand Up @@ -839,15 +847,23 @@ int wh_Client_CryptoCbDma(int devId, wc_CryptoInfo* info, void* inCtx)
} break;
#endif /* WOLFSSL_SHA384 */
#if defined(WOLFSSL_SHA512) && defined(WOLFSSL_SHA512_HASHTYPE)
case WC_HASH_TYPE_SHA512: {
case WC_HASH_TYPE_SHA512:
#if !defined(WOLFSSL_NOSHA512_224)
case WC_HASH_TYPE_SHA512_224:
#endif
#if !defined(WOLFSSL_NOSHA512_256)
case WC_HASH_TYPE_SHA512_256:
#endif
{
wc_Sha512* sha = info->hash.sha512;
const uint8_t* in = info->hash.in;
uint32_t inLen = info->hash.inSz;
uint8_t* out = info->hash.digest;

ret = wh_Client_Sha512Dma(ctx, sha, in, inLen, out);
ret = wh_Client_Sha512Dma(ctx, sha, in, inLen,
out, info->hash.type);
} break;
#endif /* WOLFSSL_SHA512 && defined(WOLFSSL_SHA512_HASHTYPE) */
#endif /* WOLFSSL_SHA512 && WOLFSSL_SHA512_HASHTYPE */
default:
ret = CRYPTOCB_UNAVAILABLE;
break;
Expand Down
161 changes: 161 additions & 0 deletions test/wh_test_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1950,6 +1950,149 @@ static int whTest_CryptoSha512(whClientContext* ctx, int devId, WC_RNG* rng)
}
return ret;
}

#if !defined(WOLFSSL_NOSHA512_224)
static int whTest_CryptoSha512_224(whClientContext* ctx, int devId,
Copy link
Copy Markdown
Contributor

@rizlik rizlik Apr 14, 2026

Choose a reason for hiding this comment

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

a multiblock test is missing

WC_RNG* rng)
{
(void)ctx;
(void)rng;
int ret = WH_ERROR_OK;
wc_Sha512 sha512[1];
/* Buffer sized for the variant, plus canary to detect overwrite */
uint8_t out[WC_SHA512_224_DIGEST_SIZE + 8];
const uint8_t canary = 0xA5;
Copy link
Copy Markdown
Contributor

@rizlik rizlik Apr 14, 2026

Choose a reason for hiding this comment

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

any reason why to rely on canary and not ASAN?

/* Same one-block input vector as the SHA512 test */
const char inOne[] =
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const uint8_t expectedOut[WC_SHA512_224_DIGEST_SIZE] = {
0x26, 0x1b, 0x94, 0xbc, 0xba, 0x55, 0x42, 0x64,
0xb3, 0xb7, 0x38, 0xe9, 0xe0, 0x9e, 0x7d, 0xc6,
0x8a, 0xc8, 0xe0, 0xb4, 0xc8, 0x51, 0x7f, 0xe9,
0xbb, 0x7c, 0x36, 0x17};

/* Fill canary bytes after the digest area */
memset(out, canary, sizeof(out));

ret = wc_InitSha512_224_ex(sha512, NULL, devId);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_InitSha512_224 on devId 0x%X: "
"%d\n", devId, ret);
}
else {
ret = wc_Sha512_224Update(sha512, (const byte*)inOne,
WC_SHA512_BLOCK_SIZE);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_Sha512_224Update %d\n", ret);
}
else {
ret = wc_Sha512_224Final(sha512, out);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_Sha512_224Final %d\n",
ret);
}
else {
if (memcmp(out, expectedOut,
WC_SHA512_224_DIGEST_SIZE) != 0) {
WH_ERROR_PRINT("SHA512/224 hash mismatch.\n");
ret = -1;
}
else {
/* Verify canary was not overwritten */
int i;
for (i = WC_SHA512_224_DIGEST_SIZE;
i < (int)sizeof(out); i++) {
if (out[i] != canary) {
WH_ERROR_PRINT("SHA512/224 overwrote "
"buffer at byte %d\n", i);
ret = -1;
break;
}
}
}
}
}
(void)wc_Sha512_224Free(sha512);
}
if (ret == 0) {
WH_TEST_PRINT("SHA512/224 DEVID=0x%X SUCCESS\n", devId);
}
return ret;
}
#endif /* !WOLFSSL_NOSHA512_224 */

#if !defined(WOLFSSL_NOSHA512_256)
static int whTest_CryptoSha512_256(whClientContext* ctx, int devId,
WC_RNG* rng)
{
(void)ctx;
(void)rng;
int ret = WH_ERROR_OK;
wc_Sha512 sha512[1];
/* Buffer sized for the variant, plus canary to detect overwrite */
uint8_t out[WC_SHA512_256_DIGEST_SIZE + 8];
const uint8_t canary = 0xA5;
/* Same one-block input vector as the SHA512 test */
const char inOne[] =
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const uint8_t expectedOut[WC_SHA512_256_DIGEST_SIZE] = {
0xb8, 0x8f, 0x97, 0xe2, 0x74, 0xf9, 0xc1, 0xd4,
0x9f, 0x18, 0x1c, 0x8c, 0xbd, 0x01, 0xa9, 0xc7,
0x49, 0x30, 0xad, 0x05, 0x5a, 0x46, 0xac, 0x44,
0x99, 0xa1, 0xd6, 0x01, 0xf1, 0xc8, 0x0b, 0xf2};

/* Fill canary bytes after the digest area */
memset(out, canary, sizeof(out));

ret = wc_InitSha512_256_ex(sha512, NULL, devId);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_InitSha512_256 on devId 0x%X: "
"%d\n", devId, ret);
}
else {
ret = wc_Sha512_256Update(sha512, (const byte*)inOne,
WC_SHA512_BLOCK_SIZE);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_Sha512_256Update %d\n", ret);
}
else {
ret = wc_Sha512_256Final(sha512, out);
if (ret != 0) {
WH_ERROR_PRINT("Failed to wc_Sha512_256Final %d\n",
ret);
}
else {
if (memcmp(out, expectedOut,
WC_SHA512_256_DIGEST_SIZE) != 0) {
WH_ERROR_PRINT("SHA512/256 hash mismatch.\n");
ret = -1;
}
else {
/* Verify canary was not overwritten */
int i;
for (i = WC_SHA512_256_DIGEST_SIZE;
i < (int)sizeof(out); i++) {
if (out[i] != canary) {
WH_ERROR_PRINT("SHA512/256 overwrote "
"buffer at byte %d\n", i);
ret = -1;
break;
}
}
}
}
}
(void)wc_Sha512_256Free(sha512);
}
if (ret == 0) {
WH_TEST_PRINT("SHA512/256 DEVID=0x%X SUCCESS\n", devId);
}
return ret;
}
#endif /* !WOLFSSL_NOSHA512_256 */

#endif /* WOLFSSL_SHA512 */

#ifdef HAVE_HKDF
Expand Down Expand Up @@ -5915,6 +6058,24 @@ int whTest_CryptoClientConfig(whClientConfig* config)
i++;
}
}
#if !defined(WOLFSSL_NOSHA512_224)
i = 0;
while ((ret == WH_ERROR_OK) && (i < WH_NUM_DEVIDS)) {
ret = whTest_CryptoSha512_224(client, WH_DEV_IDS_ARRAY[i], rng);
if (ret == WH_ERROR_OK) {
i++;
}
}
#endif /* !WOLFSSL_NOSHA512_224 */
#if !defined(WOLFSSL_NOSHA512_256)
i = 0;
while ((ret == WH_ERROR_OK) && (i < WH_NUM_DEVIDS)) {
ret = whTest_CryptoSha512_256(client, WH_DEV_IDS_ARRAY[i], rng);
if (ret == WH_ERROR_OK) {
i++;
}
}
#endif /* !WOLFSSL_NOSHA512_256 */
#endif /* WOLFSSL_SHA512 */

#ifdef HAVE_HKDF
Expand Down
Loading
Loading