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

Weijun Wang weijun at openjdk.org
Sat Sep 14 22:59:27 UTC 2024


On Fri, 13 Sep 2024 19:39:04 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:
> 
>   refine wording on DPS getInstance with params exception

Comments on the API spec in `KDF.java`. I'll need some more time to look at the implementation.

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

> 192:      * otherwise {@code null} is returned.
> 193:      *
> 194:      * @see <a href="#DelayedProviderSelection">Delayed Provider Selection</a>

Why `@see DPS`? The paragraph above has nothing to do with it.

If you meant the warning not to call it too early (which I think is not necessary), why not add the same to the `getProviderName` method?

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

> 205:      * Returns a {@code KDF} object that implements the specified algorithm.
> 206:      *
> 207:      * @see <a href="#DelayedProviderSelection">Delayed Provider Selection</a>

Put `@see` at the end. Put `@implNote` before `@param`. This is the order they are rendered in HTML javadoc.

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

> 264:      *     list
> 265:      * @throws NullPointerException
> 266:      *     if the {@code algorithm} or {@code provider} is {@code null}

Remove the `the` word. Same below.

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

> 343:      *     if at least one {@code Provider} supports a {@code KDFSpi}
> 344:      *     implementation for the specified algorithm but none of them
> 345:      *     support the specified parameters

One of the two `@throws` above uses `supports a {@code KDF} implementation` and the other uses `supports a {@code KDFSpi} implementation`. We'd better choose the same class name.

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

> 424:      * @throws InvalidAlgorithmParameterException
> 425:      *     if the specified provider does not support a {@code KDFSpi}
> 426:      *     implementation for the specified algorithm and parameters

Do not mention `algorithm` in this `@throws`. It's already covered by `@throws NSAE`. Same below.

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

> 528:      *     results in something invalid
> 529:      * @throws NoSuchAlgorithmException
> 530:      *     if {@code alg} is empty or invalid

Is it easy to tell precisely what falls into "combination of alg and derivationSpec results in something invalid" and what falls into "alg is invalid"?

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

> 553: 
> 554:     /**
> 555:      * Obtains raw data from a key derivation function.

The first sentences of the two `derive` methods use different verbs: `Derives` and `Obtains`. Is it possible to use a same one?

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

PR Review: https://git.openjdk.org/jdk/pull/20301#pullrequestreview-2305207448
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759845500
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759869223
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759877457
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759878466
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759878857
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759883966
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759882929


More information about the security-dev mailing list