RFR: 8297878: KEM: Implementation [v2]

[Sean Mullan]

On Thu, 13 Apr 2023 22:29:34 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> The KEM API and DHKEM impl. Note that this PR uses new methods in https://github.com/openjdk/jdk/pull/13250.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   spec change, getAlgorithm

src/java.base/share/classes/javax/crypto/KEM.java line 49:

> 47:  * If a provider is not specified in the {@code getInstance} method when
> 48:  * instantiating a {@code KEM} object, the {@code newEncapsulator} and
> 49:  * {@code newDecapsulator} method may return encapsulators or decapsulators

s/method/methods/

src/java.base/share/classes/javax/crypto/KEM.java line 51:

> 49:  * {@code newDecapsulator} method may return encapsulators or decapsulators
> 50:  * from different providers. The provider selected is based on the parameters
> 51:  * passed to the the {@code newEncapsulator} or {@code newDecapsulator} methods:

s/the the/the/

src/java.base/share/classes/javax/crypto/KEM.java line 54:

> 52:  * the private or public key and the optional {@code AlgorithmParameterSpec}.
> 53:  * The {@link Encapsulator#provider} and {@link Decapsulator#provider} methods
> 54:  * return the selected provider."

remove "".

src/java.base/share/classes/javax/crypto/KEM.java line 103:

> 101:          * @param key the shared secret as a key, must not be {@code null}.
> 102:          * @param encapsulation the key encapsulation message, must not be {@code null}.
> 103:          * @param params additional parameters, can be {@code null}.

Nit: remove periods.

src/java.base/share/classes/javax/crypto/KEM.java line 105:

> 103:          * @param params additional parameters, can be {@code null}.
> 104:          */
> 105:         public Encapsulated {

Should this also have an `@throws NullPointerException` clause?

src/java.base/share/classes/javax/crypto/KEM.java line 128:

> 126:          * Returns the provider.
> 127:          *
> 128:          * @return thr provider

s/thr/the/

src/java.base/share/classes/javax/crypto/KEM.java line 143:

> 141:          * @param encapsulation the key encapsulation message from the sender
> 142:          * @return the shared secret as a {@code SecretKey} with
> 143:          *          algorithm being "Generic", not {@code null}

See my comment below, I don't think you need to specify that this does not return null, it should be implied.

src/java.base/share/classes/javax/crypto/KEM.java line 160:

> 158:          * @param algorithm the algorithm for the secret key returned
> 159:          * @return a portion of the shared secret as a {@code SecretKey} with
> 160:          *          the specified algorithm, not {@code null}

Same comment as above.

src/java.base/share/classes/javax/crypto/KEM.java line 242:

> 240:          *          shared secret as a key with algorithm being "Generic",
> 241:          *          the key encapsulation message, and optional parameters.
> 242:          *          The return value should not be {@code null}.

"should" means it *could* still return null. I assume that is not what we want. Although I would be more inclined to only specify cases where null may be returned, and if it isn't mentioned, then it should be implied that null is not a legal return value. If in doubt, perhaps check with Joe/CCC for advice when you file the CSR.

This general comment applies to the other return types in this API where you say "not null". I think you can omit those.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166801682
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166802062
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166802596
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166808093
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166811613
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166826429
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166829885
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166830271
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166841781