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

Sean Mullan mullan at openjdk.org
Mon May 13 20:39:55 UTC 2024


On Mon, 13 May 2024 19:01:09 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:
> 
>   code review comments

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 47:

> 45: 
> 46: /**
> 47:  * KeyDerivation implementation for the HKDF function.

s/KeyDerivation/KDF/

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 81:

> 79:      *     if the initialization parameters are inappropriate for this {@code KDFSpi}
> 80:      */
> 81:     protected HkdfKeyDerivation(AlgorithmParameterSpec algParameterSpec)

This doesn't have to be protected. Can be package-private, or even private.

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 237:

> 235:             } catch (InvalidKeyException ike) {
> 236:                 throw new InvalidParameterSpecException(
> 237:                     "Issue encountered when combining ikm or salt values into single keys");

add cause (ike) to exception.

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 252:

> 250:             }
> 251:             // set this value in the "if"
> 252:             if ((length = anExpand.length()) <= 0) {

This check is also not necessary since it is already checked in `HKDFParameterSpec`.

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 265:

> 263:             ikms = anExtractExpand.ikms();
> 264:             salts = anExtractExpand.salts();
> 265:             if (isNullOrEmpty(ikms) && isNullOrEmpty(salts)) {

Check redundant/not needed.

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 407:

> 405:     }
> 406: 
> 407:     public static final class HkdfSHA256 extends HkdfKeyDerivation {

Can these classes be package-private instead of public?

src/java.base/share/classes/com/sun/crypto/provider/HkdfKeyDerivation.java line 411:

> 409:             throws InvalidAlgorithmParameterException {
> 410:             super(algParameterSpec);
> 411:             hmacAlgName = "HmacSHA256";

Alternatively, you could pass `hmacAlgName` into the superclass constructor as another parameter.

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

> 366:          *
> 367:          * @return a copy of the optional context and application specific
> 368:          *     information

Add ", or null if not specified"

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

> 438:          *
> 439:          * @return a copy of the optional context and application specific
> 440:          *     information

Add ", or null if not specified"

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599008452
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599018546
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599044021
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599047262
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599047927
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599015960
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599014123
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599051530
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1599051867



More information about the security-dev mailing list