RFR: 8331008: Implement JEP 478: Key Derivation Function API (Preview) [v22]

Kevin Driver kdriver at openjdk.org
Thu Sep 5 17:48:10 UTC 2024


On Wed, 4 Sep 2024 23:02:29 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   change impl class to use byte arrays rather than SecretKey objects where possible
>
> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 358:
> 
>> 356:         }
>> 357: 
>> 358:         return Arrays.copyOf(kdfOutput, outLen);
> 
> Here is an alternative solution which does not need `Arrays.copyOf()` call:
> Suggestion:
> 
>         kdfOutput = new byte[outLen];
>         int i = 0;
>         int offset = 0;
>         try {
>             while (i < rounds) {
>                 if (i > 0) {
>                     hmacObj.update(kdfOutput, offset - hmacLen, hmacLen); // add T(i-1)
>                 }
>                 hmacObj.update(info);                   // Add info
>                 hmacObj.update((byte) ++i);             // Add round number
>                 if (i == rounds && (outLen - offset < hmacLen)) {
>                     // special handling for last chunk
>                     byte[] tmp = hmacObj.doFinal();
>                     System.arraycopy(tmp, 0, kdfOutput, offset,
>                             outLen - offset);
>                     offset = outLen;
>                 } else {
>                     hmacObj.doFinal(kdfOutput, offset);
>                     offset += hmacLen;
>                 }
>             }
>         } 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);
>         }
>         return kdfOutput;

My personal opinion is that this adds complexity which is unnecessary. The `Arrays.copyOf` solution reads more simply and GC will take care of any extra allocation once it goes out of scope. I appreciate the alternate fix, but in this case the original solution is more easily understood and thus more maintainable.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1745932560


More information about the security-dev mailing list