Skip to content

Commit

Permalink
Teach RemoteIp the :clients option
Browse files Browse the repository at this point in the history
Removing the automatic addition of reserved addresses to the proxies
would introduce breaking changes (e.g., when users start getting back
127.0.0.1 because it's not being filtered as a proxy anymore).

Passing in reserved IPs as an option also seems less than ideal to me,
since you'd have to take care to add everything *except* the particular
clients you're expecting. This wasn't really the use case described by
issues like #8.

What's more, the name `:clients` has the same number of characters as
`:headers` and `:proxies`, so everything lines up all pretty. ✨

The bulk of this commit isn't so much in the logic or even the
documentation. Mostly, I just found the tests inscrutable. So I tried to
refactor them so that they'd be moderately less painful while also
adding tests for `:proxies` vs `:clients`. Essentially, instead of
cramming a million assertions into each test, I separated the tests out
from each other, grouped by forwarding header (or other general use
cases, as in the two-hop tests). Then I can just make basically one
assertion per test. Plus I fit everything back into tidy 80-character
lines. 🌈

This closes #8, closes #10, and closes #11.
  • Loading branch information
Alex Vondrak committed Oct 21, 2019
1 parent 1a77add commit 06ca198
Show file tree
Hide file tree
Showing 3 changed files with 458 additions and 243 deletions.
65 changes: 45 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

A [plug](https://github.com/elixir-lang/plug) to overwrite the [`Conn`'s](https://hexdocs.pm/plug/Plug.Conn.html) `remote_ip` based on headers such as `X-Forwarded-For`.

IPs are processed last-to-first to prevent IP spoofing, as thoroughly explained in [a blog post](http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection/) by [@gingerlime](https://github.com/gingerlime). Loopback/private IPs are always ignored and known proxies are configurable, so neither type will be erroneously treated as the original client IP. You can configure any number of arbitrary forwarding headers to use. If there's a special way to parse your particular header, the architecture of this project should [make it easy](#contributing) to open a pull request so `RemoteIp` can accommodate.
IPs are processed last-to-first to prevent IP spoofing, as thoroughly explained in [a blog post](http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection/) by [@gingerlime](https://github.com/gingerlime). Loopback/private IPs are ignored by default, but known proxies & clients are configurable, so you have full control over which IPs are considered legitimate. You can configure any number of arbitrary forwarding headers to use. If there's a special way to parse your particular header, the architecture of this project should [make it easy](#contributing) to open a pull request so `RemoteIp` can accommodate.

**If your app is not behind at least one proxy, you should not use this plug.** See [below](#algorithm) for more detailed reasoning.

Expand Down Expand Up @@ -30,23 +30,46 @@ end

Keep in mind the order of plugs in your pipeline and place `RemoteIp` as early as possible. For example, if you were to add `RemoteIp` *after* [the Plug Router](https://github.com/elixir-lang/plug#the-plug-router), your route action's logic would be executed *before* the `remote_ip` actually gets modified - not very useful!

There are 2 options that can be passed in:
There are 3 options that can be passed in:

* `:headers` - A list of strings naming the `req_headers` to use when deriving the `remote_ip`. Order does not matter. Defaults to `~w[forwarded x-forwarded-for x-client-ip x-real-ip]`.

* `:proxies` - A list of strings in [CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of known proxies. Defaults to `[]`.

For example, if you know you are behind proxies in the IP block 1.2.x.x that use the `X-Foo`, `X-Bar`, and `X-Baz` headers, you could say
[Loopback](https://en.wikipedia.org/wiki/Loopback) and [private](https://en.wikipedia.org/wiki/Private_network) IPs are always appended to this list:

* 127.0.0.0/8
* ::1/128
* fc00::/7
* 10.0.0.0/8
* 172.16.0.0/12
* 192.168.0.0/16

Since these IPs are internal, they often are not the actual client address in production, so we add them by default. To override this behavior, whitelist known client IPs using the `:clients` option.

* `:clients` - A list of strings in [CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of known clients. Defaults to `[]`.

An IP in any of the ranges listed here will never be considered a proxy. This takes precedence over the `:proxies` option, including loopback/private addresses. Any IP that is **not** covered by `:clients` or `:proxies` is assumed to be a client IP.

For example, suppose you know:
* you are behind proxies in the 1.2.x.x block
* the proxies use the `X-Foo`, `X-Bar`, and `X-Baz` headers
* but the IP 1.2.3.4 is actually a client, not one of the proxies

Then you could say

```elixir
defmodule MyApp do
use Plug.Builder

plug RemoteIp, headers: ~w[x-foo x-bar x-baz], proxies: ~w[1.2.0.0/16]
plug RemoteIp,
headers: ~w[x-foo x-bar x-baz],
proxies: ~w[1.2.0.0/16],
clients: ~w[1.2.3.4/32]
end
```

Note that, due to limitations in the [inet_cidr](https://github.com/Cobenian/inet_cidr) library used to parse them, `:proxies` **must** be written in full CIDR notation, even if specifying just a single IP. So instead of `"127.0.0.1"` and `"a:b::c:d"`, you would use `"127.0.0.1/32"` and `"a:b::c:d/128"`.
Note that, due to limitations in the [inet_cidr](https://github.com/Cobenian/inet_cidr) library used to parse them, `:proxies` and `:clients` **must** be written in full CIDR notation, even if specifying just a single IP. So instead of `"127.0.0.1"` and `"a:b::c:d"`, you would use `"127.0.0.1/32"` and `"a:b::c:d/128"`.

## Background

Expand Down Expand Up @@ -80,7 +103,7 @@ Note that the field is _meant_ to be overwritten. Plug does not actually do any
None of the available solutions I have seen are ideal. In this sort of plug, you want:

* **Configurable Headers:** With so many different headers being used, you should be able to configure the ones you need with minimal work.
* **Configurable Proxies:** With multiple proxy hops, there may be several IPs in the forwarding headers. Without being able to tell the plug which of those IPs are actually known to be proxies, you may get one of them back as the `remote_ip`.
* **Configurable Proxies and Clients:** With multiple proxy hops, there may be several IPs in the forwarding headers. Without being able to tell the plug which of those IPs are actually known to be proxies, you may get one of them back as the `remote_ip`.
* **Correctness:** Parsing forwarding headers can be surprisingly subtle. Most available libraries get it wrong.

The table below summarizes the problems with existing packages.
Expand Down Expand Up @@ -158,32 +181,34 @@ Not only are known proxies' headers trusted, but also requests forwarded for [lo
* 172.16.0.0/12
* 192.168.0.0/16

These IPs are filtered because they are used internally and are thus guaranteed not to be the actual client address in production.
These IPs are filtered because they are used internally and are thus generally not the actual client address in production.

However, if (say) your app is only deployed in a [VPN](https://en.wikipedia.org/wiki/Virtual_private_network)/[LAN](https://en.wikipedia.org/wiki/Local_area_network), then your clients might actually have these internal IPs. To prevent loopback/private addresses from being considered proxies, configure them as known clients using the [`:clients` option](#usage).

### :warning: Caveats :warning:

1. **Only use `RemoteIp` if your app is behind at least one proxy.** Because the last forwarding header is always tacitly trusted, it would be trivial to spoof an IP if your app _wasn't actually_ behind a proxy: just set a forwarding header. Besides, there isn't much to be gained from this library if your app isn't behind a proxy.

2. The relative order of IPs can still be messed up by proxies amending prior headers. For instance,

* Request starts from IP 1.1.1.1 (no forwarding headers)
* Proxy 1 with IP 2.2.2.2 adds `Forwarded: for=1.1.1.1`
* Proxy 2 with IP 3.3.3.3 adds `X-Forwarded-For: 2.2.2.2`
* Proxy 3 with IP 4.4.4.4 adds to `Forwarded` so it says `Forwarded: for=1.1.1.1, for=3.3.3.3`
* Request starts from IP 1.1.1.1 (no forwarding headers)
* Proxy 1 with IP 2.2.2.2 adds `Forwarded: for=1.1.1.1`
* Proxy 2 with IP 3.3.3.3 adds `X-Forwarded-For: 2.2.2.2`
* Proxy 3 with IP 4.4.4.4 adds to `Forwarded` so it says `Forwarded: for=1.1.1.1, for=3.3.3.3`

Thus, `RemoteIp` processes the request from 4.4.4.4 with the first-to-last list of forwarded IPs
Thus, `RemoteIp` processes the request from 4.4.4.4 with the first-to-last list of forwarded IPs

```elixir
[{1, 1, 1, 1}, {3, 3, 3, 3}, {2, 2, 2, 2}] # what we get
```
```elixir
[{1, 1, 1, 1}, {3, 3, 3, 3}, {2, 2, 2, 2}] # what we get
```

even though the _actual_ order was
even though the _actual_ order was

```elixir
[{1, 1, 1, 1}, {2, 2, 2, 2}, {3, 3, 3, 3}] # actual forwarding order
```
```elixir
[{1, 1, 1, 1}, {2, 2, 2, 2}, {3, 3, 3, 3}] # actual forwarding order
```

The solution to this problem is to add both 2.2.2.2 and 3.3.3.3 as known proxies. Then either way the original client address will be reported as 1.1.1.1. As always, be sure to test in your particular environment.
The solution to this problem is to add both 2.2.2.2 and 3.3.3.3 as known proxies. Then either way the original client address will be reported as 1.1.1.1. As always, be sure to test in your particular environment.

## Contributing

Expand Down
86 changes: 68 additions & 18 deletions lib/remote_ip.ex
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
defmodule RemoteIp do
@moduledoc """
A plug to overwrite the `Plug.Conn`'s `remote_ip` based on headers such as
`X-Forwarded-For`.
A plug to overwrite the `Plug.Conn`'s `remote_ip` based on request headers.
To use, add the `RemoteIp` plug to your app's plug pipeline:
Expand All @@ -13,7 +12,13 @@ defmodule RemoteIp do
end
```
There are 2 options that can be passed in:
Keep in mind the order of plugs in your pipeline and place `RemoteIp` as
early as possible. For example, if you were to add `RemoteIp` *after* [the
Plug Router](https://github.com/elixir-lang/plug#the-plug-router), your route
action's logic would be executed *before* the `remote_ip` actually gets
modified - not very useful!
There are 3 options that can be passed in:
* `:headers` - A list of strings naming the `req_headers` to use when
deriving the `remote_ip`. Order does not matter. Defaults to `~w[forwarded
Expand All @@ -23,22 +28,57 @@ defmodule RemoteIp do
[CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of
known proxies. Defaults to `[]`.
For example, if you know you are behind proxies in the IP block 1.2.x.x that
use the `X-Foo`, `X-Bar`, and `X-Baz` headers, you could say
[Loopback](https://en.wikipedia.org/wiki/Loopback) and
[private](https://en.wikipedia.org/wiki/Private_network) IPs are always
appended to this list:
* 127.0.0.0/8
* ::1/128
* fc00::/7
* 10.0.0.0/8
* 172.16.0.0/12
* 192.168.0.0/16
Since these IPs are internal, they often are not the actual client
address in production, so we add them by default. To override this
behavior, whitelist known client IPs using the `:clients` option.
* `:clients` - A list of strings in
[CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of
known clients. Defaults to `[]`.
An IP in any of the ranges listed here will never be considered a proxy.
This takes precedence over the `:proxies` option, including
loopback/private addresses. Any IP that is **not** covered by `:clients`
or `:proxies` is assumed to be a client IP.
For example, suppose you know:
* you are behind proxies in the 1.2.x.x block
* the proxies use the `X-Foo`, `X-Bar`, and `X-Baz` headers
* but the IP 1.2.3.4 is actually a client, not one of the proxies
Then you could say
```elixir
defmodule MyApp do
use Plug.Builder
plug RemoteIp, headers: ~w[x-foo x-bar x-baz], proxies: ~w[1.2.0.0/16]
plug RemoteIp,
headers: ~w[x-foo x-bar x-baz],
proxies: ~w[1.2.0.0/16],
clients: ~w[1.2.3.4/32]
end
```
Note that, due to limitations in the
[inet_cidr](https://github.com/Cobenian/inet_cidr) library used to parse
them, `:proxies` **must** be written in full CIDR notation, even if
specifying just a single IP. So instead of `"127.0.0.1"` and `"a:b::c:d"`,
you would use `"127.0.0.1/32"` and `"a:b::c:d/128"`.
them, `:proxies` and `:clients` **must** be written in full CIDR notation,
even if specifying just a single IP. So instead of `"127.0.0.1"` and
`"a:b::c:d"`, you would use `"127.0.0.1/32"` and `"a:b::c:d/128"`.
For more details, refer to the
[README](https://github.com/ajvondrak/remote_ip/blob/master/README.md) on
GitHub.
"""

@behaviour Plug
Expand All @@ -52,6 +92,8 @@ defmodule RemoteIp do

@proxies []

@clients []

# https://en.wikipedia.org/wiki/Loopback
# https://en.wikipedia.org/wiki/Private_network
@reserved ~w[
Expand All @@ -66,37 +108,45 @@ defmodule RemoteIp do
def init(opts \\ []) do
headers = Keyword.get(opts, :headers, @headers)
headers = MapSet.new(headers)

proxies = Keyword.get(opts, :proxies, @proxies) ++ @reserved
proxies = proxies |> Enum.map(&InetCidr.parse/1)

{headers, proxies}
clients = Keyword.get(opts, :clients, @clients)
clients = clients |> Enum.map(&InetCidr.parse/1)

{headers, proxies, clients}
end

def call(conn, {headers, proxies}) do
case last_forwarded_ip(conn, headers, proxies) do
def call(conn, {headers, proxies, clients}) do
case last_forwarded_ip(conn, headers, proxies, clients) do
nil -> conn
ip -> %{conn | remote_ip: ip}
end
end

defp last_forwarded_ip(conn, headers, proxies) do
defp last_forwarded_ip(conn, headers, proxies, clients) do
conn
|> ips_from(headers)
|> last_ip_forwarded_through(proxies)
|> last_ip_forwarded_through(proxies, clients)
end

defp ips_from(%Plug.Conn{req_headers: headers}, allowed) do
RemoteIp.Headers.parse(headers, allowed)
end

defp last_ip_forwarded_through(ips, proxies) do
defp last_ip_forwarded_through(ips, proxies, clients) do
ips
|> Enum.reverse
|> Enum.find(&forwarded?(&1, proxies))
|> Enum.find(&forwarded?(&1, proxies, clients))
end

defp forwarded?(ip, proxies, clients) do
client?(ip, clients) || !proxy?(ip, proxies)
end

defp forwarded?(ip, proxies) do
!proxy?(ip, proxies)
defp client?(ip, clients) do
Enum.any?(clients, fn client -> InetCidr.contains?(client, ip) end)
end

defp proxy?(ip, proxies) do
Expand Down
Loading

0 comments on commit 06ca198

Please sign in to comment.