On Fri, 16 Aug 2024 19:29:36 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> 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`.
Sure, I added an if-check to skip the `hmacObj.update(...)` call for the i=0
case. This way, you don't need to set `tLength` to 0 and then changing it to
`hmacLen` afterwards. I find it easier to understand.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724130421
