Add HMAC multi-mode support (SHA-2 384/512 digest sizes and variable key lengths) #89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexJones0
force-pushed
the
hmac_lengths
branch
from
b891b6b to
348f406
Compare
AlexJones0
requested review from
luismarques,
loiclefort,
jwnrt and
rivos-eblot
and removed request for
luismarques
rivos-eblot
requested changes
Dec 13, 2024
AlexJones0
force-pushed
the
hmac_lengths
branch
from
348f406 to
1a1ddd9
Compare
|
@rivos-eblot Thank you for the thorough review. I believe I've addressed all the comments now. Please let me know if there are issues with how I've added lowRISC/myself to the file caption - I'm not sure if there are any specific standards or best practices you would prefer for me to follow there. I've re-run the tests described in my initial comment again and they are all still passing with the latest changes. |
loiclefort
requested changes
Dec 16, 2024
AlexJones0
force-pushed
the
hmac_lengths
branch
2 times, most recently
from
f603875 to
d690930
Compare
|
Latest force push should resolve all further comments and fix a couple of other minor issues I noticed. I've tested again on the same tests and still see all tests passing. |
Hi @AlexJones0, Otherwise, LGTM. |
This removes the last remaining TODO from `ot_hmac` (after the multi-mode SHA384/SHA512 support is added). Looking at the RTL, it wipes the secret key regardless of whether the engine is active or not, even if mid-operation. So this TODO can be safely removed to mimic the real hardware. Also fixes a comment line length issue from a previous commit.
This commit implements initial register interface support for the key length and digest size fields in the config register of OpenTitan's HMAC. It checks that the values written are valid one-hot encodings, and maps them as is done in current hardware if not. This also implements some additional checks that the combination of (key length, digest size, HMAC enable) is valid when attempting to run the START/CONTINUE command, otherwise producing a (new type of) software error. Also fixes some comment formatting (code style).
…igest size This commit updates the size of the key and digest registers to their new maximum sizes in support of HMAC Multi-Mode (SHA-2 256/384/512 Engine), which is 64 bytes for the digest and 128 bytes for the key. The current HMAC logic which processes the input padding and performs the outer hash (using the outer padding) by default assumes that the key length is 64 bytes (512 bits), and that you are hence using the entire set of registers. They also assume that the digest is 32 bytes (256 bits), and that you are again hence using the entire register set. The new logic now uses the key_length and digest_size config register fields to determine the exact padding and digest sizes that should be used in the HMAC calculations.
AlexJones0
force-pushed
the
hmac_lengths
branch
from
d690930 to
866ab4e
Compare
This commit introduces full support for using the SHA-2 384 and SHA-2 512 algorithms for hashing alongside the existing SHA-2 256 algorithm, by setting the `digest_size` field in the configuration register. It likewise adds support for using a range of key sizes (128/256/384/512/ 1024), and not just always using a 512 bit key - this is set in the `key_length` field in the configuration register. The existing tomcrypt library is used to support the additional hash functionality, which uses `sha512` functionality for both sha384 and sha512 in its implementation. This commit also fixes an issue previously introduced where two of the `bswap`s on the digest were the wrong way around, cancelling each other out but technically producing an incorrect intermediary result. This fixes that behaviour, and updates the documentation to make the reasoning behind each endian swap clear.
…nt hash This commit updates the functionality for the HMAC's digest size configuration register field, updating it to match the current hardware behaviour. The hardware uses a `digest_size_started` field to allow the user to write a digest size at any time, while ensuring that if a hash is in progress it will carry on using the existing digest size. A similar field is introduced into the context/state struct to allow the QEMU HMAC model to keep track of the digest size when the last START/CONTINUE command was sent, and use that in place of checking the register value every time.
AlexJones0
force-pushed
the
hmac_lengths
branch
from
866ab4e to
81e5181
Compare
loiclefort
approved these changes
Dec 16, 2024
engdoreis
merged commit ec86037
into
lowRISC:dev/ot-earlgrey-1.0.0-updates
6 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR completes the remaining work to get Earlgrey's HMAC to match the latest hardware functionality. May be easier to review commit-by-commit. See commit messages for more details.
Adds support to the HMAC for variable digest sizes (SHA-2 256/384/512) and key lengths (128/256/384/512/1024 bits) by setting the corresponding fields in the config register. Related checks are added, and existing functionality replaced which assumed SHA-2 256 & key size <= 512 bits. Also cleans up the last minor TODO.
Tested against existing passing tests:
hmac_enc_test,hmac_sha256_functest,sha256_functest,hmac_smoketest,wots_test,verify_test_hardcoded,fors_test, etc.Also tested to now make previously failing tests pass:
hmac_functest,sha384_functest,hmac_sha384_functest,sha512_functestandhmac_sha512_functest.All tests were built from master on OpenTitan.