On Thu, 2 Oct 2025 22:52:49 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. src/java.base/share/classes/com/sun/crypto/provider/DH.java line 224: > 222: } else if (k instanceof XECKey xkey > 223: && xkey.getParams() instanceof NamedParameterSpec > ns) { > 224: if (ns.getName().equalsIgnoreCase("X25519")) { Maybe use NamedParameterSpec.X25519? src/java.base/share/classes/sun/security/ssl/NamedGroup.java line 218: > 216: PredefinedDHParameterSpecs.ffdheParams.get(8192)), > 217: > 218: ML_KEM_512(0x0200, "MLKEM512", Are they needed for this Jep? src/java.base/share/classes/sun/security/ssl/NamedGroup.java line 653: > 651: NAMED_GROUP_XDH("XDH", XDHScheme.instance), > 652: > 653: NAMED_GROUP_KEM("PQC", KEMScheme.instance), That Choice of Name needs probably an explaining comment if it is for pure PQC and/ormhybrid? src/java.base/share/classes/sun/security/ssl/NamedGroup.java line 802: > 800: FFDHE_3072, > 801: FFDHE_4096, > 802: FFDHE_6144, Unrelated change? src/java.base/share/classes/sun/security/ssl/NamedGroup.java line 906: > 904: NamedGroup[] groups = new NamedGroup[] { > 905: > 906: // Hybrid key agreements It feels like all the infra for X448MLKEM1024 is there, so rather than removing x448 from this patch, why not implement it (it’s more obvious than P511 Variants) src/java.base/share/classes/sun/security/util/Hybrid.java line 75: > 73: > 74: private static AlgorithmParameterSpec getSpec(String name) { > 75: if (name.startsWith("secp")) { This seems to be repeated multiple times including the case sensitive string, maybe have an APS.isGenericEC() helper? src/java.base/share/classes/sun/security/util/Hybrid.java line 230: > 228: private static int leftPublicLength(String name) { > 229: return switch (name.toLowerCase(Locale.ROOT)) { > 230: case "secp256r1" -> 65; Isn’t that Part formte named groups, maybe Look it up instead? src/java.base/share/classes/sun/security/util/Hybrid.java line 290: > 288: private static byte[] concat(byte[]... inputs) { > 289: ByteArrayOutputStream o = new ByteArrayOutputStream(); > 290: Arrays.stream(inputs).forEach(o::writeBytes); Do we really want a non presized buffer and a stream in the handshake hot path? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402691528 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402706225 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402713652 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402721266 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402719813 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402730803 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402738310 PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2402742473
