Repository navigation
hpke: set the scheme on hybrid KEM keys - #706
Open
stanleys12 wants to merge 1 commit into
Open
stanleys12 wants to merge 1 commit into
stanleys12 wants to merge 1 commit into
Conversation
DeriveKeyPair and the two unmarshalers left the scheme field of hybridKEMPrivKey and hybridKEMPubKey nil, so Scheme() returned a nil kem.Scheme and calling any method on it panicked. Co-Authored-By: Claude Opus 5 <[email protected]>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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
1 participant
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.
hybridKEMPrivKeyandhybridKEMPubKeyboth declare ascheme kem.Schemefield that theirScheme()methods return, but none of the three places that build those structs ever assigned it:DeriveKeyPair,UnmarshalBinaryPrivateKeyandUnmarshalBinaryPublicKeyall used struct literals without ascheme:entry. The field was therefore always a nil interface, and(*hybridKEMPrivKey).Public()propagated the same nil value into the public key it returns.The practical effect is that
Scheme()returns nil for every key produced byKEM_X25519_KYBER768_DRAFT00, and calling any method on that result panics with a nil pointer dereference.Scheme()is part of thekem.PublicKey/kem.PrivateKeyinterface contract documented inkem/kem.go, so algorithm-agnostic code written against those interfaces hits this on ordinary use.Changes
hpke/hybridkem.go: setscheme: hat the three construction sites, the same wayxkem.goandshortkem.gopopulate the field on their key types.Public()needs no change since it already forwardsk.scheme.hpke/kem_test.go: addTestKemKeysScheme, which assertsScheme().Name()matches the originating scheme for the private key, the public key, the key returned byPublic(), and both keys after aMarshalBinary/Unmarshalround trip. The KEM id list is hoisted into a package-levelkemIDsvar so both tests share it, and the generate-key-pair setup is factored intogenKeyPair.The list stays the same six KEMs
TestKemKeysExactLengthalready covered;KEM_XWINGis left out because it was not in that list and its keys come fromkem/xwing, which already sets the field.Testing
TestKemKeysSchemepanics onX25519_KYBER768before the fix and passes after.go test ./...passes for the whole repository.AI assistance (Claude) was used to draft this change;
go test ./hpke/...was run locally with and without the fix.