ajvondrak/remote_ip

A plug to rewrite the Plug.Conn's remote_ip based on forwarding headers.

remote_ip's Issues

Question about mapped-ipv4 ipv6 format

Hello,

I have an issue with inconsistent behavior when parsing private ipv4-mapped ipv6 ips using Nginx in Kubernetes with x-forwarded-for. In Kubernetes, with internal forwarding the pod ip appended to the header is an ipv6 address in mapped-ipv4 format. Here is an example of a logged X-Forwarded-For header that is correctly being parsed:

97.126.47.5, 172.33.29.126,::ffff:172.33.253.26, 34.228.243.32
# ^remote --^internal host - ^internal proxy host - ^host

In this setup I am not adding any known proxies or clients, and as long as the two middle ips in the chain are internal, the algorithm seems to trust them. However, when I attempt to make a similar request across two clusters the following logged header is not correctly parsed:

97.126.47.5, 34.97.78.79,::ffff:10.116.4.32, 35.245.31.68
# ^remote -- ^load-balancer - ^internal proxy host - ^host

Here, the public addresses 34.97.78.79, 35.245.31.68 are both specified in the proxies list. If I do not specify the rightmost ip, then the plug returns it as the remote. Once I specify both, the plug returns the ipv6 address, which is a private range, as

{0, 0, 0, 0, 0, 65535, 2676, 3080}

Does the algorithm account for private ranges in this notation format? Is it just a fluke that the first configuration works? Can you please advise on how to whitelist private ranges declared in ipv4 that are mapped to ipv6?

using:

remote_ip, "0.2.1", "cd27cd8ea54ecaaf3532776ff4c5e353b3804e710302e88c01eadeaaf42e7e24"

thanks much

Add inet_cidr to included_applications

inet_cidr should be added to included_applications, otherwise it will not be added in the release and the application will crash on startup.

X-Forwarded-For is parsed incorrectly!

Given an X-Forwarded-For header like 203.0.113.195, 70.41.3.18, 150.172.238.178

remote_ip will currently replace the request's IP with 150.172.238.178.

(e.g.

iex> RemoteIp.from([{"x-forwarded-for", "203.0.113.195, 70.41.3.18, 150.172.238.178"}])
{150, 172, 238, 178}

)

However, this is incorrect. according to MDN the client IP is the first one in this list.

Parse X-Forwarded-Port and X-Forwarded-Proto

Besides the real client IP there are other headers that are also interesting to parse such as the real remote port and protocol used. This is useful in situations in which the proxy is terminating the SSL for you.

problem with rewritten IP

