RFR: 8331008: Implement JEP 478: Key Derivation Function API (Preview) [v15]

Sean Mullan mullan at openjdk.org
Tue Aug 27 11:34:19 UTC 2024


On Fri, 23 Aug 2024 21:48:44 GMT, Kevin Driver <kdriver at 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 one additional commit since the last revision:
> 
>   code review comments and test renaming

src/java.base/share/classes/javax/crypto/KDF.java line 230:

> 228: 
> 229:     /**
> 230:      * Returns a {@code KDF} instance initialized with the specified algorithm from

"initialized with the specified algorithm" is not really accurate. I would change this text to "that implements the specified algorithm". Applies to all `getInstance` methods.

src/java.base/share/classes/javax/crypto/KDFSpi.java line 133:

> 131:      * <p>
> 132:      * The {@code engineDeriveData} method may be called multiple times on a
> 133:      * particular {@code KDFSpi} instance.

You will need to change the wording on this, and the `engineDeriveData` method, so as not to imply this supports concurrent access.

src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 216:

> 214:          * <p>
> 215:          * An implementation should concatenate the salt into a single value
> 216:          * once all components are available.

What do you mean by "An implementation"? Are you referring to the HKDF implementation or the implementation of this method?

src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 244:

> 242:          *
> 243:          * @param salt
> 244:          *     the salt value

It's not clear that these are wrapped in`SecretKey` objects and returned in the `salts()` method of `Extract` and `ExtractThenExpand` - I think that detail needs to be noted in the API, otherwise users might find it surprising. Same comment for `addIKM(byte[])`.

src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 265:

> 263:      * {@code ExtractThenExpand} objects.
> 264:      *
> 265:      * @return a {@code Builder} to mutate

I would just say "a new Builder".

src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 272:

> 270: 
> 271:     /**
> 272:      * Creates an {@code Expand} object

Missing period.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732624331
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732635341
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732653767
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732668426
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732641948
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1732639591



More information about the security-dev mailing list