Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Connection issue via Websocket to a node behind Nginx proxy #4181

Closed
niczy opened this issue Jul 10, 2023 · 11 comments
Closed

Connection issue via Websocket to a node behind Nginx proxy #4181

niczy opened this issue Jul 10, 2023 · 11 comments
Labels
kind/stale need/author-input Needs input from the original author

Comments

@niczy
Copy link

niczy commented Jul 10, 2023

Description

When I try to connect my libp2p node to another libp2p node behind nginx proxy via websocket, I got following error:

Custom { kind: Other, error: Transport(Handshake("server rejected handshake; status code = 404"))

Seems the issue is that nginx depends on information of HTTP headers like host, path do decide which backed to route the traffic to. Current websocket transport impl doesn't send those information

See https://github.com/libp2p/rust-libp2p/pull/4180/files for the DNS/WS transport configuration.

Motivation

Allow us to run multiple libp2p node behind a nginx proxy over Websocket.

Current Implementation

Currently we won't be able to connect to libp2p node behind a nginx proxy with virtual host.

Are you planning to do it yourself in a pull request?

Maybe. Tried this, need to research more what we should send after a socket connection is established.
https://github.com/libp2p/rust-libp2p/pull/4180/files

@mxinden
Copy link
Member

mxinden commented Jul 20, 2023

Seems the issue is that nginx depends on information of HTTP headers like host, path do decide which backed to route the traffic to. Current websocket transport impl doesn't send those information

Correct. The current websocket implementation does not support custom paths or HTTP headers. Can you load balance / multiplex based on different URLs, i.e. subdomains in your use-case?

@niczy
Copy link
Author

niczy commented Jul 24, 2023

Correct. The current websocket implementation does not support custom paths or HTTP headers. Can you load balance / multiplex based on different URLs, i.e. subdomains in your use-case?

That could work. But not very scalable for us if we want to run some IPFS node in production. Normally in a mid-size to large-size company, the LB is maintained by a dedicated team and LB relies on the virtual host to dispatch request to different services.

What we did is to have nginx to route traffic from example.com to a gateway service. The gateway service then dispatches the request further to the corresponding IPFS node based on the path "/p2p/XXXXX". When we start a new IPFS node, the node would register its peer id to the gateway service. This allows us to scale up/down the IPFS cluster automatically. Scaling would be more difficult with subdomains, because we would need to programmatically update DNS.

This approach works with js-libp2p since the connection was established with "wss://example.com/p2p/XXXXX".

@mxinden
Copy link
Member

mxinden commented Jul 24, 2023

This approach works with js-libp2p since the connection was established with "wss://example.com/p2p/XXXXX".

Indeed rust-libp2p's websocket implementation falls short here. See also #2708. I am happy to accept patches to libp2p-websocket in case you are interested in contributing @niczy.

@niczy
Copy link
Author

niczy commented Jul 24, 2023

yeah, I can try

@niczy
Copy link
Author

niczy commented Jul 31, 2023

Seems we just need to pass the original "host_name" and "resource" param to the sokettio Client here:

let mut client = handshake::Client::new(stream, &addr.host_port, addr.path.as_ref());

Passing the correct "resource" param should fix the other issue #2708.

However websocket transport is wrapped within the dns transport which removes the information about the hostname.

I think we would need to refactor the code to stop treating DnsConfig as a Transport trait and use it as a helper function. Websocket crate can call dns library to resolve the IP address and pass the correct hostname and path to sokettio Client later.

@thomaseizinger
Copy link
Contributor

I think we would need to refactor the code to stop treating DnsConfig as a Transport trait and use it as a helper function. Websocket crate can call dns library to resolve the IP address and pass the correct hostname and path to sokettio Client later.

I don't think this will be an option, DNS resolution should be composable with any transport.

@niczy
Copy link
Author

niczy commented Aug 3, 2023

Any suggestion how we can solve this? ( I am both very new to rust and rust-libp2p codebase :-) ).
Today websocket transport is wrapped inside DNSConfig, and doesn't have any information about the DNS name since the multiaddr is re-written by DNSConfig.

Other implementations like golang has this function to parse the original multiaddr to a URL that can be passed into net.Dial
https://github.com/libp2p/go-libp2p/blob/master/p2p/transport/websocket/addrs.go#L107

@thomaseizinger
Copy link
Contributor

Any suggestion how we can solve this?

Unfortunately, the interface is constrained to just passing a multiaddr.

@mxinden Do you know if we have the same problem in other transports? It seems worthwhile solving that each inner transport of DNS would want to know that we originally received a domain name as an argument.

Can we (ab)-use the /sni multiaddr protocol here? The DNS transport could append that to a resolved address to indicate to the inner transport which original domain name we received.

@mxinden
Copy link
Member

mxinden commented Aug 16, 2023

Today websocket transport is wrapped inside DNSConfig, and doesn't have any information about the DNS name since the multiaddr is re-written by DNSConfig.

It should be the other way around. I.e. websocket should wrap DNSConfig. See below as an example @niczy:

https://github.com/mxinden/rust-libp2p-server/blob/6b4c36a322127b0e5e4be8a09c8771b3cb92432b/src/main.rs#L109-L118

@thomaseizinger thomaseizinger added the need/author-input Needs input from the original author label Sep 20, 2023
@github-actions
Copy link

Oops, seems like we needed more information for this issue, please comment with more details or this issue will be closed in 7 days.

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

This issue was closed because it is missing author input.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
kind/stale need/author-input Needs input from the original author
Projects
None yet
Development

No branches or pull requests

3 participants