On Mon, 19 Aug 2024 21:39:17 GMT, Kevin Driver <kdri...@openjdk.org> wrote:

>> 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.

Still has typo "Objects", should be "object".
BTW, I still strongly feel that it can be combined into a byte[] instead of a 
SecretKey object.

>> 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.

Yes, 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.

Yes, resolved.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724137162
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724138095
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724137512

Reply via email to