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

Weijun Wang weijun at openjdk.org
Thu Dec 4 15:39:39 UTC 2025


On Thu, 4 Dec 2025 00:03:49 GMT, Hai-May Chao <hchao at openjdk.org> 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 one additional commit since the last revision:
> 
>   Remove null check to not assume key is returned

Some comments.

src/java.base/share/classes/sun/security/ssl/DHasKEM.java line 214:

> 212:             if (from == 0 && to == params.secretLen) {
> 213:                 return key;
> 214:             } else if ("RAW".equalsIgnoreCase(key.getFormat())) {

I think it's not worth supporting key slicing because it should never happen. Otherwise there might be a programming error. Throw an AssertionError here.

src/java.base/share/classes/sun/security/ssl/Hybrid.java line 406:

> 404:         // getFormat() returns "RAW" if both keys are X509Key; otherwise null.
> 405:         @Override
> 406:         public String getFormat() {

My understanding is that this will always return "RAW" even if `left` or `right` comes from 3rd-party providers and is not `X509Key`.

src/java.base/share/classes/sun/security/ssl/Hybrid.java line 420:

> 418:             if (left instanceof X509Key xk1 && right instanceof X509Key xk2) {
> 419:                 return concat(xk1.getKeyAsBytes(), xk2.getKeyAsBytes());
> 420:             } else {

Here, we should be prepared for 3rd-party providers -- just call `getEncoded` and strip the ASN.1 headers of algorithm identifiers.

src/java.base/share/classes/sun/security/ssl/Hybrid.java line 459:

> 457: 
> 458:         static boolean isXDH(String name) {
> 459:             return name != null && name.equals("X25519");

Can `name` in the two methods above be null? This might be hiding a bug.

Also, I think it's not worth putting them into a separate class `APS`.

src/java.base/share/classes/sun/security/ssl/KAKeyDerivation.java line 184:

> 182:                     KEM.getInstance(algorithmName, provider) :
> 183:                     KEM.getInstance(algorithmName);
> 184:             KEM.Encapsulator e = kem.newEncapsulator(pk);

`newEncapsulator` is called without an `SecureRandom` argument. Is this intended? Otherwise, we had a chance to pass in a user-provided random when `KEMSenderPossession` is created and it can be passed here.

src/java.base/share/classes/sun/security/ssl/KEMKeyExchange.java line 116:

> 114:         private final PublicKey publicKey;
> 115: 
> 116:         KEMReceiverPossession(NamedGroup namedGroup, SecureRandom random) {

`random` is ignored. Is this intended?

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

PR Review: https://git.openjdk.org/jdk/pull/27614#pullrequestreview-3540372956
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589356191
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589475112
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589478265
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589389536
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589439762
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2589412239


More information about the security-dev mailing list