search

ajvondrak / remote_ip

A plug to rewrite the Plug.Conn's remote_ip based on forwarding headers.
MIT License
252 stars 31 forks source link

Unsure of Implementation #3

Closed jakehasler closed 7 years ago

jakehasler commented 7 years ago

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?

ajvondrak commented 7 years ago

What do the req_headers look like in the Plug.Conn when a request comes through your load balancer?

The selection algorithm is described in this section of the README. My guess - though I can't be sure - is that you need to specify the :proxies option for your setup. (If this poses a problem, I'd love to know how the interface could be improved!)

adsteel commented 7 years ago

I'm having the same issue. My request headers look like this:

[
  {"host", "blah.herokuapp.com"},
  {"connection", "close"},
  {"accept", "application/json, text/javascript, */*; q=0.01"},
  {"origin", "http://blah.netlify.com"},
  {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36"},
  {"content-type", "multipart/form-data; boundary=----WebKitFormBoundaryxd0LvDA6YTTuBemj"},
  {"referer", "http://blah.netlify.com/contact"},
  {"accept-encoding", "gzip, deflate, br"},
  {"accept-language", "en-US,en;q=0.8"},
  {"x-request-id", "7debd32d-ec89-4079-9245-55d535eae6e2"},
  {"x-forwarded-for", "75.171.251.123"},
  {"x-forwarded-proto", "https"},
  {"x-forwarded-port", "443"},
  {"via", "1.1 vegur"},
  {"connect-time", "1"},
  {"x-request-start", "1488493100272"},
  {"total-route-time", "2001"},
  {"content-length", "929"}
]
ajvondrak commented 7 years ago

:thinking: Quite odd, because in isolation it appears to be fine:

iex(2)> conn = %Plug.Conn{remote_ip: {127,0,0,1}, req_headers: headers}
%Plug.Conn{adapter: {Plug.Conn, :...}, assigns: %{}, before_send: [],
 body_params: %Plug.Conn.Unfetched{aspect: :body_params},
 cookies: %Plug.Conn.Unfetched{aspect: :cookies}, halted: false,
 host: "www.example.com", method: "GET", owner: nil,
 params: %Plug.Conn.Unfetched{aspect: :params}, path_info: [], peer: nil,
 port: 0, private: %{},
 query_params: %Plug.Conn.Unfetched{aspect: :query_params}, query_string: "",
 remote_ip: {127, 0, 0, 1}, req_cookies: %Plug.Conn.Unfetched{aspect: :cookies},
 req_headers: [{"host", "blah.herokuapp.com"}, {"connection", "close"},
  {"accept", "application/json, text/javascript, */*; q=0.01"},
  {"origin", "http://blah.netlify.com"},
  {"user-agent",
   "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36"},
  {"content-type",
   "multipart/form-data; boundary=----WebKitFormBoundaryxd0LvDA6YTTuBemj"},
  {"referer", "http://blah.netlify.com/contact"},
  {"accept-encoding", "gzip, deflate, br"},
  {"accept-language", "en-US,en;q=0.8"},
  {"x-request-id", "7debd32d-ec89-4079-9245-55d535eae6e2"},
  {"x-forwarded-for", "75.171.251.123"}, {"x-forwarded-proto", "https"},
  {"x-forwarded-port", "443"}, {"via", "1.1 vegur"}, {"connect-time", "1"},
  {"x-request-start", "1488493100272"}, {"total-route-time", "2001"},
  {"content-length", "929"}], request_path: "", resp_body: nil,
 resp_cookies: %{},
 resp_headers: [{"cache-control", "max-age=0, private, must-revalidate"}],
 scheme: :http, script_name: [], secret_key_base: nil, state: :unset,
 status: nil}

iex(3)> RemoteIp.call(conn, RemoteIp.init).remote_ip
{75, 171, 251, 123}

Indeed, RemoteIp has tests to the effect.

I also couldn't reproduce in a new Phoenix app:

diff --git a/lib/hello_phoenix/endpoint.ex b/lib/hello_phoenix/endpoint.ex
index 2e33241..3929269 100644
--- a/lib/hello_phoenix/endpoint.ex
+++ b/lib/hello_phoenix/endpoint.ex
@@ -3,6 +3,8 @@ defmodule HelloPhoenix.Endpoint do

   socket "/socket", HelloPhoenix.UserSocket

+  plug RemoteIp
+
   # Serve at "/" the static files from "priv/static" directory.
   #
   # You should set gzip to true if you are running phoenix.digest
diff --git a/mix.exs b/mix.exs
index 6be9357..5797663 100644
--- a/mix.exs
+++ b/mix.exs
@@ -37,7 +37,8 @@ defmodule HelloPhoenix.Mixfile do
      {:phoenix_html, "~> 2.6"},
      {:phoenix_live_reload, "~> 1.0", only: :dev},
      {:gettext, "~> 0.11"},
-     {:cowboy, "~> 1.0"}]
+     {:cowboy, "~> 1.0"},
+     {:remote_ip, "~> 0.1"}]
   end

   # Aliases are shortcuts or tasks specific to the current project.
