On Tue, 3 Mar 2026 23:08:37 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.
src/java.base/share/classes/sun/security/ssl/DHasKEM.java line 282:
> 280: }
> 281: }
> 282: if (allZero) {
Is this if statement needed? Do you think returning from line 279 instead of
breaking would make the code both simpler and use less memory? Then you can
throw directly after the loop that checks for zeros. Something like this:
// RFC 8446 section 7.4.2: checks for all-zero
// X25519/X448 shared secret.
if (kaAlgorithm.equals("X25519") ||
kaAlgorithm.equals("X448")) {
byte[] s = secret.getEncoded();
for (byte b : s) {
if (b != 0) {
return secret;
}
}
// Trigger ILLEGAL_PARAMETER alert
throw new IllegalArgumentException(
"All-zero shared secret");
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2953170077
