Incremental Keccak API #4259
Incremental Keccak API #4259
Conversation
ph4r05
commented
Aug 14, 2018
ph4r05
commented
- needed for TREZOR integration
src/crypto/keccak.c
Outdated
| memcpy(md, ctx->hash, KECCAK_DIGESTSIZE); | ||
| } | ||
|
|
||
| memset(ctx, 0, sizeof(KECCAK_CTX)); |
There was a problem hiding this comment.
Could possibly do with a memwipe. Not sure if really necessary.
There was a problem hiding this comment.
I would leave the call whether necessary or not to you, you know better
There was a problem hiding this comment.
Probably unneeded, since there's no free there, though the caller might. We can add it later if it's found to be needed.
src/crypto/keccak.c
Outdated
| } | ||
| while (inlen >= KECCAK_BLOCKLEN) { | ||
| uint64_t* aligned_message_block; | ||
| if (IS_ALIGNED_64(in)) { |
There was a problem hiding this comment.
KECCAK_BLOCKLEN is a multiple of 8, so this calc is only needed once. In fact, if not aligned, a prefix can be memcpy'd, then an aligned loop can follow. Might end up more complicated, not sure. It'd also allow the round function to xor a uint64_t at a time in that case.
There was a problem hiding this comment.
Fixed the uint64_t xor and computation of IS_ALIGNED_64 only once.
But I don't see how to make unaligned input data aligned as in this state the leftover message buffer is empty and one Keccak round needs 17*8=136 bytes at a time.
src/crypto/keccak.c
Outdated
| } | ||
|
|
||
| void keccak_update(KECCAK_CTX * ctx, const uint8_t *in, size_t inlen){ | ||
| size_t idx = (size_t)ctx->rest; |
There was a problem hiding this comment.
Maybe make rest size_t, it'd avoid needless word size conversions.
src/crypto/keccak.h
Outdated
| // 1600 bits algorithm hashing state | ||
| uint64_t hash[25]; | ||
| // 1536-bit buffer for leftovers | ||
| uint64_t message[24]; |
There was a problem hiding this comment.
That seems too large. Looks like it needs 136 bytes, so 17 uint64_t. Is this wrong ?
src/crypto/keccak.c
Outdated
| void keccak_update(KECCAK_CTX * ctx, const uint8_t *in, size_t inlen){ | ||
| size_t idx = (size_t)ctx->rest; | ||
|
|
||
| if (ctx->rest & KECCAK_FINALIZED) return; // too late for additional input |
There was a problem hiding this comment.
That might be worth a hard error, it's a programming error.
src/crypto/keccak.c
Outdated
| if (IS_ALIGNED_64(in)) { | ||
| // the most common case is processing of an already aligned message | ||
| // without copying it | ||
| aligned_message_block = (uint64_t*)(void*)in; |
There was a problem hiding this comment.
The double cast seems unneeded.
src/crypto/keccak.c
Outdated
| inlen -= left; | ||
| } | ||
| while (inlen >= KECCAK_BLOCKLEN) { | ||
| uint64_t* aligned_message_block; |
There was a problem hiding this comment.
Could be const, it's input.
|
Thanks for review. I will squash eventually. |
- needed for TREZOR integration
|
Done |
|
FYI: #4275 |
|
Aah thanks for the tests! I wanted to add tests after merge.
So next time.
pá 17. 8. 2018 v 18:51 odesílatel moneromooo-monero <
notifications@github.com> napsal:
… FYI: #4275 <#4275>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4259 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABAQWZQdpDN5gjJi5qDQFEscryDU4sWEks5uRvR-gaJpZM4V9NAx>
.
|
|
Can we have this merged if there is no opposition and tests are ready too? |
4e08100 Incremental Keccak API added (pr4r05)
