On Sat, 21 Mar 2026 02:04:15 GMT, Bradford Wetmore <[email protected]> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update with Mikhail's comment > > 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.) The IETF spec (for non-hybrid): ML-KEM Post-Quantum Key Agreement for TLS 1.3, defines ML-KEM-512, ML-KEM-768, and ML-KEM-1024. https://datatracker.ietf.org/doc/draft-ietf-tls-mlkem/ In section 4.2, it specifies that: For all parameter sets, the server MUST perform the encapsulation key check described in Section 7.2 of [FIPS203] on the client's encapsulation key, and abort with an illegal_parameter alert if it fails. For all parameter sets, the client MUST check if the ciphertext length matches the selected parameter set, 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. So ML-KEM (non-hybrid) has the same behavior requirements as hybrid ECDHE-MLKEM, and the exceptions mapping to TLS alerts covers both. The difference is that ECDH validation is only applicable to hybrid ECDHE-MLKEM. > 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...) Added. > 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). Added. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2979939897 PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2979940438 PR Review Comment: https://git.openjdk.org/jdk/pull/30039#discussion_r2979940690
