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

[Sean Mullan]

On Thu, 9 May 2024 20:24:19 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   some code review comments
>
> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 80:
> 
>> 78:      * @throws InvalidAlgorithmParameterException
>> 79:      *     if the initialization parameters are inappropriate for this {@code KDFSpi}
>> 80:      */
> 
> This constructor should contain `hmacAlgName` as a parameter, and its assignment moved here, thus make it final.

Yes, had the same comment.

> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 227:
> 
>> 225:             this.ikms = anExtract.ikms();
>> 226:             this.salts = anExtract.salts();
>> 227:             if (isNullOrEmpty(ikms) && isNullOrEmpty(salts)) {
> 
> As I said earlier, I think this restriction is not necessary.

Yes, it is not possible for the params to be null as `HKDFParameterSpec.Extract` is final and always passes in non-null lists to them.

> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 244:
> 
>> 242:             HKDFParameterSpec.Expand anExpand = (HKDFParameterSpec.Expand) kdfParameterSpec;
>> 243:             // set this value in the "if"
>> 244:             if ((pseudoRandomKey = anExpand.prk()) == null) {
> 
> This could not happen here.

Right, `HKDFParameterSpec.Expand` is final and is never created with a null prk.

> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 249:
> 
>> 247:             }
>> 248:             // set this value in the "if"
>> 249:             if ((info = anExpand.info()) == null) {
> 
> I would disallow null when the parameter is created.

info is optional though.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599017285
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599040966
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599045337
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599052304