On Wed, 26 Nov 2025 18:10:03 GMT, Bradford Wetmore <[email protected]> wrote:
>> 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. One other nit, currently the `Params` class doesn't actually handle `DH`, just `ECDH`/`XDH`. Should you remove `DH` from the `DH/ECDH/XDH` javadoc? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2566314147
