On Wed, 26 Nov 2025 00:22:28 GMT, Bradford Wetmore <[email protected]> wrote:
>> Hai-May Chao has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Update names to uppercase >> - Remove fallback in engineGeneratePublic >> - Change default named group list to have only X25519MLKEM768 > > src/java.base/share/classes/com/sun/crypto/provider/DH.java line 64: > >> 62: * key exchange. It models DH/ECDH/XDH as KEMs, like post-quantum >> algorithms, >> 63: * so DH/ECDH/XDH can be used in hybrid key exchange, alongside >> post-quantum >> 64: * KEMs. > > The purpose of this class from the opening javadoc was confusing on my first > several reads. I expected that this class just created a `KEM` layer for DH, > but surprise(!), it also crammed in a Hybrid provider implementation too! > > My preference is to break the `DH` and `Provider`+`HybridService` code into > separate files for cleaner abstractions, but if not, then maybe use something > like this for the description? > > > /** > * The DH class presents a KEM abstraction layer over traditional > * DH-based key exchange, which can be used for either straight > * DH/ECDH/XDH or TLS hybrid key exchanges. > * > * This class can be alongside standard full post-quantum KEMs > * when hybrid implementations are required. > */ Could this class could be renamed to something more meaningful? e.g. `DHasKEM`, `DHasaKEM` or something similar. A class name of `DH` by itself hints this will be a DH implementation. I would expect a `KeyAgreement` impl, not as a wrapper. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2566027313