even though I get the right IP in the "x-forwarded-for" header, the remote IP is not rewritten properly. Maybe an additional header is problematic here? (I'm behind cloudfllare)
Here is what I get
remote_ip is {10, 255, 0, 2}
forwarded IP, 197.245.12.24

Here is my config (I just took this from somebody else using cloudflare)

    # https://github.com/ajvondrak/remote_ip/issues/6
    plug(RemoteIp,
      headers: ~w(forwarded x-forwarded-for x-client-ip x-real-ip cf-connecting-ip),
      proxies:
        ~w(103.21.244.0/22 103.22.200.0/22 103.31.4.0/22 104.16.0.0/12 108.162.192.0/18 131.0.72.0/22 141.101.64.0/18 162.158.0.0/15 172.64.0.0/13 173.245.48.0/20 188.114.96.0/20 190.93.240.0/20 197.234.240.0/22 198.41.128.0/17 2400:cb00::/32 2405:b500::/32 2606:4700::/32 2803:f800::/32 2c0f:f248::/32 2a06:98c0::/29)
    )

am I missing something?

Good article on `x-forwarded-for` parsing

https://adam-p.ca/blog/2022/03/x-forwarded-for/

I’m going to be looking at this in substantially more depth, but this is probably worth looking at for this library as well to see if it’s worth adding references to the documentation.

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

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?

Dialyzer warning

When I run dialyzer on my project, I get the following warning which looks like it emanates from remote_ip:

lib/project/endpoint.ex:1:call_with_opaque
The call RemoteIp.call(__@1::#{'__struct__':='Elixir.Plug.Conn', 'adapter':={atom(),_}, 'assigns':=#{atom()=>_}, 'before_send':=[fun((_) -> any())], 'body_params':=#{'__struct__'=>'Elixir.Plug.Conn.Unfetched', 'aspect'=>atom(), binary()=>_}, 'cookies':=#{'__struct__'=>'Elixir.Plug.Conn.Unfetched', 'aspect'=>atom(), binary()=>_}, 'halted':=_, 'host':=binary(), 'method':=binary(), 'owner':=pid(), 'params':=#{'__struct__'=>'Elixir.Plug.Conn.Unfetched', 'aspect'=>atom(), binary()=>_}, 'path_info':=[binary()], 'path_params':=#{binary()=>binary() | [any()] | map()}, 'port':=char(), 'private':=#{atom()=>_}, 'query_params':=#{'__struct__'=>'Elixir.Plug.Conn.Unfetched', 'aspect'=>atom(), binary()=>binary() | [any()] | map()}, 'query_string':=binary(), 'remote_ip':={byte(),byte(),byte(),byte()} | {char(),char(),char(),char(),char(),char(),char(),char()}, 'req_cookies':=#{'__struct__'=>'Elixir.Plug.Conn.Unfetched', 'aspect'=>atom(), binary()=>binary()}, 'req_headers':=[{_,_}], 'request_path':=binary(), 'resp_body':='nil' | binary() | maybe_improper_list(binary() | maybe_improper_list(any(),binary() | []) | byte(),binary() | []), 'resp_cookies':=#{binary()=>map()}, 'resp_headers':=[{_,_}], 'scheme':='http' | 'https', 'script_name':=[binary()], 'secret_key_base':='nil' | binary(), 'state':='chunked' | 'file' | 'sent' | 'set' | 'set_chunked' | 'set_file' | 'unset', 'status':='nil' | non_neg_integer()},{'Elixir.MapSet':t(_),[any()]}) contains an opaque term in 2nd argument when terms of different types are expected in these positions}.

I haven't looked at fixing this yet, but I'll try and update when I get to it.

@reserved is incorrect

I have a server at work and I need to only allow some internal IP's on specific paths, however I only get the proxy's IP (same machine, 127.0.0.1) instead of the actual IP's (10.1.1.28 and 192.168.3.143 as actual examples), yet external IP's are correct. This means that I cannot only allow some paths to certain subnets (like restricting one path to 10.1.0.0/16) unless I want to parse the header myself, which is of course not particularly safe as I have to be careful to do it right.

The plug is defined in the endpoint as:

  plug RemoteIp, headers: ["x-forwarded-for"], proxies: ["127.0.0.1/32"]

And yet it is not rewriting all IP's that I need to handle.

RFC1918 IPs shouldn't be discarded by default

Hi,
I started using this plug in applications that are only deployed in a VPN/LAN. However, it discards all the clients IPs as they are RFC1918 IPs (mostly 10/8, some 192.168/16).

It would be nice to allow an option to not discard some of theses reserved networks. Maybe even sub-ranges if that makes more sense (e.g. our VPN is 10.42/16).

Doesn't Work Running Server in Docker Container

Here is the diff when I use RemoteIP:

--- a/lib/endpoint.ex
+++ b/lib/endpoint.ex
@@ -10,6 +10,9 @@ defmodule APITournament.Endpoint do
   use Absinthe.Phoenix.Endpoint
   use Plug.ErrorHandler
   use Sentry.Plug
+  use Plug.Builder
+  plug RemoteIP

   socket "/sock", APITournament.MainSocket

--- a/mix.exs
+++ b/mix.exs
@@ -89,6 +89,7 @@ defmodule APITournament.Mixfile do
       {:sentry,           "~> 6.0.0"},
       {:ip2country,       "~> 1.1"},
+      {:remote_ip,        "~> 0.1.0"},
     ]
   end

I then build the Docker image and run it locally with port 4000 exposed and query it using curl with a forwarded IP header:

docker build -t api-tournament .
sudo docker run -p 4000:4000 -it api-tournament
curl --header "X-Forwarded-For: 192.168.0.2" http://localhost:4000/

Here is the output I get when I execute the curl command:

# conn.req_headers
[{"host", "localhost:4000"}, {"user-agent", "curl/7.52.1"}, {"accept", "*/*"},
 {"x-forwarded-for", "192.168.0.2"}]
# conn.remote_ip
{0, 0, 0, 0, 0, 65535, 44049, 1}
# to_string(:inet_parse.ntoa(conn.remote_ip))
::ffff:172.17.0.1

Here is the diff when I use plug_forwarded_peer:

--- a/lib/endpoint.ex
+++ b/lib/endpoint.ex
@@ -10,6 +10,9 @@ defmodule APITournament.Endpoint do
   use Absinthe.Phoenix.Endpoint
   use Plug.ErrorHandler
   use Sentry.Plug
+  use Plug.Builder
+  plug PlugForwardedPeer

   socket "/sock", APITournament.MainSocket

--- a/mix.exs
+++ b/mix.exs
@@ -89,6 +89,7 @@ defmodule APITournament.Mixfile do
       {:sentry,           "~> 6.0.0"},
       {:ip2country,       "~> 1.1"},
       {:remote_ip,        "~> 0.1.0"},
+      {:plug_forwarded_peer, "~> 0.0.2"},
     ]
   end

