On Mon, 24 Nov 2025 07:51:40 GMT, Hai-May Chao <[email protected]> wrote:
>> Implement hybrid key exchange support for TLS 1.3 by adding three >> post-quantum hybrid named groups: X25519MLKEM768, SecP256r1MLKEM768, and >> SecP384r1MLKEM1024. >> Please see [JEP 527](https://openjdk.org/jeps/527) for details about this >> change. > > Hai-May Chao has updated the pull request incrementally with three additional > commits since the last revision: > > - Update names to uppercase > - Remove fallback in engineGeneratePublic > - Change default named group list to have only X25519MLKEM768 test/jdk/sun/security/ssl/CipherSuite/NamedGroupsWithCipherSuite.java line 84: > 82: }; > 83: > 84: private static final String[] HYBRID_NAMEDGROUPS = { Somewhat of a nit, but you only access this as a `List` in the code below, so it would be better to just use `List.of` here. In fact, these 3 arrays could all be lists and initialized with `List.of`. test/jdk/sun/security/ssl/CipherSuite/SupportedGroups.java line 51: > 49: > 50: private static volatile int index; > 51: private static final String[][][] protocolForClassic = { Nit: suggest naming this `protocolsForClassic`. test/jdk/sun/security/ssl/CipherSuite/SupportedGroups.java line 58: > 56: }; > 57: > 58: private static final String[][][] protocolForHybrid = { Nit: suggest naming this `protocolsForHybrid`. test/micro/org/openjdk/bench/javax/crypto/full/KEMBench.java line 94: > 92: } > 93: > 94: protected static Provider getInternalJce() { Can this just be private? test/micro/org/openjdk/bench/javax/crypto/full/KEMBench.java line 97: > 95: try { > 96: Class<?> dhClazz = > Class.forName("com.sun.crypto.provider.DH"); > 97: return (Provider) (dhClazz.getField("PROVIDER").get(null)); Nit: params around `dhClazz.getField("PROVIDER").get(null)` not necessary. test/micro/org/openjdk/bench/javax/crypto/full/KEMBench.java line 120: > 118: > 119: public static class MLKEM extends KEMBench { > 120: @Param({"ML-KEM-512", "ML-KEM-768", "ML-KEM-1024" }) Not a JMH expert, but what is the difference between these parameters and the ones on line 48? It looks like a duplicate test to me. test/micro/org/openjdk/bench/javax/crypto/full/KeyPairGeneratorBench.java line 59: > 57: try { > 58: Class<?> dhClazz = > Class.forName("com.sun.crypto.provider.DH"); > 59: return (Provider) (dhClazz.getField("PROVIDER").get(null)); Nit: don't think you need the parens around `dhClazz.getField("PROVIDER").get(null)` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565417693 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565198775 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565199669 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565573576 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565575198 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565569737 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2565123722
