On Tue, 15 Oct 2024 22:17:26 GMT, Ben Perez <bpe...@openjdk.org> wrote:
>> Java implementation of ML-KEM, the [FIPS >> 203](https://csrc.nist.gov/pubs/fips/203/final) post-quantum KEM scheme. >> Depends on https://github.com/openjdk/jdk/pull/21167 > > Ben Perez has updated the pull request incrementally with one additional > commit since the last revision: > > NamedParameterSpec constants src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 29: > 27: private ML_KEM_EncapsulationKey encapsulationKey = null; > 28: private ML_KEM_DecapsulationKey decapsulationKey = null; > 29: private SecureRandom secureRandom = null; I don't see where these are used. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 405: > 403: > 404: public record K_PKE_DecryptionKey(byte[] keyBytes) { > 405: static K_PKE_DecryptionKey from(ML_KEM_DecapsulationKey key) { Unused method as far as I can tell. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 525: > 523: throw new DecapsulateException("Invalid ciphertext"); > 524: } > 525: I don't see the hash check mentioned in section 7.3 of the spec. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 638: > 636: new K_PKE_EncryptionKey(pkEncoded)); > 637: > 638: return kPkekp; You could return output from K_PKE_KeyPair() on line 634 and avoid the local variable. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 739: > 737: Arrays.fill(ofs, 0); > 738: short[][] aij = new short[nrPar][]; > 739: Shake128Parallel parXof = new Shake128Parallel(xofBufArr);; Double semicolons. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 1080: > 1078: // the result. It also returns (the modified) a so that it can be > used > 1079: // in an expression. > 1080: // The coefficiens in all polynomials of both vectors are supposed > to be coefficiens src/java.base/share/classes/com/sun/crypto/provider/ML_KEM.java line 1327: > 1325: private short[] decodePoly(int l, byte[] input, int index) { > 1326: short[] poly = new short[mlKem_n]; > 1327: short[] poly1 = new short[mlKem_n]; poly1 is unused. src/java.base/share/classes/com/sun/crypto/provider/ML_KEM_Provider.java line 152: > 150: var kpkeCipherText = new ML_KEM.K_PKE_CipherText(cipherText); > 151: > 152: byte[] decapsulateResult = null; Unnecessary initialization. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1825998841 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826009580 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826186198 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826231562 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826248244 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826265816 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826262511 PR Review Comment: https://git.openjdk.org/jdk/pull/21478#discussion_r1826280582
