On Tue, 9 Dec 2025 03:48:13 GMT, Bradford Wetmore <[email protected]> wrote:

>> 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?

Split up the long line. Yes, because of NSS-FIPS not support it, and added a 
comment.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2601262949

Reply via email to