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

[Kevin Driver]

On Mon, 9 Sep 2024 20:50:47 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   updated comments around locking mechanism
>
> src/java.base/share/classes/java/security/KDFParameters.java line 39:
> 
>> 37:  * the
>> 38:  * {@link javax.crypto.KDF#getInstance(String, KDFParameters) KDF.getInstance}
>> 39:  * methods. The {@code getInstance} method returns a {@code KDF}.
> 
> Suggest changing the above sentence to:
> 
> The {@code getInstance} method returns a {@code KDF} that is initialized with the specified parameters.

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

> src/java.base/share/classes/java/security/KDFParameters.java line 45:
> 
>> 43:  * parameters were not supplied and can be generated by the {@code KDF}
>> 44:  * object, these may be supplied by the implementation. For additional
>> 45:  * information, see: {@link javax.crypto.KDF#getParameters()}.
> 
> I would simplify this as:
> 
> The {@code KDFParameters} used for initialization are returned by {@link javax.crypto.KDF#getParameters()} and may contain additional default or random parameter values used by the underlying KDF implementation.

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

> src/java.base/share/classes/javax/crypto/KDF.java line 225:
> 
>> 223:      *     if {@code algorithm} is {@code null}
>> 224:      */
>> 225:     public static KDF getInstance(String algorithm)
> 
> One thing that is missing from this method is an Implementation Note describing how the `jdk.security.provider.preferred` property affects the search algorithm. See any other JCE API - you can just copy/paste the text. Also affects the `getInstance` method that takes an algorithm and params.

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

> src/java.base/share/classes/javax/crypto/KDF.java line 352:
> 
>> 350:                         new InvalidAlgorithmParameterException(
>> 351:                             "No provider can be found that supports the "
>> 352:                             + "specified parameters"));
> 
> Slight tweak in message: s/specified parameters/specified algorithm and parameters/

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

> src/java.base/share/classes/javax/crypto/KDF.java line 369:
> 
>> 367:                 lastException =
>> 368:                     new NoSuchAlgorithmException(
>> 369:                         new InvalidAlgorithmParameterException(
> 
> Slight tweak in message: s/specified parameters/specified algorithm and parameters/

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

> src/java.base/share/classes/javax/crypto/KDF.java line 507:
> 
>> 505:      * <p>
>> 506:      * The {@code deriveKey} method may be called multiple times on a particular
>> 507:      * {@code KDF} instance, but it is not considered thread-safe.
> 
> My preference would be to remove this sentence and the same one in deriveData now that you have the "Concurrent Access" section in the Class Summary which I think describes the concurrency behavior of the whole API better.

@seanjmullan: Resolved in https://github.com/openjdk/jdk/pull/20301/commits/82791ac01fb7f597a7e814403261c7a50e8a08df.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755775749
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755776023
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755776724
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755776255
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755776497
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1755777144