RFR: 8331008: KDF Implementation (Preview) [v28]

[Sean Mullan]

On Mon, 13 May 2024 03:46:50 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).
>
> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
> 
>   re-enable preview annotations

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

> 44: 
> 45: /**
> 46:  * This class provides the functionality of a key derivation algorithm for JCE.

I think it's important to spell out the KDF acronym in the first sentence, and then you can use KDF subsequently instead of "Key derivation". Suggest:

"This class provides the functionality of a Key Derivation Function (KDF). {@code KDF} objects are instantiated with the {@code getInstance} family of methods.  KDF algorithm names follow a naming convention of
 <I>Algorithm</I>with<I>PRF</I>. ..."

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

> 49:  * of methods.  Key derivation algorithm names follow a naming convention of
> 50:  * <I>Algorithm</I>with<I>PRF</I>.  The algorithm field is the KDF algorithm
> 51:  * (e.g. HKDF, etc.), while the PRF specifier identifies the underlying

s/specifier/field/

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

> 51:  * (e.g. HKDF, etc.), while the PRF specifier identifies the underlying
> 52:  * pseudorandom function (e.g. HmacSHA256).  For instance, a KDF implementation
> 53:  * of HKDF using HMAC-SHA256 will have an algorithm string of

s/will have/has/
s/string/name/

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

> 58:  * {@snippet lang = java:
> 59:  *    KDF kdfHkdf = KDF.getInstance("HKDFWithHmacSHA256",
> 60:  *                                       (AlgorithmParameterSpec) null);

I think it would be clearer to call the one-arg `getInstance` w/o null params here.

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

> 79:                                                           "Provider");
> 80:     private static final boolean skipDebug = Debug.isOn("engine=")
> 81:                                              && !Debug.isOn("kdf");

None of these debug variables are used AFAICT.

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

> 87:     private KDFSpi spi;
> 88: 
> 89:     // The name of the MAC algorithm.

s/MAC/KDF/

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

> 155: 
> 156:     /**
> 157:      * Returns a {@code KDF} object that implements the specified algorithm..

Extra period at end of sentence.

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

> 207:         } catch (InvalidAlgorithmParameterException e) {
> 208:             throw new NoSuchAlgorithmException(
> 209:                 "Received an InvalidParameterSpecException. Does this "

s/InvalidParameterSpecException/InvalidAlgorithmParameterException/
Also, preserve the cause when throwing `NoSuchAlgorithmException` (pass `e` as the 2nd param).

Same comment applies to other getInstance methods.

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

> 312:         Objects.requireNonNull(algorithm, "null algorithm name");
> 313:         try {
> 314:             Instance instance = GetInstance.getInstance("KDF", KDFSpi.class,

I think you can call `JceSecurity.getInstance` here, and then you don't need lines 318-322. Same comment applies to other `getInstance` methods.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598479103
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598484799
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598482080
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598489265
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598494982
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598496701
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598499646
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598509971
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1598523001