Refactor: Simplify base functionality of Hybrid KEMs#5715
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Botan’s hybrid KEM foundation to assume exactly two component algorithms (instead of N≥2), and lifts the KEX→KEM adapter usage into the hybrid_kem base so multiple hybrid implementations (e.g., TLS and future composites) can reuse it.
Changes:
- Refactor
hybrid_kembase keys/ops to operate onstd::pair-based key storage (PairOfPublicKeys/PairOfPrivateKeys) and corresponding paired ciphertext/shared-secret handling. - Update the TLS 1.3 PQ-hybrid key implementation to the new two-key hybrid foundation and rename TLS-specific key types (
Hybrid_TLS_KEM_*). - Adjust tests and module dependency declarations to match the new architecture (and remove the now-unused
stl_util.h::reducehelper).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/test_tls_hybrid_kem_key.cpp | Updates TLS hybrid KEM tests to the new two-key API and renamed TLS hybrid key types. |
| src/lib/utils/stl_util.h | Removes the internal reduce helper previously used by the N-key hybrid implementation. |
| src/lib/tls/tls13_pqc/info.txt | Updates module dependencies after moving KEX→KEM adapter usage into hybrid_kem. |
| src/lib/tls/tls13_pqc/hybrid_public_key.h | Renames TLS hybrid key classes and adapts interfaces to pair-based hybrid keys. |
| src/lib/tls/tls13_pqc/hybrid_public_key.cpp | Refactors TLS hybrid key parsing/generation/ops wiring to the new two-key base and paired ops types. |
| src/lib/tls/tls_callbacks.cpp | Switches TLS callbacks to use the renamed TLS hybrid KEM key classes. |
| src/lib/pubkey/hybrid_kem/info.txt | Declares kex_to_kem_adapter as a required dependency for the refactored hybrid KEM base. |
| src/lib/pubkey/hybrid_kem/hybrid_kem.h | Refactors hybrid KEM base key abstractions to take exactly two keys via std::pair. |
| src/lib/pubkey/hybrid_kem/hybrid_kem.cpp | Implements pair-based hybrid key construction, auto-wrapping KEX keys into KEX→KEM adapters. |
| src/lib/pubkey/hybrid_kem/hybrid_kem_ops.h | Refactors hybrid KEM combiner ops interfaces to pair-based ciphertext/shared-secret handling. |
| src/lib/pubkey/hybrid_kem/hybrid_kem_ops.cpp | Implements the pair-based encryption/decryption combiner scaffolding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b61079c to
242d961
Compare
Before, the implementation allowed for hybridization of an arbitrary number of algorithms (n>=2). This introduced unnecessary complexity given that is pretty unlikely that we'll see general-purpose hybridization of more than two algorithms. Additionally, this lifts the KeyExchange-to-KeyEncapsulation adapter from the concrete TLS 1.3 hybridization implementation into the base class so that it may be reused by other concrete hybridization implementations. Most notably by draft-ietf-lamps-pq-composite-kem.
Before, the Hybrid_KEM_PublicKey contained an unfortunate default c'tor that would be silently called in the most-derived class to initialize the class in a diamond-shape virtual inhertance relationship. This resulted in unexpected internal states of the public key portion of the object. Instead, by removing the default c'tor, we now force a more verbose but easier to understand workaround on the user.
This lets user explicitly unwrap the wrapped KEX-key as necessary.
For TLS 1.3 it is currently just an implementation detail. But for the Composite ML-KEM implementation it will have to become accessible via the public API.
The old name 'hybrid_public_key.h' was too generic and might become ambiguouos in the future.
8a8e0c2 to
547378b
Compare
falko-strenzke
left a comment
falko-strenzke
left a comment
There was a problem hiding this comment.
This is only a superficial review. The reason is that I don't really see a clearly convincing value in the hybrid_kem and hybrid_kem_ops. Most of the functions they implemenent are one-liners and thus hardly save much code in the derived classes. They make one assumption that may not always hold, namely that the artifacts are concatenated. As a downside they create a lot of interdependency for trivial things. Of course, on the plus side, they add some functions to the child classes that ideally do what is needed.
Regarding this PR, I am not convinced that restricting the number of component schemes to 2 is good idea. I agree that we will probably not see standardization of multi-algorithm combinations beyond 2 component schemes. However, custom algorithm-combinations might in my view choose to combine more than 2.
|
I concur that this base class doesn't provide an awful lot of shared code. However, I think there's value in the standardized approach towards hybrid keys. Regarding the serialization assumptions: those don't have to be used and can be overridden in concrete subclasses. |
As promised, this is a rework of the Hybrid KEM foundation that we already support in the library. The patch is rather large, but it is mostly moving stuff around and simplifying things as described below.
Before, the base class in the
hybrid_kemmodule allowed for hybridization of an arbitrary number of algorithms (n>=2). This introduced unnecessary complexity given that it is pretty unlikely that we'll see general-purpose hybridization of more than two algorithms any time soon.Additionally, this lifts the KeyExchange-to-KeyEncapsulation adapter from the concrete TLS 1.3 hybridization implementation into the base class so that it may be reused by other concrete hybridization implementations. Most notably by draft-ietf-lamps-pq-composite-kem (see #5686). For that use case, the Hybrid KEM base class has to be publish as a public API so that the public ML-KEM-Composite classes can inherit from it.
The basic architecture is as follows:
KEX_to_KEM_Adapter_PublicKey/KEX_to_KEM_Adapter_PrivateKey(in thekex_to_kem_adaptermodule)These classes take ownership of a Key-Exchange keypair (i.e. DH, ECDH or X25519/X448) and build a canonical Key-Encapsulation API on their basic functionality. Essentially "KeyGen -> KeyGen", "Encaps -> KeyGen + Agree", "Decaps -> Agree". This module existed before and is essentially unchanged in this patch.
Hybrid_KEM_PublicKey/Hybrid_KEM_PrivateKey(in thehybrid_kemmodule)These abstract classes now take exactly two keypairs (of either KEX or KEM algorithms) and implement the low-level mechanics to provide a hybrid KEM. Concrete hybridization implementations can use them as base classes and modify the combination specifics as needed. Most notably, key serialization, algorithm identification and shared secret combination.
Hybrid_TLS_KEM_PublicKey/Hybrid_TLS_KEM_PrivateKey(in thetls13_pqcmodule)Those implement the concrete TLS 1.3 PQ-hybrid as specified in draft-ietf-tls-hybrid-design. They are concrete subclasses of
Hybrid_KEM_PublicKey/Hybrid_KEM_PrivateKey.