-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add a multiselect 2.0 spec #227
base: master
Are you sure you want to change the base?
Conversation
|
||
Note that this negotiation scheme allows peers to negotiate a "monoplexed" connection, i.e. a connection that doesn't use any stream multiplexer. Endpoints can offer support for monoplexed connections by offering the `/monoplex` stream multiplexer. | ||
|
||
**TODO**: Do we need to define a way to send an error code / error string? Or do we have something like that in libp2p already? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to be able to send an error message if, e.g., the responder doesn't support any of the initiator's multiplexers. Perhaps that would be a potential use-case for a Reject
message, even in the streaming case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we have any libp2p-wide error codes. I think they should be protocol specific.
connections/multiselect2.md
Outdated
|
||
#### 0-RTT | ||
|
||
When using 0-RTT session resumption as offered by TLS 1.3 and some variants of Noise (**TODO**: specify which), the endpoints MUST remember the negotiated stream multiplexer used on the original connection. This ensures that the client can send application data in the first flight when resuming a connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Noise IK
handshake, which is part of the "Noise Pipes" pattern described in the spec, supports 0-RTT encryption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take into account that multiplexers can change across restarts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raulk That's on us to define. We could say that if you resume a connection, you have to use the same multiplexer. This would save us a roundtrip for negotiation the stream multiplexer in the common case.
In the rare case when a peer dropped support for a muxer between two connections, you would just reject 0-RTT and fall back to the 1-RTT handshake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to sanely account for simultaneous open, as it is a hard requirement for achieving TCP hole punching.
We can't possibly deploy a protocol that doesn't allow us to deal with this very important issue.
Note that multiselect-1.0 can handle simultaneous open with a simple protocol extension; see #196. |
@vyzo If I understand #196 correctly, it works by adding (yet) one more round trip to multistream/1.0. I'd like to avoid adding round trips to multiselect 2.0 in the common case (where no simultaneous open is needed). What do you think of this strawman proposal: If a handshake fails due to an unexpected message error (e.g. in the case of TLS, when the client receives a ClientHello in response to a ClientHello), both peers clear their handshake state and start over again. The roles are determined by the following rule: Both peers calculate the hash of the message(s) they sent and they received during the last handshake attempt. The peer that sent the message that hashes to the smaller value will then act as a client, the other one as the server in the subsequent handshake. A protocol along those lines would make sure that peers that don't use simultaneous connect don't suffer any round-trip penalty, while peers that do simultaneous connect would pay the cost of a single additional round-trip. |
#196 does not add a round-trip to the protocol; it is pipelined together with the protocol selection. |
@vyzo The crypto protocol selection, which we're getting rid of here. This negotiation made (makes) us vulnerable to a trivial MITM attack, which was one the motivations to develop a new version of this protocol that fixes this vulnerability (not to mention that removing it also speeds up the handshake by one round-trip). |
@marten-seemann the strawman proposal is a hack, the TLS handshake can fail for all sorts of reasons, and the "unexpected message error" is not necessarily indicative of an unexpected "ClientHello". Why can't we add an |
@vyzo The problem with a
Regarding the TLS handshake,
I wouldn't use a loaded work like "hack" for this. It's a way to solve the problem that seems consistent with our design goals. You could easily wrap a TLS connection and parse the first byte of the first incoming message, if you're uncomfortable with relying of the error message returned by the TLS stack. That would tell you if the message you received was a ClientHello or a ServerHello. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is great for the streaming case. In my opinion, it accomplishes the most important goals of #95, the original spec. As you mentioned in our chat the other day, it doesn't really cover the packet-oriented case at all, and it feels as though it'd be worth establishing a separate or--as I previously referred to it--an extension protocol for the packet case.
I think it's worth having different message types, likely defined in a separate proto file within the same directory, for that case. The differences that come to mind include:
- Sender and receiver both need to establish dynamic IDs for protocols.
- The protocol needs explicit reject messages. In my opinion this includes a "reject protocol" message as well as a potential "reject transport" message, since "closing" isn't a concept that is guaranteed to exist.
- All protocol messages should support the optional inclusion of a payload. You made a great point that this could simply follow the var-int delimited protobuf message in the packet.
I'd like us to get another draft going that begins to tackle this. Perhaps we could have a secondary PR targeting this branch?
connections/multiselect2.md
Outdated
|
||
### Handshake Protocol Selection | ||
|
||
Handshake protocols are not being negotiated, but are announced in the peers' multiaddrs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably mention that they could be negotiated in some corner case, but that the general case will be to assume a secure transport by the time multiselect is initiated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is that corner case you're referring to? I'm not sure if we should even build that flexibility into our stack, even if we could come up with a situation where a negotiation wouldn't be susceptible to a MITM attack. If it's not there, it can't be exploited.
|
||
Handshake protocols are not being negotiated, but are announced in the peers' multiaddrs. | ||
|
||
**TODO**: Do we need to describe the format here? I guess we don't, but we will probably need another document for that change, and we can link to it from here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a link to some documentation on this would be nice! I don't believe it exists at the moment, so may be worth keeping this here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need an extension to the multiaddr spec + multicodec table to add secure channels as an atom. Also, @yusefnapora was writing this document to specify the semantics of multiaddrs: #191.
|
||
Note that this negotiation scheme allows peers to negotiate a "monoplexed" connection, i.e. a connection that doesn't use any stream multiplexer. Endpoints can offer support for monoplexed connections by offering the `/monoplex` stream multiplexer. | ||
|
||
**TODO**: Do we need to define a way to send an error code / error string? Or do we have something like that in libp2p already? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we have any libp2p-wide error codes. I think they should be protocol specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial responses to the current conversation.
connections/multiselect2.md
Outdated
|
||
#### 0-RTT | ||
|
||
When using 0-RTT session resumption as offered by TLS 1.3 and some variants of Noise (**TODO**: specify which), the endpoints MUST remember the negotiated stream multiplexer used on the original connection. This ensures that the client can send application data in the first flight when resuming a connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take into account that multiplexers can change across restarts.
I just rewrote the Simultaneous Open section and specified how the hashing of handshake messages works to determine the roles in a subsequent handshake attempt. PTAL. |
connections/multiselect2.md
Outdated
|
||
Since secio doesn't provide this property, secio cannot be used with Multiselect 2.0. | ||
To determine the roles in the second handshake attempt, endpoints calculate the SHA-256 hash of the handshake messages that were sent and received (including any error message(s) that the handshake protocol might have sent) during the failed handshake attempt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these handshake messages available to the application? It seems to me that these will be internal to the TLS implementation and not actually available to hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just wrap the Read
and Write
of the net.Conn
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true in go, but what about other languages?
Also, does the "wrap the Read/Write" imply that we hash all the bytes sent/received to make the decision?
It's still now clear how an implementor would implement this part of the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the bytes sent in the clear over the wire. I'm not familiar with the TLS / Noise implementations in other languages, but I'd be surprised if this posed a major challenge in any language.
@tomaka, @yusefnapora Can you shed some light on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any language that uses a TLS library that reads/writes to the file descriptor will have problems with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an additional detail we need to specify: When receiving the unexpected handshake message, TLS does two things:
- It sends a TLS alert to the peer, and
- it aborts the handshake and returns an error message.
If we just restart the handshake when the handshake message is returned, the peer's alert will still be in flight, and typically will be received after we initiated the second handshake, leading to a handshake failure. There are three ways we can solve this:
- Modify the TLS stack to not send the alert,
- Filter out the alert before it reaches the TLS stack
- Wait for the alert to arrive, and then start the second connection attempt afterwards.
I don't think 1. is very practical, since it requires us to modify the TLS stack. 2. would be doable, but requires wrapping of the connection. 3. costs us half a roundtrip, but could be implemented without modifications to the TLS stack or wrapping of the connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: I just implemented option 3 in https://github.com/libp2p/go-libp2p-tls/compare/tcp-sim-open-option3, and unfortunately, it's inherently racy: When tls.Conn().Handshake()
returns, we have no idea if this is before or after the TLS alert from the peer was received. We therefore can't reliably catch the TLS alert afterwards, since it might never arrive.
I think there's an option 4:
- Filter out the
ClientHello
before it arrives at the TLS stack. If we receive one, abort the current handshake (taking care that no abort alert is sent out), and start anew.
Unfortunately, this option requires wrapping of the TLS connection, but at least it should be race-free. Furthermore, it doesn't cost us any additional (half) round-trips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an implementation of option 4, which seems to work: libp2p/go-libp2p-tls#38.
The downside is that we're duplicating a bit of the record layer parsing code of TLS, since we need to make sure that we read the whole ClientHello
from the connection before we can start the second connection attempt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The concern here is again other language implementations and whether they can do that in reasonable fashion.
Other than that, I am fine with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't verified this at all, so we shouldn't assume this is correct... but the docs for node's tls.connect() show that you can pass in any duplex Stream as the underlying socket which will be used for the connection.
So it seems like we should be able to wrap a net.Socket
with some code similar to @marten-seemann's go code which parses the first handshake message and aborts with a custom error if the server sends a ClientHello
. Then we can do the peer id comparison and retry the connection when we get the special error type.
This won't work in browsers, but I don't think we have a clear path to supporting TLS 1.3 at all there, and we don't have access to raw TCP in browsers anyway, so it's kind of a moot point.
To determine the roles in the second handshake attempt, endpoints calculate the SHA-256 hash of the handshake messages that were sent and received (including any error message(s) that the handshake protocol might have sent) during the failed handshake attempt. | ||
The peer that sent the messages resulting in the numerically smaller hash value acts a client in the second handshake attempt, the peer that sent the messages resulting in the numerically larger hash value acts as a server. | ||
|
||
Since secio assign roles during the handshake, it is not possible to detect a Simultaneous Open in this case. Therefore, secio MUST NOT be used with Multiselect 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, this is not quite right; we need something for secio too, unless we are actively deprecating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which part of it is not right?
We've been talking about deprecating it for a long time, and the main reason we've been sticking to it is for backwards compatibility. Since Multiselect 2.0 is not backwards compatible anyways, this would be a good opportunity to finally phase it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the push towards deprecation, as seems to be the growing consensus. Multiselect 2.0 presents a convenient opportunity to upgrade our secure channels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no path for backwards compatibility here with multistream 1 correct? Pairing the secio deprecation with multiselect 2 is going to put a lot of stress on limited environments (browsers) to upgrade, or segregate them from the rest of the network. I don't think it warrants blocking the spec, especially if providing that compatibility hampers the performance/feature gains, but we should be cognizant and clear of the network rollout time table and its impact on the network.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secio is actively being deprecated and removed from the network, so this no longer needs to be a consideration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, even Noise needs an initiator and a responder. So, even after deprecating SecIO, we still need to assign roles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to update the document and remove any mention of secio, provided there's interest in moving forward with this document. It's been quiet for long time...
To determine the roles in the second handshake attempt, endpoints calculate the SHA-256 hash of the handshake messages that were sent and received (including any error message(s) that the handshake protocol might have sent) during the failed handshake attempt. | ||
The peer that sent the messages resulting in the numerically smaller hash value acts a client in the second handshake attempt, the peer that sent the messages resulting in the numerically larger hash value acts as a server. | ||
|
||
Since secio assign roles during the handshake, it is not possible to detect a Simultaneous Open in this case. Therefore, secio MUST NOT be used with Multiselect 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the push towards deprecation, as seems to be the growing consensus. Multiselect 2.0 presents a convenient opportunity to upgrade our secure channels.
|
||
TLS as well as Noise will fail the handshake if both endpoints act as clients. In case of such a handshake failure, the two endpoints need to restart the handshake. Endpoints MUST NOT close the underlying TCP connection in this case. Implementations SHOULD specifically test for this type of handshake failure, and not treat any handshake failure as a potential Simultaneous Open. | ||
|
||
To determine the roles in the second handshake attempt, endpoints compare the SHA-256 hashes of their peer IDs. The peer with the numerically smaller hash value acts as a client in the second handshake attempt, the peer with the numerically larger hash value acts as a server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forces us to transfer our peer ID in the initial handshake message, right?
While this is normally good, it also means we can't for example fool proxies by pretending that our traffic in regular HTTP3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling TCP simultaneous connections seems a bit off-topic to me for multistream-select, and could be specific to each transport and/or encryption protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you dial a peer, you already know it’s peer ID. And of course you also know your own peer ID, so we don’t have to send anything extra on the wire.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomaka I tend to agree with this being slightly off-topic. The reason it's in here is because ms-2.0 also changes the connection bootstrapping, and #196 won't work any more (at least not if we want to be able to mask our traffic).
What do you think of saying in this document
Secure Channels MUST define how to handle TCP simultaneous open, if they can be used over TCP.
and then moving the text here to the TLS document?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you dial a peer, you already know it’s peer ID. And of course you also know your own peer ID, so we don’t have to send anything extra on the wire.
I may be missing something, but how does the responder learn the peer ID without a hard requirement for the crypto handshake to transmit it on the first message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a simultaneous dial, you have both peer IDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this layer, you only know you have established a connection and have exchanged some bytes. You think you know the other side’s peer ID, but it’s not authenticated. After exchanging those initial bytes, you notice the other party sends you a message you didn’t expect. Does each party work on their assumption of the peer ID of the other party? That leaves the system open to a series of attacks where a peer advertises a very large/small peer ID for itself, then responds to all SYN packets with another SYN + the initiator message, to force a conflict resolution pathway it knows it’ll always win. I much rather introduce a vector of randomness per-session.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raulk What's the attack here? Sure, an attacker can go through the hassle you're describing to make sure it ends up in the client role (from the viewpoint of the cryptographic handshake). During the cryptographic handshake both peer validate each other's peer IDs, so it seems to me that the attacker gains nothing from this attack at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not worried about peers lying about their identity. I'm worried about making the conflict resolution 100% deterministic, and the transitive attack surface that might expose going forward. We're specifying against our future selves, not against the situations we foresee now.
If there's a bug exploitable in this circumstance, an attack can be crafted that works 100% of the time, by precomputing a large/small peer identity.
I admit I may be thinking too far. But if we want to make conflict resolution probabilistic (which I think is the right way, thinking from first principles), then we can make the protocol can send a random nonce and have peers XOR their nonces to calculate who wins, or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure I understand the argument here. The security of our handshake relies on the TLS handshake. If TLS is broken, we don’t gain a lot from reducing the cost of an attack from succeeding in 100% of the cases to 50% of the cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran out of time to review the protocol specification in-depth, but I think there's enough feedback here to issue another revision for people to review a more refined doc.
|
||
Handshake protocols are not being negotiated, but are announced in the peers' multiaddrs. | ||
|
||
**TODO**: Do we need to describe the format here? I guess we don't, but we will probably need another document for that change, and we can link to it from here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need an extension to the multiaddr spec + multicodec table to add secure channels as an atom. Also, @yusefnapora was writing this document to specify the semantics of multiaddrs: #191.
|
||
**TODO**: Do we need to describe the format here? I guess we don't, but we will probably need another document for that change, and we can link to it from here. | ||
|
||
Peers advertising a multiaddr that includes a handshake protocol MUST support Multiselect 2.0 as described in this document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we had discussed this assumption as a way to simplify things, but I do think it'll end up being short-sighted, and in some ways, a regression compared to multistream-select 1.0, which does announce its version (although admittedly the implementations are not ready to support multiple versions, but the protocol is).
Something I've been thinking about is to create an extension to multistream-select 1.0 / upgraders that would allow us to go straight into a cryptographic handshake, as a way to deliver censorship resistance to downstream users that require it before we realistically ship ms2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this would work without either defining another multicoded or requiring an additional round-trip to negotiate.
|
||
![](handshake.png) | ||
|
||
Handshake protocols (or implementations of handshake protocols) that don't support sending of Early Data will have to run the stream multiplexer selection after the handshake completes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, I see that you added the fallback. We need to define how that'll work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if there's anything we need to define. "Early data" is not a separate byte stream, it's the same byte stream as the rest of the connection. The only difference is that the data is sent earlier (and, depending on the handshake protocol, might use a different set of keys).
To the application Early Data and Late (?) Data is not distinguishable at all.
|
||
#### 0-RTT | ||
|
||
When using 0-RTT session resumption as offered by TLS 1.3 and Noise, the endpoints MUST remember the negotiated stream multiplexer used on the original connection. This ensures that the client can send application data in the first flight when resuming a connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Stream multiplexers can change over time.
- Peers may forget other peers' protocols.
We may need some form of opaque token with which we can verify that our assertions about the other party still remain valid, upon reconnecting. If the other party NACKs, we fall back to full connection bootstrapping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TLS session ticket is exactly that opaque token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But multiselect 2.0 doesn't know about concrete secure channels. If there's a handshake-specific token that we can bind to, it needs to percolate up to this layer.
Co-Authored-By: Raúl Kripalani <raul@protocol.ai>
In Multiselect 2 the server makes use of Early Data by sending a list of stream multiplexers. This ensures that the client can choose a stream multiplexer as soon as the handshake completes (or fail the connection if it doesn't support any stream multiplexer offered by the server). | ||
|
||
When using TLS 1.3, the server can send Early Data after it receives the ClientHello. Early Data is encrypted, but at this point of the handshake the client's identity is not yet verified. | ||
While Noise in principle allows sending of unencrypted data, endpoints MUST NOT use this to send their list of stream multiplexers. An endpoint MAY send it as soon it is possible to send encrypted data, even if the peers' identity is not verified at that point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While Noise in principle allows sending of unencrypted data, endpoints MUST NOT use this to send their list of stream multiplexers. An endpoint MAY send it as soon it is possible to send encrypted data, even if the peers' identity is not verified at that point. | |
While Noise in principle allows sending of unencrypted data, endpoints MUST NOT use this to send their list of stream multiplexers. An endpoint MAY send it as soon as it is possible to send encrypted data, even if the peers' identity is not verified at that point. |
To determine the roles in the second handshake attempt, endpoints calculate the SHA-256 hash of the handshake messages that were sent and received (including any error message(s) that the handshake protocol might have sent) during the failed handshake attempt. | ||
The peer that sent the messages resulting in the numerically smaller hash value acts a client in the second handshake attempt, the peer that sent the messages resulting in the numerically larger hash value acts as a server. | ||
|
||
Since secio assign roles during the handshake, it is not possible to detect a Simultaneous Open in this case. Therefore, secio MUST NOT be used with Multiselect 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no path for backwards compatibility here with multistream 1 correct? Pairing the secio deprecation with multiselect 2 is going to put a lot of stress on limited environments (browsers) to upgrade, or segregate them from the rest of the network. I don't think it warrants blocking the spec, especially if providing that compatibility hampers the performance/feature gains, but we should be cognizant and clear of the network rollout time table and its impact on the network.
Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
This reverts commit 86030ed.
select the stream muxers by intersecting the lists of supported muxers
I have not had time to review all the commentary here and the resulting changes. We should move with this rapidly, knowing that this spec is going to continue change. I'm happy to lock in a baseline so we can start a PoC, but not before we see the following. If these are implemented/rejected, please synthesise and explain (reiterating I have no time to go through the commentary and doc, but I have strong ideas):
|
Compressors came up on #230. It was not part of our requirement document, and to be honest, I don't really understand the use case well enough to even write down the requirements for this, let alone a specification.
We discussed this, and decided to remove this from the specification. The reason is that the cost of sending the string for one round-trip (and learning a new ID for this protocol) is minor, and doesn't justify the cost of indexing a mapping and verifying that this index is still valid.
It's possible to |
I disagree here. This assertion can be sent in early data, therefore amortised in terms of segmentation. A normal IPFS peer will need to agree on identify, dht, pubsub, bitswap, etc. Imagine the network is spotty and this peer keeps connecting and reconnecting frequently. Why would we want to renegotiate protocol indices every time, when we can exchange opaque table IDs upfront and just assert that we both maintain the same table?
This was captured in the design doc. "A simpler wire format" is not a convincing answer for a feature that's required; of course things are going to be simple if you remove features. Aren't we talking about an |
Well, you merged that PR without consensus. And now we're having the discussion again, which was predictable ;-) It's a mistake to make the early data a multiselect 2.0 message. Early data forms part of connection bootstrapping, which is really a process that's decoupled from ms2.0 -- that's the way we're designing it. Therefore, connection bootstrapping needs its own specific payload, which IMO should include: enum CapabilityType {
MULTIPLEXER
COMPRESSOR
ERASURE_CODER
}
enum Scope {
CONN
STREAM
BOTH
}
message SessionCapGroup {
CapabilityType type = 1;
repeated string supported = 2;
repeated Scope scopes = 3;
}
message ConnectionBootstrap {
fixed32 protocol_table_id = 1; // an opaque string that identifies the version of the protocol table we are using.
repeated SessionCapGroup session_capabilities = 2;
} |
Because the overhead of it is a few bytes, for a single roundtrip. I suggest we first show that this is actually a problem before designing a complicated solution. Synchronizing state across connections is not trivial, and will add significant complexity, both in terms of specification as well as implementation.
It was marked "tentative" in the design doc, so it's hard to argue that this is a feature that was a hard requirement in the protocol design. The only comment I received about this tentative point was that it would be complex. Considering that we don't have the concept of protocol upgrades at all in multistream 1 at all, I'm not sure how we know that improving the current situation to a one round-trip per connection wait for protocol upgrades would be prohibitively expensive.
I don't think it's that easy. First of all, it breaks with the concept of ms2.0 to use protobufs for protocol messages, but not application data. Furthermore, it's unclear how this interacts with stream flow control. We might also want to think about an API that would generate this kind of data first.
I merged #230 into this PR, not into master. Consensus would have been nice, but difficult to achieve if so few people actually review the PR. Compressors were unrelated to #230 anyway, so this is the better place to discuss this anyway. Or at least as far as I understand them. This is a totally new requirement to me, I don't know how they're supposed to work, so I have no idea how I should argue about a specific wire format at this point. |
I tried to condense everything down to make the new protocol as simple as possible, while providing an extension point that will allow us to deploy all of the optimizations that people have already suggested in the future.
The document here describes only the stream-based use case, and doesn't cover the packet-based use case yet. I believe that we can use a very similar format for unreliable connections, but there seem to be some subtle differences that would prevent us from reusing the exact wire format of the
Use
andOffer
message defined in this document:Offer
can offer multiple protocols. This makes sense when establishing communication on a reliable channel, but it doesn't really work out in an unreliable protocol. For packet-based protocols, it probably makes sense to restrictOffer
such that it's only possible to offer a single protocol.Use
uses aoneof
ofname
andid
. This works well when a stream correlates theUse
to anOffer
sent before, but isn't enough in the packet-based use case, where you might have multipleOffer
s in flight. Here, it would make sense to define aUse
message such that it includes both aname
and anid
.Reject
message to communicate that a certain protocol is not supported.