Plug.Conn also has get_peer_data, which returns the original ip

[dvic]

I realised with a LiveView project that overwriting the remote_ip is not enough because Plug.Conn has a get_peer_data function and that's what's being called by LiveView. This value is

@type peer_data :: %{
          address: :inet.ip_address(),
          port: :inet.port_number(),
          ssl_cert: binary | nil
        }

What is the way forward here? Wrap the adapter implementation and wrap the get_peer_data to override the ip?

[dvic]

The relevant calls: https://hexdocs.pm/plug/Plug.Conn.html#get_peer_data/1 and https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html#get_connect_info/2

[ajvondrak]

Taking plug_cowboy as our canonical implementation, the way it works:

• Cowboy manipulates an underlying Req object: https://ninenines.eu/docs/en/cowboy/2.6/manual/cowboy_req/#_req
• A plug adapter translates the backing library's object into a Plug.Conn struct that Plug will understand. The original object is still available under the :adapter key: https://github.com/elixir-plug/plug_cowboy/blob/0227c58613d02d4d6885dd4df01d297ccdb95c68/lib/plug/cowboy/conn.ex#L17
• A callback that plug adapters are responsible for implementing is Plug.Conn.Adapter.get_peer_data/1, which takes in the underlying payload (here, the Cowboy Req) and returns a map: https://github.com/elixir-plug/plug/blob/12dbfb817d1951c955afe90d0f4f4bf4bfd4e8b9/lib/plug/conn/adapter.ex#L150-L153
• The plug_cowboy implementation can fetch the :address for the peer data directly from the Req's :peer: https://github.com/elixir-plug/plug_cowboy/blob/0227c58613d02d4d6885dd4df01d297ccdb95c68/lib/plug/cowboy/conn.ex#L122-L129
• Plug.Conn.get_peer_data/1 delegates to the adapter callback: https://github.com/elixir-plug/plug/blob/5997804c57d7bb2e1c7570b5bb0b3ec13ce677a7/lib/plug/conn.ex#L636-L642

So ultimately, with a given Plug.Conn, the way we'd override the info returned by Plug.Conn.get_peer_data/1 would have to be adapter-specific. For Cowboy, this would involve manipulating the :peer key of the underlying conn.adapter payload (the Cowboy Req).

I believe clobbering this value would be a bad idea. My impression is that this data is used by Cowboy to actually orchestrate the HTTP response, which is why Plug.Conn exposes the :remote_ip as a separate entity that's meant to be overwritten.

I'm also under the impression that Phoenix LiveView relies on this "raw" data because of the low-level manipulation of sockets that it has to do. I'm unsure about those details.

The general workaround for this is to pass the forwarding headers to RemoteIp.from/2 in your Phoenix sockets, as sketched in the docs: https://hexdocs.pm/remote_ip/RemoteIp.html#module-usage I don't know enough about Phoenix to provide any more in-depth guidance than that. :sweat_smile:

One of the main issues people have bumped into with this is that Phoenix sockets only expose the x- headers, which won't work so well if you're using a header like Forwarded or Fly-Client-IP. There has been some amount of discussion within Phoenix about exposing more data, but I take it there are security concerns:

• https://github.com/ajvondrak/remote_ip/issues/30#issuecomment-1143804155
• https://github.com/phoenixframework/phoenix/pull/3815#issuecomment-621852644
• https://github.com/phoenixframework/phoenix/pull/3481#issuecomment-511496947
• https://github.com/phoenixframework/phoenix/issues/3524#issuecomment-521855670
• https://github.com/phoenixframework/phoenix/issues/3427
• https://github.com/phoenixframework/phoenix/pull/2889#discussion_r209181416

Given all that, I'm inclined to close this issue, since (a) I don't think we should be clobbering the raw peer data and (b) getting the right information for RemoteIp.from/2 to work (e.g., non-x- headers) would need to be solved on the Phoenix end. Let me know if you think differently, though.

[dvic]

Thanks for the elaborate response!

RemoteIp.from/2 will work for us because we're dealing with the x-forwarded-for header (Google Cloud k8s ingress).

I believe clobbering this value would be a bad idea. My impression is that this data is used by Cowboy to actually orchestrate the HTTP response, which is why Plug.Conn exposes the :remote_ip as a separate entity that's meant to be overwritten.

I did not think of this, then indeed it's a bad idea. We can go ahead and close this issue. Once again, thanks for the explanation 👍