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

Kevin Driver kdriver at openjdk.org
Fri Aug 23 21:48:45 UTC 2024


On Wed, 21 Aug 2024 20:21:47 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   addresses delayed provider selection where parameters are involved
>
> test/jdk/com/sun/crypto/provider/KDF/Functions.java line 36:
> 
>> 34: import java.util.HexFormat;
>> 35: 
>> 36: public class Functions {
> 
> Since this is strictly HKDF, it'd be clearer for the test to have HKDF in its name, e.g. "HKDFFunctions.java" or "BasicHKDF,java".

Addressed in https://github.com/openjdk/jdk/pull/20301/commits/9f050b6a1a4a83d8623e206323071c2c77c90bb2. Please indicate if resolved.

> test/jdk/com/sun/crypto/provider/KDF/TestHKDF.java line 55:
> 
>> 53:             testName = Objects.requireNonNull(name);
>> 54:             algName = Objects.requireNonNull(algStr);
>> 55:             IKM = hex2bin(Objects.requireNonNull(ikmStr));
> 
> why is "IKM" all uppercases? "ikm" matches the naming convention of other fields better.

Addressed in https://github.com/openjdk/jdk/pull/20301/commits/9f050b6a1a4a83d8623e206323071c2c77c90bb2. Please indicate if resolved.

> test/jdk/com/sun/crypto/provider/KDF/TestHKDF.java line 241:
> 
>> 239:             System.out.println("\t* Key output: FAIL");
>> 240:             System.out.println("Expected:\n" +
>> 241:                                dumpHexBytes(outData, 16, "\n", " "));
> 
> `outData `should be `expectedOut`.

Addressed in https://github.com/openjdk/jdk/pull/20301/commits/9f050b6a1a4a83d8623e206323071c2c77c90bb2. Please indicate if resolved.

> test/jdk/com/sun/crypto/provider/KDF/TestHKDF.java line 302:
> 
>> 300:         }
>> 301:         return data;
>> 302:     }
> 
> What is the difference between this method and `HexFormat.of().parseHex(hex)` ? We should just reuse existing java APIs if possible.

Addressed in https://github.com/openjdk/jdk/pull/20301/commits/9f050b6a1a4a83d8623e206323071c2c77c90bb2. Please indicate if resolved.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1729544932
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1729544857
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1729544711
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1729544504



More information about the security-dev mailing list