p2p/conn: simplify secret connection handshake malleability fix with merlin #4185
Conversation
Introduces new dependencies on github.com/gtank/merlin and sha3 as a cryptographic primitive This also only uses the transcript hash as a MAC.
tessr
changed the title
tessr
changed the title
Codecov Report
@@ Coverage Diff @@
## master #4185 +/- ##
==========================================
+ Coverage 67.43% 67.56% +0.13%
==========================================
Files 221 221
Lines 18666 18658 -8
==========================================
+ Hits 12587 12607 +20
+ Misses 5125 5102 -23
+ Partials 954 949 -5
|
|
Nice. I'm going to merge this, but I'd also love to read more about Merlin vs. HKDF, and it could be a useful thing to include in the comments of this PR. @zmanian do you have any resources handy? (I personally have some understanding of Merlin--thanks Henry!--but don't know anything about HKDF, and I imagine others would find it useful too.) |
|
dev session? |
|
That would be fun. Maybe we could get Henry or George to come, too. |
|
Transcript hashing and exchanging a transcript derived authenticator Merlin is kind of the ideal transcript hashing API. It's safe accumulate and extract state in the function. TLS 1.3 uses a strategy of concatenating all messages together. Noise uses HKDF to derive both encryption and MAC keys from the transcript. IMO Merlin is the nicest and easiest to understand API for doing thing. Our needs are basically like TLS 1.3. We don't really need to ability to accumulate and extract so we do have some downsiders to not doing the TLS thing. What I like about Merlin is that it's really unlikely that I made a mistake that would introduce malleability. One downside of this is that it would make alternative language implementations harder. Merlin is available in go and rust. And likely c soonish. |
|
@zmanian can we get this set of changes written up in an RFC in https://github.com/tendermint/spec so we have a permanent place for the rationale to live. A bit backwards this time since the changes have already been merged, but we should follow the process. Also need to update the specs for this in that repo. Thanks! |
|
Yes will do. |
This simplifies transcript hashing using the merlin protocol rather than HKDF.
It's a cleaner API
This also only uses the transcript hash as a MAC. Fixes #4140 as well.