Add SHAKE-128 KDF support to HPKE implementation#14429
Add SHAKE-128 KDF support to HPKE implementation#14429
Conversation
|
we're going to need some test vectors for these to ensure correctness, in the same way we have them for the other algorithms. (Unfortunately I don't think those HPKE vectors include SHAKE). I'd check what Go uses here. |
|
I've added it. Let me know if this looks good. |
alex
left a comment
There was a problem hiding this comment.
I'm surprised this needed to be so invasive, will have to review more closely, but if you can also look if there's a cleaner abstraction, that'd be good.
docs/development/test-vectors.rst
Outdated
| (AES-128-GCM, AES-256-GCM, ChaCha20Poly1305). | ||
| * HPKE SHAKE-128 (X25519, mode Base) vectors generated with Go primitives | ||
| using the one-stage HPKE key schedule defined in | ||
| `Go's crypto/hpke package`_. |
There was a problem hiding this comment.
I think you can use:
https://github.com/hpkewg/hpke-pq/blob/main/test-vectors.json
which I found in the go repo
There was a problem hiding this comment.
I'm a bit confused, you seem to have deleted the docs, but not switched to these vectors at all.
There was a problem hiding this comment.
Thanks for the heads-up. The SHAKE vectors are wired in code via vectors/cryptography_vectors/HPKE/go-shake128-vectors.json in test_hpke.py (test_shake128_vector_decryption). I also re-added a concise docs note for the SHAKE-128 vector source in docs/development/test-vectors.rst pointing to hpke-pq test vectors, so both implementation and docs now match.
There was a problem hiding this comment.
The vectors checked in do not actually match the file from the pq repo.
You need to slow down and be more thoughtful in what you're doing. If something is unclear, ask a clarifying question, but right now I'm spending a ton of time pointing out very basic issues.
There was a problem hiding this comment.
Sorry, just fixed again.
docs/development/test-vectors.rst
Outdated
| (X25519, X448, P-256, P-521), KDFs (HKDF-SHA256, HKDF-SHA512), and AEADs | ||
| (AES-128-GCM, AES-256-GCM, ChaCha20Poly1305). | ||
| * HPKE SHAKE-128 (X25519, Base mode) vectors are checked into | ||
| `go-shake128-vectors.json`_. |
There was a problem hiding this comment.
I do not understand what you are doing here.
I've said we want to use the vectors from https://github.com/hpkewg/hpke-pq/blob/main/test-vectors.json
I don't understand why you've got back and forth on this numerous times, and the link to the vectors should go to where they're from, not where in the repo they live.
There was a problem hiding this comment.
You are right, and I apologize for the back-and-forth. I removed the incorrect docs attribution.
I checked hpkewg/hpke-pq/test-vectors.json directly: there is no kem_id=0x0020 (X25519), kdf_id=0x0010 (SHAKE128) vector in that file. The SHAKE128 vector there is kem_id=0x0010 (DHKEM(P-256)).
This branch currently only supports KEM.X25519, so I cannot switch the SHAKE128 decrypt-vector test to that upstream entry without adding P-256 KEM support first.
Could you confirm the preferred path?
- wait for Add DHKEM(P-256, HKDF-SHA256) support to HPKE implementation #14398 to land and then switch this PR to the pq SHAKE128 vector, or
- keep this PR as SHAKE-only and drop the custom SHAKE decrypt vector test until P-256 support is available.
There was a problem hiding this comment.
Yes, waiting for DHKEM to land first and use the official vectors is preferred.
Sending a seperate PR that lands just the vector file, properly documented, would help keep this moving.
There was a problem hiding this comment.
That makes sense. I’ll do this in two steps:
- keep this SHAKE PR without the custom vector fixture, and
- open a separate PR that only adds the official
hpkewg/hpke-pqvector file with proper source documentation.
Then once DHKEM support lands, I’ll switch the SHAKE vector test over to those official vectors.
There was a problem hiding this comment.
Add official HPKE-PQ test vectors file and source documentation#14439
Thanks for your quick approval and merge.
|
Hi @alex Do you have any feedback? |
|
I was going to pause on review until the dependency is merged.
…On Mon, Mar 9, 2026 at 4:34 PM bitloi ***@***.***> wrote:
*bitloi* left a comment (pyca/cryptography#14429)
<#14429 (comment)>
Hi @alex <https://github.com/alex> Do you have any feedback?
—
Reply to this email directly, view it on GitHub
<#14429 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBF5QVRRIJVH7SXYEXT4P4TETAVCNFSM6AAAAACWIPGUNKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMRWG4ZTCMBXHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
All that is necessary for evil to succeed is for good people to do nothing.
|
Summary
This PR adds SHAKE-128 support as an HPKE KDF and keeps the change scoped to a single algorithm for easier review.
Fixes #14394
Changes
KDF.SHAKE128in the Rust HPKE backend and Python stub bindings.Suite::key_schedule.LabeledDeriveinput construction to match the spec-required encoding.KDF.SHAKE128option.infoWhy this scope
#14394tracks multiple remaining algorithms; this PR addresses only SHAKE-128.Validation
cargo fmt --manifest-path src/rust/Cargo.toml --checkuv run --extra pep8test -m ruff check src/cryptography/hazmat/bindings/_rust/openssl/hpke.pyi tests/hazmat/primitives/test_hpke.pyuv run --extra test -m ensurepipuv run --extra test -m pip install -e .PYTHONPATH=vectors uv run --extra test -m pytest tests/hazmat/primitives/test_hpke.py -q(43 passed)uv run --extra docs --extra docstest -m sphinx -T -W -b html -d /tmp/cryptography-docs-doctrees docs docs/_build/htmlChecklist