RFR: 8331008: Implement JEP 478: Key Derivation Function API (Preview) [v11]
Kevin Driver
kdriver at openjdk.org
Fri Aug 16 19:32:57 UTC 2024
On Fri, 16 Aug 2024 01:19:15 GMT, Valerie Peng <valeriep at openjdk.org> wrote:
>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>>
>> addressed several review comments, namely: - renaming the getParameters method - renaming the AlgorithmParameterSpec object - address some javadoc exception messages - add some information to KDF class private constructor javadocs - other general cleanup
>
> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 226:
>
>> 224: byte[] extractResult = hkdfExtract(inputKeyMaterial, (salt
>> 225: == null) ? null : salt.getEncoded());
>> 226: pseudoRandomKey = new SecretKeySpec(extractResult, "RAW");
>
> Use `hmacAlgName` instead of `RAW` as the key algorithm? Or is it well-established fact that most if not all Hmac impls doesn't care or check the key algorithm?
@wangweij felt this was advantageous.
> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 369:
>
>> 367: throw new RuntimeException(sbe);
>> 368: }
>> 369: }
>
> `tLength` may not be necessary. Variables only used inside the loop can also be moved to the loop.
> The loop can be modified as:
>
>
> for (int i = 0, offset = 0; i < rounds; i++, offset += hmacLen) {
> // Calculate this round
> try {
> if (i > 0) {
> hmacObj.update(kdfOutput, offset - hmacLen, hmacLen); // add T(i-1)
> }
> hmacObj.update(info); // Add info
> hmacObj.update((byte) (i + 1)); // Add round number
> hmacObj.doFinal(kdfOutput, offset);
> } catch (ShortBufferException sbe) {
> // This really shouldn't happen given that we've
> // sized the buffers to their largest possible size up-front,
> // but just in case...
> throw new RuntimeException(sbe);
> }
> }
I'll need to double-check the logic of this snippet. For example, in the first iteration of the loop, isn't `tLength` == 0? Yet, you have it always set to `hmacLen`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1720263369
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1720260930
More information about the security-dev
mailing list