-
Notifications
You must be signed in to change notification settings - Fork 832
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.
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
Added crypto callback for SHA3. #7670
Conversation
Retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, but I'd like to see the existing pattern of using the p (block count) and len to determine the type instead of modifying and adding a member to track it. Also, there's a bug in InitSha3 that clears the devId, so the end of Final should either invoke the wc_InitSha3(..., sha3->devId) or InitSha3 should be updated to remember the devId.
(void)devId; | ||
#elif defined(WOLF_CRYPTO_CB) | ||
sha3->devId = devId; | ||
sha3->type = type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the other cases (async, sw), the p parameter passed into SHA3_Update and SHA3_Final controls which type (256, 384, 512) of SHA3 is being used. Consider letting the the processing functions determine the info->hash->type based on that and remove the type parameter/member here. Comments below.
if (sha3->devId != INVALID_DEVID) | ||
#endif | ||
{ | ||
ret = wc_CryptoCb_Sha3Hash(sha3, sha3->type, NULL, 0, hash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider:
int type = 0;
switch(len) {
case WC_SHA3_224_DIGEST_SIZE: type = WC_HASH_TYPE_SHA3_224; break;
case WC_SHA3_256_DIGEST_SIZE: type = WC_HASH_TYPE_SHA3_256; break;
case WC_SHA3_384_DIGEST_SIZE: type = WC_HASH_TYPE_SHA3_384; break;
case WC_SHA3_512_DIGEST_SIZE: type = WC_HASH_TYPE_SHA3_512; break;
default: return WC_ERROR_BAD_ARG;
}
ret = wc_CryptoCb_Sha3Hash(sha, type, NULL, 0, hash);
if (sha3->devId != INVALID_DEVID) | ||
#endif | ||
{ | ||
ret = wc_CryptoCb_Sha3Hash(sha3, sha3->type, data, len, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider:
int type = 0;
switch(p) {
case WC_SHA3_224_COUNT: type = WC_HASH_TYPE_SHA3_224; break;
case WC_SHA3_256_COUNT: type = WC_HASH_TYPE_SHA3_256; break;
case WC_SHA3_384_COUNT: type = WC_HASH_TYPE_SHA3_384; break;
case WC_SHA3_512_COUNT: type = WC_HASH_TYPE_SHA3_512; break;
default: return WC_ERROR_BAD_ARG;
}
ret = wc_CryptoCb_Sha3Hash(sha, type, NULL, 0, hash);
Description
Added crypto callback for SHA3 and extended the test.c tests for it in cryptocb_test.
Testing