RFR: 8314323: Implement JEP 527: TLS 1.3 Hybrid Key Exchange [v9]

[Bradford Wetmore]

On Mon, 1 Dec 2025 16:13:52 GMT, Sean Mullan <mullan at openjdk.org> 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
>
> src/java.base/share/classes/com/sun/crypto/provider/DH.java line 66:
> 
>> 64:  * KEMs.
>> 65:  */
>> 66: public class DH implements KEMSpi {
> 
> This class includes more than a DH KEM wrapper implementation. The provider is registering all the hybrid algorithms, including the ML-KEM parts. It feels odd to be hiding the provider inside this class. I suggest creating a separate `HybridProvider` class that is in `sun.security.ssl` package and either encapsulating the DH impl inside that, or better yet, create a separate class for it.

Agreed.

> src/java.base/share/classes/sun/security/ssl/KEMKeyExchange.java line 28:
> 
>> 26: 
>> 27: import java.io.IOException;
>> 28: import java.security.*;
> 
> Probably clearer not to use wildcard imports so we can see all the classes needed, same comment on line 32.

I vaguely remember somewhere the guidance that if there are <= 5 imports, you should list them, otherwise use the wildcard.

> test/jdk/javax/net/ssl/TLSv13/HRRKeyShares.java line 33:
> 
>> 31:  * @summary Use two key share entries
>> 32:  * @library /test/lib
>> 33:  * @run main/othervm -Djdk.tls.namedGroups=x25519,secp256r1,secp384r1,X25519MLKEM768,SecP256r1MLKEM768,SecP384r1MLKEM1024 HRRKeyShares
> 
> Long line, break up into multiple lines.

I noted quite a few <= 80 comments throughout, so fully agree here!  ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2578840216
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2578857585
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2578864356