RFR: 8331008: KDF Implementation [v6]

[Sean Mullan]

On Thu, 9 May 2024 15:04:53 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Kevin Driver has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   change algorithm standard name for HKDFs in SunJCE provider
>
> src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 70:
> 
>> 68:          */
>> 69:         public Extract extractOnly() {
>> 70:             if (this.ikms.isEmpty() && this.salts.isEmpty()) {
> 
> I don't think this check is necessary? While it's probably unsafe to provide no IKM, providing no salt is quite common. Anyway, no need to restrict on both, IMHO

I agree. Also, if we do want to validate arguments (and I don't know if we need to), then I think the `Extract` constructor should be responsible for doing that, not the `Builder`. Doing it in `Extract` is safer since it is done after the fields are cloned.

> src/java.base/share/classes/javax/crypto/spec/HKDFParameterSpec.java line 108:
> 
>> 106:          *
>> 107:          * @param ikm
>> 108:          *     the ikm value (null values will not be added)
> 
> Are you sure about allowing `null`? Any valid use case? Same question for the other `add` methods.

`null` should not be silently ignored. The method should throw `NullPointerException` (and that should be added to the javdoc in an `@throws` clause).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1596861849
PR Review Comment: https://git.openjdk.org/jdk/pull/18924#discussion_r1596866987