RFR: 8297878: KEM: Implementation [v9]

[Weijun Wang]

On Wed, 26 Apr 2023 17:56:12 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   small spec change
>
> src/java.base/share/classes/javax/crypto/KEMSpi.java line 127:
> 
>> 125:          *          the key encapsulation message, and optional parameters.
>> 126:          * @throws ArrayIndexOutOfBoundsException if {@code from < 0},
>> 127:          *     {@code from > to}, or {@code to > secretSize()}
> 
> Should this throw an `IndexOutOfBoundsException` instead here? That's what the DHKEM impl does and this seems more reasonable since you aren't actually trying to access an array.

Yes, you're right. I actually didn't realize the difference between IOOBE and AIOOBE at all. In fact, I checked for IOOBE in the `Compliance.java` test.

> src/java.base/share/classes/javax/crypto/KEMSpi.java line 168:
> 
>> 166:          * <p>
>> 167:          * An implementation must support the case where {@code from} is 0,
>> 168:          * {@code from} is the same as the output of {@code secretSize()},
> 
> Same comment as above.

Will fix.

> src/java.base/share/classes/javax/crypto/KEMSpi.java line 184:
> 
>> 182:          * @throws DecapsulateException if an error occurs during the
>> 183:          *          decapsulation process
>> 184:          * @throws ArrayIndexOutOfBoundsException if {@code from < 0},
> 
> Same comment as above.

Will fix.

> src/java.base/share/classes/javax/crypto/KEMSpi.java line 242:
> 
>> 240:      * @see KEM#newDecapsulator(PrivateKey, AlgorithmParameterSpec)
>> 241:      */
>> 242:     DecapsulatorSpi engineNewDecapsulator(PrivateKey sk, AlgorithmParameterSpec spec)
> 
> Bit of a nit, but suggest changing variable names to be more descriptive: `privKey` (or `privateKey`) and `pubKey` (or `publicKey`), for the `engineEncapsulate` method. I assume `sk` is taken from RFC 9180, but I don't think we need to use the same variable names.

OK, I'll change them.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178254497
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178256035
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178255927
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178247018