Host origin IP can be spoofed via X-Forwarded-For

th3Bak3r commented 2 months ago

When visiting configured HTTP endpoints, it is currently possible for the origin IP of the visiting user to be spoofed. This is achieved by sending a spoofed IP address in the X-Forwarded-For HTTP request header to the endpoints.

As an example, the host origin IP address will be seen as 127.0.0.1 in the monitoring logs for the following HTTP request:

$ curl "https://demo.local/demo1" -H "X-Forwarded-For: 127.0.0.1, 192.168.1.123"

The underlying issue is in line 393 of delivery/handlers.py, where the first element in the list after the split() call would be used as the host origin IP:

...
        if 'X-Forwarded-For' in request.headers:
            # Accepted practice is to put the original client IP address in the leftmost position
            # Truncate at 50 characters in case we get some really weird invalid value
            request_log.request_ip = request.headers['X-Forwarded-For'].split(',')[0][:50]
...

Depending on the CDN vendors used, the way for host origin IPs to get relayed to the server may be different. However, a good starting point to investigate might be to take the last element in the list after the split() call as opposed to the first element.

inzlain commented 2 months ago

Sushi Train expects redirectors to explicitly set the X-Forwarded-For header to a trusted value as documented . For traditional redirectors like NGINX, this value is explicitly set to a trusted value in the templated redirector configuration (i.e. this issue only affects redirectors deployed with the templated CloudFront configurations). However, when it comes to CDN redirectors like CloudFront and Azure CDN it's not always ideal (or even possible with some CDNs) to set this header to a trusted value.

This is particularly relevant when chaining multiple CDNs. For example, if you are have another "dumb" passthrough CDN in front of your CloudFront redirector, then you probably don't actually to take the right-most value because it would be IP address of the first CDN. It can also be useful to see the full X-Forwarded-For chain for troubleshooting why requests via CDNs aren't working as expected. For these reasons, I have generally been reluctant to explicitly set the header to a trusted value on CDN redirectors in the templated redirector configurations provided. The expectation was that users would either explicitly set the header themselves (i.,e. modify the template for their needs), or if the CDN didn't allow it, the user should use the CDN as a "dumb" passthrough and place a NGINX redirector behind it (which can set a trusted header value).

In practice though, most use cases of CDN redirectors from users are just using them as a single hop directly into Sushi Train (using the default unmodified template I provide). So I've changed the default behaviour to select the right-most IP address from the X-Forwarded-For header. I will see if there are any user reports of newly introduced problems due this change. Potentially, this could be a configurable option for redirectors in the future to choose the IP selection strategy (e.g. left-most, left-most non-private, right-most, or right-most non-private).

Note: This doesn't remove the requirement for non-CDN redirectors to still have to supply a trusted value for the X-Forwarded-For header. It just results in better behaviour for CDNs that do provide a "trusted" value in the right-most position.

th3Bak3r commented 2 weeks ago

It turns out that the rightmost value for X-Forwarded-For used in Azure CDN (classic) would be the Azure dynamic IPs, which would not be optimal for our purposes. Did some digging and found this reference, which essentially creates some Nginx mapping to extract the X-Forwarded-For value we want.

While it's mostly a bandage fix, we could potentially do something like the following:

Create Custom Mapping in /etc/nginx/nginx.conf

...
# Make sure our code is in the http block
http {
...
    map $http_x_forwarded_for $x_real_ip_leftmost {
      "~^([^,]+).*" $1;
      default $http_x_forwarded_for;
    }

    map $http_x_forwarded_for $x_real_ip_rightmost {
      ".*([^,]+)$" $1;
      default $http_x_forwarded_for;
    }
...
}

Create if conditions to use values based on CDN domain in /etc/nginx/sites-enabled/default

server {
...
location = /delivery {

            set $processed_x_forwarded_for $http_x_forwarded_for;

            if ($http_x_redirector_hostname ~* "\.azureedge\.net$")
            {
                    set $processed_x_forwarded_for $x_real_ip_leftmost;
            }
            proxy_set_header X-Forwarded-For $processed_x_forwarded_for;

            proxy_pass http://sushitrain:9000/delivery
}
}

inzlain commented 2 weeks ago

Feel free to add feedback here, this needs more than just picking one option over the other, or implementing bandaid fixes in the redirector configurations: https://github.com/inzlain/SushiTrain/issues/2

inzlain / SushiTrain

Host origin IP can be spoofed via X-Forwarded-For #1