On Wed, 26 Apr 2023 14:29:25 GMT, Weijun Wang <wei...@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: > > small spec change src/java.base/share/classes/javax/crypto/KEMSpi.java line 111: > 109: * The key encapsulation function. > 110: * <p> > 111: * Each invocation of this method must generates a new secret > key and key s/generates/generate/ src/java.base/share/classes/javax/crypto/KEMSpi.java line 115: > 113: * <p> > 114: * An implementation must support the case where {@code from} is > 0, > 115: * {@code from} is the same as the output of {@code > secretSize()}, Should "from" be "to"? Maybe also say "return value" instead of "output". src/java.base/share/classes/javax/crypto/KEMSpi.java line 125: > 123: * @return an {@link KEM.Encapsulated} object containing a > portion of > 124: * the shared secret as a key with the specified > algorithm, > 125: * the key encapsulation message, and optional > parameters. Remove "the". 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. 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. 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. 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178210665 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178212708 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178214239 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178219972 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178221846 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178220363 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178207872
