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

Incorrect handling of X-Forwarded-For #1735

Open
rhbvkleef opened this issue May 7, 2024 · 0 comments
Open

Incorrect handling of X-Forwarded-For #1735

rhbvkleef opened this issue May 7, 2024 · 0 comments

Comments

@rhbvkleef
Copy link
Contributor

As per an email conversation with @tkurki, I am posting this issue here as the security aspect of this issue is not so critical that some other flow should be used.

SignalK always prefers using the exact value of the X-Forwarded-For header as the originating IP-address. This has several issues.

  1. The X-Forwarded-For header is poorly standardized, and commonly has several different possible types of values, namely:
    1. A comma-separated list of IP-addresses
    2. A singular IP address
      With IPv6 addresses maybe bracketized, and IP-addresses maybe being postfixed by a port number. See the linked Wikipedia article for more info.
  2. The X-Forwarded-For header is not always used. Occasionally, a reverse proxy may create a Forwarded header instead. See RFC 7239 (linked)
  3. A client may spoof either the X-Forwarded-For or the Forwarded header. It is therefore necessary to check whether the originating IP address of the request is a trusted proxy.

Possible solutions for the security issue

Boolean configuration option enabling the usage of X-Forwarded-For or Forwarded

This is by far the simplest solution. It is also relatively common. A configuration option would need to be added that either enables the current behaviour, but defaulting to a safer behaviour, namely just using the source IP address as per the IP header.

List of trusted proxies for only the first hop

This is also a relatively common solution. A list of IP-subnets can be specified. If the source IP address as per the IP header is in the list, then the headers are used, and otherwise the source IP address as per the IP header is used.

Possible solutions for the correctness issues

The first problem is choosing between X-Forwarded-For and Forwarded. A configuration option could be added for this, or one or the other can be preferred. I personally think that if we're going to prefer one header over the other, we should prefer Forwarded as it is well-specified as opposed to X-Forwarded-For.

The second is choosing the correct IP if a list is specified. This is relatively simple. Choose the last IP address in the list.

See

Problematic sites:

const ip = req.headers['x-forwarded-for'] || req.connection.remoteAddress

spark.request.headers['x-forwarded-for'] ||

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant