On Fri, 16 Aug 2024 21:12:02 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java >> line 242: >> >>> 240: } >>> 241: throw new InvalidAlgorithmParameterException( >>> 242: "an HKDF could not be initialized with the given >>> KDFParameterSpec"); >> >> It's clearer to state that the given `KDFParameterSpec` must be >> `HKDFParameterSpec`. Also, given that KDF.getInstance() takes a >> KDFParameters parameters, I'd avoid the word "initialized" as it may confuse >> people which parameters you are talking about. >> I'd suggest something like "HKDF data/key derivatopn requires >> HKDFParameterSpec, not " + derivationParameterSpec.getClass() >> Also, for readability, it may be better to check the specified >> `derivationParameterSpec` is an instanceof `HKDFParameterSpec` in the >> beginning. > > Addressed in > https://github.com/openjdk/jdk/pull/20301/commits/c6f491cd05c76088e6431b2ba9d4ab42b29e4055. > Please indicate if this is resolved. Yes, resolved. >> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java >> line 279: >> >>> 277: >>> 278: /** >>> 279: * Perform the HMAC-Extract operation. >> >> typo: 'HMAC' should be 'HKDF' > > Addressed in > https://github.com/openjdk/jdk/pull/20301/commits/c6f491cd05c76088e6431b2ba9d4ab42b29e4055. > Please indicate if this is resolved. Yes, resolved. >> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java >> line 310: >> >>> 308: >>> 309: /** >>> 310: * Perform the HMAC-Expand operation. At the end of the >>> operation, the >> >> typo: 'HMAC' should be 'HKDF'. > > Addressed in > https://github.com/openjdk/jdk/pull/20301/commits/c6f491cd05c76088e6431b2ba9d4ab42b29e4055. > Please indicate if this is resolved. Yes, resolved. >> src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java >> line 331: >> >>> 329: * or derived during the generation of the PRK. >>> 330: */ >>> 331: protected byte[] hkdfExpand(SecretKey prk, byte[] info, int outLen) >> >> Same here, can be made 'private'. > > Addressed in > https://github.com/openjdk/jdk/pull/20301/commits/c6f491cd05c76088e6431b2ba9d4ab42b29e4055. > Please indicate if this is resolved. Yes, resolved. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724089634 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724091242 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724091341 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1724091033
