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