-
Notifications
You must be signed in to change notification settings - Fork 281
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
*: Address security-protocol-in-multiaddr and relay conflict #359
*: Address security-protocol-in-multiaddr and relay conflict #359
Conversation
This patch addresses the problems below by detailing the solution below in the corresponding specifications. > Taxonomy: > > * `A`: The destination node. > > * `B`: The source node. > > * `R`: The relay node. > > * outer connection: The connection from `R` to `A`. > > * relayed (inner) connection: The relayed (inner) connection from `B` to > `A`. > > > Problems we need to solve: > > 1. `B` and `A` need to know which security protocol to use to upgrade the > relayed (inner) connection. > > 2. In the case of active relaying, `R` needs to know which security > protocol to use to upgrade the outer connection to the destination. > > 3. `A` should be able to offer different security protocols for the > relayed (inner) connection. `A` can use different layer 4 ports to > demultiplex security protocols for the outer connection. `A` has no > demultiplexing mechanism for the security protocols used for the > relayed (inner) connection. Thus if `A` advertises different > multiaddresses with different security protocols for the relayed > (inner) connection, it has no way to determine which one to use on > incoming relayed (inner) connections. > > > We propose two mechanisms to solve the issues above: > > 1. We have to somehow embed both outer and inner security protocol in the > multiaddr. What we could do is introduce another separator, e.g. > `p2p-circuit-inner` (name yet to be determined). > > * **Solving (1)**: A multiaddr for a passive relayed connection could > then look like: > `/ip4/../tcp/../tls/p2p/QmRelay/p2p-circuit/p2p/QmA/p2p-circuit-inner/noise`, > with `noise` being used to secure the relayed (inner) connection. * > * **Solving (2)**: A multiaddr for an active relayed connection could > then look like: > `/ip4/../tcp/../tls/p2p/QmRelay/p2p-circuit/ip6/::1/tls/p2p/QmA/p2p-circuit-inner/noise`, > with `tls` being used to secure the outer connection and `noise` > being used to secure the relayed (inner) connection. > > 2. **Solving (3)**: In order for `A` to be able to advertise different > multiaddresses with different security protocols for the relayed > (inner) connection, while still being able to choose the right security > protocol on incoming relayed (inner) connections, we need to include > the security protocol for the relayed (inner) connection in the circuit > relay v2 CONNECT message from `B` to `A`. See libp2p#349 (comment) for details.
@marten-seemann friendly ping. Sorry for the confusion around this being opened as a Draft. |
|
||
- `<relay-R1-multiaddr>/p2p-circuit/p2p/QmTarget/p2p-circuit-inner/noise` | ||
- `<relay-R2-multiaddr>/p2p-circuit/p2p/QmTarget/p2p-circuit-inner/noise` | ||
|
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.
Should we say that the relay SHOULD abort the connection if it receives a CONNECT
with different security protocols, and the target MUST abort the connection attempt (i.e. reset the stream) if it receives such a CONNECT
?
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.
Should we say that the relay SHOULD abort the connection if it receives a CONNECT with different security protocols,
I would go by Be strict when sending and tolerant when receiving (RFC 1958). We might have a case in the future where sending a CONNECT
with different security protocols is a valid use-case. Say we take the relaxed approach today, we would only have to roll-out an upgrade to the endpoints, not the relays, in the future.
the target MUST abort the connection attempt (i.e. reset the stream) if it receives such a CONNECT?
Sounds good to me.
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.
Second suggestion applied via 7844421.
#### Relay addresses and multiaddr security component | ||
|
||
Instead of negotiating the security protocol in-band, security protocols should | ||
be encapsulated in the multiaddr (see [The multiaddr security component |
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.
Do we want to make this a prerequisite for circuit v2, or does that clash with our experimental deployment of v2?
be encapsulated in the multiaddr (see [The multiaddr security component | |
Instead of negotiating the security protocol in-band, security protocols are | |
encapsulated in the multiaddr (see [The multiaddr security component |
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.
That would make Project Flare dependent on security-protocol-in-multiaddr. Given the large number of live libp2p networks, I would prefer to keep the two decoupled to allow flexible roll-outs.
Do you see issues with the decoupled approach @marten-seemann?
##### Security protocol selection for the relayed connection | ||
|
||
Instead of negotiating the security protocol in-band, security protocols should | ||
be encapsulated in the multiaddr (see [The multiaddr security component |
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.
Same here.
Note: This pull request targets the
addresses-with-security-protocol
branch (#353) and not themaster
branch.This patch addresses the problems below by detailing the solution below in the
corresponding specifications.
See #349 (comment) for details.