Repository navigation
Hold HPKE key and context buffers out of line - #3637
Draft
WillChilds-Klein wants to merge 4 commits into
Draft
WillChilds-Klein wants to merge 4 commits into
WillChilds-Klein wants to merge 4 commits into
Conversation
EVP_HPKE_KEY and EVP_HPKE_CTX are public, stack-allocatable types, so their sizes are ABI. Adding the ML-KEM KEMs grew EVP_HPKE_KEY from 72 to 4808 bytes, which overflows the object a caller built against an earlier header reserves. Sizing every buffer per algorithm and holding it behind a pointer makes both sizes independent of the algorithms on offer.
Contributor
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3637 +/- ##
==========================================
+ Coverage 78.46% 78.69% +0.22%
==========================================
Files 705 705
Lines 130645 130738 +93
Branches 17912 17925 +13
==========================================
+ Hits 102516 102889 +373
+ Misses 27150 26869 -281
- Partials 979 980 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The bounds were hand-picked, so they pin nothing a reviewer would not already see in the header diff.
This reverts commit f0a06de.
Its layout is unchanged between FIPS 5.x and main, so reshaping it here would introduce an ABI break rather than repair one. Only EVP_HPKE_KEY grew when the ML-KEM KEMs landed.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
EVP_HPKE_KEYis a public type the header invites callers to stack-allocate, so its size is ABI.EVP_HPKE_CTXis left alone. Its layout is identical on FIPS 5.x andmain, so reshaping it would add an ABI break rather than repair one.Call-outs
ABI_VERSIONbump is a release-level decision I have not made here. The same question was raised in Add ML-KEM support to HPKE (draft-ietf-hpke-pq-05) #3277 and left open.public_keyfield. Landing a KEM that large while the key material is still inline growsEVP_HPKE_KEYagain, and the overflow with it.main. No release branch or FIPS snapshot carries it, so no shipped artifact is affected.ScopedEVP_HPKE_KEY, force libssl's ECH config onto the heap, and diverge from upstream.Testing / verification
Sizes, measured with
sizeofagainst each tree's own headers:EVP_HPKE_KEYEVP_HPKE_CTXfips-2026-06-26-snapshot)maintodaymain+ this PRWhat a caller built against the FIPS 5.x header does when the library underneath it changes, which is the upgrade-without-rebuild case. The caller uses the public API only, and reserves the 5.x sizes:
malloc(sizeof)main, without this PRmain, with this PREvery case that does not crash completes a correct HPKE round trip. Plain X25519 is enough to trigger the overflow, because
public_keymoved to the offset one byte past the old object's end.abidiff, same flags the ABI Monitoring jobs use:maintoday (i.e. #3277)evp_hpke_key_st72 -> 4808main+ this PRevp_hpke_key_st72 -> 32maintoday ->main+ this PRevp_hpke_key_st4808 -> 32evp_hpke_key_must_not_hold_inline_buffers, confirming the guard bites.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.