libp2p / specs

Technical specifications for the libp2p networking stack
https://libp2p.io
1.58k stars 275 forks source link

discussion: should we remove Noise Pipes from the noise-libp2p spec? #249

Closed yusefnapora closed 3 years ago

yusefnapora commented 4 years ago

Context: #246 is currently blocking a correct implementation of Noise Pipes.

The issue is resolvable, but @Stebalien and I had a sync chat just now and are wondering if Noise Pipes is worth the cost in terms of complexity.

In our design for Noise Pipes, the IK handshake is 1-RTT, not 0-RTT. While we can send early data in the handshake payload, we can't really trust that data until the handshake completes. Since we're not planning to send arbitrary application data in the early payload, we have to wait for the handshake to complete before handing the conn off to the app layer.

From a client's perspective, the 0.5 RTT difference isn't meaningful. If a client does an IK handshake, they need to receive one handshake message from the server before they send their first transport message.

With XX they still only need to receive one message from the server, because the client can send their final XX handshake message immediately followed by transport messages containing their actual data.

Since in the majority of cases, it's the client who wants to send the first data after connection establishment, most connections won't benefit from using IK in practice.

Given that the benefits aren't that amazing, it's worth considering if we should shelve Noise Pipes, and just use XX as our only handshake. Doing so would remove a lot of complexity from the spec and implementations, and we could always revisit in a new version if we decide that the extra 0.5 RTT is worth it after all.

noise interest group: @raulk, @tomaka, @romanb, @shahankhatch, @Mikerah, @djrtwo, @dryajov, @mpetrunic, @AgeManning, @morrigan, @araskachoi, @mhchia

raulk commented 4 years ago

I'll rephrase and provide some perspective to check if I've understood correctly.


Conclusion: IK makes sense if we're sending arbitrary application data we want the other party to be able to process IMMEDIATELY, and even respond to in their handshake response. Since that's not the case here -- as the client needs to intersect its capabilities with the responder's in all cases -- we can simplify the handshake if we assume the pipelining is always available and supported, such that the client can always append its muxer selection + first stream open + first app message to the third and and final message of XX.

romanb commented 4 years ago

I'm in favour of removing it, at least for the time being. I've stated similar reasons for why rust-libp2p does not (and likely will not for some time) support Noise Pipes and XXfallback anyway here. I just don't think it is pulling its weight and simpler specifications are always beneficial because they are much more likely to be fully (and correctly) implemented by many parties.

raulk commented 4 years ago

@yusefnapora on reading the spec, it's not clear to me which part is optional: IK or Noise Pipes? Regardless, the spec doesn't propose a good strategy to deal with this optionality.

yusefnapora commented 4 years ago

@raulk the way the spec is currently written, if you don't support Noise Pipes, you also don't support IK.

The idea is that the non-Pipes peer always does XX. If someone sends them an IK message, they just treat it like XX and send an XX response back. The other peer will try to decrypt the response as IK, but if that doesn't work it will "fallback" to XX. Unfortunately, that's not quite right according to the Noise framework spec (see #246).

Anyway, the worst-case scenario in the current spec is always a 1.5-RTT handshake. We never terminate the connection or start over from scratch; the initiator's first message is always used.

If we do remove Noise Pipes, then we would get rid of IK entirely and just always use XX.

Stebalien commented 4 years ago

if we assume the pipelining is always available and supported, such that the client can always append its muxer selection + first stream open + first app message to the third and and final message of XX.

In terms of RTTs, even if pipelining (sending the muxer selection, etc., along with the final handshake message) isn't supported, the initiator should be able to send another message without waiting yet another round trip, right?

raulk commented 4 years ago

@Stebalien yep 👍


I posted my analysis of the practical issues that motivated us to reconsider Noise Pipes support, to begin with.

Now, as for the decision, I am in favour of simplifying the first version of this handshake by making it support only XX.

Reasons:

  1. We declared XX as compulsory and Noise Pipes as optional. As a result, other implementations (Python, Nim, js, etc.) have prioritised XX support and some have decided to forgo Noise Pipes. Faced with the dilemma of choosing one or another (cannot have both), this reason alone is enough for choosing XX for this first version.
  2. The technical discussion above has convinced me. In material terms, we won't see compelling gains from IK given how we use it in libp2p (and the latest conversations around ms2.0 and connection bootstrapping).
  3. Once we have secure channels encoded in the multiaddr, it'll be trivial to resolve the "what does the other node support?" question. We can introduce multicodecs for noise-xx (current) and noise-pipes (future). We'll still need to figure out how to distinguish when each is used, but let's cross that bridge when we get to it.
yusefnapora commented 4 years ago

@raulk sound good to me - I'll make a PR to go-libp2p-noise to remove Pipes and update the spec.

mxinden commented 3 years ago

I am closing here since https://github.com/libp2p/specs/pull/260 removed noise pipes and the IK handshake from the specification.