RFR: 8297878: KEM: Implementation [v4]

[Weijun Wang]

On Fri, 14 Apr 2023 14:33:56 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> src/java.base/share/classes/javax/crypto/KEM.java line 94:
>> 
>>> 92:      * @see KEM#newEncapsulator(PublicKey, AlgorithmParameterSpec, SecureRandom)
>>> 93:      */
>>> 94:     public record Encapsulated(SecretKey key, byte[] encapsulation, byte[] params) {
>> 
>> We need to decide if the encapsulation and params arrays should be defensively cloned. I would lean towards cloning it since immutability is a feature of this API, and I think it would be surprising if this type was not. 
>> 
>> We can potentially switch to frozen arrays later.
>
> If we need to clone defensively I'll switch back to normal class, and then we can clone in both the constructor and the getters. IMO records are meant to be deadly simple.

Rewrite in plain normal class. Clone on both sides.

>> src/java.base/share/classes/javax/crypto/KEMSpi.java line 211:
>> 
>>> 209:      * The caller of this method has already validated the parameters to
>>> 210:      * ensure that {@code pk} is not {@code null}. Therefore an implementation
>>> 211:      * of this method does not to validate it.
>> 
>> s/not to/not need to/
>> 
>> Also, suggest saying who the caller is, "The caller (KEM.newEncapsulator) of this method ..."
>
> Fixed.
> 
> Not sure if the caller needs to be written out. `KEM.newEncapsulator` is the ultimate caller, but for delayed provider selection, the direct caller is in `Delayed`. Also, this can be considered as implementation detail. If I write out the name, then we cannot change.
> 
> `engineEncapsulate` and `engineDecapsulate` also use "the caller of this method".

Removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1170288830
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1170290779