Reproduce incorrect IP address with multiple reverse proxies

[acelaya]

This is a reproduction of #2351, where the incorrect visitor IP address is resolved when Shlink is served behind a chain of two or more reverse proxies.

In here, three reverse proxies are configured in front of Shlink, with the next IP addresses:

• visitor - 172.20.16.1
• shlink_proxy_three - 172.20.16.18
• shlink_proxy_two - 172.20.16.17
• shlink_proxy_one - 172.20.16.15
• shlink - not relevant

I tried to visit every proxy, and see what IP addresses were resolved and set in every relevant header, and these are the results:

akrabat/ip-address-middleware ^2.5

shlink (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.1
X-Real-IP: -
X-Forwarded-For: - 

shlink_proxy_one (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.1
X-Forwarded-For: 172.20.16.1

shlink_proxy_two (trusted_proxies: [])

Resolved IP: 172.20.16.17 (wrong 🔴)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.17

shlink_proxy_three (trusted_proxies: [])

Resolved IP: 172.20.16.17 (wrong 🔴)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.18, 172.20.16.17

shlink_proxy_one (trusted_proxies: ['172.20.16.15'])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.1
X-Forwarded-For: 172.20.16.1

shlink_proxy_two (trusted_proxies: ['172.20.16.15', '172.20.16.17'])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.17

shlink_proxy_three (trusted_proxies: ['172.20.16.15', '172.20.16.17', '172.20.16.18'])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.18, 172.20.16.17

akrabat/ip-address-middleware 2.4.0

shlink (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.1
X-Real-IP: -
X-Forwarded-For: - 

shlink_proxy_one (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.1
X-Forwarded-For: 172.20.16.1

shlink_proxy_two (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.17

shlink_proxy_three (trusted_proxies: [])

Resolved IP: 172.20.16.1 (right 🟢)
REMOTE_ADDR: 172.20.16.15
X-Real-IP: 172.20.16.17
X-Forwarded-For: 172.20.16.1, 172.20.16.18, 172.20.16.17

Conclusion

akrabat/ip-address-middleware: ^2.5 introduced a regression in order to fix a potential security issue. This is unfortunate, but there was probably not backwards-compatible way to solve it.

The result is that, while before, the first IP from the left was picked from X-Forwarded-For, now the first IP from the right which is not a trusted proxy is picked instead, so now it's required to set trusted proxies to retain the previous behavior.

Also, changing the order in which headers are checked to inspect X-Real-IP first would not solve the problem, as every proxy sets the IP of the previous one there, so we would still not resolve the visitor IP address when there's two or more proxies.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.77%. Comparing base (e511e15) to head (d8db6d1).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff             @@
##             develop    #2357   +/-   ##
==========================================
  Coverage      93.77%   93.77%           
  Complexity      1702     1702           
==========================================
  Files            276      276           
  Lines           5895     5895           
==========================================
  Hits            5528     5528           
  Misses           367      367           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.