On Thu, 15 Aug 2024 20:12:09 GMT, Valerie Peng <valer...@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 124: > >> 122: List<SecretKey> salts; >> 123: SecretKey inputKeyMaterial; >> 124: SecretKey salt; > > Looking at the implementation, it seems you can just use byte[] for > `inputKeyMaterial` and `salt`. Why bother packaging the bytes into a > `SecretKey` object and later calling `getEncoded()` to retrieve it again? We use SecretKey, because sometimes the raw bytes may not be available to us, for example if it's a hardware key. > src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java > line 151: > >> 149: try { >> 150: return hkdfExtract(inputKeyMaterial, >> 151: (salt == null) ? null : >> salt.getEncoded()); > > If you use byte[] for `salt`, then you can just pass it to `hkdfExtract()` > and no need for this null-check and `getEncoded()` call. See above comment. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1718993177 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1718993640