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

[Weijun Wang]

On Sun, 12 May 2024 14:39:40 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   commenting out until better understood -- causing failures
>
> src/java.base/share/classes/javax/crypto/KDF.java line 398:
> 
>> 396:      * <p>
>> 397:      * Delayed provider selection is also supported such that the provider
>> 398:      * performing the derive is not selected until the method is called.
> 
> Delayed provider selection is an important enough topic that it probably should be in the class summary. However it is complicated to word correctly as there is also the case if someone calls `getProviderName` beforehand which locks the provider to the first one supporting the algorithm. I would probably also avoid "delayed provider" as that is not a term currently used in the javadocs. Suggest something like:
> 
> If a provider is not specified in the getInstance method when instantiating a KDF object, the provider is selected the first time the deriveKey or deriveData method is called and a provider is chosen that supports the parameters passed to the deriveKey or deriveData method, for example the initial key material. However, if getProviderName is called before calling the deriveKey or deriveData methods, the first provider supporting the KDF algorithm is chosen which may not be the desired one; therefore it is recommended to not call getProviderName until after a key derivation operation.

This is because the selection occurs just once. Should we explicitly mention this?

> src/java.base/share/classes/javax/crypto/KDF.java line 414:
> 
>> 412:      *
>> 413:      */
>> 414:     public SecretKey deriveKey(String alg, KDFParameterSpec kdfParameterSpec)
> 
> Are there cases where the parameters are correct, but the derive operation can still fail for other reasons? If so, I'm not sure we should be wrapping those in InvalidParameterSpecException. That exception should be thrown by the method initially when it inspects the parameters and before the derive operation begins.
> 
> This is why I mentioned previously if it makes sense to add a DerivationException class.

First, very wrong parameters (say, null info, negative length) should not be create-able at all.

Then, in some cases, "correct" parameters could still be "invalid". For example, HKDF expand key length cannot exceed HashLen * 255, but HashLen is determined by the KDF algorithm. In this case, maybe an `InvalidAlgorithmParameterException` should be thrown because it does not conform to the spec.

Sometimes the implementation just does not support some parameters. For example, in PKCS #11 you cannot provide an arbitrary string as the algorithm name. Also, only if HKDF expand "info" is a well-known value that's recognized by the underlying implementation, `deriveData` is able to return a byte array. See 7a in https://docs.oasis-open.org/pkcs11/pkcs11-profiles/v3.1/os/pkcs11-profiles-v3.1-os.html#_Toc142307348. In these cases, maybe an `UnsupportedOperationException` should be thrown because the implementation does not support them.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1597686355
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1597685778