Switch from SECIO to Noise

[Stebalien]

This PR switches from the SECIO security transport, to Noise. This is a network braking change.

Users who still need SECIO, can re-enable it by passing the Security(secio.ID, secio.New) option.

Fixes #957.

Blocked on verifying interop with rust-libp2p (@tomaka?) and js-libp2p (@vasco-santos?).

[tomaka]

verifying interop with rust-libp2p (@tomaka?)

If you could point me to an IP address or a docker image with a node that supports noise, I can quickly test the interoperability.

[Stebalien]

If you could point me to an IP address or a docker image with a node that supports noise, I can quickly test the interoperability.

go-ipfs 0.6.0 supports noise. You should be able to use one of the bootstrappers (e.g., /ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ).

[tomaka]

Opened libp2p/rust-libp2p#1631

[jacobheun]

JS and Go are interoperable. I created a PR in libp2p/interop, libp2p/interop#41, to switch over to Noise only by default.

[tomaka]

See details in libp2p/rust-libp2p#1631, but in my opinion it would have been more correct to add a length prefix to the protobuf handshake.
This makes it possible to consider the Noise decoded stream of bytes as a stream, and not as a mix between a single handshake packet followed with a stream.

Considering that:

• It's in my opinion a better thing to do.
• rust-libp2p, Substrate and Polkadot use a length-prefixed protobuf message in production, and removing it would be quite painful.
• go-libp2p and js-libp2p could still modify the protocol without breaking the world.

Would it be acceptable to change the specs?

[Stebalien]

go-ipfs uses noise in production, in the latest release. However, we support multiple security transports so bumping the protocol version isn't a huge issue.

But, as far as I can tell, the handshake in go uses a length prefix as well. Does rust have two length prefixes?

[tomaka]

But, as far as I can tell, the handshake in go uses a length prefix as well. Does rust have two length prefixes?

The Noise packets (the ones that gets sent over the socket) are delimited using length prefixes.
Once decoded, these Noise packets form a stream of bytes. Within that decoded stream of bytes, Rust also puts a length prefix in front of the initial protobuf handshake message, otherwise we don't know where the protobuf message stops and the stream of data starts.

Without that second length prefix, you can no longer treat to decoded Noise packets purely as a stream. Instead, you have to assume that the first Noise packet contains the protobuf message and only the protobuf message, in its entirety. It's kind of a leak of one "OSI layer" to the next one.

[Stebalien]

The handshake is not a part of the rest of the stream as it has different security/privacy guarantees. I'm not sure if your method is actually safe (it might be, but it's important to treat handshake messages different).

[tomaka]

I don't want this question to just rot in the corner as usual.
Unless there's a comment saying either "yes we would consider changing the specs" or "person X will look into it in the near future", I'm going to treat the lack of response as "no, we're not going to change the specs".
Which is fine as well, I just do want an answer.

[Stebalien]

This question needs a decision from @raulk.

[raulk]

@tomaka we had plenty of community time to review the specs, and the rust-libp2p devs participated. It's quite unfortunate that this piece of feedback didn't make it sooner.

• go-libp2p and js-libp2p could still modify the protocol without breaking the world.
  Would it be acceptable to change the specs?

There are 6 other libraries that already have theoretically spec-compliant implementations, so it wouldn't be a trivial change.

---

Regarding the change itself, I don't think it's worth or desirable to pursue. The Protobuf serialisation format does not define/require/impose a length-prefixing strategy. For that reason, I don't think the analogy of "leakage between OSI layers" follows logic: Protobuf does not require us to encapsulate any headers locally that we are omitting/leaking out to the outer context in this case.

The Protobuf-encoded early messages we include in Noise handshake messages do NOT form a stream. They are discrete messages embedded in a concrete choreography of messages which is already length-delimited. They have no standalone bearing.

Think of it like writing a Protobuf to a file: you wouldn't length-prefix it because the filesize already tells you the length of the payload.

[raulk]

@tomaka That's to say, you really shouldn't process these embedded early messages as a generic stream. They are messages in the strict sense of the word. Users can't set them arbitrarily, they follow a very specific format, and the stage at which they are exchanged is strictly defined:

These payloads MUST be inserted into the first message of the handshake pattern that guarantees secrecy. In practice, this means that the initiator must not send a payload in their first message. Instead, the initiator will send its payload in message 3 (closing message), whereas the responder will send theirs in message 2 (their only message).

They are, to all effects, part of the handshake procedure only (and strictly separated from the subsequent cypher stream).

[tomaka]

Thanks for the answer.

We will need to come up with a plan to eventually fix that in rust-libp2p without breaking Polkadot.

[jacobheun]

This is no longer blocked, all implementations should now be interoperable with go-libp2p-noise.

[raulk]

LGTM, but tests are failing?

Community notice here: https://discuss.libp2p.io/t/notice-upstream-security-vulnerability-please-upgrade-go-and-go-libp2p-immediately/629 (thanks @jacobheun!)

[Stebalien]