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