On Wed, 18 Mar 2026 19:14:25 GMT, Hai-May Chao <[email protected]> wrote:
>> This change implements behavior required by the specification Post-quantum
>> hybrid ECDHE-MLKEM Key Agreement for TLSv1.3. The specification defines
>> several validation checks during the hybrid key exchange that require
>> aborting the connection with either an illegal_parameter alert or an
>> internal_error alert.
>>
>> In 4.2. Server share section specifies the following checks:
>> For all groups, the server MUST perform the encapsulation key check
>> described in Section 7.2 of [NIST-FIPS-203] on the client’s encapsulation
>> key, and abort with an illegal_parameter alert if it fails.
>>
>> For all groups, the client MUST check if the ciphertext length matches the
>> selected group, and abort with an illegal_parameter alert if it fails. If
>> ML-KEM decapsulation fails for any other reason, the connection MUST be
>> aborted with an internal_error alert.
>>
>> For all groups, both client and server MUST process the ECDH part as
>> described in Section 4.2.8.2 of [RFC8446], including all validity checks,
>> and abort with an illegal_parameter alert if it fails.
>>
>> In 4.3. Shared secret section specifies the following check:
>> For all groups, both client and server MUST calculate the ECDH part of the
>> shared secret as described in Section 7.4.2 of [RFC8446], including the
>> all-zero shared secret check for X25519, and abort the connection with an
>> illegal_parameter alert if it fails.
>>
>> This implementation propagates exceptions raised during ECDH and ML-KEM
>> operations in client and server sides from the Hybrid and DHasKEM classes
>> (which implement KEMSpi) to the TLS handshake layer, where they are mapped
>> to the corresponding TLS fatal alerts.
>
> Hai-May Chao has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update with Mikhail's comment
Marked as reviewed by wetmore (Reviewer).
src/java.base/share/classes/sun/security/ssl/KAKeyDerivation.java line 1:
> 1: /*
This logic is specific to the current Hybrid KEM draft. Is there a chance a
different (e.g. non-hybrid) KEM might have different behavior specified?
(I don't know the answer, but wanted to check.)
src/java.base/share/classes/sun/security/ssl/KAKeyDerivation.java line 207:
> 205: throw context.conContext.fatal(Alert.INTERNAL_ERROR, e);
> 206: } catch (RuntimeException e) {
> 207: throw context.conContext.fatal(Alert.INTERNAL_ERROR, e);
Consider adding a similar comment here for RTE as on lines 203/204 ? I like
seeing what options were considered.
If not, then you could combine lines 202+206 in a multi-catch. (but meh...)
src/java.base/share/classes/sun/security/ssl/KAKeyDerivation.java line 245:
> 243: // cryptographic failure
> 244: throw context.conContext.fatal(Alert.INTERNAL_ERROR,
> e);
> 245: } catch (RuntimeException e) {
Same as above (add comment).
-------------
PR Review: https://git.openjdk.org/jdk/pull/30039#pullrequestreview-3984971629
PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2968683675
PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2968694936
PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2968696641
