Skip to content

hpke: draft-ietf-hpke-pq support#1539

Open
franziskuskiefer wants to merge 8 commits into
mainfrom
franziskus/hpke-pq
Open

hpke: draft-ietf-hpke-pq support#1539
franziskuskiefer wants to merge 8 commits into
mainfrom
franziskus/hpke-pq

Conversation

@franziskuskiefer

Copy link
Copy Markdown
Contributor

Adds the post-quantum and PQ/T-hybrid HPKE algorithms from draft-ietf-hpke-pq-05, behind a new draft-ietf-hpke-pq feature (libcrux provider only).

Added

  • Single-stage KDFs Shake128 = 0x0010 and Shake256 = 0x0011, with the one-stage HPKE key schedule (KeySchedule, Export, DeriveKeyPair).
  • Pure ML-KEM: MlKem512 = 0x0040 (plus existing MlKem768/MlKem1024 repointed to the IETF draft).
  • PQ/T-hybrid KEMs MlKem768P256 = 0x0050, MlKem1024P384 = 0x0051, and MLKEM768-X25519 = 0x647a, per draft-irtf-cfrg-concrete-hybrid-kems-04.

Deprecated

  • draft-connolly-cfrg-hpke-mlkem, superseded by draft-ietf-hpke-pq. The two are mutually exclusive (build warning + docs.rs pins a working feature set).

Notes

  • MLKEM1024-P384 also needs rustcrypto-p-curves for now.
  • Adds a bench_pq benchmark and test_hpke_pq suite with the draft's vectors.
  • Updates kats to check for explicit ciphersuite (kem) support.

To test the new code run cargo test -p hpke-rs -F draft-ietf-hpke-pq,rustcrypto-p-curves.

Claude helped push some of the code around.

Implement the post-quantum algorithms of draft-ietf-hpke-pq-04, gated
behind a new `draft-ietf-hpke-pq` cargo feature in the libcrux provider
only (rust_crypto is untouched beyond declining the new algorithms).

Algorithms:
- SHAKE128 (0x0010) and SHAKE256 (0x0011) one-stage KDFs
- ML-KEM-768 / ML-KEM-1024
- hybrids MLKEM768-P256 (0x0050) and MLKEM1024-P384 (0x0051)
- MLKEM768-X25519 (0x647a) via libcrux XWingKemDraft06

Core changes (feature-gated):
- One-stage (SHAKE) HPKE key schedule and Export: single-stage KDFs are
  not Extract/Expand, so `key_schedule` and `Context::export` branch on
  `kdf::is_one_stage` and use `LabeledDerive` (src/kdf.rs, src/lib.rs).
- DeriveKeyPair for ML-KEM/hybrid/X-Wing always uses SHAKE256.LabeledDerive,
  independent of the HPKE KDF, computed directly via libcrux_sha3 so it is
  provider-independent (src/kem.rs).

Provider (libcrux_provider):
- SHAKE128/256 via shake128/256_ema; ML-KEM private key is the seed
  (decaps re-expands).
- Hybrids match concrete-hybrid-kem GC: seed-as-dk, SHAKE256 PRG expansion,
  RustCrypto p256/p384 rejection sampling + uncompressed SEC1, x-coord ss_T,
  SHA3-256 C2PRI combiner.
- Sender-side encaps is derandomized (encapsulate_derand) so the KAT can
  inject the vector's ikmE without routing through the libcrux RNG.

Deprecate `draft-connolly-cfrg-hpke-mlkem` (superseded; build.rs warning)
and forbid enabling it together with `draft-ietf-hpke-pq` via compile_error!.
Because the two are mutually exclusive, `--all-features` does not build;
docs.rs metadata pins a working feature set in both Cargo.tomls.
Check for the exact list of supported cipersuites.
This is not implemented by any provider in the repo.
@franziskuskiefer
franziskuskiefer requested a review from keks July 16, 2026 10:37
@franziskuskiefer franziskuskiefer self-assigned this Jul 16, 2026
keks
keks previously requested changes Jul 17, 2026

@keks keks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a pretty good start! I left some comments. Most things are pretty minor, but I think the handling of singe stage KDFs could be improved, and that might be a little bit more involved.

