Skip to content
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

TCP connection multiplexing (tcpreuse package) doesn't work with pnet #3119

Open
aschmahmann opened this issue Dec 20, 2024 · 2 comments
Open

Comments

@aschmahmann
Copy link
Collaborator

The tcpreuse package was introduced so that the same TCP port could be reused across the TCP, WS, WSS, etc. transports like we have with UDP and QUIC + WebRTC (-Direct). Doing this requires looking at the first packet for the connection to figure out which transport to associate the connection with.

Unfortunately, this does not integrate nicely with the private networks / pnet feature since in the case of the TCP transport + pnet the first few bytes are encrypted which means we can't use our current check which checks for the start of the multistream protocol. Similarly, we also would no longer be able to rely on the WS, WSS, etc. checks because the encryption could be "unlucky" and collide with the first bytes from HTTP or TLS.

To reproduce this start 2 libp2p nodes with pnet and have one use the tcpreuse package and listen on TCP. Try to open a stream and send a message and you'll see a failure. The bug was discovered integrating the shared tcp listener into kubo ipfs/kubo#10565 (comment).

Some options here:

  1. Mark the tcpreuse feature as unsupported for pnet until enough people want it to warrant the effort, and add errors in the host constructor like we do for pnet + quic/webrtc.
  2. Forbid pnet nonce's from coming from the list of supported starting bytes to switch on (e.g. the first 3 bytes that could come from multistream, HTTP, or TLS). If the inbound packet is "unknown" assume it's the TCP transport If in the future new ones are added pnet will need to be updated, and rely on the assumption that pnet networks are able to update in lockstep (not totally crazy given the keys could leak and need to be rotated, but some people might just be using pnet as a hack to create namespaced networks and not be able to rotate as easily).
  3. Expand the number of bytes we sniff to figure out the transport to be enough that we have very high confidence that something that looks like HTTP or TLS is actually one of those and not just an unlucky set of random nonce bytes that happen to look like HTTP or TLS. Not sure if this is even really doable.

At the moment it seems like just marking the combination of tcpreuse + pnet as unsupported seems like way to go. The complexity of the other two options doesn't seem like a good use of time unless there's demand for it.

@gammazero
Copy link
Contributor

I agree we should consider the current version of pnet (v0) to be incompatible with tcpreuse. A future pnet (v1) can propose adding its own identifying first bytes. This will also allow for pnet to do version detection in the future.

@lidel
Copy link
Member

lidel commented Dec 20, 2024

+1, because:

  • PNET is already limited to TCP (UDP/QUIC is not supported, as noted in quic: private network support #1432)
  • Secure WebSockets will effectively leak the existence of private swarm to CA that provides TLS cert + require client in browser to have shared key, leaking entire thing – due to this we have probably 0 people who use PNET with Websockets.

Just to reduce confusion, we should ensure PNET == raw TCP, and error when go-libp2p user attempts to use anything else.

Would it be possible for go-libp2p itself to detect when PNET is enabled, and force-disable, or error when unsupported transport other than raw TCP is present in libp2p config?

We do it in Kubo right now, but by having this in go-libp2p we would save all users from spending time debugging issues like ipfs/kubo#10565 (comment).

@lidel lidel mentioned this issue Dec 21, 2024
47 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants