Skip to content

websocket: record X-Real-IP header #1437

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

Open
magik6k opened this issue Jun 7, 2019 · 3 comments · May be fixed by #3235
Open

websocket: record X-Real-IP header #1437

magik6k opened this issue Jun 7, 2019 · 3 comments · May be fixed by #3235
Labels
exp/expert Having worked on the specific codebase is important good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature

Comments

@magik6k
Copy link
Contributor

magik6k commented Jun 7, 2019

We currently don't support TLS here, so a reverse proxy needs to be set up, which has the side effect of loosing real ip addresses in ipfs swarm peers.

We could check if the X-Real-IP header is set when accepting connections and change the ip address we report as the remote (and probably only do that when we proxy from localhost)

@Jorropo
Copy link
Contributor

Jorropo commented Jul 12, 2019

That also should come with a way to set trusted reverse proxy because else someone could do a real ws conn with a X-Real-IP and change his ip to someone else for you.
(the reverse proxy would be an ip or ip range (/ip4/127.0.0.1 by default) and if the connection is made via here we can know X-Real-IP should be real)

@Stebalien Stebalien added kind/enhancement A net-new feature or improvement to an existing feature help wanted Seeking public contribution on this issue labels Jul 25, 2019
@marten-seemann marten-seemann transferred this issue from libp2p/go-ws-transport Apr 22, 2022
@marten-seemann marten-seemann changed the title Record X-Real-IP header websocket: record X-Real-IP header Apr 22, 2022
@MarcoPolo MarcoPolo added exp/expert Having worked on the specific codebase is important good first issue Good issue for new contributors labels Jun 19, 2023
@MarcoPolo
Copy link
Collaborator

Here's how I think we could implement this:

  • Add option to always trust this header
  • By default, trust this header only of private network/localhost

@snichols
Copy link

snichols commented Apr 17, 2024

Another issue that comes from using a proxy is that the host thinks that remote websocket connections are coming from localhost which causes the identify protocol to share private addresses with websocket peers. When this happens, browser peers react poorly and refuse to connect to the proxied peer after identify is completed. I'm still narrowing down that piece and will report to the libp2p-js repo.

Relevant filtering code in identify:

func filterAddrs(addrs []ma.Multiaddr, remote ma.Multiaddr) []ma.Multiaddr {

That being said, this issue is a thorn in the side of anyone that wants to easily provide browser-friendly websocket transports. I'm using ngrok -> nginx -> libp2p to wrangle my web traffic. This would be exacerbated if I were deploying to Kubernetes as web ingress relies on standard proxying to communicate details about the external connection. Accepting standard headers seems like a necessary feature to me and would help make adoption of libp2p easier in production environments, IMO.

@MarcoPolo has the right idea here. Trust the header when coming from a private network. Optionally add a config setting for fine-grained control over the behavior.

@jclab-joseph jclab-joseph linked a pull request Mar 14, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/expert Having worked on the specific codebase is important good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants