noise-libp2p: introduce "handshake seal"; more. #234
Conversation
The current construction is vulnerable to replay attacks by a MITM agent that records previous handshakes, and transplants old user data from those handshakes into new ones, as long as the static key hasn't changed. We fix this vulnerability by leveraging Noise channel binding, in order to "seal" the handshake and assert that the exchanged data was witnessed equally by both parties. This commit also simplifies protobuf field naming. Finally, we formalise in which Noise messages of IK and XX the message payload is to be shared, to guarantee secrecy, integrity and authentication.
raulk
requested review from
romanb,
yusefnapora,
tomaka,
AgeManning and
shahankhatch
|
@yusefnapora awesome! Let's wait for a few more approvals. @burges -- would appreciate thoughts, if any. |
| efficiency and enhanced security purposes, we choose to adopt the [channel | ||
| binding][npf-channel-binding] technique defined in the Noise Protocol Framework. | ||
|
|
||
| The Noise Protocol state machine tracks the handshake hash throughout the entire |
There was a problem hiding this comment.
Can you be more precise which data should be hashed? Also, do you want to specify this to be SHA256?
There was a problem hiding this comment.
The handshake hash is defined in the Noise spec: https://noiseprotocol.org/noise.html#overview-of-handshake-state-machine. The noise library you use should be computing the hash -- you shouldn't need to do it in the noise-libp2p implementation.
|
🚃🚃🚃🚃🚃 Merge train leaving tomorrow morning. Please get your remarks in today! |
|
Could someone elaborate (e.g. with concrete examples) on
as well as
Given that all "early data" is sent as part of encrypted handshake payloads using an AEAD scheme and the current handshake hash I'm probably missing the obvious, but I currently don't see what the signing of the handshake hash is supposed to accomplish on top of signing the static DH public key as far as authentication is concerned. I mean, I can understand why one would rather want to sign the handshake hash instead of the static DH public key (e.g. to prevent signature replays when a static DH private key is compromised, allowing impersonating the owner of the private identity key) but I fail to see why you would want to do both. If someone could enlighten me with more details on the attack vectors addressed here and why both, signing the static DH key as well as the handshake hash is necessary, I would be very grateful. |
|
@romanb I think you're right. I missed the fact that For some reason, I thought only the keys were mixed in, but I was wrong. Consequently, the AD (authentication data) already includes the payload ciphertexts we've witnessed, and by introducing this handshake seal we are not proposing any extra guarantee that is not already awarded by the AEAD scheme that encompasses the payloads. If an intermediary constructs a transplant or tamper attack, I'm confident it'll be caught by an authentication tag mismatch. I'm going to retract this RFC, and open a new PR with the other changes. |
This PR supersedes #210, and implements the outcome of that discussion.
The current construction is vulnerable to replay attacks by a MITM agent that records previous handshakes, and transplants old user data from those handshakes into new ones, as long as the static key hasn't changed.
We fix this vulnerability by leveraging Noise channel binding, in order to "seal" the handshake and assert that the exchanged data was witnessed equally by both parties.
This commit also simplifies protobuf field naming.
Finally, we formalise in which Noise messages of IK and XX the message payload is to be shared, to guarantee secrecy, integrity and authentication.
ping: @shahankhatch @Mikerah @djrtwo @dryajov @mpetrunic @morrigan @araskachoi -- please review too (can't assign you as a reviewer as you're not part of the libp2p org yet).