[hmac, rtl] Fix digest reloading bug #23018
Conversation
See lowRISC#23014 for more context. Signed-off-by: Fatih Balli <[email protected]>
hw/ip/hmac/doc/programmers_guide.md
Outdated
| 1. Repeat steps 1-5 for message stream B. | ||
| 1. Before restoring context, make sure that `sha_en` in `CFG` remains disabled. | ||
| 1. Restore the context of message stream A by writing the `CFG`, `DIGEST_0`..`15`, and `MSG_LENGTH_`{`LOWER`,`UPPER`} registers. | ||
| 1. Enable `sha_en` of CFG, which locks down `DIGEST_0`..`15` registers. |
There was a problem hiding this comment.
How about "Enable sha_en of CFG, which enables HMAC again and locks down writing to the DIGEST_0..15 registers from SW."?
| 1. Write an arbitrary number of message blocks to HMAC's `MSG_FIFO`. | ||
| 1. Stop HMAC by setting the `CMD.hash_stop` bit and wait for the `hmac_done` interrupt (or poll the interrupt status register). | ||
| 1. Save the context by reading the `DIGEST_0`..`15` and `MSG_LENGTH_`{`LOWER`,`UPPER`} registers. (The values in the `CFG` register must also be preserved, but that is purely SW-controlled so doesn't need to be read from HW.) | ||
| 1. Disable `sha_en` by updating `CFG` register, in order to clear the digest from stream A. |
There was a problem hiding this comment.
I would add here that "This is necessary to also prevent leakage of intermediate context of one SW thread to another."
ballifatih
force-pushed
the
hmac-fix-digest-reloading
branch
from
88b6eca
to
84611bd
Compare
|
Thanks @gdessouky, I made the editorial changes in the doc as you suggested. |
hw/ip/hmac/doc/programmers_guide.md
Outdated
| 9. Continue this with as many message blocks and parallel message streams as needed. The final hash for any message stream can be obtained at any time (no need for complete blocks) by setting `CMD.hash_process` and waiting for the `hmac_done` interrupt / status bit, finally reading the digest from the `DIGEST` registers. | ||
| 1. Write an arbitrary number of message blocks to HMAC's `MSG_FIFO`. | ||
| 1. Stop HMAC by setting the `CMD.hash_stop` bit and wait for the `hmac_done` interrupt (or poll the interrupt status register). | ||
| 1. Save the context by reading the `DIGEST_0`..`15` and `MSG_LENGTH_`{`LOWER`,`UPPER`} registers. (The values in the `CFG` register must also be preserved, but that is purely SW-controlled so doesn't need to be read from HW.) |
There was a problem hiding this comment.
I think the key registers should also be preserved as values can be changed
There was a problem hiding this comment.
I think you're right, because the key not only gets hashed before the message (in the first round) but also after the message (in the second round) before the final digest is computed.
| 1. Disable `sha_en` by updating `CFG` register, in order to clear the digest from stream A. This is necessary to also prevent leakage of intermediate context of one SW thread to another. | ||
| 1. Repeat steps 1-5 for message stream B. | ||
| 1. Before restoring context, make sure that `sha_en` in `CFG` remains disabled. | ||
| 1. Restore the context of message stream A by writing the `CFG`, `DIGEST_0`..`15`, and `MSG_LENGTH_`{`LOWER`,`UPPER`} registers. |
There was a problem hiding this comment.
And the key registers?
There was a problem hiding this comment.
indeed, and CFG includes the key_length field.
There was a problem hiding this comment.
LGTM modulo @martin-velay's suggestion. Thx for these improvements / fixes, @ballifatih 👍
During streaming, sha_en signal needs to be carefully managed. See lowRISC#23014. Signed-off-by: Fatih Balli <[email protected]>
ballifatih
force-pushed
the
hmac-fix-digest-reloading
branch
from
84611bd
to
e995480
Compare
|
Thanks @martin-velay for pointing out that |
ballifatih
added
the
Status:Ready to merge
Fixes #23014 and updates the programmers guide.
The gist is that we need deassert
sha_enas a clean-up after HMAC operation/streaming is done.sha_enis only asserted onceDIGESTregisters are restored.I tested the streaming with my draft driver implementation, but only with few vectors.
In the doc, I updated the numbering, so this is why the diff looks unnecessarily large.