Refactor of HMACParams - #121
Conversation
d2a9b35 to
d1dcf75
Compare
|
Okay, so it looked fine when I looked at it earlier, but it looks like it's based on the old release/0.1.3alpha tip, so it's picked up the extra commits. It'll need to be re-based back to the current 0.1.3alpha tip. To be honest, I recommend re-basing it off #113 or #115, it will probably be easier and likely give a cleaner result. Once the re-basing is done, this can be considered approved. |
dghgit
left a comment
dghgit
left a comment
There was a problem hiding this comment.
It's been affected by the re-base. See comment at bottom of discussion page.
(cherry picked from commit 1f4a84e) Replayed onto the HMAC/HKDF utility-crate split, which had restructured both crate doc headers. Adapted while resolving the conflict: * link [`SHA256Internal`] / [`SHA512Internal`] and [`SHA3Internal`] rather than [`SHA2`] / [`SHA3`]; the latter two are not type names in either crate and would have been dead intra-doc links. * keep the "## HMAC" pointer that the split added to the sha3 header. * move sha2's "Memory Usage" and "Security Considerations" below "Suspending and resuming execution", so that both crates' doc headers present the same section order. sha3 was already in that order. Assisted-by: Claude Code:claude-opus-5 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aliases
The HMAC test suite exercised HMAC over SHA-2 and SHA-3, which forced
bouncycastle-hmac to carry bouncycastle-sha2, -sha3, -rng and -hex as
dev-dependencies purely in order to test itself. That was a deliberate cycle
through dev-dependencies, flagged with a todo, left over from the
utility-crate split.
Split the suite by the hash it exercises and move each half to the crate that
publishes the aliases under test:
* crypto/sha2/tests/hmac_tests.rs -- HMAC-SHA224/256/384/512: the full
RFC 4231 suite, suspend/resume, keygen, and weak-RNG rejection.
* crypto/sha3/tests/hmac_tests.rs -- HMAC-SHA3-224/256/384/512: the
143-byte long-key regression (SHA3-224 has the largest block at 144
bytes), truncated-tag rejection, suspend/resume, and keygen.
No test bodies or expected values were changed; the suite was partitioned,
not rewritten. bouncycastle-hmac's [dev-dependencies] are gone entirely and
with them the dependency cycle. The hex dev-dependency follows the tests to
bouncycastle-sha2, which did not previously have one.
This leaves bouncycastle-hmac with no tests of its own. That is inherent to
it being a utility crate rather than a gap in coverage: HMAC<H> cannot be
instantiated without a hash, and this crate must not depend on the hash
crates. Every behaviour it had is still exercised, from the two suites above.
Assisted-by: Claude Code:claude-opus-5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ating it Assisted-by: Claude Code:claude-opus-5
1f4a84e to
3cdca65
Compare
|
@dghgit Right. We rebased release/0.1.3alpha to take the AES, SM3, and SHA512/t stuff off it. I have now rebased this to match. While I was at it, I decided to have a go at simplifying how the HMACParams trait is structured, and that lead me to reading SP 800-107r1 §5.3.4 and I realized that it allows the HMAC's security strength to be higher
An artifact of #122 (merged earlier today), which moved the HMAC_SHA2 and HMAC_SHA3 pub types from the HMAC crate to the SHA2 / SHA3 crates is that the HMAC integration tests should have moved with them. That has been done on this PR as well. |
ounsworth
changed the title
ounsworth
changed the title
2 participants
Original issue:
Adds docs for cloning a hash function mid-stream so that you now have two hash objects that are identical up to this point, but you can feed different suffix input into before finalizing.
I put this next to the existing sections for
[Suspendable], since it's closely-related functionality. (and re-arranged order to put these before Memory Usage and Security Consideration sections).Extension
During development of that, I realized that the pub interface flip of HMAC and HKDF introduced in #122 meant that the way HMACParams was defined was now really clunky. A refactor of that lead to realizing that actually, SP 800-107r1 specifies the security strength of an HMAC higher than the collision strength of the underlying hash function, so I re-worked that as well.