Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change ActionDispatch::RemoteIp to not use HTTP_X_FORWARDED_FOR when REMOTE_ADDR is not a trusted proxy #51610

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ClearlyClaire
Copy link
Contributor

Motivation / Background

As pointed out in the documentation, using RemoteIp while accessible without a reverse proxy opens you to IP spoofing. Returning REMOTE_ADDR when that is not considered a trusted proxy should make that less likely.

Detail

The current implementation of RemoteIp assumes the Rails server is only directly accessible through a trusted reverse-proxy and requires users to filter the header or remove the middleware when that is not the case.

While such misconfigurations are unlikely, it is also somewhat of an unsafe fallback mode, and returning REMOTE_ADDR instead of trusting X-Forwarded-For from an untrusted client should be better.

Additional information

In the test suite, there are cases where REMOTE_ADDR is unset. I don't know in which real-life situations this could happen, but if it does, the code essentially falls back to the old behavior of trusting the last X-Forwarded-For.

An alternative might be to raise an error when REMOTE_ADDR does not match one of the trusted proxies, as is done when Client-Ip and X-Forwarded-For are mismatched.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionpack label Apr 19, 2024
…hen `REMOTE_ADDR` is not a trusted proxy

As pointed out in the documentation, using `RemoteIp` while accessible without a reverse proxy opens you to IP
spoofing. Returning `REMOTE_ADDR` when that is not considered a trusted proxy should make that less likely.
@ClearlyClaire ClearlyClaire force-pushed the fixes/remote-ip-x-forwarded-for-ordering branch from e403810 to 30e14eb Compare April 19, 2024 13:47
@rails-bot rails-bot bot added the railties label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant