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