diff --git a/mix.lock b/mix.lock
index ac61ff7..b1d9de8 100644
--- a/mix.lock
+++ b/mix.lock
@@ -1,4 +1,5 @@
-%{"connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], []},
+%{"combine": {:hex, :combine, "0.9.6", "8d1034a127d4cbf6924c8a5010d3534d958085575fa4d9b878f200d79ac78335", [:mix], []},
+  "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], []},
   "cowboy": {:hex, :cowboy, "1.1.2", "61ac29ea970389a88eca5a65601460162d370a70018afe6f949a29dca91f3bb0", [:rebar3], [{:cowlib, "~> 1.0.2", [hex: :cowlib, optional: false]}, {:ranch, "~> 1.3.2", [hex: :ranch, optional: false]}]},
   "cowlib": {:hex, :cowlib, "1.0.2", "9d769a1d062c9c3ac753096f868ca121e2730b9a377de23dec0f7e08b1df84ee", [:make], []},
   "db_connection": {:hex, :db_connection, "1.1.1", "f9d246e8f65b9490945cf7360875eee18fcec9a0115207603215eb1fd94c39ef", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]},
@@ -6,6 +7,7 @@
   "ecto": {:hex, :ecto, "2.1.3", "ffb24e150b519a4c0e4c84f9eabc8587199389bc499195d5d1a93cd3b2d9a045", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, optional: true]}]},
   "fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], []},
   "gettext": {:hex, :gettext, "0.13.1", "5e0daf4e7636d771c4c71ad5f3f53ba09a9ae5c250e1ab9c42ba9edccc476263", [:mix], []},
+  "inet_cidr": {:hex, :inet_cidr, "1.0.2", "1acc8b89670aaf4aec7dd36d08f33d6ba6155be0fa30169abdb86b8431ecb175", [:mix], []},
   "mime": {:hex, :mime, "1.1.0", "01c1d6f4083d8aa5c7b8c246ade95139620ef8effb009edde934e0ec3b28090a", [:mix], []},
   "phoenix": {:hex, :phoenix, "1.2.1", "6dc592249ab73c67575769765b66ad164ad25d83defa3492dc6ae269bd2a68ab", [:mix], [{:cowboy, "~> 1.0", [hex: :cowboy, optional: true]}, {:phoenix_pubsub, "~> 1.0", [hex: :phoenix_pubsub, optional: false]}, {:plug, "~> 1.1", [hex: :plug, optional: false]}, {:poison, "~> 1.5 or ~> 2.0", [hex: :poison, optional: false]}]},
   "phoenix_ecto": {:hex, :phoenix_ecto, "3.2.2", "2e51c57614528b130d54019e78349f83a5b5d53b3b2ef775dbc30df7e1fecb45", [:mix], [{:ecto, "~> 2.1", [hex: :ecto, optional: false]}, {:phoenix_html, "~> 2.9", [hex: :phoenix_html, optional: false]}, {:plug, "~> 1.0", [hex: :plug, optional: false]}]},
@@ -16,4 +18,5 @@
   "poison": {:hex, :poison, "2.2.0", "4763b69a8a77bd77d26f477d196428b741261a761257ff1cf92753a0d4d24a63", [:mix], []},
   "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], []},
   "postgrex": {:hex, :postgrex, "0.13.1", "ebf17b7348922af9db8b4d70f946328ab9ef5123526bf05567d9306393fce104", [:mix], [{:connection, "~> 1.0", [hex: :connection, optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, optional: false]}]},
-  "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], []}}
+  "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], []},
+  "remote_ip": {:hex, :remote_ip, "0.1.3", "9175e63ce23eece9253bdd17edbd2262bd91c5b700987f6ce0044e504ba42089", [:mix], [{:combine, "~> 0.9.2", [hex: :combine, optional: false]}, {:inet_cidr, "~> 1.0", [hex: :inet_cidr, optional: false]}, {:plug, "~> 1.2", [hex: :plug, optional: false]}]}}
diff --git a/web/templates/page/index.html.eex b/web/templates/page/index.html.eex
index 8ff4b81..dc00fb3 100644
--- a/web/templates/page/index.html.eex
+++ b/web/templates/page/index.html.eex
@@ -1,5 +1,5 @@
 <div class="jumbotron">
-  <h2><%= gettext "Welcome to %{name}", name: "Phoenix!" %></h2>
+  <h2>Welcome, <%= @conn.remote_ip |> :inet.ntoa |> to_string %></h2>
   <p class="lead">A productive web framework that<br />does not compromise speed and maintainability.</p>
 </div>
$ mix phoenix.server

$ curl localhost:4000 -s | grep Welcome
  <h2>Welcome, 127.0.0.1</h2>

$ curl localhost:4000 -s -H'X-Forwarded-For: 1.2.3.4' | grep Welcome
  <h2>Welcome, 1.2.3.4</h2>

My remaining guess is that it has something to do with whatever other plugs you're using. Maybe you should try logging the IP in between every plug in your pipeline? :man_shrugging: Let me know how it goes!

adsteel commented 7 years ago

Well, I wrote my own, simpler plug and did some logging. My route action was getting called before my plug! Turns out I added my plug below plug :match, which is where I had added RemoteIp as well. I moved my plug up above plug :match and everything worked as expected. So, noobie mistake, but it may be worth adding to the readme.

ajvondrak commented 7 years ago

Oh, cool! Glad you got it working, @adsteel. Hadn't thought of that issue, but it makes sense. Will definitely update the readme when I get a moment.