Using the same docker build and curl:

[info] GET /
# conn.req_headers
[{"host", "localhost:4000"}, {"user-agent", "curl/7.52.1"}, {"accept", "*/*"},
 {"x-forwarded-for", "192.168.0.2"}]
# conn.remote_ip
{192, 168, 0, 2}
# to_string(:inet_parse.ntoa(conn.remote_ip))
192.168.0.2

This is the correct output, but I can't figure out why it appears RemoteIP does nothing with the x-forwarded-for header when it's present.

Export `RemoteIp.Block` as its own package?

I’m getting ready to release a new IP access control plug and I had been using InetCidr when I looked at the latest 1.0 update to remote_ip. At this point, I threw out the parsing and implementation that I had and explicitly refer to RemoteIp.Block for my IP address inclusion test, which means that instead of remote_ip being an optional dependency of my plug, it is now a required dependency of my plug (its use is recommended whenever someone is behind a proxy server).

This is compounded by the fact that RemoteIp.Block is an implementation detail for the RemoteIp plug (that is, you’ve set it @moduledoc false). I don’t see you changing the API in a way that would break my use of it, but I’m not 100% comfortable relying on such a detail.

This leaves three options:

1. I copy the code from RemoteIp.Block into IpAccessControl.Block. I’m not fond of this approach, but it is the least amount of work for you.
2. I convince you to document RemoteIp.Block so that I can treat it as an official API that I can depend on.
3. I convince you to document RemoteIp.Block and make it its own Hex package similar to InetCidr so that it is usable without depending on internal details of a tangentially-related package.

I’d rather not do 1; the implementation for RemoteIp.Block is really well done and it feels wrong to duplicate this code in my package. I haven’t yet released the package to Hex, but I’d like to do so in the next week or so (I need to test this on a local package).

Let me know what you’d prefer; the current implementation can be found here: https://github.com/KineticCafe/ip_access_control

Any way to pass runtime information to `init`

Hi, as a proxy list I need to pass information coming from an environment variable, but they are present only in the runtime environment. By the nature of Phoenix plugs, init is called only in compile time.

Do you have an idea how would I solve this issue?

Unsure of Implementation

Hey there,

I just wanted to make sure that this is implemented properly, because I can't seem to get it working like I planned on.

Endpoint.ex

defmodule MyApp.Endpoint do
  use Phoenix.Endpoint, otp_app: :my_app

  plug CORSPlug

  plug RemoteIp

  plug Plug.Static,
    at: "/", from: :lotus, gzip: false,
    only: ~w(css fonts images js favicon.ico robots.txt)

   # Rest of Plug info here
end   

MyController.ex

  def create(conn, %{"id" => id} = params) do
    # Accessing the `remote_ip` here, but its still a "10.x.x.x" address
    ip_address = conn.remote_ip |> Tuple.to_list |> Enum.join(".")
  end

My understanding was that by implementing the plug RemoteIp, that it would change the remote_ip field that comes through on the conn, however this is not the case as it appears that I keep getting the IP of the load balancer that sits in front of the app itself --
Is there anything I'm missing here?

:combine should not be in :included_applications

There is already a comment suggesting this in the commit: a5fb55b#commitcomment-20730235

My reason for bringing it up: it breaks Docker Elixir builds, see also elixir-lang/elixir#9163 (comment)

Support for `Fly-Client-IP`?

This repo doesn't seem to work with Phoenix apps deployed to Fly.io, which is one of the larger Elixir webhosts. They instead provide a [Fly-Client-IP](https://fly.io/docs/reference/runtime-environment/#fly-client-ip) header. Would be cool if this project could support that, if possible.