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

[Weijun Wang]

On Tue, 14 May 2024 22:14:47 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 Delayed Provider test

Some more comments on `HkdfKeyDerivation.java`.

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

> 95:             throw new InvalidParameterSpecException(
> 96:                 "the algorithm for the resultant SecretKey may not be null or"
> 97:                 + " empty");

I think this should be an `IllegalArgumentException`.

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

> 134:             // we should be able to combine these Lists of keys into single
> 135:             // SecretKey Objects,
> 136:             // unless we were passed something bogus or an unexportable P11 key

There is a bug here. `consolidateKeyMaterial()` returns the key if it's the only one in `ikms`, without checking if it's "bogus". Then, it is passed into `hkdfExtract` and very unfortunately `hmacObj.doFinal(null)` doesn't make any noise.

Same in ExtractThenExpand.

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

> 154:                 // This is bubbling up from the getInstance of the Mac/Hmac.
> 155:                 // Since we're defining these values internally, it should be
> 156:                 // safe to "eat" this NSAE.

This could happen if someone filters out the required algorithms. In this case, we usually throw a `ProviderException`. 

There are similar cases below.

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

> 362:         }
> 363: 
> 364:         return kdfOutput;

Since `deriveKey` is now calling `deriveData`, I suggest checking if `outLen` is the same as the length `kdfOutput` here and call `copyOf` if not. There is no need to call it in `deriveData`.

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

> 368:         public HkdfSHA256(AlgorithmParameterSpec algParameterSpec)
> 369:             throws InvalidAlgorithmParameterException,
> 370:                    NoSuchAlgorithmException {

You should not throw `NoSuchAlgorithmException` here. This constructor will be directly called by JCA framework and must conform to the spec described in `KDFSpi` that it can only throw `InvalidAlgorithmParameterException`.

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

PR Review: https://git.openjdk.org/jdk/pull/18924#pullrequestreview-2056627780
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1600771294
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1600771763
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1600772561
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1600774835
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1600775715