Conversation
The secp256r1 ECIES variant computed its HMAC-SHA512 over the ciphertext plus a fixed 8-byte constant but NOT over the 16-byte IV, which is appended to the message and used for AES-256-CBC decryption. Since CBC yields P0 = D(C0) XOR IV, an attacker able to modify a stored/transmitted blob could XOR a chosen delta into the unauthenticated IV to flip the first plaintext block while the MAC still validated - a targeted first-block integrity break. The secp256k1 and X25519 variants already authenticate the IV. Include the IV in the HMAC input on both encrypt and decrypt. Note: this changes the MAC input, so it is not wire-compatible with blobs produced by the previous code (or the referenced crypto-ecies-js format that omits the IV from the MAC). If cross-implementation compatibility must be preserved, coordinate the same change on the other side. Co-authored-by: Ty Schenk <schenkty@users.noreply.github.com>
larseidsvoll
left a comment
larseidsvoll
left a comment
There was a problem hiding this comment.
Crypto fix correct: MAC now covers IV (CBC first-block malleability closed). REQUEST_CHANGES: deliberate wire break vs crypto-ecies-js — need explicit interop decision (coordinate JS, versioned format, or documented break) before Ready. Draft + Ty lock — no merge.
|
Security look @ No HIGH+. Fix is correct (IV must be in HMAC for AES-CBC; matches secp256k1/X25519). Underlying issue is MEDIUM (first-block malleability if blob is attacker-modifiable). Go-hold justified. Wire MAC change breaks |
|
Acknowledging REQUEST_CHANGES / HOLD. Crypto fix (IV in HMAC) stays correct. This PR remains draft until Ty/Elrond pick an interop path: coordinated |
3 participants
HELD — CTO lock (do not undraft / merge)
Cryptographically correct (authenticate IV in HMAC, matching secp256k1 / X25519), but a breaking ECIES-secp256r1 wire change vs old node-rs and
crypto-ecies-js.Decision: HOLD until a coordinated
crypto-ecies-jschange and a versioned format (new alg id; legacy decrypt-only). No silent wire break.Problem
Unauthenticated IV under AES-CBC lets an attacker flip bits in the first plaintext block while the MAC still validates.
Fix on this branch
Include the IV in the HMAC on encrypt and decrypt (
mac.update(iv)before ciphertext).Tests
secp256r1_rejects_iv_tamperingplus existing ECIES round-trips.Fixes nothing until interop plan ships; leave draft.