On Wed, 28 Aug 2024 20:44:10 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). >> >> Work was begun in [another PR](https://github.com/openjdk/jdk/pull/18924). > > Kevin Driver has updated the pull request incrementally with two additional > commits since the last revision: > > - consistency with wording for addIKM and addSalt > - another round of code review comments Some comments on the latest commits. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 163: > 161: * > 162: * @implNote An implementation should concatenate the input > keying > 163: * materials into a single value once all components are > available. I'm not sure what "once all components are available" means. Technically, it's "...into a single value and pass it to the HKDF-Extract step". Also, this method is for end users. The `@implNote` should be added to `ikms` which is called by an implementation. There you can tell the implementation to do the concatenation. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 287: > 285: * > 286: * @implNote HKDF implementations will enforce that the length is > less than > 287: * 255 * HMAC length. It will also check the size of `prk`. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 322: > 320: * Returns an unmodifiable {@code List} of input keying material > values > 321: * in the order they were added. Returns an empty list if there > are no > 322: * input keying material values. Sean asked about where do the byte array IKMs go. Here you should mention that an IKM added by `{@link #addIKM(byte[])}` is converted to a `SecretKeySpec` object. Same for `salts`. Same as in `ExtractThenExpand`. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 365: > 363: * modification > 364: * @param length > 365: * the length of the output keying material (must be > 0 and > < 255 * Make the same change on the upper limit of `length` as line 286. src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 431: > 429: * modification > 430: * @param length > 431: * the length of the output keying material (must be > 0 and > < 255 * Maybe you don't need to talk about the length restriction here. The rule is at input. ------------- PR Review: https://git.openjdk.org/jdk/pull/20301#pullrequestreview-2267476500 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1735433495 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1735427784 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1735428673 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1735429361 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1735429901
