RFR: 8297878: KEM: Implementation [v10]
Weijun Wang
weijun at openjdk.org
Thu Apr 27 18:33:26 UTC 2023
On Thu, 27 Apr 2023 13:22:38 GMT, Sean Mullan <mullan at openjdk.org> wrote:
>> I checked for our implementations, and until now all I found is IKE. Even the PBEWithMD5AndDES algorithm mentioned in https://bugs.openjdk.org/browse/JDK-4953551 also throws an IKE.
>>
>> I'd rather follow the existing style. If it’s not clear enough, I can update the line to
>>
>> @throws InvalidKeyException if {@code publicKey} is {@code null} or invalid
>>
>>
>> If we are worried that a security provider might throw an NPE, how about we add a check on the user API side:
>>
>> public Encapsulator newEncapsulator(PublicKey publicKey,
>> AlgorithmParameterSpec spec, SecureRandom secureRandom)
>> throws InvalidAlgorithmParameterException, InvalidKeyException {
>> if (publicKey == null) {
>> throw new InvalidKeyException("Null key provided");
>> }
>> return delayed != null
>> ? delayed.newEncapsulator(publicKey, spec, secureRandom)
>> : new Encapsulator(spi.engineNewEncapsulator(publicKey, spec, secureRandom), provider);
>> }
>
>> I checked for our implementations, and until now all I found is IKE. Even the PBEWithMD5AndDES algorithm mentioned in https://bugs.openjdk.org/browse/JDK-4953551 also throws an IKE.
>>
>> I'd rather follow the existing style. If it’s not clear enough, I can update the line to
>>
>> ```
>> @throws InvalidKeyException if {@code publicKey} is {@code null} or invalid
>> ```
>
> I'll leave it up to you, or consensus from others. This one particular issue is probably not going to overly affect the API much.
>
> The APIs you referenced were created a long time ago before more accepted API best practices evolved. Also, this API already throws NPE in other methods when a required parameter is `null`, so these methods seem inconsistent with the way it treats `null` parameters.
The new `KEM` class still follows the API/SPI design principal. I gave up designing `DecapsulteException` as an uncheck exception and I still use class instead of record for `Encapsulated`. So let's consistently traditional.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1179528648
More information about the security-dev
mailing list