On Tue, 25 Nov 2025 20:33:04 GMT, Sean Mullan <[email protected]> wrote:
>> 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/pkcs11/tls/fips/FipsModeTLS.java line 38: > >> 36: * @comment SunPKCS11 does not support (TLS1.2) >> SunTlsExtendedMasterSecret yet. >> 37: * Stateless resumption doesn't currently work with NSS-FIPS, see >> JDK-8368669 >> 38: * @run main/othervm/timeout=120 -Djdk.tls.client.protocols=TLSv1.3 >> -Djdk.tls.namedGroups=x25519,secp256r1,secp384r1,secp521r1,x448,ffdhe2048,ffdhe3072,ffdhe4096,ffdhe6144,ffdhe8192 >> FipsModeTLS > > Long line, break up into more than one line. > > Also instead of setting the system property, suggest using the > `SSLParameters.getNamedGroups()` API to read the default list of named > groups, remove X25519MLKEM768 and then set the list back. This way if the > other defaults change in the future (like removing some of the ffdhe groups) > the code will still be ok and reflect the default list. > > It looks like the code already does that for other groups in > `createSSLEngine`. Looks like this long line comment got missed. (or alternatively use the API.)_ Could you also add a comment about why you're forcing the namedGroups here? Guessing that FIPS doesn't support MLKEM? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2600982537
