Repository navigation
kem/hybrid: derive the same key pair from a seed every time - #699
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| for ctr := 0; ctr < 256; ctr++ { | ||
| _, _ = h.Read(candidate) | ||
| candidate[0] &= bitmask | ||
| privKey, err := sch.curve.NewPrivateKey(candidate) | ||
| if err != nil { | ||
| // Out of range or zero; draw the next candidate. | ||
| continue | ||
| } | ||
| sk := cPrivateKey{scheme: sch, key: privKey} | ||
| pk := cPublicKey{scheme: sch, key: privKey.PublicKey()} | ||
| return &pk, &sk |
There was a problem hiding this comment.
🟨 Key derivation leaks rejection timing
DeriveKeyPair exits after the first valid secret-derived scalar. Its branch and runtime expose how many candidates were rejected, violating the constant-time policy.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is the sampling loop RFC 9180 section 7.1.3 specifies, and the one hpke's shortKEM.DeriveKeyPair already runs on these curves. A candidate is rejected only when it falls outside [1, n-1], with probability about 2^-32 for P-256 and P-384 and far smaller for P-521, and the rejected bytes are discarded, so the count reveals nothing about the accepted scalar.
| pk := cPublicKey{scheme: sch, key: pubKey} | ||
|
|
||
| return &pk, &sk | ||
| // crypto/ecdh's GenerateKey may read an extra byte from the reader |
There was a problem hiding this comment.
More precisely: ecdh GenerateKey sometimes reads an extra byte on purpose to break exactly what we tried to do here.
|
Adjust the comment and this is good to merge. |
|
Adjusted the comment. |
cScheme.DeriveKeyPairread its seed throughcrypto/ecdh'sGenerateKey, which callsrandutil.MaybeReadByteand so consumes an extra byte of the reader on a random subset of calls. One seed produced different key pairs, andEncapsulateDeterministicallyinherited it. The P-256 hybrid KEMs are built on this scheme.The scalar is now sampled from the SHAKE stream directly and handed to
NewPrivateKey, with the top byte masked for P-521 and out-of-range candidates redrawn.Two tests derive from one seed repeatedly; on main they fail within a few derivations.