SignalK / signalk-server

An implementation of a Signal K central server for boats.
http://signalk.org
Apache License 2.0
298 stars 150 forks source link

Incorrect handling of X-Forwarded-For #1735

Open rhbvkleef opened 1 month ago

rhbvkleef commented 1 month ago

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: https://github.com/SignalK/signalk-server/blob/448944169a7bcb09ef3ca2bbc7be2b041481ad5d/src/serverroutes.ts#L436 https://github.com/SignalK/signalk-server/blob/448944169a7bcb09ef3ca2bbc7be2b041481ad5d/src/interfaces/ws.js#L363