RFR: 8331008: Implement JEP 478: Key Derivation Function API (Preview) [v11]
Kevin Driver
kdriver at openjdk.org
Mon Aug 19 21:42:18 UTC 2024
On Fri, 16 Aug 2024 18:16:07 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 202:
>
>> 200: salts = anExtractThenExpand.salts();
>> 201: // we should be able to combine these Lists of keys into single
>> 202: // SecretKey Objects,
>
> "Single SecretKey objects" => "a byte[]"
> "List of keys" is really "a list of key segments" which are combined into one key. Same goes for salts. The API is designed with the protocol usage in mind, but the naming we have here does not directly line up with RFC5869 which only mentions singular "IKM" and "SALT".
Addressed in https://github.com/openjdk/jdk/pull/20301/commits/48395b86ba8e1cda663ae326e06ae2556f4b905a. Please indicate if this is resolved.
> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 328:
>
>> 326: *
>> 327: * @throws InvalidKeyException
>> 328: * if an invalid key was provided through the {@code HkdfParameterSpec}
>
> Clarify "key" with "{@code prk}" and get rid of the trailing description.
Addressed in https://github.com/openjdk/jdk/pull/20301/commits/48395b86ba8e1cda663ae326e06ae2556f4b905a. Please indicate if this is resolved.
> src/java.base/share/classes/javax/crypto/KDF.java line 124:
>
>> 122:
>> 123: int DERIVE_KEY = 0;
>> 124: int DERIVE_DATA = 1;
>
> These two variables are a waste of space... `chooseProvider()` impl can just use a local boolean variable to remember whether the derivation is for key or for data.
Addressed in https://github.com/openjdk/jdk/pull/20301/commits/48395b86ba8e1cda663ae326e06ae2556f4b905a. Please indicate if this is resolved.
> test/jdk/com/sun/crypto/provider/KDF/TestHKDF.java line 79:
>
>> 77:
>> 78: public static final List<TestData> testList = new LinkedList<TestData>() {{
>> 79: add(new TestData("RFC 5689 Test Case 1", "HKDFWithHmacSHA256",
>
> typo: "5689" should be "5869". Sames goes to other test datum.
Addressed in https://github.com/openjdk/jdk/pull/20301/commits/48395b86ba8e1cda663ae326e06ae2556f4b905a. Please indicate if this is resolved.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1722376679
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1722376314
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1722376862
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1722376789
More information about the security-dev
mailing list