Comment thread crates/protocols/hpke/CHANGELOG.md Outdated
Comment on lines +299 to +321
/// Benchmark the post-quantum ciphersuites.
///
/// Restricted to `Base` mode: the PQ KEMs return `UnsupportedKemOperation` for the
/// `Auth`/`AuthPsk` modes, and PSK is out of scope here. Key generation is timed in
/// addition to the usual operations.
fn benchmark_post_quantum<Crypto: HpkeCrypto + 'static>(c: &mut Criterion) {
for aead_mode in AEAD_IDS {
if Crypto::supports_aead(aead_mode).is_err() {
continue;
}
for kdf_mode in PQ_KDF_IDS {
if Crypto::supports_kdf(kdf_mode).is_err() {
continue;
}
for &kem_mode in PQ_KEM_IDS {
if Crypto::supports_kem(kem_mode).is_err() {
continue;
}
bench_suite::<Crypto>(c, HpkeMode::Base, kem_mode, kdf_mode, aead_mode, true);
}
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100% sure, but it seems bench.rs and bench-pq.rs are the same except that the IDs that are being iterated over are different and the non-pq version does not have an option to benchmark key generation.

Are there deeper differences? Otherwise I would be in favour of merging the files and just have different top-level functions for pq and non-pq (where non-pq can use the flag to disable benchmarking key generation).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought this could make sense but you're right that it ended up with too much duplicate code.

Comment thread crates/protocols/hpke/src/kdf.rs Outdated
Comment on lines +37 to +57
#[inline]
/// Build the `LabeledDerive` input for the single-stage KDFs:
/// `ikm ‖ "HPKE-v1" ‖ suite_id ‖ I2OSP(len(label),2) ‖ label ‖ I2OSP(L,2) ‖ context`.
///
/// requires: len <= u16::MAX.
pub(crate) fn shake256_labeled_ikm(
suite_id: &[u8],
label: &str,
context: &[u8],
ikm: &[u8],
len: usize,
) -> Vec<u8> {
concat(&[
ikm,
HPKE_VERSION,
suite_id,
&length_prefixed(label.as_bytes()),
&(len as u16).to_be_bytes(),
context,
])
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not convinced that this function is needed. In the draft they also just call concat from labeled_derive. I would just inline it.

https://datatracker.ietf.org/doc/html/draft-ietf-hpke-hpke-03#name-labeled-derivation-function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it's a bit more verbose than needed.

Comment on lines +51 to +71
fn kdf_extract(alg: KdfAlgorithm, salt: &[u8], ikm: &[u8]) -> Result<Vec<u8>, Error> {
let alg = kdf_algorithm_to_libcrux_hkdf_algorithm(alg);
// SHAKE256 is a single-stage (XOF) KDF: there is no separate extraction
// step, so `Extract(salt, ikm)` derives `Nh` bytes from `salt || ikm`.
// See draft-ietf-hpke-pq Section 5.
#[cfg(feature = "draft-ietf-hpke-pq")]
if let Some(nh) = shake_nh(alg) {
return Ok(shake_derive(alg, &[salt, ikm], nh));
}

let alg = kdf_algorithm_to_libcrux_hkdf_algorithm(alg)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The draft defines separate interfaces for these kinds of algorithms, and I don't think we should force them all into a single API. So maybe we should instead

  • define a new enum SingleStageKdfAlgorithm for the Shake algorithms (not a nice name, maybe there is something nicer)
  • define a new enum TwoStageKdfAlgorithm for the HKDF-based et al.
  • change the trait methods kdf_extract and kdf_expand to take the TwoStageKdfAlgorithm
  • define a new trait method kdf_derive(alg:SingleStageKdfAlgorithm, ikm: &[u8], l: usize) -> Result<Vec<u8>, Error>; and implement that here, and use that in labeled_derive
  • Give the caller a way to convert the KdfAlgorithm into either SingleStageKdfAlgorithm or a TwoStageKdfAlgorithm -- or maybe just define
    enum KdfAlgorithm {
      SingleStage(SingleStageKdfAlgorithm),
      TwoStage(TwoStageKdfAlgorithm),
    }
  • otherwise leave these functions as-is
  • make the callers pick the function to call based on whether it's single-stage or two-stage

Of course changing the trait breaks compatibility, but so does changing the enum, so I think it's fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this could be a bit cleaner. I don't want to change wire types and make that too complicated. But pulling this apart a bit more makes sense.
This is much more churn though. So I'm not sure if it's actually better. I tried some options and aren't super happy with either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just add a kdf_derive and we return an error if the wrong kind of method is called. Then at least we are not squeezing it into the wrong interface.

Comment thread crates/protocols/hpke/libcrux_provider/src/lib.rs

/// The nominal group (traditional component) of a hybrid KEM.
#[derive(Clone, Copy)]
enum NistCurve {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be called NominalGroup? That would leave the door for X25519 open. Actually, why is X25519 not in here? Because it is in a separate X-Wing impl? Maybe we could merge these? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can use NominalGroup. x25519 isn't used in any combiner like this and is handled pretty differently. So no need for it in here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay. In the RFC it looked like doing the normal combiner with X25519 would just be XWing, but I didn't look to closely.

### Changed

- [#154](https://github.com/celabshq/hpke-rs/pull/154): `HpkeCrypto::HpkePrng` now has `rand` v0.10 bound `CryptoRng`, instead of `rand` v0.9 bound `RngCore + CryptoRng`
- Add `KemAlgorithm::MlKem512 = 0x0040` (per `draft-ietf-hpke-pq`). Repoint the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a PR number? Also, aren't we adding more algorithms?

///
/// Single-stage KDFs use [`labeled_derive`] and a different key-schedule shape
/// than the two-stage HKDF KDFs (which use [`labeled_extract`]/[`labeled_expand`]).
pub fn is_one_stage(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be is_single_stage (or made obsolete by refactoring the whole single-stage vs two-stage mechanism)

@franziskuskiefer
franziskuskiefer requested a review from keks July 17, 2026 12:20
@github-actions
github-actions Bot dismissed keks’s stale review July 17, 2026 12:20

Review re-requested

@keks keks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this generally looks good. There seems to be another missing PR number in the change logs, and I have one question about whether something should be change where I expect the answer is "no", so approving with a caveat 👍


### Deprecated

- The `draft-connolly-cfrg-hpke-mlkem` feature is deprecated in favour of

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also have a PR number?

Comment on lines -585 to +586
let kdf_alg: KdfAlgorithm = alg.into();
let kdf_alg = two_stage_kdf(alg)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment in the From impl says that the caller can override the KDF algorithm. Can/should we respect that override here?

// Post-quantum hybrid KEMs default to the SHAKE256 KDF, per
// draft-ietf-hpke-pq. Note that callers construct HPKE with an
// explicit `kdf_id`, so this mapping is only a default.
KemAlgorithm::MlKem768P256 | KemAlgorithm::MlKem1024P384 => KdfAlgorithm::Shake256,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants