On Mon, 13 May 2024 19:01:09 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> Introduce an API for Key Derivation Functions (KDFs), which are >> cryptographic algorithms for deriving additional keys from a secret key and >> other data. See [JEP 478](https://openjdk.org/jeps/478). > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > code review comments src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 47: > 45: > 46: /** > 47: * KeyDerivation implementation for the HKDF function. s/KeyDerivation/KDF/ src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 81: > 79: * if the initialization parameters are inappropriate for this > {@code KDFSpi} > 80: */ > 81: protected HkdfKeyDerivation(AlgorithmParameterSpec algParameterSpec) This doesn't have to be protected. Can be package-private, or even private. src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 237: > 235: } catch (InvalidKeyException ike) { > 236: throw new InvalidParameterSpecException( > 237: "Issue encountered when combining ikm or salt values > into single keys"); add cause (ike) to exception. src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 252: > 250: } > 251: // set this value in the "if" > 252: if ((length = anExpand.length()) <= 0) { This check is also not necessary since it is already checked in `HKDFParameterSpec`. src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 265: > 263: ikms = anExtractExpand.ikms(); > 264: salts = anExtractExpand.salts(); > 265: if (isNullOrEmpty(ikms) && isNullOrEmpty(salts)) { Check redundant/not needed. src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 407: > 405: } > 406: > 407: public static final class HkdfSHA256 extends HkdfKeyDerivation { Can these classes be package-private instead of public? src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 411: > 409: throws InvalidAlgorithmParameterException { > 410: super(algParameterSpec); > 411: hmacAlgName = "HmacSHA256"; Alternatively, you could pass `hmacAlgName` into the superclass constructor as another parameter. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 368: > 366: * > 367: * @return a copy of the optional context and application > specific > 368: * information Add ", or null if not specified" src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 440: > 438: * > 439: * @return a copy of the optional context and application > specific > 440: * information Add ", or null if not specified" ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599008452 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599018546 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599044021 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599047262 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599047927 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599015960 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599014123 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599051530 PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599051867
